On Thu, Nov 26, 2020 at 03:54:03PM +0000, David Brazdil wrote: > When the a CPU is booted in EL2, the kernel checks for VHE support and > initializes the CPU core accordingly. For nVHE it also installs the stub > vectors and drops down to EL1. > > Once KVM gains the ability to boot cores without going through the > kernel entry point, it will need to initialize the CPU the same way. > Extract the relevant bits of el2_setup into an init_el2_state macro > with an argument specifying whether to initialize for VHE or nVHE. > > No functional change. Size of el2_setup increased by 148 bytes due > to duplication. As a heads-up, this will conflict with my rework which is queued in the arm64 for-next/uaccess branch. I reworked an renamed el2_setup to initialize SCTLR_ELx and PSTATE more consistently as a prerequisite for the set_fs() removal. I'm afraid this is going to conflict, and I reckon this needs to be rebased atop that. I think the actual conflicts are logically trivial, but the diff is going to be painful. I'm certainly in favour of breaking this down into manageable chunks, especially as that makes the branch naming easier to follow, but I have a couple of concerns below. > +/* GICv3 system register access */ > +.macro __init_el2_gicv3 > + mrs x0, id_aa64pfr0_el1 > + ubfx x0, x0, #ID_AA64PFR0_GIC_SHIFT, #4 > + cbz x0, 1f > + > + mrs_s x0, SYS_ICC_SRE_EL2 > + orr x0, x0, #ICC_SRE_EL2_SRE // Set ICC_SRE_EL2.SRE==1 > + orr x0, x0, #ICC_SRE_EL2_ENABLE // Set ICC_SRE_EL2.Enable==1 > + msr_s SYS_ICC_SRE_EL2, x0 > + isb // Make sure SRE is now set > + mrs_s x0, SYS_ICC_SRE_EL2 // Read SRE back, > + tbz x0, #0, 1f // and check that it sticks > + msr_s SYS_ICH_HCR_EL2, xzr // Reset ICC_HCR_EL2 to defaults > +1: > +.endm In the head.S code, this was under an ifdef CONFIG_ARM_GIC_V3, but that ifdef wasn't carried into the macro here, or into its use below. I'm not sure of the impact, but that does seem to be a functional change. > + > +.macro __init_el2_hstr > + msr hstr_el2, xzr // Disable CP15 traps to EL2 > +.endm Likewise, this used to be be guarded by CONFIG_COMPAT, but that's not carried into the macro or its use. If the intent was to remove the conditionality, then that should be mentioned in the commit message, since it is a potential functional change. Thanks, Mark.
