[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 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). > --- 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. > @@ -267,11 +270,75 @@ static void check_pdev(const struct pci_dev *pdev) > } > break; > > + case PCI_HEADER_TYPE_NORMAL: > + nbars = PCI_HEADER_NORMAL_NR_BARS; > + rom_pos = PCI_ROM_ADDRESS; > + break; > + > case PCI_HEADER_TYPE_CARDBUS: > /* TODO */ > break; > } > #undef PCI_STATUS_CHECK > + > + /* Check if BARs overlap with other memory regions. */ > + val = pci_conf_read16(pdev->sbdf, PCI_COMMAND); > + if ( !(val & PCI_COMMAND_MEMORY) || pdev->ignore_bars ) > + return; > + > + pci_conf_write16(pdev->sbdf, PCI_COMMAND, val & ~PCI_COMMAND_MEMORY); > + for ( i = 0; i < nbars; ) > + { > + uint64_t addr, size; > + unsigned int reg = PCI_BASE_ADDRESS_0 + i * 4; > + int rc = 1; > + > + if ( (pci_conf_read32(pdev->sbdf, reg) & PCI_BASE_ADDRESS_SPACE) != > + PCI_BASE_ADDRESS_SPACE_MEMORY ) > + goto next; > + > + rc = pci_size_mem_bar(pdev->sbdf, reg, &addr, &size, > + (i == nbars - 1) ? PCI_BAR_LAST : 0); > + if ( rc < 0 ) > + /* Unable to size, better leave memory decoding disabled. */ > + return; > + if ( size && !is_iomem_range(addr, size) ) > + { > + /* > + * Return without enabling memory decoding if BAR position is not > + * in IO suitable memory. Let the hardware domain re-position the > + * BAR. > + */ > + printk(XENLOG_WARNING > +"%pp disabled: BAR%u [%" PRIx64 ", %" PRIx64 ") overlaps with memory map\n", I guess despite its length this still wants indenting properly. Maybe you could fold this and ... > + &pdev->sbdf, i, addr, addr + size); > + return; > + } > + > + next: > + ASSERT(rc > 0); > + i += rc; > + } > + > + if ( rom_pos && > + (pci_conf_read32(pdev->sbdf, rom_pos) & PCI_ROM_ADDRESS_ENABLE) ) > + { > + uint64_t addr, size; > + int rc = pci_size_mem_bar(pdev->sbdf, rom_pos, &addr, &size, > + PCI_BAR_ROM); > + > + if ( rc < 0 ) > + return; > + if ( size && !is_iomem_range(addr, size) ) > + { > + printk(XENLOG_WARNING > +"%pp disabled: ROM BAR [%" PRIx64 ", %" PRIx64 ") overlaps with memory > map\n", .. this into static const char warn[] = "%pp disabled: %sBAR [%" PRIx64 ", %" PRIx64 ") overlaps with memory map\n"; to limit indentation and redundancy at the same time? Could then also be re-used later for the SR-IOV BAR check. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |