Hi,
On 2/21/24 18:17, Nabih Estefan wrote:
> From: Roque Arcudia Hernandez <roq...@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 <roq...@google.com>
> Signed-off-by: Nabih Estefan <nabiheste...@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);
> }
>
> /**
