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

[Yi Liu <yi.l.liu@intel.com>]

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.

> 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