[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 1/2] xen/vpci: header: status register handler



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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.