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

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


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Date: Thu, 23 Nov 2023 07:57:21 -0500
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=citrix.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=IFMKOYpWh97J33kiNDv1TLuF8rDGTo7G3bOUBlPBr70=; b=CbYPYrHj5AAySXlYZ48arjjvQMniaWuO1EQ1c+B6W+b8siVuweTQyirEUtKZS01VTSt8vIwDbbwwFxcj0vARQ374jnAk7athL3idzVJuYZ3xTCOidFEFDDuTshBlStQfS8GsK01HdxEJszaVT8MRzTHjIHUwSJp4e9TdFJE/MsOQ+IVTWXiKDknOAl2Pd6cf23SkmSnW5EnV3xH3n1eHlNOlKnwxf5NVVCEzgYhtgJPgZYiYRaE+JCix/FSYeNQqmfjUL4nSzcBsTPVR+xX3faqOVd5wZ5xkAqugiu/J3JB68MQZ9uPfI90T99vQSwQ3g/jXQ/HFaaUSr50+FFYnww==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mI+CIZVOJ7SPtesLjDX5FOcjwe5cBeqHwzRUr8O8nIjPUafcChBuQW8fyv//fIbQ8bE+BQxSY/hmeaEnOXY3Hes9ElzQVyVVDnIwCEamMkn8ih9CUTVEXYcw+3DZs3frdXKamndheWkBhkeRfcSp/8c+Ba32xWsPNBncARfM37ck8RYez7aaccuak4rtdgwDoZY16fB76JOP+VtXcQgflStJjdorg45JxRKdJDOj2l99aFMHT3pMsh6hdq+dUHN7cjERxX9oglH6PFIcmAkRLPQF+vpUxXSPbgBaCSqlI1fE2BB/qWvhOMEWXV+2icO0dh0/LDxebyZ7isqALVn8aA==
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Thu, 23 Nov 2023 12:57:36 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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



 


Rackspace

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