Re: [RFC PATCH 04/16] hw/arm/smmuv3: Add a system property to choose translation stage

[Eric Auger]

Hi,

On 2/5/23 10:43, Mostafa Saleh wrote:
> Add a new system property for smmuv3 to choose what translation
> stages to advertise.
>
> The property added arm-smmuv3.stage can have 3 values:
> - "1": Stage-1 is only advertised.
Stage-1 only is advertised
> - "2": Stage-2 is only advertised.
> - "all": Stage-1 + Stage-2 are supported, which is not implemented in
> this patch series.
>
> If not passed or an unsupported value is passed, it will default to
> stage-1.
>
> The property is not used in this patch as stage-2 has not been
> enabled yet.
Usually the user knob (ie. the property) is introduced at the last
stage, ie. at 16/16.

Thanks

Eric
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>  hw/arm/smmuv3-internal.h |  5 +++++
>  hw/arm/smmuv3.c          | 28 +++++++++++++++++++++++++++-
>  include/hw/arm/smmuv3.h  |  1 +
>  3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 170e88c24a..ec64fb43a0 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -329,6 +329,11 @@ enum { /* Command completion notification */
>          })
>  
>  #define SMMU_FEATURE_2LVL_STE (1 << 0)
> +#define SMMU_FEATURE_STAGE1   (1 << 1)
> +#define SMMU_FEATURE_STAGE2   (1 << 2)
> +
> +#define STAGE1_SUPPORTED(f)   (f & SMMU_FEATURE_STAGE1)
> +#define STAGE2_SUPPORTED(f)   (f & SMMU_FEATURE_STAGE2)
>  
>  /* Events */
>  
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 955b89c8d5..54dd8e5ec1 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -21,6 +21,7 @@
>  #include "hw/irq.h"
>  #include "hw/sysbus.h"
>  #include "migration/vmstate.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/qdev-core.h"
>  #include "hw/pci/pci.h"
>  #include "cpu.h"
> @@ -238,6 +239,19 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo 
> *info)
>  
>  static void smmuv3_init_regs(SMMUv3State *s)
>  {
> +    /*
> +     * Based on system property, the stages supported in smmu will be 
> advertised.
> +     * At the moment "all" is not supported.
> +     * Default stage is 1.
> +     */
> +    s->features = SMMU_FEATURE_STAGE1;
> +    if (s->stage && !strcmp("2", s->stage)) {
> +        s->features = SMMU_FEATURE_STAGE2;
> +    } else if (s->stage && !strcmp("all", s->stage)) {
> +        qemu_log_mask(LOG_UNIMP,
> +                          "SMMUv3 S1 and S2 nesting not supported, 
> defaulting to S1\n");
> +    }
> +
>      /**
>       * IDR0: stage1 only, AArch64 only, coherent access, 16b ASID,
>       *       multi-level stream table
> @@ -276,7 +290,6 @@ static void smmuv3_init_regs(SMMUv3State *s)
>      s->eventq.cons = 0;
>      s->eventq.entry_size = sizeof(struct Evt);
>  
> -    s->features = 0;
>      s->sid_split = 0;
>      s->aidr = 0x1;
>      s->cr[0] = 0;
> @@ -1514,6 +1527,18 @@ static const VMStateDescription vmstate_smmuv3 = {
>      },
>  };
>  
> +static Property smmuv3_properties[] = {
> +    /*
> +     * Stages of translation advertised.
> +     * "1": Stage 1
> +     * "2": Stage 2
> +     * "all": Stage 1 + Stage 2
> +     * Defaults to stage 1
> +     */
> +    DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
>  static void smmuv3_instance_init(Object *obj)
>  {
>      /* Nothing much to do here as of now */
> @@ -1530,6 +1555,7 @@ static void smmuv3_class_init(ObjectClass *klass, void 
> *data)
>                                         &c->parent_phases);
>      c->parent_realize = dc->realize;
>      dc->realize = smmu_realize;
> +    device_class_set_props(dc, smmuv3_properties);
>  }
>  
>  static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index f1921fdf9e..9b9e1c7baf 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -62,6 +62,7 @@ struct SMMUv3State {
>  
>      qemu_irq     irq[4];
>      QemuMutex mutex;
> +    char *stage;
>  };
>  
>  typedef enum {