ARM SMMUv3 StreamID Implementation

[PATCH 2/2] hw/arm/smmu-common: Create virtual function for implementation defined StreamID

Posted by Nabih Estefan 9 months, 1 week ago

From: Roque Arcudia Hernandez <roqueh@google.com>

According to the SMMU specification the StreamID construction and size is
IMPLEMENTATION DEFINED, the size being between 0 and 32 bits.

This patch creates virtual functions get_sid and get_iommu_mr to allow different
implementations of how the StreamID is constructed via inheritance. The default
implementation of these functions will match the current ones where the BDF is
used directly as StreamID.

Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Signed-off-by: Nabih Estefan <nabihestefan@google.com>
---
 hw/arm/smmu-common.c         | 12 ++++++++++++
 include/hw/arm/smmu-common.h | 16 +++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 4caedb4998..14b3240a88 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -621,6 +621,11 @@ static const PCIIOMMUOps smmu_ops = {
 };
 
 IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid)
+{
+    return ARM_SMMU_GET_CLASS(s)->get_iommu_mr(s, sid);
+}
+
+static IOMMUMemoryRegion *smmu_base_iommu_mr(SMMUState *s, uint32_t sid)
 {
     uint8_t bus_n, devfn;
     SMMUPciBus *smmu_bus;
@@ -659,6 +664,11 @@ void smmu_inv_notifiers_all(SMMUState *s)
     }
 }
 
