Re: [PATCH 07/59] KVM: arm64: nv: Add EL2 system registers to vcpu context

Marc Zyngier
2019-07-03T12:21:04Z

On 24/06/2019 13:54, Dave Martin wrote:
> On Fri, Jun 21, 2019 at 10:37:51AM +0100, Marc Zyngier wrote:
>> From: Jintack Lim <jintack.lim@linaro.org>
>>
>> ARM v8.3 introduces a new bit in the HCR_EL2, which is the NV bit. When
>> this bit is set, accessing EL2 registers in EL1 traps to EL2. In
>> addition, executing the following instructions in EL1 will trap to EL2:
>> tlbi, at, eret, and msr/mrs instructions to access SP_EL1. Most of the
>> instructions that trap to EL2 with the NV bit were undef at EL1 prior to
>> ARM v8.3. The only instruction that was not undef is eret.
>>
>> This patch sets up a handler for EL2 registers and SP_EL1 register
>> accesses at EL1. The host hypervisor keeps those register values in
>> memory, and will emulate their behavior.
>>
>> This patch doesn't set the NV bit yet. It will be set in a later patch
>> once nested virtualization support is completed.
>>
>> Signed-off-by: Jintack Lim <jintack.lim@linaro.org>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm64/include/asm/kvm_host.h | 37 +++++++++++++++-
>> arch/arm64/include/asm/sysreg.h | 50 ++++++++++++++++++++-
>> arch/arm64/kvm/sys_regs.c | 74 ++++++++++++++++++++++++++++---
>> 3 files changed, 154 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 4bcd9c1291d5..2d4290d2513a 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -173,12 +173,47 @@ enum vcpu_sysreg {
>> APGAKEYLO_EL1,
>> APGAKEYHI_EL1,
>>
>> - /* 32bit specific registers. Keep them at the end of the range */
>> + /* 32bit specific registers. */
>
> Out of interest, why did we originally want these to be at the end?
> Because they're not at the end any more...

I seem to remember the original assembly switch code used that property.
This is a long gone requirement, thankfully.

>
>> DACR32_EL2, /* Domain Access Control Register */
>> IFSR32_EL2, /* Instruction Fault Status Register */
>> FPEXC32_EL2, /* Floating-Point Exception Control Register */
>> DBGVCR32_EL2, /* Debug Vector Catch Register */
>>
>> + /* EL2 registers sorted ascending by Op0, Op1, CRn, CRm, Op2 */
>> + FIRST_EL2_SYSREG,
>> + VPIDR_EL2 = FIRST_EL2_SYSREG,
>> + /* Virtualization Processor ID Register */
>> + VMPIDR_EL2, /* Virtualization Multiprocessor ID Register */
>> + SCTLR_EL2, /* System Control Register (EL2) */
>> + ACTLR_EL2, /* Auxiliary Control Register (EL2) */
>> + HCR_EL2, /* Hypervisor Configuration Register */
>> + MDCR_EL2, /* Monitor Debug Configuration Register (EL2) */
>> + CPTR_EL2, /* Architectural Feature Trap Register (EL2) */
>> + HSTR_EL2, /* Hypervisor System Trap Register */
>> + HACR_EL2, /* Hypervisor Auxiliary Control Register */
>> + TTBR0_EL2, /* Translation Table Base Register 0 (EL2) */
>> + TTBR1_EL2, /* Translation Table Base Register 1 (EL2) */
>> + TCR_EL2, /* Translation Control Register (EL2) */
>> + VTTBR_EL2, /* Virtualization Translation Table Base Register */
>> + VTCR_EL2, /* Virtualization Translation Control Register */
>> + SPSR_EL2, /* EL2 saved program status register */
>> + ELR_EL2, /* EL2 exception link register */
>> + AFSR0_EL2, /* Auxiliary Fault Status Register 0 (EL2) */
>> + AFSR1_EL2, /* Auxiliary Fault Status Register 1 (EL2) */
>> + ESR_EL2, /* Exception Syndrome Register (EL2) */
>> + FAR_EL2, /* Hypervisor IPA Fault Address Register */
>> + HPFAR_EL2, /* Hypervisor IPA Fault Address Register */
>> + MAIR_EL2, /* Memory Attribute Indirection Register (EL2) */
>> + AMAIR_EL2, /* Auxiliary Memory Attribute Indirection Register (EL2) */
>> + VBAR_EL2, /* Vector Base Address Register (EL2) */
>> + RVBAR_EL2, /* Reset Vector Base Address Register */
>> + RMR_EL2, /* Reset Management Register */
>> + CONTEXTIDR_EL2, /* Context ID Register (EL2) */
>> + TPIDR_EL2, /* EL2 Software Thread ID Register */
>> + CNTVOFF_EL2, /* Counter-timer Virtual Offset register */
>> + CNTHCTL_EL2, /* Counter-timer Hypervisor Control register */
>> + SP_EL2, /* EL2 Stack Pointer */
>> +
>
> I wonder whether we could make these conditionally present somehow. Not
> worth worrying about for now to save 200-odd bytes per vcpu though.

With 8.4-NV, these 200 bytes turn into a whole 8kB (4kB page, plus
almost 4kB of padding that I need to reduce one way or another). So I'm
not too worried about this for now.

I really want the NV code to always be present though, in order to avoid
configuration related regressions. I'm not sure how to make this better.

Thanks,

M.
--
Jazz is not dead. It just smells funny...