[PATCH 10/12] KVM: arm64: nv: Add SW walker for AT S1 emulation

[Alexandru Elisei]

Hi Marc,

On Wed, Jul 31, 2024 at 04:43:16PM +0100, Marc Zyngier wrote:
> On Wed, 31 Jul 2024 15:33:25 +0100,
> Alexandru Elisei <alexandru.elisei at arm.com> wrote:
> > 
> > Hi Marc,
> > 
> > On Mon, Jul 08, 2024 at 05:57:58PM +0100, Marc Zyngier wrote:
> > > In order to plug the brokenness of our current AT implementation,
> > > we need a SW walker that is going to... err.. walk the S1 tables
> > > and tell us what it finds.
> > > 
> > > Of course, it builds on top of our S2 walker, and share similar
> > > concepts. The beauty of it is that since it uses kvm_read_guest(),
> > > it is able to bring back pages that have been otherwise evicted.
> > > 
> > > This is then plugged in the two AT S1 emulation functions as
> > > a "slow path" fallback. I'm not sure it is that slow, but hey.
> > > 
> > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > ---
> > >  arch/arm64/kvm/at.c | 538 ++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 520 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> > > index 71e3390b43b4c..8452273cbff6d 100644
> > > --- a/arch/arm64/kvm/at.c
> > > +++ b/arch/arm64/kvm/at.c
> > > @@ -4,9 +4,305 @@
> > >   * Author: Jintack Lim <jintack.lim at linaro.org>
> > >   */
> > >  
> > > +#include <linux/kvm_host.h>
> > > +
> > > +#include <asm/esr.h>
> > >  #include <asm/kvm_hyp.h>
> > >  #include <asm/kvm_mmu.h>
> > >  
> > > +struct s1_walk_info {
> > > +	u64	     baddr;
> > > +	unsigned int max_oa_bits;
> > > +	unsigned int pgshift;
> > > +	unsigned int txsz;
> > > +	int 	     sl;
> > > +	bool	     hpd;
> > > +	bool	     be;
> > > +	bool	     nvhe;
> > > +	bool	     s2;
> > > +};
> > > +
> > > +struct s1_walk_result {
> > > +	union {
> > > +		struct {
> > > +			u64	desc;
> > > +			u64	pa;
> > > +			s8	level;
> > > +			u8	APTable;
> > > +			bool	UXNTable;
> > > +			bool	PXNTable;
> > > +		};
> > > +		struct {
> > > +			u8	fst;
> > > +			bool	ptw;
> > > +			bool	s2;
> > > +		};
> > > +	};
> > > +	bool	failed;
> > > +};
> > > +
> > > +static void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool ptw, bool s2)
> > > +{
> > > +	wr->fst		= fst;
> > > +	wr->ptw		= ptw;
> > > +	wr->s2		= s2;
> > > +	wr->failed	= true;
> > > +}
> > > +
> > > +#define S1_MMU_DISABLED		(-127)
> > > +
> > > +static int setup_s1_walk(struct kvm_vcpu *vcpu, struct s1_walk_info *wi,
> > > +			 struct s1_walk_result *wr, const u64 va, const int el)
> > > +{
> > > +	u64 sctlr, tcr, tg, ps, ia_bits, ttbr;
> > > +	unsigned int stride, x;
> > > +	bool va55, tbi;
> > > +
> > > +	wi->nvhe = el == 2 && !vcpu_el2_e2h_is_set(vcpu);
> > > +
> > > +	va55 = va & BIT(55);
> > > +
> > > +	if (wi->nvhe && va55)
> > > +		goto addrsz;
> > > +
> > > +	wi->s2 = el < 2 && (__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_VM);
> > > +
> > > +	switch (el) {
> > > +	case 1:
> > > +		sctlr	= vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> > > +		tcr	= vcpu_read_sys_reg(vcpu, TCR_EL1);
> > > +		ttbr	= (va55 ?
> > > +			   vcpu_read_sys_reg(vcpu, TTBR1_EL1) :
> > > +			   vcpu_read_sys_reg(vcpu, TTBR0_EL1));
> > > +		break;
> > > +	case 2:
> > > +		sctlr	= vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> > > +		tcr	= vcpu_read_sys_reg(vcpu, TCR_EL2);
> > > +		ttbr	= (va55 ?
> > > +			   vcpu_read_sys_reg(vcpu, TTBR1_EL2) :
> > > +			   vcpu_read_sys_reg(vcpu, TTBR0_EL2));
> > > +		break;
> > > +	default:
> > > +		BUG();
> > > +	}
> > > +
> > > +	/* Let's put the MMU disabled case aside immediately */
> > > +	if (!(sctlr & SCTLR_ELx_M) ||
> > > +	    (__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_DC)) {
> > > +		if (va >= BIT(kvm_get_pa_bits(vcpu->kvm)))
> > 
> > As far as I can tell, if TBI, the pseudocode ignores bits 63:56 when checking
> > for out-of-bounds VA for the MMU disabled case (above) and the MMU enabled case
> > (below). That also matches the description of TBIx bits in the TCR_ELx
> > registers.
> 
> Right. Then the check needs to be hoisted up and the VA sanitised
> before we compare it to anything.
> 
> Thanks for all your review comments, but I am going to ask you to stop
> here. You are reviewing a pretty old code base, and although I'm sure
> you look at what is in my tree, I'd really like to post a new version
> for everyone to enjoy.

Got it.

Thanks,
Alex