Re: [RFC PATCH v2 10/11] hw/arm/smmuv3: Populate OAS based on CPU PARANGE

[Eric Auger]

Hi Mostafa,

On 3/21/23 14:06, Mostafa Saleh wrote:
> Hi Eric,
>
>>> +     * According to 6.3.6 SMMU_IDR5, OAS must match the system physical 
>>> address
>>> +     * size.
>>> +     */
>>> +    ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));
>>> +    uint8_t oas = FIELD_EX64(armcpu->isar.id_aa64mmfr0, ID_AA64MMFR0, 
>>> PARANGE);
>> is this working in accelerated mode?
> I didn't try with accel, I will give it a try, but from what I see, that
> ARM_CPU() is used to get the CPU in traget/arm/kvm.c which is used from
> accel/kvm-all.c, so it seems this would work for accelerated mode.

yeah I ma not familiar enough with that code but it is worth to be checked.
>
>>> +
>>>      /**
>>>       * IDR0: stage1 only, AArch64 only, coherent access, 16b ASID,
>>>       *       multi-level stream table
>>> @@ -265,7 +272,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
>>>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, 1);
>>>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
>>>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN64K, 1);
>>> -    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS); /* 44 
>>> bits */
>>> +    s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, oas);
>> I am not sure you can change that easily. In case of migration this is
>> going to change the behavior of the device, no?
> I see IDR registers are not migrated. I guess we can add them in a
> subsection and if they were not passed (old instances) we set OAS to
> 44.
> Maybe this should be another change outside of this series.
Indeed tehy are not migrated so it can lead to inconsistent behavior in
both source and dest. This deserves more analysis to me. In case you
would decide to migrate IDR regs this would need to be done in that
series I think. Migration must not be broken by this series.

Thanks

Eric
>
> Thanks,
> Mostafa
>