Re: [PATCH ats_vtd v5 06/22] pcie: add helper to declare PASID capability for a pcie device

[CLEMENT MATHIEU--DRIF]

On 03/07/2024 14:04, Yi Liu wrote:
> Caution: External email. Do not open attachments or click links, 
> unless this email comes from a known sender and you know the content 
> is safe.
>
>
> On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> ---
>>   hw/pci/pcie.c                             | 24 +++++++++++++++++++++++
>>   include/hw/pci/pcie.h                     |  6 +++++-
>>   include/hw/pci/pcie_regs.h                |  3 +++
>>   include/standard-headers/linux/pci_regs.h |  1 +
>>   4 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>> index 4b2f0805c6..d6a052b616 100644
>> --- a/hw/pci/pcie.c
>> +++ b/hw/pci/pcie.c
>> @@ -1177,3 +1177,27 @@ void pcie_acs_reset(PCIDevice *dev)
>>           pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 
>> 0);
>>       }
>>   }
>> +
>> +/* PASID */
>> +void pcie_pasid_init(PCIDevice *dev, uint16_t offset, uint8_t 
>> pasid_width,
>> +                     bool exec_perm, bool priv_mod)
>> +{
>> +    assert(pasid_width <= PCI_EXT_CAP_PASID_MAX_WIDTH);
>> +    static const uint16_t control_reg_rw_mask = 0x07;
>> +    uint16_t capability_reg = pasid_width;
>> +
>> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_PASID, PCI_PASID_VER, 
>> offset,
>> +                        PCI_EXT_CAP_PASID_SIZEOF);
>> +
>> +    capability_reg <<= PCI_PASID_CAP_WIDTH_SHIFT;
>> +    capability_reg |= exec_perm ? PCI_PASID_CAP_EXEC : 0;
>> +    capability_reg |= priv_mod  ? PCI_PASID_CAP_PRIV : 0;
>> +    pci_set_word(dev->config + offset + PCI_PASID_CAP, capability_reg);
>> +
>> +    /* Everything is disabled by default */
>> +    pci_set_word(dev->config + offset + PCI_PASID_CTRL, 0);
>> +
>> +    pci_set_word(dev->wmask + offset + PCI_PASID_CTRL, 
>> control_reg_rw_mask);
>> +
>> +    dev->exp.pasid_cap = offset;
>> +}
>
> seems no user of this helper in this series. If yes, you may drop this
> patch and include it when there is a caller of it.
You are right, I will move it to the series that implements the SVM demo 
device
>
>> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
>> index 5eddb90976..b870958c99 100644
>> --- a/include/hw/pci/pcie.h
>> +++ b/include/hw/pci/pcie.h
>> @@ -72,8 +72,9 @@ struct PCIExpressDevice {
>>       uint16_t aer_cap;
>>       PCIEAERLog aer_log;
>>
>> -    /* Offset of ATS capability in config space */
>> +    /* Offset of ATS and PASID capabilities in config space */
>>       uint16_t ats_cap;
>> +    uint16_t pasid_cap;
>>
>>       /* ACS */
>>       uint16_t acs_cap;
>> @@ -150,4 +151,7 @@ void pcie_cap_slot_unplug_cb(HotplugHandler 
>> *hotplug_dev, DeviceState *dev,
>>                                Error **errp);
>>   void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,
>>                                        DeviceState *dev, Error **errp);
>> +
>> +void pcie_pasid_init(PCIDevice *dev, uint16_t offset, uint8_t 
>> pasid_width,
>> +                     bool exec_perm, bool priv_mod);
>>   #endif /* QEMU_PCIE_H */
>> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
>> index 9d3b6868dc..0a86598f80 100644
>> --- a/include/hw/pci/pcie_regs.h
>> +++ b/include/hw/pci/pcie_regs.h
>> @@ -86,6 +86,9 @@ typedef enum PCIExpLinkWidth {
>>   #define PCI_ARI_VER                     1
>>   #define PCI_ARI_SIZEOF                  8
>>
>> +/* PASID */
>> +#define PCI_PASID_VER                   1
>> +#define PCI_EXT_CAP_PASID_MAX_WIDTH     20
>>   /* AER */
>>   #define PCI_ERR_VER                     2
>>   #define PCI_ERR_SIZEOF                  0x48
>> diff --git a/include/standard-headers/linux/pci_regs.h 
>> b/include/standard-headers/linux/pci_regs.h
>> index a39193213f..406dce8e82 100644
>> --- a/include/standard-headers/linux/pci_regs.h
>> +++ b/include/standard-headers/linux/pci_regs.h
>> @@ -935,6 +935,7 @@
>>   #define  PCI_PASID_CAP_EXEC 0x0002  /* Exec permissions Supported */
>>   #define  PCI_PASID_CAP_PRIV 0x0004  /* Privilege Mode Supported */
>>   #define  PCI_PASID_CAP_WIDTH        0x1f00
>> +#define  PCI_PASID_CAP_WIDTH_SHIFT  8
>>   #define PCI_PASID_CTRL              0x06    /* PASID control 
>> register */
>>   #define  PCI_PASID_CTRL_ENABLE      0x0001  /* Enable bit */
>>   #define  PCI_PASID_CTRL_EXEC        0x0002  /* Exec permissions 
>> Enable */
>
> -- 
> Regards,
> Yi Liu