linux-kernel - Re: [PATCH v1 13/24] kvm: arm64: Add CPU entry point in nVHE hyp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201116114952.ybj24wx44lofqs25@google.com>
Date:   Mon, 16 Nov 2020 11:49:52 +0000
From:   David Brazdil <dbrazdil@...gle.com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Dennis Zhou <dennis@...nel.org>,
        Tejun Heo <tj@...nel.org>, Christoph Lameter <cl@...ux.com>,
        Mark Rutland <mark.rutland@....com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Quentin Perret <qperret@...gle.com>,
        Andrew Scull <ascull@...gle.com>,
        Andrew Walbran <qwandor@...gle.com>, kernel-team@...roid.com
Subject: Re: [PATCH v1 13/24] kvm: arm64: Add CPU entry point in nVHE hyp

> >  #ifdef CONFIG_CPU_PM
> >    DEFINE(CPU_CTX_SP,		offsetof(struct cpu_suspend_ctx, sp));
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > index 1697d25756e9..f999a35b2c8c 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> > @@ -6,6 +6,7 @@
> > 
> >  #include <linux/arm-smccc.h>
> >  #include <linux/linkage.h>
> > +#include <linux/irqchip/arm-gic-v3.h>
> 
> This should probably be included from the file that provides init_el2_state.

Agreed. This is a workaround for the fact that the arm-gic* headers don't play
nice with each other (define the same constants). Including arm-gic-v3.h in
kvm_asm.h will trigger macro redefine warnings in vgic*-v2.c because they
include arm-gic.h.

Another option is to create a header just for el2 init. Would that be
preferable? Other ideas?

> 
> > 
> >  #include <asm/alternative.h>
> >  #include <asm/assembler.h>
> > @@ -159,6 +160,35 @@ alternative_else_nop_endif
> >  	ret
> >  SYM_CODE_END(___kvm_hyp_init)
> > 
> > +SYM_CODE_START(__kvm_hyp_cpu_entry)
> > +	msr	SPsel, #1			// We want to use SP_EL{1,2}
> > +
> > +	/*
> > +	 * Check that the core was booted in EL2. Loop indefinitely if not
> > +	 * because it cannot be safely given to the host without installing
> > KVM.
> > +	 */
> > +	mrs	x1, CurrentEL
> > +	cmp	x1, #CurrentEL_EL2
> > +	b.ne	.
> 
> This is a bit brutal. Consider using a WFE/WFI loop as we have in other
> places already (see __secondary_too_slow for example).

Ack

> > +
> > +	/* Initialize EL2 CPU state to sane values. */
> > +	mov	x29, x0
> > +	init_el2_state nvhe
> > +	mov	x0, x29
> > +
> > +	/*
> > +	 * Load hyp VA of C entry function. Must do so before switching on the
> > +	 * MMU because the struct pointer is PA and not identity-mapped in
> > hyp.
> > +	 */
> > +	ldr	x29, [x0, #NVHE_INIT_PSCI_CPU_ENTRY_FN]
> > +
> > +	/* Enable MMU, set vectors and stack. */
> > +	bl	___kvm_hyp_init
> > +
> > +	/* Leave idmap. */
> > +	br	x29
> 
> To a point I made against an earlier patch: psci_cpu_entry_fn seems to be a
> HYP
> VA, and really needs to be documented as such, because this is pretty hard
> to
> follow otherwise.
> 
> > +SYM_CODE_END(__kvm_hyp_cpu_entry)
> > +
> >  SYM_CODE_START(__kvm_handle_stub_hvc)
> >  	cmp	x0, #HVC_SOFT_RESTART
> >  	b.ne	1f
> 
> Thanks,
> 
>         M.
> -- 
> Jazz is not dead. It just smells funny...