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 <smost...@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 {
>
