| Message ID | 20230205094411.793816-5-smostafa@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add stage-2 translation for SMMUv3 | expand |
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 {
On Wed, Feb 15, 2023 at 05:29:04PM +0100, Eric Auger wrote: > > > > The property added arm-smmuv3.stage can have 3 values: > > - "1": Stage-1 is only advertised. > Stage-1 only is advertised I will update it. > > - "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. I will split this commit, move the knobs to the end and keep features definition as it is used by other commits. I think it would also make sense to merge the knobs commit with last one([RFC PATCH 16/16] hw/arm/smmuv3: Enable stage-2 support). Thanks, Mostafa
On 2/16/23 13:58, Mostafa Saleh wrote: > On Wed, Feb 15, 2023 at 05:29:04PM +0100, Eric Auger wrote: >>> The property added arm-smmuv3.stage can have 3 values: >>> - "1": Stage-1 is only advertised. >> Stage-1 only is advertised > I will update it. > >>> - "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. > I will split this commit, move the knobs to the end and keep features > definition as it is used by other commits. > I think it would also make sense to merge the knobs commit with last > one([RFC PATCH 16/16] hw/arm/smmuv3: Enable stage-2 support). OK! Eric > > > Thanks, > Mostafa >
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 {
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. - "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. 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(-)