Re: [PATCH] vPCI: avoid bogus "overlap in extended cap list" warnings

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Dec 18, 2025 at 08:56:24AM +0100, Jan Beulich wrote:
> 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>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

With the U suffix added to the constant, as noted by Stewart.

> ---
> 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?

I think that's likely too much - for the hardware domain we want to
allow the domain to access all the PCI config space, regardless of
Xen's thinking there's nothing there.

> 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)?

If there's no explicitly handler added, the behavior for domU will
already be to drop writes, and return reads as all 1s, which is fine
for the rest of the extended config space?  We just need to return 0
for the first 32bits to avoid seeming to have extended capability
support.

Maybe we want to keep the same behavior as expected from native for
legacy devices and just return all 1s consistency for the extended
space?

Hence we don't need to special case this region, as it's already
covered by how unhandled accesses are resolved for domUs.

Or is there something else I'm missing?

> Should we perhaps also log a warning if we exit the loop with non-zero
> "pos"?

Possibly?  The spec says no next capability must be signaled by 00,
returning any other value below the extended space is a bug in the
device, but I have no idea whether that would be a common bug, and
whether things would get too noisy.

Thanks, Roger.