Re: [PATCH v2 30/37] KVM: PPC: Book3S HV: Implement radix prefetch workaround by disabling MMU

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Fabiano Rosas's message of March 3, 2021 7:21 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> Rather than partition the guest PID space and catch and flush a rogue
>> guest, instead work around this issue by ensuring the MMU is always
>> disabled in HV mode while the guest MMU context is switched in.
>>
>> This may be a bit less efficient, but it is a lot less complicated and
>> allows the P9 path to trivally implement the workaround too. Newer CPUs
>> are not subject to this issue.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/include/asm/mmu_context.h   |  6 ----
>>  arch/powerpc/kvm/book3s_hv.c             | 10 ++++--
>>  arch/powerpc/kvm/book3s_hv_interrupt.c   | 14 ++++++--
>>  arch/powerpc/kvm/book3s_hv_rmhandlers.S  | 34 ------------------
>>  arch/powerpc/mm/book3s64/radix_pgtable.c | 27 +++++---------
>>  arch/powerpc/mm/book3s64/radix_tlb.c     | 46 ------------------------
>>  arch/powerpc/mm/mmu_context.c            |  4 +--
>>  7 files changed, 28 insertions(+), 113 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
>> index 652ce85f9410..bb5c7e5e142e 100644
>> --- a/arch/powerpc/include/asm/mmu_context.h
>> +++ b/arch/powerpc/include/asm/mmu_context.h
>> @@ -122,12 +122,6 @@ static inline bool need_extra_context(struct mm_struct *mm, unsigned long ea)
>>  }
>>  #endif
>>
>> -#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_RADIX_MMU)
>> -extern void radix_kvm_prefetch_workaround(struct mm_struct *mm);
>> -#else
>> -static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) { }
>> -#endif
>> -
>>  extern void switch_cop(struct mm_struct *next);
>>  extern int use_cop(unsigned long acop, struct mm_struct *mm);
>>  extern void drop_cop(unsigned long acop, struct mm_struct *mm);
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index ad16331c3370..c3064075f1d7 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -806,6 +806,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
>>  		/* KVM does not support mflags=2 (AIL=2) */
>>  		if (mflags != 0 && mflags != 3)
>>  			return H_UNSUPPORTED_FLAG_START;
>> +		/* Prefetch bug */
>> +		if (cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG) &&
>> +				kvmhv_vcpu_is_radix(vcpu) && mflags == 3)
>> +			return H_UNSUPPORTED_FLAG_START;
> 
> So does this mean that if the host has the prefetch bug, all of its
> guests will run with AIL=0 all the time?

All radix guests will, yes.

> And what we're avoiding here is
> a guest setting AIL=3 which would (since there's no HAIL) cause
> hypervisor interrupts to be taken with MMU on, is that it?

Yes that's right.

> Do we need to add this verification to kvmppc_set_lpcr as well? QEMU
> could in theory call the KVM_SET_ONE_REG ioctl and set AIL to any value.

Yeah I guess so. We don't restrict other AIL values there by the looks
but maybe we should.

Thanks,
Nick