diff mbox series

[RFC,v3,17/18] hw/arm/smmuv3: Add property for OAS

Message ID 20240429032403.74910-18-smostafa@google.com
State New
Headers show
Series SMMUv3 nested translation support | expand

Commit Message

Mostafa Saleh April 29, 2024, 3:24 a.m. UTC 
Add property that sets the OAS of the SMMU, this in not used in this
patch.

Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 hw/arm/smmuv3-internal.h |  3 ++-
 hw/arm/smmuv3.c          | 29 ++++++++++++++++++++++++++++-
 include/hw/arm/smmuv3.h  |  1 +
 3 files changed, 31 insertions(+), 2 deletions(-)

Comments

Eric Auger May 21, 2024, 9:32 a.m. UTC | #1 
Hi Mostafa,

On 4/29/24 05:24, Mostafa Saleh wrote:
> Add property that sets the OAS of the SMMU, this in not used in this
> patch.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>  hw/arm/smmuv3-internal.h |  3 ++-
>  hw/arm/smmuv3.c          | 29 ++++++++++++++++++++++++++++-
>  include/hw/arm/smmuv3.h  |  1 +
>  3 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 0ebf2eebcf..dd91807624 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -111,7 +111,8 @@ REG32(IDR5,                0x14)
>       FIELD(IDR5, VAX,        10, 2);
>       FIELD(IDR5, STALL_MAX,  16, 16);
>  
> -#define SMMU_IDR5_OAS 4
> +#define SMMU_IDR5_OAS_DEF 4 /* 44 bits. */
> +#define SMMU_IDR5_OAS_MAX 5 /* 48 bits. */
>  
>  REG32(IIDR,                0x18)
>  REG32(AIDR,                0x1c)
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 4ac818cf7a..39d03e7e24 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -299,7 +299,9 @@ static void smmuv3_init_regs(SMMUv3State *s)
>      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
>      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
>  
> -    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS); /* 44 bits */
> +    /* PTW doesn't support 52 bits. */
remove the point
> +    s->oas = MIN(s->oas, SMMU_IDR5_OAS_MAX);
> +    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, s->oas);
>      /* 4K, 16K and 64K granule support */
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, 1);
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
> @@ -1901,11 +1903,34 @@ static const VMStateDescription vmstate_gbpa = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_oas = {
> +    .name = "smmuv3/oas",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
don't you need a .needed function?

I tested backward migration and this fails
qemu-system-aarch64: error while loading state for instance 0x0 of
device 'smmuv3'
qemu-system-aarch64: load of migration failed: No such file or directory
post-processing ...

Thanks

Eric
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_INT32(oas, SMMUv3State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static int smmuv3_preload(void *opaque)
> +{
> +    SMMUv3State *s = opaque;
> +
> +    /*
> +     * In case it wasn't migrated, use the value used
> +     * by older QEMU.
> +     */
> +    s->oas = SMMU_IDR5_OAS_DEF;
> +    return 0;
> +}
> +
>  static const VMStateDescription vmstate_smmuv3 = {
>      .name = "smmuv3",
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .priority = MIG_PRI_IOMMU,
> +    .pre_load = smmuv3_preload,
>      .fields = (const VMStateField[]) {
>          VMSTATE_UINT32(features, SMMUv3State),
>          VMSTATE_UINT8(sid_size, SMMUv3State),
> @@ -1933,6 +1958,7 @@ static const VMStateDescription vmstate_smmuv3 = {
>      },
>      .subsections = (const VMStateDescription * const []) {
>          &vmstate_gbpa,
> +        &vmstate_oas,
>          NULL
>      }
>  };
> @@ -1945,6 +1971,7 @@ static Property smmuv3_properties[] = {
>       * Defaults to stage 1
>       */
>      DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> +    DEFINE_PROP_INT32("oas", SMMUv3State, oas, SMMU_IDR5_OAS_DEF),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index d183a62766..00a9eb4467 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -63,6 +63,7 @@ struct SMMUv3State {
>      qemu_irq     irq[4];
>      QemuMutex mutex;
>      char *stage;
> +    int32_t oas;
>  };
>  
>  typedef enum {
Mostafa Saleh June 27, 2024, 11:50 a.m. UTC | #2 
Hi Eric,

On Tue, May 21, 2024 at 11:32:48AM +0200, Eric Auger wrote:
> Hi Mostafa,
> 
> On 4/29/24 05:24, Mostafa Saleh wrote:
> > Add property that sets the OAS of the SMMU, this in not used in this
> > patch.
> >
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> >  hw/arm/smmuv3-internal.h |  3 ++-
> >  hw/arm/smmuv3.c          | 29 ++++++++++++++++++++++++++++-
> >  include/hw/arm/smmuv3.h  |  1 +
> >  3 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> > index 0ebf2eebcf..dd91807624 100644
> > --- a/hw/arm/smmuv3-internal.h
> > +++ b/hw/arm/smmuv3-internal.h
> > @@ -111,7 +111,8 @@ REG32(IDR5,                0x14)
> >       FIELD(IDR5, VAX,        10, 2);
> >       FIELD(IDR5, STALL_MAX,  16, 16);
> >  
> > -#define SMMU_IDR5_OAS 4
> > +#define SMMU_IDR5_OAS_DEF 4 /* 44 bits. */
> > +#define SMMU_IDR5_OAS_MAX 5 /* 48 bits. */
> >  
> >  REG32(IIDR,                0x18)
> >  REG32(AIDR,                0x1c)
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index 4ac818cf7a..39d03e7e24 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -299,7 +299,9 @@ static void smmuv3_init_regs(SMMUv3State *s)
> >      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
> >      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
> >  
> > -    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS); /* 44 bits */
> > +    /* PTW doesn't support 52 bits. */
> remove the point
> > +    s->oas = MIN(s->oas, SMMU_IDR5_OAS_MAX);
> > +    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, s->oas);
> >      /* 4K, 16K and 64K granule support */
> >      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, 1);
> >      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
> > @@ -1901,11 +1903,34 @@ static const VMStateDescription vmstate_gbpa = {
> >      }
> >  };
> >  
> > +static const VMStateDescription vmstate_oas = {
> > +    .name = "smmuv3/oas",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> don't you need a .needed function?
> 
> I tested backward migration and this fails
> qemu-system-aarch64: error while loading state for instance 0x0 of
> device 'smmuv3'
> qemu-system-aarch64: load of migration failed: No such file or directory
> post-processing ...

Yes, I think we need a .needed which returns if oas is not as default,
that should be backward compatiable (at least for this patch) and would
break when oas changes.

Thanks,
Mostafa
> 
> Thanks
> 
> Eric
> > +    .fields = (const VMStateField[]) {
> > +        VMSTATE_INT32(oas, SMMUv3State),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static int smmuv3_preload(void *opaque)
> > +{
> > +    SMMUv3State *s = opaque;
> > +
> > +    /*
> > +     * In case it wasn't migrated, use the value used
> > +     * by older QEMU.
> > +     */
> > +    s->oas = SMMU_IDR5_OAS_DEF;
> > +    return 0;
> > +}
> > +
> >  static const VMStateDescription vmstate_smmuv3 = {
> >      .name = "smmuv3",
> >      .version_id = 1,
> >      .minimum_version_id = 1,
> >      .priority = MIG_PRI_IOMMU,
> > +    .pre_load = smmuv3_preload,
> >      .fields = (const VMStateField[]) {
> >          VMSTATE_UINT32(features, SMMUv3State),
> >          VMSTATE_UINT8(sid_size, SMMUv3State),
> > @@ -1933,6 +1958,7 @@ static const VMStateDescription vmstate_smmuv3 = {
> >      },
> >      .subsections = (const VMStateDescription * const []) {
> >          &vmstate_gbpa,
> > +        &vmstate_oas,
> >          NULL
> >      }
> >  };
> > @@ -1945,6 +1971,7 @@ static Property smmuv3_properties[] = {
> >       * Defaults to stage 1
> >       */
> >      DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> > +    DEFINE_PROP_INT32("oas", SMMUv3State, oas, SMMU_IDR5_OAS_DEF),
> >      DEFINE_PROP_END_OF_LIST()
> >  };
> >  
> > diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> > index d183a62766..00a9eb4467 100644
> > --- a/include/hw/arm/smmuv3.h
> > +++ b/include/hw/arm/smmuv3.h
> > @@ -63,6 +63,7 @@ struct SMMUv3State {
> >      qemu_irq     irq[4];
> >      QemuMutex mutex;
> >      char *stage;
> > +    int32_t oas;
> >  };
> >  
> >  typedef enum {
>
diff mbox series

Patch

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 0ebf2eebcf..dd91807624 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -111,7 +111,8 @@  REG32(IDR5,                0x14)
      FIELD(IDR5, VAX,        10, 2);
      FIELD(IDR5, STALL_MAX,  16, 16);
 
-#define SMMU_IDR5_OAS 4
+#define SMMU_IDR5_OAS_DEF 4 /* 44 bits. */
+#define SMMU_IDR5_OAS_MAX 5 /* 48 bits. */
 
 REG32(IIDR,                0x18)
 REG32(AIDR,                0x1c)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 4ac818cf7a..39d03e7e24 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -299,7 +299,9 @@  static void smmuv3_init_regs(SMMUv3State *s)
     s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
     s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
 
-    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS); /* 44 bits */
+    /* PTW doesn't support 52 bits. */
+    s->oas = MIN(s->oas, SMMU_IDR5_OAS_MAX);
+    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, s->oas);
     /* 4K, 16K and 64K granule support */
     s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, 1);
     s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
@@ -1901,11 +1903,34 @@  static const VMStateDescription vmstate_gbpa = {
     }
 };
 
+static const VMStateDescription vmstate_oas = {
+    .name = "smmuv3/oas",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (const VMStateField[]) {
+        VMSTATE_INT32(oas, SMMUv3State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int smmuv3_preload(void *opaque)
+{
+    SMMUv3State *s = opaque;
+
+    /*
+     * In case it wasn't migrated, use the value used
+     * by older QEMU.
+     */
+    s->oas = SMMU_IDR5_OAS_DEF;
+    return 0;
+}
+
 static const VMStateDescription vmstate_smmuv3 = {
     .name = "smmuv3",
     .version_id = 1,
     .minimum_version_id = 1,
     .priority = MIG_PRI_IOMMU,
+    .pre_load = smmuv3_preload,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32(features, SMMUv3State),
         VMSTATE_UINT8(sid_size, SMMUv3State),
@@ -1933,6 +1958,7 @@  static const VMStateDescription vmstate_smmuv3 = {
     },
     .subsections = (const VMStateDescription * const []) {
         &vmstate_gbpa,
+        &vmstate_oas,
         NULL
     }
 };
@@ -1945,6 +1971,7 @@  static Property smmuv3_properties[] = {
      * Defaults to stage 1
      */
     DEFINE_PROP_STRING("stage", SMMUv3State, stage),
+    DEFINE_PROP_INT32("oas", SMMUv3State, oas, SMMU_IDR5_OAS_DEF),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index d183a62766..00a9eb4467 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -63,6 +63,7 @@  struct SMMUv3State {
     qemu_irq     irq[4];
     QemuMutex mutex;
     char *stage;
+    int32_t oas;
 };
 
 typedef enum {