[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] xen/pci: detect when BARs are not suitably positioned
On 27.01.2022 10:47, Roger Pau Monné wrote: > On Thu, Jan 27, 2022 at 09:48:16AM +0100, Jan Beulich wrote: >> On 27.01.2022 09:22, Roger Pau Monne wrote: >>> --- a/xen/arch/arm/mm.c >>> +++ b/xen/arch/arm/mm.c >>> @@ -1625,6 +1625,17 @@ bool is_iomem_page(mfn_t mfn) >>> return !mfn_valid(mfn); >>> } >>> >>> +bool is_iomem_range(paddr_t start, uint64_t size) >>> +{ >>> + unsigned int i; >>> + >>> + for ( i = 0; i < PFN_UP(size); i++ ) >>> + if ( !is_iomem_page(_mfn(PFN_DOWN(start) + i)) ) >>> + return false; >>> + >>> + return true; >>> +} >> >> I'm afraid you can't very well call this non-RFC as long as this doesn't >> scale. Or if you're building on this still being dead code on Arm, then >> some kind of "fixme" annotation would be needed here (or the function be >> omitted altogether, for Arm folks to sort out before they actually start >> selecting the HAS_PCI Kconfig setting). > > I was expecting the lack of 'RFC' tag to attract the attention of the > maintainers for feedback. Oh, I see. >>> --- 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_iomem_range(paddr_t start, uint64_t size) >>> +{ >>> + 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 < entry->addr + entry->size && >>> + entry->addr < start + size ) >>> + return false; >>> + } >>> + >>> + return true; >>> +} >> >> I should have realized (and pointed out) earlier that with the type >> check dropped the function name no longer represents what the >> function does. It now really is unsuitable for about anything other >> that the checking of BARs. > > is_devmem_range would be more suitable? I'd make it even more tight and use e.g. is_pci_bar_range(). Or maybe is_uncovered_range()? > I will wrap this in an #ifdef HAS_PCI section now. What would be the point of that, as long as X86 selects HAS_PCI unconditionally? (Arguably one may want to turn off PCI support in shim-only builds.) Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |