Re: [PATCH v3 07/11] vpci: Hide extended capability when it fails to initialize

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, May 08, 2025 at 09:16:49AM +0000, Chen, Jiqian wrote:
> On 2025/4/23 00:06, Jan Beulich wrote:
> > On 21.04.2025 08:18, Jiqian Chen wrote:
> >> --- a/xen/include/xen/pci_regs.h
> >> +++ b/xen/include/xen/pci_regs.h
> >> @@ -449,6 +449,7 @@
> >>  #define PCI_EXT_CAP_ID(header)		((header) & 0x0000ffff)
> >>  #define PCI_EXT_CAP_VER(header)		(((header) >> 16) & 0xf)
> >>  #define PCI_EXT_CAP_NEXT(header)	(((header) >> 20) & 0xffc)
> >> +#define PCI_EXT_CAP_NEXT_MASK		0xFFC00000U
> > 
> > To avoid introducing redundancy, imo this addition calls for
> > 
> > #define PCI_EXT_CAP_NEXT(header)	MASK_EXTR(header, PCI_EXT_CAP_NEXT_MASK)
> When I tested this locally, I encountered errors: every next position address loss two bits zero.
> The next register has 12 bits, according to PCI spec, the bottom two bits are reserved zero,
> so "#define PCI_EXT_CAP_NEXT_MASK 0xFFC00000U" is fine,
> but if change this "#define PCI_EXT_CAP_NEXT(header) MASK_EXTR(header, PCI_EXT_CAP_NEXT_MASK)",
> I need to change PCI_EXT_CAP_NEXT_MASK to be 0xFFF00000U, is it fine?

Oh, I see.  You might want to do:

#define PCI_EXT_CAP_NEXT_MASK		0xFFF00000U
/* Bottom two bits of next capability position are reserved. */
#define PCI_EXT_CAP_NEXT(header)	(MASK_EXTR(header,
					           PCI_EXT_CAP_NEXT_MASK)
					 & 0xFFCU)

The spec says:

"The bottom 2 bits of this offset are Reserved and must be implemented
as 00b although software must mask them to allow for future uses of
these bits."

So we need to make sure they are masked.

Thanks, Roger.