From: Marc Zyngier <marc.zyngier@arm.com>
To: Dave Martin <Dave.Martin@arm.com>, kvmarm@lists.cs.columbia.edu
Cc: Okamoto Takayuki <tokamoto@jp.fujitsu.com>,
Christoffer Dall <cdall@kernel.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 13/25] KVM: arm64/sve: System register context switch and access support
Date: Fri, 18 Jan 2019 16:42:07 +0000 [thread overview]
Message-ID: <3d6da606-d7ae-5232-8317-4c550370b469@arm.com> (raw)
In-Reply-To: <1547757219-19439-14-git-send-email-Dave.Martin@arm.com>
On 17/01/2019 20:33, 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>
> ---
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/include/asm/sysreg.h | 3 ++
> arch/arm64/kvm/fpsimd.c | 8 ++-
> arch/arm64/kvm/hyp/switch.c | 3 ++
> arch/arm64/kvm/sys_regs.c | 111 ++++++++++++++++++++++++++++++++++++--
> 5 files changed, 121 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index af625a8..c32f195 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -112,6 +112,7 @@ enum vcpu_sysreg {
> SCTLR_EL1, /* System Control Register */
> ACTLR_EL1, /* Auxiliary Control Register */
> CPACR_EL1, /* Coprocessor Access Control */
> + ZCR_EL1, /* SVE Control */
> TTBR0_EL1, /* Translation Table Base Register 0 */
> TTBR1_EL1, /* Translation Table Base Register 1 */
> TCR_EL1, /* Translation Control Register */
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 72dc4c0..da38491 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -449,6 +449,9 @@
> #define SYS_ICH_LR14_EL2 __SYS__LR8_EL2(6)
> #define SYS_ICH_LR15_EL2 __SYS__LR8_EL2(7)
>
> +/* VHE encodings for architectural EL0/1 system registers */
> +#define SYS_ZCR_EL12 sys_reg(3, 5, 1, 2, 0)
> +
> /* Common SCTLR_ELx flags. */
> #define SCTLR_ELx_DSSBS (_BITUL(44))
> #define SCTLR_ELx_ENIA (_BITUL(31))
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index 1cf4f02..5ff2d90 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -103,6 +103,8 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
> void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
> {
> unsigned long flags;
> + bool host_has_sve = system_supports_sve();
> + bool guest_has_sve = vcpu_has_sve(vcpu);
>
> local_irq_save(flags);
>
> @@ -110,7 +112,11 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
> /* Clean guest FP state to memory and invalidate cpu view */
> fpsimd_save();
> fpsimd_flush_cpu_state();
> - } else if (system_supports_sve()) {
> +
> + if (guest_has_sve)
> + vcpu->arch.ctxt.sys_regs[ZCR_EL1] =
> + read_sysreg_s(SYS_ZCR_EL12);
nit: Please keep assignments on a single line.
> + } else if (host_has_sve) {
> /*
> * The FPSIMD/SVE state in the CPU has not been touched, and we
> * have SVE (and VHE): CPACR_EL1 (alias CPTR_EL2) has been
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index b0b1478..9f07403 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -347,6 +347,9 @@ static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
>
> __fpsimd_restore_state(&vcpu->arch.ctxt.gp_regs.fp_regs);
>
> + if (vcpu_has_sve(vcpu))
> + write_sysreg_s(vcpu->arch.ctxt.sys_regs[ZCR_EL1], SYS_ZCR_EL12);
> +
> /* Skip restoring fpexc32 for AArch64 guests */
> if (!(read_sysreg(hcr_el2) & HCR_RW))
> write_sysreg(vcpu->arch.ctxt.sys_regs[FPEXC32_EL2],
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index bca32d5..c8df730 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1036,10 +1036,7 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> (u32)r->CRn, (u32)r->CRm, (u32)r->Op2);
> u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
>
> - if (id == SYS_ID_AA64PFR0_EL1) {
> - if (val & (0xfUL << ID_AA64PFR0_SVE_SHIFT))
> - kvm_debug("SVE unsupported for guests, suppressing\n");
> -
> + if (id == SYS_ID_AA64PFR0_EL1 && !vcpu_has_sve(vcpu)) {
> val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
> } else if (id == SYS_ID_AA64ISAR1_EL1) {
> const u64 ptrauth_mask = (0xfUL << ID_AA64ISAR1_APA_SHIFT) |
> @@ -1091,6 +1088,105 @@ static int reg_from_user(u64 *val, const void __user *uaddr, u64 id);
> static int reg_to_user(void __user *uaddr, const u64 *val, u64 id);
> static u64 sys_reg_to_index(const struct sys_reg_desc *reg);
>
> +#ifdef CONFIG_ARM64_SVE
> +static bool sve_check_present(const struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + return vcpu_has_sve(vcpu);
> +}
> +
> +static bool access_zcr_el1(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + /*
> + * ZCR_EL1 access is handled directly in Hyp as part of the FPSIMD/SVE
> + * context, so we should only arrive here for non-SVE guests:
> + */
> + WARN_ON(vcpu_has_sve(vcpu));
> +
> + kvm_inject_undefined(vcpu);
> + return false;
> +}
> +
> +static int get_zcr_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + if (!vcpu_has_sve(vcpu))
> + return -ENOENT;
> +
> + return reg_to_user(uaddr, &vcpu->arch.ctxt.sys_regs[ZCR_EL1],
> + reg->id);
> +}
> +
> +static int set_zcr_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + if (!vcpu_has_sve(vcpu))
> + return -ENOENT;
> +
> + return reg_from_user(&vcpu->arch.ctxt.sys_regs[ZCR_EL1], uaddr,
> + reg->id);
> +}
> +
> +/* Generate the emulated ID_AA64ZFR0_EL1 value exposed to the guest */
> +static u64 guest_id_aa64zfr0_el1(const struct kvm_vcpu *vcpu)
> +{
> + if (!vcpu_has_sve(vcpu))
> + return 0;
> +
> + return read_sanitised_ftr_reg(SYS_ID_AA64ZFR0_EL1);
> +}
> +
> +static bool access_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write)
> + return write_to_read_only(vcpu, p, rd);
> +
> + p->regval = guest_id_aa64zfr0_el1(vcpu);
> + return true;
> +}
> +
> +static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + u64 val;
> +
> + if (!vcpu_has_sve(vcpu))
> + return -ENOENT;
> +
> + val = guest_id_aa64zfr0_el1(vcpu);
> + return reg_to_user(uaddr, &val, reg->id);
> +}
> +
> +static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + const u64 id = sys_reg_to_index(rd);
> + int err;
> + u64 val;
> +
> + if (!vcpu_has_sve(vcpu))
> + return -ENOENT;
> +
> + err = reg_from_user(&val, uaddr, id);
> + if (err)
> + return err;
> +
> + /* This is what we mean by invariant: you can't change it. */
> + if (val != guest_id_aa64zfr0_el1(vcpu))
> + return -EINVAL;
> +
> + return 0;
> +}
> +#endif /* CONFIG_ARM64_SVE */
> +
> /*
> * cpufeature ID register user accessors
> *
> @@ -1278,7 +1374,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> ID_SANITISED(ID_AA64PFR1_EL1),
> ID_UNALLOCATED(4,2),
> ID_UNALLOCATED(4,3),
> +#ifdef CONFIG_ARM64_SVE
> + { SYS_DESC(SYS_ID_AA64ZFR0_EL1), access_id_aa64zfr0_el1, .get_user = get_id_aa64zfr0_el1, .set_user = set_id_aa64zfr0_el1, .check_present = sve_check_present },
> +#else
> ID_UNALLOCATED(4,4),
> +#endif
Can't we always have this code present and fallback to the "unallocated"
behaviour at runtime?
> ID_UNALLOCATED(4,5),
> ID_UNALLOCATED(4,6),
> ID_UNALLOCATED(4,7),
> @@ -1315,6 +1415,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>
> { SYS_DESC(SYS_SCTLR_EL1), access_vm_reg, reset_val, SCTLR_EL1, 0x00C50078 },
> { SYS_DESC(SYS_CPACR_EL1), NULL, reset_val, CPACR_EL1, 0 },
> +#ifdef CONFIG_ARM64_SVE
> + { SYS_DESC(SYS_ZCR_EL1), access_zcr_el1, reset_unknown, ZCR_EL1, ~0xfUL, .get_user = get_zcr_el1, .set_user = set_zcr_el1, .check_present = sve_check_present },
> +#endif
> { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 },
> { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 },
> { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 },
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2019-01-18 16:42 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-17 20:33 [PATCH v4 00/25] KVM: arm64: SVE guest support Dave Martin
2019-01-17 20:33 ` [PATCH v4 01/25] KVM: Documentation: Document arm64 core registers in detail Dave Martin
2019-01-17 20:33 ` [PATCH v4 02/25] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush Dave Martin
2019-01-17 20:33 ` [PATCH v4 03/25] KVM: arm64: Delete orphaned declaration for __fpsimd_enabled() Dave Martin
2019-01-17 20:33 ` [PATCH v4 04/25] KVM: arm64: Refactor kvm_arm_num_regs() for easier maintenance Dave Martin
2019-01-17 20:33 ` [PATCH v4 05/25] KVM: arm64: Add missing #include of <linux/bitmap.h> to kvm_host.h Dave Martin
2019-01-17 20:33 ` [PATCH v4 06/25] arm64/sve: Check SVE virtualisability Dave Martin
2019-01-17 20:33 ` [PATCH v4 07/25] arm64/sve: Clarify role of the VQ map maintenance functions Dave Martin
2019-01-17 20:33 ` [PATCH v4 08/25] arm64/sve: Enable SVE state tracking for non-task contexts Dave Martin
2019-01-17 20:33 ` [PATCH v4 09/25] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest Dave Martin
2019-01-17 20:33 ` [PATCH v4 10/25] KVM: arm64: Propagate vcpu into read_id_reg() Dave Martin
2019-01-17 20:33 ` [PATCH v4 11/25] KVM: arm64: Extend reset_unknown() to handle mixed RES0/UNKNOWN registers Dave Martin
2019-01-17 20:33 ` [PATCH v4 12/25] KVM: arm64: Support runtime sysreg filtering for KVM_GET_REG_LIST Dave Martin
2019-01-17 20:33 ` [PATCH v4 13/25] KVM: arm64/sve: System register context switch and access support Dave Martin
2019-01-18 16:42 ` Marc Zyngier [this message]
2019-01-22 16:27 ` Dave Martin
2019-01-17 20:33 ` [PATCH v4 14/25] KVM: arm64/sve: Context switch the SVE registers Dave Martin
2019-01-18 17:15 ` Marc Zyngier
2019-01-22 17:12 ` Dave Martin
2019-01-17 20:33 ` [PATCH v4 15/25] KVM: Allow 2048-bit register access via ioctl interface Dave Martin
2019-01-17 20:33 ` [PATCH v4 16/25] KVM: arm64: Reject ioctl access to FPSIMD V-regs on SVE vcpus Dave Martin
2019-01-17 20:33 ` [PATCH v4 17/25] KVM: arm64/sve: Add SVE support to register access ioctl interface Dave Martin
2019-01-18 17:58 ` Marc Zyngier
2019-01-22 17:24 ` Dave Martin
2019-01-17 20:33 ` [PATCH v4 18/25] KVM: arm64: Enumerate SVE register indices for KVM_GET_REG_LIST Dave Martin
2019-01-17 20:33 ` [PATCH v4 19/25] arm64/sve: In-kernel vector length availability query interface Dave Martin
2019-01-17 20:33 ` [PATCH v4 20/25] KVM: arm/arm64: Add hook to finalize the vcpu configuration Dave Martin
2019-01-17 20:33 ` [PATCH v4 21/25] KVM: arm64/sve: Add pseudo-register for the guest's vector lengths Dave Martin
2019-01-17 20:33 ` [PATCH v4 22/25] KVM: arm64/sve: Allow userspace to enable SVE for vcpus Dave Martin
2019-01-17 20:33 ` [PATCH v4 23/25] KVM: arm64: Add a capabillity to advertise SVE support Dave Martin
2019-01-17 20:33 ` [PATCH v4 24/25] KVM: Document errors for KVM_GET_ONE_REG and KVM_SET_ONE_REG Dave Martin
2019-01-17 20:33 ` [PATCH v4 25/25] KVM: arm64/sve: Document KVM API extensions for SVE Dave Martin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3d6da606-d7ae-5232-8317-4c550370b469@arm.com \
--to=marc.zyngier@arm.com \
--cc=Dave.Martin@arm.com \
--cc=ard.biesheuvel@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=cdall@kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=tokamoto@jp.fujitsu.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox