Subject: Re: [PATCH] KVM: PPC: Book3S HV: H_ENTER filter out reserved HPTE[B] value

From: Fabiano Rosas <[email-masked]>
Date: Mon, 11 Oct 2021 11:47:45 -0300

Nicholas Piggin <[email-masked]> writes:

> The HPTE B field is a 2-bit field with values 0b10 and 0b11 reserved.
> This field is also taken from the HPTE and used when KVM executes
> TLBIEs to set the B field of those instructions.
>
> Disallow the guest setting B to a reserved value with H_ENTER by
> rejecting it. This is the same approach already taken for rejecting
> reserved (unsupported) LLP values. This prevents the guest from being
> able to induce the host to execute TLBIE with reserved values, which
> is not known to be a problem with current processors but in theory it
> could prevent the TLBIE from working correctly in a future processor.
>
> Signed-off-by: Nicholas Piggin <[email-masked]>

The ISA says:

B Segment Size Selector
0b00 - 256 MB (s=28)
0b01 - 1 TB (s=40)
0b10 - reserved
0b11 - reserved

So that looks good. I couldn't find any other guest initiated PTE
modifications, so I think we're covered.

Reviewed-by: Fabiano Rosas <[email-masked]>

> ---
>  arch/powerpc/include/asm/kvm_book3s_64.h | 4 ++++
>  arch/powerpc/kvm/book3s_hv_rm_mmu.c      | 9 +++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 19b6942c6969..fff391b9b97b 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -378,6 +378,10 @@ static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
>  		rb |= 1;		/* L field */
>  		rb |= r & 0xff000 & ((1ul << a_pgshift) - 1); /* LP field */
>  	}
> +	/*
> +	 * This sets both bits of the B field in the PTE. 0b1x values are
> +	 * reserved, but those will have been filtered by kvmppc_do_h_enter.
> +	 */
>  	rb |= (v >> HPTE_V_SSIZE_SHIFT) << 8;	/* B field */
>  	return rb;
>  }
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> index 632b2545072b..2c1f3c6e72d1 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> @@ -207,6 +207,15 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
>
>  	if (kvm_is_radix(kvm))
>  		return H_FUNCTION;
> +	/*
> +	 * The HPTE gets used by compute_tlbie_rb() to set TLBIE bits, so
> +	 * these functions should work together -- must ensure a guest can not
> +	 * cause problems with the TLBIE that KVM executes.
> +	 */
> +	if ((pteh >> HPTE_V_SSIZE_SHIFT) & 0x2) {
> +		/* B=0b1x is a reserved value, disallow it. */
> +		return H_PARAMETER;
> +	}
>  	psize = kvmppc_actual_pgsz(pteh, ptel);
>  	if (!psize)
>  		return H_PARAMETER;