Hi Gavin,
On Fri, Dec 13, 2024 at 10:03:08PM +1000, Gavin Shan wrote:
> Hi Jean,
>
> On 11/26/24 5:56 AM, Jean-Philippe Brucker wrote:
> > When RME is enabled, the upper GPA bit is used to distinguish protected
> > from unprotected addresses. Reserve it when setting up the guest memory
> > map.
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> > hw/arm/virt.c | 14 ++++++++++++--
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 9836dfbdfb..eb94997914 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -3035,14 +3035,24 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
> > VirtMachineState *vms = VIRT_MACHINE(ms);
> > int rme_vm_type = kvm_arm_rme_vm_type(ms);
> > int max_vm_pa_size, requested_pa_size;
> > + int rme_reserve_bit = 0;
> > bool fixed_ipa;
> > - max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa);
> > + if (rme_vm_type) {
> > + /*
> > + * With RME, the upper GPA bit differentiates Realm from NS memory.
> > + * Reserve the upper bit to ensure that highmem devices will fit.
> > + */
> > + rme_reserve_bit = 1;
> > + }
> > +
> > + max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa) -
> > + rme_reserve_bit;
>
> For realm, @max_vm_pa_size is decreased by 1 ...
>
> > /* we freeze the memory map to compute the highest gpa */
> > virt_set_memmap(vms, max_vm_pa_size);
> > - requested_pa_size = 64 - clz64(vms->highest_gpa);
> > + requested_pa_size = 64 - clz64(vms->highest_gpa) + rme_reserve_bit;
>
> ... For realm, @requested_pa_size is increased by 1, meaning there are two bits in
> the gap.
I think it's a 1-bit gap: max_vm_pa_size is decreased by 1 for the purpose
of memory map calculation, and here we increase by 1 what comes out of
that calculation, for the KVM IPA size setting
>
> > /*
> > * KVM requires the IPA size to be at least 32 bits.
>
> One bit instead of two bits seems the correct gap for the followup check?
Yes this check seems wrong for realm, since (requested_pa_size ==
max_vm_pa_size + 1) should be valid in this case. I'll fix this.
Thanks,
Jean
>
> if (requested_pa_size > max_vm_pa_size) {
> error_report("-m and ,maxmem option values "
> "require an IPA range (%d bits) larger than "
> "the one supported by the host (%d bits)",
> requested_pa_size, max_vm_pa_size);
> return -1;
> }
>
> Thanks,
> Gavin
>