Re: [PATCH v3 01/41] KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2
Excerpts from Fabiano Rosas's message of March 9, 2021 1:26 am:
> Nicholas Piggin writes:
>
>> These are already disallowed by H_SET_MODE from the guest, also disallow
>> these by updating LPCR directly.
>>
>> AIL modes can affect the host interrupt behaviour while the guest LPCR
>> value is set, so filter it here too.
>>
>> Suggested-by: Fabiano Rosas
>> Signed-off-by: Nicholas Piggin
>> ---
>> arch/powerpc/kvm/book3s_hv.c| 11 +--
>> arch/powerpc/kvm/book3s_hv_nested.c | 7 +--
>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 13bad6bf4c95..c40eeb20be39 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -803,7 +803,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu,
>> unsigned long mflags,
>> vcpu->arch.dawrx1 = value2;
>> return H_SUCCESS;
>> case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
>> -/* KVM does not support mflags=2 (AIL=2) */
>> +/*
>> + * KVM does not support mflags=2 (AIL=2) and AIL=1 is reserved.
>> + * Keep this in synch with kvmppc_set_lpcr.
>> + */
>> if (mflags != 0 && mflags != 3)
>> return H_UNSUPPORTED_FLAG_START;
>> return H_TOO_HARD;
>> @@ -1667,8 +1670,12 @@ static void kvmppc_set_lpcr(struct kvm_vcpu *vcpu,
>> u64 new_lpcr,
>> * On POWER8 and POWER9 userspace can also modify AIL (alt. interrupt
>> loc.).
>> */
>> mask = LPCR_DPFD | LPCR_ILE | LPCR_TC;
>> -if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> +if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>> mask |= LPCR_AIL;
>> +/* LPCR[AIL]=1/2 is disallowed */
>> +if ((new_lpcr & LPCR_AIL) && (new_lpcr & LPCR_AIL) !=
>> LPCR_AIL_3)
>> +new_lpcr &= ~LPCR_AIL;
>> +}
>> /*
>> * On POWER9, allow userspace to enable large decrementer for the
>> * guest, whether or not the host has it enabled.
>> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c
>> b/arch/powerpc/kvm/book3s_hv_nested.c
>> index 2fe1fea4c934..b496079e02f7 100644
>> --- a/arch/powerpc/kvm/book3s_hv_nested.c
>> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
>> @@ -139,9 +139,12 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu,
>> struct hv_guest_state *hr)
>
> We're missing the patch that moves the lpcr setting into
> sanitise_hv_regs.
Oh yes sorry, mistyped the format-patch command.
>>
>> /*
>> * Don't let L1 change LPCR bits for the L2 except these:
>> + * Keep this in sync with kvmppc_set_lpcr.
>> */
>> -mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
>> -LPCR_LPES | LPCR_MER;
>> +mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_LD | LPCR_LPES | LPCR_MER;
>
> I think this line's change belongs in patch 33 doesn't it? Otherwise you
> are clearing a bit below that is not present in the mask so it would
> never be used anyway.
Ah yes, thank you. Will fix.
>
>> +/* LPCR[AIL]=1/2 is disallowed */
>> +if ((hr->lpcr & LPCR_AIL) && (hr->lpcr & LPCR_AIL) != LPCR_AIL_3)
>> +hr->lpcr &= ~LPCR_AIL;
>> hr->lpcr = (vc->lpcr & ~mask) | (hr->lpcr & mask);
>>
>> /*
>
Re: [PATCH v3 01/41] KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2
Nicholas Piggin writes:
> These are already disallowed by H_SET_MODE from the guest, also disallow
> these by updating LPCR directly.
>
> AIL modes can affect the host interrupt behaviour while the guest LPCR
> value is set, so filter it here too.
>
> Suggested-by: Fabiano Rosas
> Signed-off-by: Nicholas Piggin
> ---
> arch/powerpc/kvm/book3s_hv.c| 11 +--
> arch/powerpc/kvm/book3s_hv_nested.c | 7 +--
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 13bad6bf4c95..c40eeb20be39 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -803,7 +803,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu,
> unsigned long mflags,
> vcpu->arch.dawrx1 = value2;
> return H_SUCCESS;
> case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
> - /* KVM does not support mflags=2 (AIL=2) */
> + /*
> + * KVM does not support mflags=2 (AIL=2) and AIL=1 is reserved.
> + * Keep this in synch with kvmppc_set_lpcr.
> + */
> if (mflags != 0 && mflags != 3)
> return H_UNSUPPORTED_FLAG_START;
> return H_TOO_HARD;
> @@ -1667,8 +1670,12 @@ static void kvmppc_set_lpcr(struct kvm_vcpu *vcpu, u64
> new_lpcr,
>* On POWER8 and POWER9 userspace can also modify AIL (alt. interrupt
> loc.).
>*/
> mask = LPCR_DPFD | LPCR_ILE | LPCR_TC;
> - if (cpu_has_feature(CPU_FTR_ARCH_207S))
> + if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
> mask |= LPCR_AIL;
> + /* LPCR[AIL]=1/2 is disallowed */
> + if ((new_lpcr & LPCR_AIL) && (new_lpcr & LPCR_AIL) !=
> LPCR_AIL_3)
> + new_lpcr &= ~LPCR_AIL;
> + }
> /*
>* On POWER9, allow userspace to enable large decrementer for the
>* guest, whether or not the host has it enabled.
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c
> b/arch/powerpc/kvm/book3s_hv_nested.c
> index 2fe1fea4c934..b496079e02f7 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -139,9 +139,12 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu,
> struct hv_guest_state *hr)
We're missing the patch that moves the lpcr setting into
sanitise_hv_regs.
>
> /*
>* Don't let L1 change LPCR bits for the L2 except these:
> + * Keep this in sync with kvmppc_set_lpcr.
>*/
> - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
> - LPCR_LPES | LPCR_MER;
> + mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_LD | LPCR_LPES | LPCR_MER;
I think this line's change belongs in patch 33 doesn't it? Otherwise you
are clearing a bit below that is not present in the mask so it would
never be used anyway.
> + /* LPCR[AIL]=1/2 is disallowed */
> + if ((hr->lpcr & LPCR_AIL) && (hr->lpcr & LPCR_AIL) != LPCR_AIL_3)
> + hr->lpcr &= ~LPCR_AIL;
> hr->lpcr = (vc->lpcr & ~mask) | (hr->lpcr & mask);
>
> /*