+static uint32_t smmu_base_get_sid(SMMUDevice *sdev)
+{
+    return PCI_BUILD_BDF(pci_bus_num(sdev->bus), sdev->devfn);
+}
+
 static void smmu_base_realize(DeviceState *dev, Error **errp)
 {
     SMMUState *s = ARM_SMMU(dev);
@@ -709,6 +719,8 @@ static void smmu_base_class_init(ObjectClass *klass, void *data)
     device_class_set_parent_realize(dc, smmu_base_realize,
                                     &sbc->parent_realize);
     rc->phases.hold = smmu_base_reset_hold;
+    sbc->get_sid = smmu_base_get_sid;
+    sbc->get_iommu_mr = smmu_base_iommu_mr;
 }
 
 static const TypeInfo smmu_base_info = {
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 5ec2e6c1a4..d53121fe37 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -131,6 +131,9 @@ typedef struct SMMUIOTLBKey {
     uint8_t level;
 } SMMUIOTLBKey;
 
+#define TYPE_ARM_SMMU "arm-smmu"
+OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
+
 struct SMMUState {
     /* <private> */
     SysBusDevice  dev;
@@ -147,6 +150,9 @@ struct SMMUState {
     PCIBus *primary_bus;
 };
 
+typedef uint32_t GetSidFunc(SMMUDevice *obj);
+typedef IOMMUMemoryRegion *GetIommuMr(SMMUState *s, uint32_t sid);
+
 struct SMMUBaseClass {
     /* <private> */
     SysBusDeviceClass parent_class;
@@ -154,19 +160,19 @@ struct SMMUBaseClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
+    GetSidFunc *get_sid;
+    GetIommuMr *get_iommu_mr;
 
 };
 
-#define TYPE_ARM_SMMU "arm-smmu"
-OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
-
 /* Return the SMMUPciBus handle associated to a PCI bus number */
 SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num);
 
 /* Return the stream ID of an SMMU device */
-static inline uint16_t smmu_get_sid(SMMUDevice *sdev)
+static inline uint32_t smmu_get_sid(SMMUDevice *sdev)
 {
-    return PCI_BUILD_BDF(pci_bus_num(sdev->bus), sdev->devfn);
+    SMMUState *s = sdev->smmu;
+    return ARM_SMMU_GET_CLASS(s)->get_sid(sdev);
 }
 
 /**
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [PATCH 2/2] hw/arm/smmu-common: Create virtual function for implementation defined StreamID

Posted by Eric Auger 9 months, 1 week ago

Hi,

On 2/21/24 18:17, Nabih Estefan wrote:
> From: Roque Arcudia Hernandez <roqueh@google.com>
>
> According to the SMMU specification the StreamID construction and size is
> IMPLEMENTATION DEFINED, the size being between 0 and 32 bits.
>
> This patch creates virtual functions get_sid and get_iommu_mr to allow different
> implementations of how the StreamID is constructed via inheritance. The default
> implementation of these functions will match the current ones where the BDF is
> used directly as StreamID.

The patch itself looks good to me but it lacks a concrete derived
implementation of get_sid() and get_iommu_mr(). Until you do not
upstream it I don't see the point in introducing those callbacks. Thanks
Eric
>
> Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> ---
>  hw/arm/smmu-common.c         | 12 ++++++++++++
>  include/hw/arm/smmu-common.h | 16 +++++++++++-----
>  2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 4caedb4998..14b3240a88 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -621,6 +621,11 @@ static const PCIIOMMUOps smmu_ops = {
>  };
>  
>  IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid)
> +{
> +    return ARM_SMMU_GET_CLASS(s)->get_iommu_mr(s, sid);
> +}
> +
> +static IOMMUMemoryRegion *smmu_base_iommu_mr(SMMUState *s, uint32_t sid)
>  {
>      uint8_t bus_n, devfn;
>      SMMUPciBus *smmu_bus;
> @@ -659,6 +664,11 @@ void smmu_inv_notifiers_all(SMMUState *s)
>      }
>  }
>  
> +static uint32_t smmu_base_get_sid(SMMUDevice *sdev)
> +{
> +    return PCI_BUILD_BDF(pci_bus_num(sdev->bus), sdev->devfn);
> +}
> +
>  static void smmu_base_realize(DeviceState *dev, Error **errp)
>  {
>      SMMUState *s = ARM_SMMU(dev);
> @@ -709,6 +719,8 @@ static void smmu_base_class_init(ObjectClass *klass, void *data)
>      device_class_set_parent_realize(dc, smmu_base_realize,
>                                      &sbc->parent_realize);
>      rc->phases.hold = smmu_base_reset_hold;
> +    sbc->get_sid = smmu_base_get_sid;
> +    sbc->get_iommu_mr = smmu_base_iommu_mr;
>  }
>  
>  static const TypeInfo smmu_base_info = {
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 5ec2e6c1a4..d53121fe37 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -131,6 +131,9 @@ typedef struct SMMUIOTLBKey {
>      uint8_t level;
>  } SMMUIOTLBKey;
>  
> +#define TYPE_ARM_SMMU "arm-smmu"
> +OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
> +
>  struct SMMUState {
>      /* <private> */
>      SysBusDevice  dev;
> @@ -147,6 +150,9 @@ struct SMMUState {
>      PCIBus *primary_bus;
>  };
>  
> +typedef uint32_t GetSidFunc(SMMUDevice *obj);
> +typedef IOMMUMemoryRegion *GetIommuMr(SMMUState *s, uint32_t sid);
> +
>  struct SMMUBaseClass {
>      /* <private> */
>      SysBusDeviceClass parent_class;
> @@ -154,19 +160,19 @@ struct SMMUBaseClass {
>      /*< public >*/
>  
>      DeviceRealize parent_realize;
> +    GetSidFunc *get_sid;
> +    GetIommuMr *get_iommu_mr;
>  
>  };
>  
> -#define TYPE_ARM_SMMU "arm-smmu"
> -OBJECT_DECLARE_TYPE(SMMUState, SMMUBaseClass, ARM_SMMU)
> -
>  /* Return the SMMUPciBus handle associated to a PCI bus number */
>  SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num);
>  
>  /* Return the stream ID of an SMMU device */
> -static inline uint16_t smmu_get_sid(SMMUDevice *sdev)
> +static inline uint32_t smmu_get_sid(SMMUDevice *sdev)
>  {
> -    return PCI_BUILD_BDF(pci_bus_num(sdev->bus), sdev->devfn);
> +    SMMUState *s = sdev->smmu;
> +    return ARM_SMMU_GET_CLASS(s)->get_sid(sdev);
>  }
>  
>  /**

[PATCH 1/2] hw/arm/smmuv3: Check StreamIDs against SMMU_IDR1.SIDSIZE value
[PATCH 2/2] hw/arm/smmu-common: Create virtual function for implementation defined StreamID