|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] vPCI: avoid bogus "overlap in extended cap list" warnings
Legacy PCI devices don't have any extended config space. Reading any part
thereof may very well return all ones. That then necessarily means we
would think we found a "loop", when there simply is nothing.
Fixes: a845b50c12f3 ("vpci/header: Emulate extended capability list for dom0")
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
This is the minimalistic change to get rid of "overlap in extended cap
list" warnings I'm observing. We may want to avoid any attempt to access
extended config space when there is none - see Linux'es
pci_cfg_space_size() and it helper pci_cfg_space_size_ext(). This would
then also avoid us interpreting as an extended cap list what isn't one at
all (some legacy PCI devices don't decode register address bits 9-11, some
return other non-0, non-all-ones data). Including the risk of reading a
register with read side effects. Thoughts?
The DomU part of the function worries me as well. Rather than making it
"read 0, write ignore" for just the first 32 bits, shouldn't we make it so
for the entire extended config space, and shouldn't we also make it "read
all ones, write ignore" when there is no extended config space in the
first place (then in particular also for the first 32 bits)?
Should we perhaps also log a warning if we exit the loop with non-zero
"pos"?
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -839,6 +839,15 @@ static int vpci_init_ext_capability_list
uint32_t header = pci_conf_read32(pdev->sbdf, pos);
int rc;
+ if ( header == 0xffffffff )
+ {
+ if ( pos != PCI_CFG_SPACE_SIZE )
+ printk(XENLOG_WARNING
+ "%pd %pp: broken extended cap list, offset %#x\n",
+ pdev->domain, &pdev->sbdf, pos);
+ return 0;
+ }
+
rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
pos, 4, (void *)(uintptr_t)header);
if ( rc == -EEXIST )
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |