Re: [PATCH v7 12/27] KVM: arm64/sve: System register context switch and access support

Andrew Jones
2019-04-04T08:59:23Z

On Fri, Mar 29, 2019 at 01:00:37PM +0000, Dave Martin wrote:
> This patch adds the necessary support for context switching ZCR_EL1
> for each vcpu.
>
> ZCR_EL1 is trapped alongside the FPSIMD/SVE registers, so it makes
> sense for it to be handled as part of the guest FPSIMD/SVE context
> for context switch purposes instead of handling it as a general
> system register. This means that it can be switched in lazily at
> the appropriate time. No effort is made to track host context for
> this register, since SVE requires VHE: thus the hosts's value for
> this register lives permanently in ZCR_EL2 and does not alias the
> guest's value at any time.
>
> The Hyp switch and fpsimd context handling code is extended
> appropriately.
>
> Accessors are added in sys_regs.c to expose the SVE system
> registers and ID register fields. Because these need to be
> conditionally visible based on the guest configuration, they are
> implemented separately for now rather than by use of the generic
> system register helpers. This may be abstracted better later on
> when/if there are more features requiring this model.
>
> ID_AA64ZFR0_EL1 is RO-RAZ for MRS/MSR when SVE is disabled for the
> guest, but for compatibility with non-SVE aware KVM implementations
> the register should not be enumerated at all for KVM_GET_REG_LIST
> in this case. For consistency we also reject ioctl access to the
> register. This ensures that a non-SVE-enabled guest looks the same
> to userspace, irrespective of whether the kernel KVM implementation
> supports SVE.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Reviewed-by: Julien Thierry <julien.thierry@arm.com>
> Tested-by: zhang.lei <zhang.lei@jp.fujitsu.com>
>
> ---
>
> Changes since v5:
>
> * Port to the renamed visibility() framework.
>
> * Swap visiblity() helpers so that they appear by the relevant accessor
> functions.
>
> * [Julien Grall] With the visibility() checks, {get,set}_zcr_el1()
> degenerate to doing exactly what the common code does, so drop them.
>
> The ID_AA64ZFR0_EL1 handlers are still needed to provide contitional
> RAZ behaviour. This could be moved to the common code too, but since
> this is a one-off case I don't do this for now. We can address this
> later if other regs need to follow the same pattern.
>
> * [Julien Thierry] Reset ZCR_EL1 to a fixed value using reset_val
> instead of using relying on reset_unknown() honouring set bits in val
> as RES0.
>
> Most of the bits in ZCR_EL1 are RES0 anyway, and many implementations
> of SVE will support larger vectors than 128 bits, so 0 seems as good
> a value as any to expose guests that forget to initialise this
> register properly.
> ---
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/include/asm/sysreg.h | 3 ++
> arch/arm64/kvm/fpsimd.c | 9 ++++-
> arch/arm64/kvm/hyp/switch.c | 3 ++
> arch/arm64/kvm/sys_regs.c | 83 ++++++++++++++++++++++++++++++++++++---
> 5 files changed, 93 insertions(+), 6 deletions(-)
>

Reviewed-by: Andrew Jones <drjones@redhat.com>