[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] xen/pci: detect when BARs are not suitably positioned
Hi Jan,
On 02/02/2022 10:05, Jan Beulich wrote:
On 02.02.2022 10:57, Julien Grall wrote:
On 02/02/2022 09:42, Jan Beulich wrote:
On 01.02.2022 13:45, Roger Pau Monne wrote:
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -783,6 +783,23 @@ bool is_iomem_page(mfn_t mfn)
return (page_get_owner(page) == dom_io);
}
+bool is_memory_hole(unsigned long start, unsigned long end)
+{
+ unsigned int i;
+
+ for ( i = 0; i < e820.nr_map; i++ )
+ {
+ const struct e820entry *entry = &e820.map[i];
+
+ /* Do not allow overlaps with any memory range. */
+ if ( start < PFN_DOWN(entry->addr + entry->size) &&
+ PFN_DOWN(entry->addr) <= end )
+ return false;
+ }
+
+ return true;
+}
Doesn't the left side of the && need to use PFN_UP()?
I also think it would help if a brief comment ahead of the
function said that the range is inclusive. Otherwise the use
of < and >= gives the impression of something being wrong.
Then again it may be better to switch to <= anyway, as I
think you want to avoid possible zero-size regions (at which
point subtracting 1 for using <= is going to be valid).
Finally I wonder whether the function parameters wouldn't
better be named e.g. spfn and epfn, but maybe their units can
be inferred from their types being unsigned long (which,
however, would build on the assumption that we use appropriate
types everywhere).
I think this a case where mfn_t would be useful to use.
Actually I did consider to suggest it when asking to convert
to frame numbers, and specifically didn't because its use will
clutter the code here quite a bit. Which isn't to mean I'd
object if the adjustment was made ...
I thought about code cluterring, but there are two use
of the variables. So it would not look too bad.
But I care more about the external interface to be typesafe. So an
alternative would be:
unsigned long smfn_ = mfn_x();
unsigned long emfn_ = mfn_x();
Cheers,
--
Julien Grall
|
