Re: [PATCH v2 2/6] KVM: selftests: arm64: Introduce a variable default IPA size

[Marc Zyngier <maz@kernel.org>]

On Tue, 28 Dec 2021 09:26:22 +0000,
Andrew Jones <drjones@redhat.com> wrote:
> 
> On Mon, Dec 27, 2021 at 12:48:05PM +0000, Marc Zyngier wrote:
> > Contrary to popular belief, there is no such thing as a default
> > IPA size on arm64. Anything goes, and implementations are the
> > usual Wild West.
> > 
> > The selftest infrastructure default to 40bit IPA, which obviously
> > doesn't work for some systems out there.
> > 
> > Turn VM_MODE_DEFAULT from a constant into a variable, and let
> > guest_modes_append_default() populate it, depending on what
> > the HW can do. In order to preserve the current behaviour, we
> > still pick 40bits IPA as the default if it is available, and
> > the largest supported IPA space otherwise.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  .../testing/selftests/kvm/include/kvm_util.h  |  4 ++-
> >  tools/testing/selftests/kvm/lib/guest_modes.c | 30 +++++++++++++++++--
> >  2 files changed, 30 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index 2d62edc49d67..7fa0a93d7526 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -53,7 +53,9 @@ enum vm_guest_mode {
> >  
> >  #if defined(__aarch64__)
> >  
> > -#define VM_MODE_DEFAULT			VM_MODE_P40V48_4K
> > +extern enum vm_guest_mode vm_mode_default;
> > +
> > +#define VM_MODE_DEFAULT			vm_mode_default
> >  #define MIN_PAGE_SHIFT			12U
> >  #define ptes_per_page(page_size)	((page_size) / 8)
> >  
> > diff --git a/tools/testing/selftests/kvm/lib/guest_modes.c b/tools/testing/selftests/kvm/lib/guest_modes.c
> > index c330f414ef96..5e3fdbd992fd 100644
> > --- a/tools/testing/selftests/kvm/lib/guest_modes.c
> > +++ b/tools/testing/selftests/kvm/lib/guest_modes.c
> > @@ -4,22 +4,46 @@
> >   */
> >  #include "guest_modes.h"
> >  
> > +#ifdef __aarch64__
> > +enum vm_guest_mode vm_mode_default;
> > +#endif
> > +
> >  struct guest_mode guest_modes[NUM_VM_MODES];
> >  
> >  void guest_modes_append_default(void)
> >  {
> > +#ifndef __aarch64__
> >  	guest_mode_append(VM_MODE_DEFAULT, true, true);
> > -
> > -#ifdef __aarch64__
> > -	guest_mode_append(VM_MODE_P40V48_64K, true, true);
> > +#else
> >  	{
> >  		unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
> > +		int i;
> > +
> > +		vm_mode_default = NUM_VM_MODES;
> > +
> >  		if (limit >= 52)
> >  			guest_mode_append(VM_MODE_P52V48_64K, true, true);
> >  		if (limit >= 48) {
> >  			guest_mode_append(VM_MODE_P48V48_4K, true, true);
> >  			guest_mode_append(VM_MODE_P48V48_64K, true, true);
> >  		}
> > +		if (limit >= 40) {
> > +			guest_mode_append(VM_MODE_P40V48_4K, true, true);
> > +			guest_mode_append(VM_MODE_P40V48_64K, true, true);
> > +			vm_mode_default = VM_MODE_P40V48_4K;
> > +		}
> > +
> > +		/*
> > +		 * Pick the first supported IPA size if the default
> > +		 * isn't available.
> > +		 */
> > +		for (i = 0; vm_mode_default == NUM_VM_MODES && i < NUM_VM_MODES; i++) {
> > +			if (guest_modes[i].supported && guest_modes[i].enabled)
> > +				vm_mode_default = i;
> 
> Since we don't have a 'break' here, this picks the last supported size
> (of the guest_modes list), not the first, as the comment implies it should
> do.

This is checked in the for() loop condition, and the first matching
mode will cause the loop to terminate. This is the same check that
avoids scanning for a mode when VM_MODE_P40V48_4K is selected.

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm