On Thu, Nov 23, 2023 at 07:57:21AM -0500, Stewart Hildebrand wrote: > On 11/23/23 03:14, Roger Pau Monné wrote: > > On Wed, Nov 22, 2023 at 03:16:29PM -0500, Stewart Hildebrand wrote: > >> On 11/17/23 07:40, Roger Pau Monné wrote: > >>> On Wed, Sep 13, 2023 at 10:35:46AM -0400, Stewart Hildebrand wrote: > >>>> r->write(pdev, r->offset, data & (0xffffffffU >> (32 - 8 * > >>>> r->size)), > >>>> r->private); > >>>> } > >>>> diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h > >>>> index 84b18736a85d..b72131729db6 100644 > >>>> --- a/xen/include/xen/pci_regs.h > >>>> +++ b/xen/include/xen/pci_regs.h > >>>> @@ -66,6 +66,15 @@ > >>>> #define PCI_STATUS_REC_MASTER_ABORT 0x2000 /* Set on master abort */ > >>>> #define PCI_STATUS_SIG_SYSTEM_ERROR 0x4000 /* Set when we drive > >>>> SERR */ > >>>> #define PCI_STATUS_DETECTED_PARITY 0x8000 /* Set on parity error */ > >>>> +#define PCI_STATUS_RSVDZ_MASK 0x0006 > >>> > >>> In my copy of the PCIe 6 spec bit 6 is also RsvdZ, so the mask should > >>> be 0x46. > >> > >> Right, mine too. It's probably safer to follow the newer version of the > >> spec, so I'll make the change. For completeness / archaeology purposes, I > >> just want to document some relevant data points here. > >> > >> In PCIe 4 spec, it says this about bit 6: > >> "These bits were used in previous versions of the programming model. > >> Careful consideration should be given to any attempt to repurpose them." > >> > >> Going further back, PCI (old school PCI, not Express) spec 3.0 says this > >> about bit 6: > >> "This bit is reserved. *" > >> "* In Revision 2.1 of this specification, this bit was used to indicate > >> whether or not a device supported User Definable Features." > >> > >> Just above in our pci_regs.h (and equally in Linux > >> include/uapi/linux/pci_regs.h) we have this definition for bit 6: > >> #define PCI_STATUS_UDF 0x40 /* Support User Definable Features > >> [obsolete] */ > >> > >> Qemu hw/xen/xen_pt_config_init.c treats bit 6 as RO: > >> .ro_mask = 0x06F8, > > > > Right, given the implementation of ro_mask that would likely be fine. > > Reading unconditionally as 0 while preserving the value on writes > > seems the safest option. > > That would mean treating bit 6 as RsvdP, even though the PCIe 6 spec > says RsvdZ. I just want to confirm this is indeed the intent since > we both said RsvdZ just a moment ago? If so, I would add a comment > since it's deviating from spec. I would personally still vote in > favor of following PCIe 6 spec (RsvdZ).
Hm, yes, lets go with the spec and use RsdvZ. I very much doubt we passthrough any device that actually uses the UDF bit. Thanks, Roger.
