On Tue, 09 Mar 2021 20:06:06 +0000,
Andrew Jones <drjo...@redhat.com> wrote:
>
> On Tue, Mar 09, 2021 at 07:21:58PM +0000, Marc Zyngier wrote:
> > Hi Andrew,
> >
> > On Tue, 09 Mar 2021 18:59:39 +0000,
> > Andrew Jones <drjo...@redhat.com> wrote:
> > >
> > > The virt machine already checks KVM_CAP_ARM_VM_IPA_SIZE to get the
> > > upper bound of the IPA size. If that bound is lower than the highest
> > > possible GPA for the machine, then QEMU will error out. However, the
> > > IPA is set to 40 when the highest GPA is less than or equal to 40,
> > > even when KVM may only support an IPA limit as low as 32. This means
> > > KVM may fail the VM creation unnecessarily. Additionally, 40 is
> > > selected with the value 0, which means use the default, and that gets
> > > around a check in some versions of KVM, causing a difficult to debug
> > > fail. Always use the IPA size that corresponds to the highest possible
> > > GPA, unless it's lower than 32, in which case use 32.
> > >
> > > Signed-off-by: Andrew Jones <drjo...@redhat.com>
> > > ---
> > > hw/arm/virt.c | 9 ++++-----
> > > 1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 371147f3ae9c..7bf563715b4e 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -2547,14 +2547,13 @@ static int virt_kvm_type(MachineState *ms, const
> > > char *type_str)
> > > "require an IPA range (%d bits) larger than "
> > > "the one supported by the host (%d bits)",
> > > requested_pa_size, max_vm_pa_size);
> > > - exit(1);
> > > + exit(1);
> > > }
> > > +
> > > /*
> > > - * By default we return 0 which corresponds to an implicit legacy
> > > - * 40b IPA setting. Otherwise we return the actual requested PA
> > > - * logsize
> > > + * KVM requires the IPA size to be at least 32 bits.
> > > */
> > > - return requested_pa_size > 40 ? requested_pa_size : 0;
> > > + return requested_pa_size < 32 ? 32 : requested_pa_size;
> >
> > Doesn't this break on older kernels (pre 233a7cb23531, which is
> > anything up 4.19), where the 'type' parameter passed at VM creation
> > time was expected to be 0 and nothing else?
>
> Doh, of course!
>
> >
> > I had a quick go at qemu a couple of weeks back and came up with the
> > following hack, but never actually tested it (there is no way qemu can
> > fit in the initramfs I am feeding to this damn machine...).
> >
> > Thanks,
> >
> > M.
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 371147f3ae..3301bb8dba 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -2534,8 +2534,11 @@ static HotplugHandler
> > *virt_machine_get_hotplug_handler(MachineState *machine,
> > static int virt_kvm_type(MachineState *ms, const char *type_str)
> > {
> > VirtMachineState *vms = VIRT_MACHINE(ms);
> > - int max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms);
> > + int max_vm_pa_size;
> > int requested_pa_size;
> > + bool fixed_ipa;
> > +
> > + max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa);
> >
> > /* we freeze the memory map to compute the highest gpa */
> > virt_set_memmap(vms);
> > @@ -2550,11 +2553,12 @@ static int virt_kvm_type(MachineState *ms, const
> > char *type_str)
> > exit(1);
> > }
> > /*
> > - * By default we return 0 which corresponds to an implicit legacy
> > - * 40b IPA setting. Otherwise we return the actual requested PA
> > - * logsize
> > + * We return the requested PA log size, unless KVM only supports
> > + * the implicit legacy 40b IPA setting, in which case we return 0.
> > */
> > - return requested_pa_size > 40 ? requested_pa_size : 0;
> > + if (fixed_ipa)
> > + return 0;
> > + return requested_pa_size;
>
> Looks good to me. I think we should still ensure requested_pa_size is at
> least 32 though.
Yes, absolutely.
> I can add that and test this tomorrow. Should I post it with your
> authorship?
No, please keep it with yours (add a Suggested-by: tag if you really
want to). If you can give it a good shake, that'd be great. I'll try
and repost the kernel fixes tomorrow.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
