[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v7 1/2] xen/vpci: header: status register handler
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).
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |