lkml.org 
[lkml]   [2020]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 05/23] arm64: Extract parts of el2_setup into a macro
    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.

    \
     
     \ /
      Last update: 2020-11-26 19:08    [W:3.254 / U:0.252 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site