* [PATCH v3 0/2] MTE support for KVM guest
@ 2020-09-25 9:36 Steven Price
2020-09-25 9:36 ` [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers Steven Price
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Steven Price @ 2020-09-25 9:36 UTC (permalink / raw)
To: Catalin Marinas, Marc Zyngier, Will Deacon
Cc: Dr. David Alan Gilbert, qemu-devel, Dave Martin, Juan Quintela,
Richard Henderson, linux-kernel, Steven Price, Thomas Gleixner,
kvmarm, linux-arm-kernel
Version 3 of adding MTE support for KVM guests. See the previous (v2)
posting for background:
https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com
These patches add support to KVM to enable MTE within a guest. They are
based on Catalin's v9 MTE user-space support series[1] (currently in
next).
Changes since v2:
* MTE is no longer a VCPU feature, instead it is a VM cap.
* Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE).
* The cap must be set before any VCPUs are created, preventing any
shenanigans where MTE is enabled for the guest after memory accesses
have been performed.
[1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com
Steven Price (2):
arm64: kvm: Save/restore MTE registers
arm64: kvm: Introduce MTE VCPU feature
arch/arm64/include/asm/kvm_emulate.h | 3 +++
arch/arm64/include/asm/kvm_host.h | 7 +++++++
arch/arm64/include/asm/sysreg.h | 3 ++-
arch/arm64/kvm/arm.c | 9 +++++++++
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++
arch/arm64/kvm/mmu.c | 15 +++++++++++++++
arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++-----
include/uapi/linux/kvm.h | 1 +
8 files changed, 66 insertions(+), 6 deletions(-)
--
2.20.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers 2020-09-25 9:36 [PATCH v3 0/2] MTE support for KVM guest Steven Price @ 2020-09-25 9:36 ` Steven Price 2020-10-02 14:23 ` Andrew Jones 2020-09-25 9:36 ` [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature Steven Price 2020-10-02 14:36 ` [PATCH v3 0/2] MTE support for KVM guest Andrew Jones 2 siblings, 1 reply; 10+ messages in thread From: Steven Price @ 2020-09-25 9:36 UTC (permalink / raw) To: Catalin Marinas, Marc Zyngier, Will Deacon Cc: Dr. David Alan Gilbert, qemu-devel, Dave Martin, Juan Quintela, Richard Henderson, linux-kernel, Steven Price, Thomas Gleixner, kvmarm, linux-arm-kernel Define the new system registers that MTE introduces and context switch them. The MTE feature is still hidden from the ID register as it isn't supported in a VM yet. Signed-off-by: Steven Price <steven.price@arm.com> --- arch/arm64/include/asm/kvm_host.h | 4 ++++ arch/arm64/include/asm/sysreg.h | 3 ++- arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ arch/arm64/kvm/sys_regs.c | 14 ++++++++++---- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index e52c927aade5..4f4360dd149e 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -126,6 +126,8 @@ enum vcpu_sysreg { SCTLR_EL1, /* System Control Register */ ACTLR_EL1, /* Auxiliary Control Register */ CPACR_EL1, /* Coprocessor Access Control */ + RGSR_EL1, /* Random Allocation Tag Seed Register */ + GCR_EL1, /* Tag Control Register */ ZCR_EL1, /* SVE Control */ TTBR0_EL1, /* Translation Table Base Register 0 */ TTBR1_EL1, /* Translation Table Base Register 1 */ @@ -142,6 +144,8 @@ enum vcpu_sysreg { TPIDR_EL1, /* Thread ID, Privileged */ AMAIR_EL1, /* Aux Memory Attribute Indirection Register */ CNTKCTL_EL1, /* Timer Control Register (EL1) */ + TFSRE0_EL1, /* Tag Fault Status Register (EL0) */ + TFSR_EL1, /* Tag Fault Stauts Register (EL1) */ PAR_EL1, /* Physical Address Register */ MDSCR_EL1, /* Monitor Debug System Control Register */ MDCCINT_EL1, /* Monitor Debug Comms Channel Interrupt Enable Reg */ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 52eefe2f7d95..cd60677551b7 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -563,7 +563,8 @@ #define SCTLR_ELx_M (BIT(0)) #define SCTLR_ELx_FLAGS (SCTLR_ELx_M | SCTLR_ELx_A | SCTLR_ELx_C | \ - SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB) + SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \ + SCTLR_ELx_ITFSB) /* SCTLR_EL2 specific flags. */ #define SCTLR_EL2_RES1 ((BIT(4)) | (BIT(5)) | (BIT(11)) | (BIT(16)) | \ diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h index 7a986030145f..a124ffa49ba3 100644 --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h @@ -18,6 +18,11 @@ static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt) { ctxt_sys_reg(ctxt, MDSCR_EL1) = read_sysreg(mdscr_el1); + if (system_supports_mte()) { + ctxt_sys_reg(ctxt, RGSR_EL1) = read_sysreg_s(SYS_RGSR_EL1); + ctxt_sys_reg(ctxt, GCR_EL1) = read_sysreg_s(SYS_GCR_EL1); + ctxt_sys_reg(ctxt, TFSRE0_EL1) = read_sysreg_s(SYS_TFSRE0_EL1); + } } static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt) @@ -45,6 +50,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) ctxt_sys_reg(ctxt, CNTKCTL_EL1) = read_sysreg_el1(SYS_CNTKCTL); ctxt_sys_reg(ctxt, PAR_EL1) = read_sysreg(par_el1); ctxt_sys_reg(ctxt, TPIDR_EL1) = read_sysreg(tpidr_el1); + if (system_supports_mte()) + ctxt_sys_reg(ctxt, TFSR_EL1) = read_sysreg_el1(SYS_TFSR); ctxt_sys_reg(ctxt, SP_EL1) = read_sysreg(sp_el1); ctxt_sys_reg(ctxt, ELR_EL1) = read_sysreg_el1(SYS_ELR); @@ -63,6 +70,11 @@ static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt) static inline void __sysreg_restore_common_state(struct kvm_cpu_context *ctxt) { write_sysreg(ctxt_sys_reg(ctxt, MDSCR_EL1), mdscr_el1); + if (system_supports_mte()) { + write_sysreg_s(ctxt_sys_reg(ctxt, RGSR_EL1), SYS_RGSR_EL1); + write_sysreg_s(ctxt_sys_reg(ctxt, GCR_EL1), SYS_GCR_EL1); + write_sysreg_s(ctxt_sys_reg(ctxt, TFSRE0_EL1), SYS_TFSRE0_EL1); + } } static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt) @@ -106,6 +118,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) write_sysreg_el1(ctxt_sys_reg(ctxt, CNTKCTL_EL1), SYS_CNTKCTL); write_sysreg(ctxt_sys_reg(ctxt, PAR_EL1), par_el1); write_sysreg(ctxt_sys_reg(ctxt, TPIDR_EL1), tpidr_el1); + if (system_supports_mte()) + write_sysreg_el1(ctxt_sys_reg(ctxt, TFSR_EL1), SYS_TFSR); if (!has_vhe() && cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT) && diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 379f4969d0bd..a655f172b5ad 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1391,6 +1391,12 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, return false; } +static unsigned int mte_visibility(const struct kvm_vcpu *vcpu, + const struct sys_reg_desc *rd) +{ + return REG_HIDDEN_USER | REG_HIDDEN_GUEST; +} + /* sys_reg_desc initialiser for known cpufeature ID registers */ #define ID_SANITISED(name) { \ SYS_DESC(SYS_##name), \ @@ -1557,8 +1563,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_ACTLR_EL1), access_actlr, reset_actlr, ACTLR_EL1 }, { SYS_DESC(SYS_CPACR_EL1), NULL, reset_val, CPACR_EL1, 0 }, - { SYS_DESC(SYS_RGSR_EL1), access_mte_regs }, - { SYS_DESC(SYS_GCR_EL1), access_mte_regs }, + { SYS_DESC(SYS_RGSR_EL1), access_mte_regs, reset_unknown, RGSR_EL1, .visibility = mte_visibility }, + { SYS_DESC(SYS_GCR_EL1), access_mte_regs, reset_unknown, GCR_EL1, .visibility = mte_visibility }, { SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility }, { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, @@ -1584,8 +1590,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi }, { SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi }, - { SYS_DESC(SYS_TFSR_EL1), access_mte_regs }, - { SYS_DESC(SYS_TFSRE0_EL1), access_mte_regs }, + { SYS_DESC(SYS_TFSR_EL1), access_mte_regs, reset_unknown, TFSR_EL1, .visibility = mte_visibility }, + { SYS_DESC(SYS_TFSRE0_EL1), access_mte_regs, reset_unknown, TFSRE0_EL1, .visibility = mte_visibility }, { SYS_DESC(SYS_FAR_EL1), access_vm_reg, reset_unknown, FAR_EL1 }, { SYS_DESC(SYS_PAR_EL1), NULL, reset_unknown, PAR_EL1 }, -- 2.20.1 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers 2020-09-25 9:36 ` [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers Steven Price @ 2020-10-02 14:23 ` Andrew Jones 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jones @ 2020-10-02 14:23 UTC (permalink / raw) To: Steven Price Cc: Catalin Marinas, Juan Quintela, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, kvmarm, Marc Zyngier, Thomas Gleixner, Will Deacon, Dave Martin, linux-kernel, linux-arm-kernel On Fri, Sep 25, 2020 at 10:36:06AM +0100, Steven Price wrote: > Define the new system registers that MTE introduces and context switch > them. The MTE feature is still hidden from the ID register as it isn't > supported in a VM yet. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > arch/arm64/include/asm/kvm_host.h | 4 ++++ > arch/arm64/include/asm/sysreg.h | 3 ++- > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ > arch/arm64/kvm/sys_regs.c | 14 ++++++++++---- > 4 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index e52c927aade5..4f4360dd149e 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -126,6 +126,8 @@ enum vcpu_sysreg { > SCTLR_EL1, /* System Control Register */ > ACTLR_EL1, /* Auxiliary Control Register */ > CPACR_EL1, /* Coprocessor Access Control */ > + RGSR_EL1, /* Random Allocation Tag Seed Register */ > + GCR_EL1, /* Tag Control Register */ > ZCR_EL1, /* SVE Control */ > TTBR0_EL1, /* Translation Table Base Register 0 */ > TTBR1_EL1, /* Translation Table Base Register 1 */ > @@ -142,6 +144,8 @@ enum vcpu_sysreg { > TPIDR_EL1, /* Thread ID, Privileged */ > AMAIR_EL1, /* Aux Memory Attribute Indirection Register */ > CNTKCTL_EL1, /* Timer Control Register (EL1) */ > + TFSRE0_EL1, /* Tag Fault Status Register (EL0) */ > + TFSR_EL1, /* Tag Fault Stauts Register (EL1) */ > PAR_EL1, /* Physical Address Register */ > MDSCR_EL1, /* Monitor Debug System Control Register */ > MDCCINT_EL1, /* Monitor Debug Comms Channel Interrupt Enable Reg */ > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 52eefe2f7d95..cd60677551b7 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -563,7 +563,8 @@ > #define SCTLR_ELx_M (BIT(0)) > > #define SCTLR_ELx_FLAGS (SCTLR_ELx_M | SCTLR_ELx_A | SCTLR_ELx_C | \ > - SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB) > + SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \ > + SCTLR_ELx_ITFSB) > > /* SCTLR_EL2 specific flags. */ > #define SCTLR_EL2_RES1 ((BIT(4)) | (BIT(5)) | (BIT(11)) | (BIT(16)) | \ > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > index 7a986030145f..a124ffa49ba3 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > @@ -18,6 +18,11 @@ > static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt) > { > ctxt_sys_reg(ctxt, MDSCR_EL1) = read_sysreg(mdscr_el1); > + if (system_supports_mte()) { > + ctxt_sys_reg(ctxt, RGSR_EL1) = read_sysreg_s(SYS_RGSR_EL1); > + ctxt_sys_reg(ctxt, GCR_EL1) = read_sysreg_s(SYS_GCR_EL1); > + ctxt_sys_reg(ctxt, TFSRE0_EL1) = read_sysreg_s(SYS_TFSRE0_EL1); > + } > } > > static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt) > @@ -45,6 +50,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) > ctxt_sys_reg(ctxt, CNTKCTL_EL1) = read_sysreg_el1(SYS_CNTKCTL); > ctxt_sys_reg(ctxt, PAR_EL1) = read_sysreg(par_el1); > ctxt_sys_reg(ctxt, TPIDR_EL1) = read_sysreg(tpidr_el1); > + if (system_supports_mte()) > + ctxt_sys_reg(ctxt, TFSR_EL1) = read_sysreg_el1(SYS_TFSR); > > ctxt_sys_reg(ctxt, SP_EL1) = read_sysreg(sp_el1); > ctxt_sys_reg(ctxt, ELR_EL1) = read_sysreg_el1(SYS_ELR); > @@ -63,6 +70,11 @@ static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt) > static inline void __sysreg_restore_common_state(struct kvm_cpu_context *ctxt) > { > write_sysreg(ctxt_sys_reg(ctxt, MDSCR_EL1), mdscr_el1); > + if (system_supports_mte()) { > + write_sysreg_s(ctxt_sys_reg(ctxt, RGSR_EL1), SYS_RGSR_EL1); > + write_sysreg_s(ctxt_sys_reg(ctxt, GCR_EL1), SYS_GCR_EL1); > + write_sysreg_s(ctxt_sys_reg(ctxt, TFSRE0_EL1), SYS_TFSRE0_EL1); > + } > } > > static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt) > @@ -106,6 +118,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) > write_sysreg_el1(ctxt_sys_reg(ctxt, CNTKCTL_EL1), SYS_CNTKCTL); > write_sysreg(ctxt_sys_reg(ctxt, PAR_EL1), par_el1); > write_sysreg(ctxt_sys_reg(ctxt, TPIDR_EL1), tpidr_el1); > + if (system_supports_mte()) > + write_sysreg_el1(ctxt_sys_reg(ctxt, TFSR_EL1), SYS_TFSR); > > if (!has_vhe() && > cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT) && > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 379f4969d0bd..a655f172b5ad 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1391,6 +1391,12 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > return false; > } > > +static unsigned int mte_visibility(const struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd) > +{ > + return REG_HIDDEN_USER | REG_HIDDEN_GUEST; > +} > + > /* sys_reg_desc initialiser for known cpufeature ID registers */ > #define ID_SANITISED(name) { \ > SYS_DESC(SYS_##name), \ > @@ -1557,8 +1563,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { > { SYS_DESC(SYS_ACTLR_EL1), access_actlr, reset_actlr, ACTLR_EL1 }, > { SYS_DESC(SYS_CPACR_EL1), NULL, reset_val, CPACR_EL1, 0 }, > > - { SYS_DESC(SYS_RGSR_EL1), access_mte_regs }, > - { SYS_DESC(SYS_GCR_EL1), access_mte_regs }, > + { SYS_DESC(SYS_RGSR_EL1), access_mte_regs, reset_unknown, RGSR_EL1, .visibility = mte_visibility }, > + { SYS_DESC(SYS_GCR_EL1), access_mte_regs, reset_unknown, GCR_EL1, .visibility = mte_visibility }, > > { SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility }, > { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, > @@ -1584,8 +1590,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { > { SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi }, > { SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi }, > > - { SYS_DESC(SYS_TFSR_EL1), access_mte_regs }, > - { SYS_DESC(SYS_TFSRE0_EL1), access_mte_regs }, > + { SYS_DESC(SYS_TFSR_EL1), access_mte_regs, reset_unknown, TFSR_EL1, .visibility = mte_visibility }, > + { SYS_DESC(SYS_TFSRE0_EL1), access_mte_regs, reset_unknown, TFSRE0_EL1, .visibility = mte_visibility }, > > { SYS_DESC(SYS_FAR_EL1), access_vm_reg, reset_unknown, FAR_EL1 }, > { SYS_DESC(SYS_PAR_EL1), NULL, reset_unknown, PAR_EL1 }, > -- > 2.20.1 > > Reviewed-by: Andrew Jones <drjones@redhat.com> _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature 2020-09-25 9:36 [PATCH v3 0/2] MTE support for KVM guest Steven Price 2020-09-25 9:36 ` [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers Steven Price @ 2020-09-25 9:36 ` Steven Price 2020-10-02 14:30 ` Andrew Jones 2020-10-02 14:36 ` [PATCH v3 0/2] MTE support for KVM guest Andrew Jones 2 siblings, 1 reply; 10+ messages in thread From: Steven Price @ 2020-09-25 9:36 UTC (permalink / raw) To: Catalin Marinas, Marc Zyngier, Will Deacon Cc: Dr. David Alan Gilbert, qemu-devel, Dave Martin, Juan Quintela, Richard Henderson, linux-kernel, Steven Price, Thomas Gleixner, kvmarm, linux-arm-kernel Add a new VM feature 'KVM_ARM_CAP_MTE' which enables memory tagging for a VM. This exposes the feature to the guest and automatically tags memory pages touched by the VM as PG_mte_tagged (and clears the tags storage) to ensure that the guest cannot see stale tags, and so that the tags are correctly saved/restored across swap. Signed-off-by: Steven Price <steven.price@arm.com> --- arch/arm64/include/asm/kvm_emulate.h | 3 +++ arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/kvm/arm.c | 9 +++++++++ arch/arm64/kvm/mmu.c | 15 +++++++++++++++ arch/arm64/kvm/sys_regs.c | 6 +++++- include/uapi/linux/kvm.h | 1 + 6 files changed, 36 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 49a55be2b9a2..4923a566ae6e 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -79,6 +79,9 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu) if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) || vcpu_el1_is_32bit(vcpu)) vcpu->arch.hcr_el2 |= HCR_TID2; + + if (vcpu->kvm->arch.mte_enabled) + vcpu->arch.hcr_el2 |= HCR_ATA; } static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 4f4360dd149e..1379300c1487 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -110,6 +110,9 @@ struct kvm_arch { * supported. */ bool return_nisv_io_abort_to_user; + + /* Memory Tagging Extension enabled for the guest */ + bool mte_enabled; }; struct kvm_vcpu_fault_info { diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 46dc3d75cf13..624edca0a1fa 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -87,6 +87,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, r = 0; kvm->arch.return_nisv_io_abort_to_user = true; break; + case KVM_CAP_ARM_MTE: + if (!system_supports_mte() || kvm->created_vcpus) + return -EINVAL; + r = 0; + kvm->arch.mte_enabled = true; + break; default: r = -EINVAL; break; @@ -206,6 +212,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) */ r = 1; break; + case KVM_CAP_ARM_MTE: + r = system_supports_mte(); + break; default: r = kvm_arch_vm_ioctl_check_extension(kvm, ext); break; diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index ba00bcc0c884..befb9e1f0aa6 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -1949,6 +1949,21 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, if (vma_pagesize == PAGE_SIZE && !force_pte) vma_pagesize = transparent_hugepage_adjust(memslot, hva, &pfn, &fault_ipa); + if (system_supports_mte() && kvm->arch.mte_enabled && pfn_valid(pfn)) { + /* + * VM will be able to see the page's tags, so we must ensure + * they have been initialised. + */ + struct page *page = pfn_to_page(pfn); + long i, nr_pages = compound_nr(page); + + /* if PG_mte_tagged is set, tags have already been initialised */ + for (i = 0; i < nr_pages; i++, page++) { + if (!test_and_set_bit(PG_mte_tagged, &page->flags)) + mte_clear_page_tags(page_address(page)); + } + } + if (writable) kvm_set_pfn_dirty(pfn); diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index a655f172b5ad..5010a47152b4 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1132,7 +1132,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT); } else if (id == SYS_ID_AA64PFR1_EL1) { - val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); + if (!vcpu->kvm->arch.mte_enabled) + val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) { val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) | (0xfUL << ID_AA64ISAR1_API_SHIFT) | @@ -1394,6 +1395,9 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, static unsigned int mte_visibility(const struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd) { + if (vcpu->kvm->arch.mte_enabled) + return 0; + return REG_HIDDEN_USER | REG_HIDDEN_GUEST; } diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index f6d86033c4fa..87678ed82ab4 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1035,6 +1035,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_LAST_CPU 184 #define KVM_CAP_SMALLER_MAXPHYADDR 185 #define KVM_CAP_S390_DIAG318 186 +#define KVM_CAP_ARM_MTE 188 #ifdef KVM_CAP_IRQ_ROUTING -- 2.20.1 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature 2020-09-25 9:36 ` [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature Steven Price @ 2020-10-02 14:30 ` Andrew Jones 2020-10-02 15:30 ` Steven Price 0 siblings, 1 reply; 10+ messages in thread From: Andrew Jones @ 2020-10-02 14:30 UTC (permalink / raw) To: Steven Price Cc: Catalin Marinas, Juan Quintela, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, kvmarm, Marc Zyngier, Thomas Gleixner, Will Deacon, Dave Martin, linux-kernel, linux-arm-kernel On Fri, Sep 25, 2020 at 10:36:07AM +0100, Steven Price wrote: > Add a new VM feature 'KVM_ARM_CAP_MTE' which enables memory tagging > for a VM. This exposes the feature to the guest and automatically tags > memory pages touched by the VM as PG_mte_tagged (and clears the tags > storage) to ensure that the guest cannot see stale tags, and so that the > tags are correctly saved/restored across swap. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > arch/arm64/include/asm/kvm_emulate.h | 3 +++ > arch/arm64/include/asm/kvm_host.h | 3 +++ > arch/arm64/kvm/arm.c | 9 +++++++++ > arch/arm64/kvm/mmu.c | 15 +++++++++++++++ > arch/arm64/kvm/sys_regs.c | 6 +++++- > include/uapi/linux/kvm.h | 1 + > 6 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > index 49a55be2b9a2..4923a566ae6e 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -79,6 +79,9 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu) > if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) || > vcpu_el1_is_32bit(vcpu)) > vcpu->arch.hcr_el2 |= HCR_TID2; > + > + if (vcpu->kvm->arch.mte_enabled) > + vcpu->arch.hcr_el2 |= HCR_ATA; > } > > static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu) > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 4f4360dd149e..1379300c1487 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -110,6 +110,9 @@ struct kvm_arch { > * supported. > */ > bool return_nisv_io_abort_to_user; > + > + /* Memory Tagging Extension enabled for the guest */ > + bool mte_enabled; > }; > > struct kvm_vcpu_fault_info { > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 46dc3d75cf13..624edca0a1fa 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -87,6 +87,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, > r = 0; > kvm->arch.return_nisv_io_abort_to_user = true; > break; > + case KVM_CAP_ARM_MTE: > + if (!system_supports_mte() || kvm->created_vcpus) > + return -EINVAL; > + r = 0; > + kvm->arch.mte_enabled = true; > + break; > default: > r = -EINVAL; > break; > @@ -206,6 +212,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > */ > r = 1; > break; > + case KVM_CAP_ARM_MTE: > + r = system_supports_mte(); > + break; > default: > r = kvm_arch_vm_ioctl_check_extension(kvm, ext); > break; > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index ba00bcc0c884..befb9e1f0aa6 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -1949,6 +1949,21 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > if (vma_pagesize == PAGE_SIZE && !force_pte) > vma_pagesize = transparent_hugepage_adjust(memslot, hva, > &pfn, &fault_ipa); > + if (system_supports_mte() && kvm->arch.mte_enabled && pfn_valid(pfn)) { 'system_supports_mte() && kvm->arch.mte_enabled' is redundant, but I assume system_supports_mte() is there to improve the efficiency of the branch, as it's using cpus_have_const_cap(). Maybe a helper like static inline bool kvm_arm_mte_enabled(struct kvm *kvm) { return system_supports_mte() && kvm->arch.mte_enabled; } would allow both the more efficient branch and look less confusing where it gets used. > + /* > + * VM will be able to see the page's tags, so we must ensure > + * they have been initialised. > + */ > + struct page *page = pfn_to_page(pfn); > + long i, nr_pages = compound_nr(page); > + > + /* if PG_mte_tagged is set, tags have already been initialised */ > + for (i = 0; i < nr_pages; i++, page++) { > + if (!test_and_set_bit(PG_mte_tagged, &page->flags)) > + mte_clear_page_tags(page_address(page)); > + } > + } > + > if (writable) > kvm_set_pfn_dirty(pfn); > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index a655f172b5ad..5010a47152b4 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1132,7 +1132,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, > val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); > val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT); > } else if (id == SYS_ID_AA64PFR1_EL1) { > - val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); > + if (!vcpu->kvm->arch.mte_enabled) > + val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); > } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) { > val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) | > (0xfUL << ID_AA64ISAR1_API_SHIFT) | > @@ -1394,6 +1395,9 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > static unsigned int mte_visibility(const struct kvm_vcpu *vcpu, > const struct sys_reg_desc *rd) > { > + if (vcpu->kvm->arch.mte_enabled) > + return 0; > + > return REG_HIDDEN_USER | REG_HIDDEN_GUEST; > } > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index f6d86033c4fa..87678ed82ab4 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1035,6 +1035,7 @@ struct kvm_ppc_resize_hpt { > #define KVM_CAP_LAST_CPU 184 > #define KVM_CAP_SMALLER_MAXPHYADDR 185 > #define KVM_CAP_S390_DIAG318 186 > +#define KVM_CAP_ARM_MTE 188 > > #ifdef KVM_CAP_IRQ_ROUTING > > -- > 2.20.1 > > Besides the helper suggestion nit Reviewed-by: Andrew Jones <drjones@redhat.com> _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature 2020-10-02 14:30 ` Andrew Jones @ 2020-10-02 15:30 ` Steven Price 2020-10-02 16:20 ` Andrew Jones 0 siblings, 1 reply; 10+ messages in thread From: Steven Price @ 2020-10-02 15:30 UTC (permalink / raw) To: Andrew Jones Cc: Catalin Marinas, Juan Quintela, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, kvmarm, Marc Zyngier, Thomas Gleixner, Will Deacon, Dave Martin, linux-kernel, linux-arm-kernel On 02/10/2020 15:30, Andrew Jones wrote: > On Fri, Sep 25, 2020 at 10:36:07AM +0100, Steven Price wrote: >> Add a new VM feature 'KVM_ARM_CAP_MTE' which enables memory tagging >> for a VM. This exposes the feature to the guest and automatically tags >> memory pages touched by the VM as PG_mte_tagged (and clears the tags >> storage) to ensure that the guest cannot see stale tags, and so that the >> tags are correctly saved/restored across swap. >> >> Signed-off-by: Steven Price <steven.price@arm.com> >> --- >> arch/arm64/include/asm/kvm_emulate.h | 3 +++ >> arch/arm64/include/asm/kvm_host.h | 3 +++ >> arch/arm64/kvm/arm.c | 9 +++++++++ >> arch/arm64/kvm/mmu.c | 15 +++++++++++++++ >> arch/arm64/kvm/sys_regs.c | 6 +++++- >> include/uapi/linux/kvm.h | 1 + >> 6 files changed, 36 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h >> index 49a55be2b9a2..4923a566ae6e 100644 >> --- a/arch/arm64/include/asm/kvm_emulate.h >> +++ b/arch/arm64/include/asm/kvm_emulate.h >> @@ -79,6 +79,9 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu) >> if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) || >> vcpu_el1_is_32bit(vcpu)) >> vcpu->arch.hcr_el2 |= HCR_TID2; >> + >> + if (vcpu->kvm->arch.mte_enabled) >> + vcpu->arch.hcr_el2 |= HCR_ATA; >> } >> >> static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu) >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h >> index 4f4360dd149e..1379300c1487 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -110,6 +110,9 @@ struct kvm_arch { >> * supported. >> */ >> bool return_nisv_io_abort_to_user; >> + >> + /* Memory Tagging Extension enabled for the guest */ >> + bool mte_enabled; >> }; >> >> struct kvm_vcpu_fault_info { >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 46dc3d75cf13..624edca0a1fa 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -87,6 +87,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, >> r = 0; >> kvm->arch.return_nisv_io_abort_to_user = true; >> break; >> + case KVM_CAP_ARM_MTE: >> + if (!system_supports_mte() || kvm->created_vcpus) >> + return -EINVAL; >> + r = 0; >> + kvm->arch.mte_enabled = true; >> + break; >> default: >> r = -EINVAL; >> break; >> @@ -206,6 +212,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) >> */ >> r = 1; >> break; >> + case KVM_CAP_ARM_MTE: >> + r = system_supports_mte(); >> + break; >> default: >> r = kvm_arch_vm_ioctl_check_extension(kvm, ext); >> break; >> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c >> index ba00bcc0c884..befb9e1f0aa6 100644 >> --- a/arch/arm64/kvm/mmu.c >> +++ b/arch/arm64/kvm/mmu.c >> @@ -1949,6 +1949,21 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, >> if (vma_pagesize == PAGE_SIZE && !force_pte) >> vma_pagesize = transparent_hugepage_adjust(memslot, hva, >> &pfn, &fault_ipa); >> + if (system_supports_mte() && kvm->arch.mte_enabled && pfn_valid(pfn)) { > > 'system_supports_mte() && kvm->arch.mte_enabled' is redundant, but I > assume system_supports_mte() is there to improve the efficiency of the > branch, as it's using cpus_have_const_cap(). system_supports_mte() compiles to 0 when MTE support isn't built in, so this code can be removed by the compiler, whereas with kvm->arch.mte_enabled I doubt the compiler can deduce that it is never set. > Maybe a helper like > > static inline bool kvm_arm_mte_enabled(struct kvm *kvm) > { > return system_supports_mte() && kvm->arch.mte_enabled; > } > > would allow both the more efficient branch and look less confusing > where it gets used. I wasn't sure it was worth having a helper since this was the only place checking this condition. It's also a bit tricky putting this in a logical header file, kvm_host.h doesn't work because struct kvm hasn't been defined by then. Steve >> + /* >> + * VM will be able to see the page's tags, so we must ensure >> + * they have been initialised. >> + */ >> + struct page *page = pfn_to_page(pfn); >> + long i, nr_pages = compound_nr(page); >> + >> + /* if PG_mte_tagged is set, tags have already been initialised */ >> + for (i = 0; i < nr_pages; i++, page++) { >> + if (!test_and_set_bit(PG_mte_tagged, &page->flags)) >> + mte_clear_page_tags(page_address(page)); >> + } >> + } >> + >> if (writable) >> kvm_set_pfn_dirty(pfn); >> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c >> index a655f172b5ad..5010a47152b4 100644 >> --- a/arch/arm64/kvm/sys_regs.c >> +++ b/arch/arm64/kvm/sys_regs.c >> @@ -1132,7 +1132,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, >> val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); >> val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT); >> } else if (id == SYS_ID_AA64PFR1_EL1) { >> - val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); >> + if (!vcpu->kvm->arch.mte_enabled) >> + val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); >> } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) { >> val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) | >> (0xfUL << ID_AA64ISAR1_API_SHIFT) | >> @@ -1394,6 +1395,9 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, >> static unsigned int mte_visibility(const struct kvm_vcpu *vcpu, >> const struct sys_reg_desc *rd) >> { >> + if (vcpu->kvm->arch.mte_enabled) >> + return 0; >> + >> return REG_HIDDEN_USER | REG_HIDDEN_GUEST; >> } >> >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index f6d86033c4fa..87678ed82ab4 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -1035,6 +1035,7 @@ struct kvm_ppc_resize_hpt { >> #define KVM_CAP_LAST_CPU 184 >> #define KVM_CAP_SMALLER_MAXPHYADDR 185 >> #define KVM_CAP_S390_DIAG318 186 >> +#define KVM_CAP_ARM_MTE 188 >> >> #ifdef KVM_CAP_IRQ_ROUTING >> >> -- >> 2.20.1 >> >> > > Besides the helper suggestion nit > > Reviewed-by: Andrew Jones <drjones@redhat.com> > _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature 2020-10-02 15:30 ` Steven Price @ 2020-10-02 16:20 ` Andrew Jones 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jones @ 2020-10-02 16:20 UTC (permalink / raw) To: Steven Price Cc: linux-kernel, Juan Quintela, Catalin Marinas, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, linux-arm-kernel, Marc Zyngier, Thomas Gleixner, Will Deacon, kvmarm, Dave Martin On Fri, Oct 02, 2020 at 04:30:47PM +0100, Steven Price wrote: > On 02/10/2020 15:30, Andrew Jones wrote: > > On Fri, Sep 25, 2020 at 10:36:07AM +0100, Steven Price wrote: > > > + if (system_supports_mte() && kvm->arch.mte_enabled && pfn_valid(pfn)) { > > > > 'system_supports_mte() && kvm->arch.mte_enabled' is redundant, but I > > assume system_supports_mte() is there to improve the efficiency of the > > branch, as it's using cpus_have_const_cap(). > > system_supports_mte() compiles to 0 when MTE support isn't built in, so this > code can be removed by the compiler, I know. That's what I meant by "improve the efficiency of the branch" > whereas with kvm->arch.mte_enabled I > doubt the compiler can deduce that it is never set. > > > Maybe a helper like > > > > static inline bool kvm_arm_mte_enabled(struct kvm *kvm) > > { > > return system_supports_mte() && kvm->arch.mte_enabled; > > } > > > > would allow both the more efficient branch and look less confusing > > where it gets used. > > I wasn't sure it was worth having a helper since this was the only place > checking this condition. It's also a bit tricky putting this in a logical > header file, kvm_host.h doesn't work because struct kvm hasn't been defined > by then. OK, but I feel like we're setting ourselves up to revisit these types of conditions again when our memories fade or when new developers see them for the first time and ask. Thanks, drew > > Steve > > > > + /* > > > + * VM will be able to see the page's tags, so we must ensure > > > + * they have been initialised. > > > + */ > > > + struct page *page = pfn_to_page(pfn); > > > + long i, nr_pages = compound_nr(page); > > > + > > > + /* if PG_mte_tagged is set, tags have already been initialised */ > > > + for (i = 0; i < nr_pages; i++, page++) { > > > + if (!test_and_set_bit(PG_mte_tagged, &page->flags)) > > > + mte_clear_page_tags(page_address(page)); > > > + } > > > + } > > > + > > > if (writable) > > > kvm_set_pfn_dirty(pfn); > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index a655f172b5ad..5010a47152b4 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -1132,7 +1132,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, > > > val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); > > > val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT); > > > } else if (id == SYS_ID_AA64PFR1_EL1) { > > > - val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); > > > + if (!vcpu->kvm->arch.mte_enabled) > > > + val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT); > > > } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) { > > > val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) | > > > (0xfUL << ID_AA64ISAR1_API_SHIFT) | > > > @@ -1394,6 +1395,9 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > > > static unsigned int mte_visibility(const struct kvm_vcpu *vcpu, > > > const struct sys_reg_desc *rd) > > > { > > > + if (vcpu->kvm->arch.mte_enabled) > > > + return 0; > > > + > > > return REG_HIDDEN_USER | REG_HIDDEN_GUEST; > > > } > > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > > > index f6d86033c4fa..87678ed82ab4 100644 > > > --- a/include/uapi/linux/kvm.h > > > +++ b/include/uapi/linux/kvm.h > > > @@ -1035,6 +1035,7 @@ struct kvm_ppc_resize_hpt { > > > #define KVM_CAP_LAST_CPU 184 > > > #define KVM_CAP_SMALLER_MAXPHYADDR 185 > > > #define KVM_CAP_S390_DIAG318 186 > > > +#define KVM_CAP_ARM_MTE 188 > > > #ifdef KVM_CAP_IRQ_ROUTING > > > -- > > > 2.20.1 > > > > > > > > > > Besides the helper suggestion nit > > > > Reviewed-by: Andrew Jones <drjones@redhat.com> > > > > _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] MTE support for KVM guest 2020-09-25 9:36 [PATCH v3 0/2] MTE support for KVM guest Steven Price 2020-09-25 9:36 ` [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers Steven Price 2020-09-25 9:36 ` [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature Steven Price @ 2020-10-02 14:36 ` Andrew Jones 2020-10-02 15:38 ` Steven Price 2 siblings, 1 reply; 10+ messages in thread From: Andrew Jones @ 2020-10-02 14:36 UTC (permalink / raw) To: Steven Price Cc: Catalin Marinas, Juan Quintela, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, kvmarm, Marc Zyngier, Thomas Gleixner, Will Deacon, Dave Martin, linux-kernel, linux-arm-kernel On Fri, Sep 25, 2020 at 10:36:05AM +0100, Steven Price wrote: > Version 3 of adding MTE support for KVM guests. See the previous (v2) > posting for background: > > https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com > > These patches add support to KVM to enable MTE within a guest. They are > based on Catalin's v9 MTE user-space support series[1] (currently in > next). > > Changes since v2: > > * MTE is no longer a VCPU feature, instead it is a VM cap. > > * Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE). > > * The cap must be set before any VCPUs are created, preventing any > shenanigans where MTE is enabled for the guest after memory accesses > have been performed. > > [1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com > > Steven Price (2): > arm64: kvm: Save/restore MTE registers > arm64: kvm: Introduce MTE VCPU feature > > arch/arm64/include/asm/kvm_emulate.h | 3 +++ > arch/arm64/include/asm/kvm_host.h | 7 +++++++ > arch/arm64/include/asm/sysreg.h | 3 ++- > arch/arm64/kvm/arm.c | 9 +++++++++ > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ > arch/arm64/kvm/mmu.c | 15 +++++++++++++++ > arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++----- > include/uapi/linux/kvm.h | 1 + > 8 files changed, 66 insertions(+), 6 deletions(-) > > -- > 2.20.1 > > Hi Steven, These patches look fine to me, but I'd prefer we have a working implementation in QEMU before we get too excited about the KVM bits. kvmtool isn't sufficient since it doesn't support migration (at least afaik). In the past we've implemented features in KVM that look fine, but then issues have been discovered when trying to enable them from QEMU, where we also support migration. This feature looks like there's risk of issues with the userspace side. Although these two patches would probably stay the same, even if userspace requires more support. Thanks, drew _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] MTE support for KVM guest 2020-10-02 14:36 ` [PATCH v3 0/2] MTE support for KVM guest Andrew Jones @ 2020-10-02 15:38 ` Steven Price 2020-10-02 16:23 ` Andrew Jones 0 siblings, 1 reply; 10+ messages in thread From: Steven Price @ 2020-10-02 15:38 UTC (permalink / raw) To: Andrew Jones Cc: Catalin Marinas, Juan Quintela, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, kvmarm, Marc Zyngier, Thomas Gleixner, Will Deacon, Dave Martin, linux-kernel, linux-arm-kernel On 02/10/2020 15:36, Andrew Jones wrote: > On Fri, Sep 25, 2020 at 10:36:05AM +0100, Steven Price wrote: >> Version 3 of adding MTE support for KVM guests. See the previous (v2) >> posting for background: >> >> https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com >> >> These patches add support to KVM to enable MTE within a guest. They are >> based on Catalin's v9 MTE user-space support series[1] (currently in >> next). >> >> Changes since v2: >> >> * MTE is no longer a VCPU feature, instead it is a VM cap. >> >> * Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE). >> >> * The cap must be set before any VCPUs are created, preventing any >> shenanigans where MTE is enabled for the guest after memory accesses >> have been performed. >> >> [1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com >> >> Steven Price (2): >> arm64: kvm: Save/restore MTE registers >> arm64: kvm: Introduce MTE VCPU feature >> >> arch/arm64/include/asm/kvm_emulate.h | 3 +++ >> arch/arm64/include/asm/kvm_host.h | 7 +++++++ >> arch/arm64/include/asm/sysreg.h | 3 ++- >> arch/arm64/kvm/arm.c | 9 +++++++++ >> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ >> arch/arm64/kvm/mmu.c | 15 +++++++++++++++ >> arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++----- >> include/uapi/linux/kvm.h | 1 + >> 8 files changed, 66 insertions(+), 6 deletions(-) >> >> -- >> 2.20.1 >> >> > > Hi Steven, > > These patches look fine to me, but I'd prefer we have a working > implementation in QEMU before we get too excited about the KVM > bits. kvmtool isn't sufficient since it doesn't support migration > (at least afaik). In the past we've implemented features in KVM > that look fine, but then issues have been discovered when trying > to enable them from QEMU, where we also support migration. This > feature looks like there's risk of issues with the userspace side. > Although these two patches would probably stay the same, even if > userspace requires more support. I agree kvmtool isn't a great test because it doesn't support migration. The support in this series is just the basic support for MTE in a guest and we'd need to wait for the QEMU implementation before deciding whether we need any extra support (e.g. kernel interfaces for reading/writing tags as discussed before). However, I don't think there's much danger of the support in this series changing - so extra support can be added when/if it's needed, but I don't think we need to block these series on that - QEMU can just probe for whatever additional support it needs before enabling MTE in a guest. I plan to rebase/repost after -rc1 when the user space support has been merged. Steve _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/2] MTE support for KVM guest 2020-10-02 15:38 ` Steven Price @ 2020-10-02 16:23 ` Andrew Jones 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jones @ 2020-10-02 16:23 UTC (permalink / raw) To: Steven Price Cc: linux-kernel, Juan Quintela, Catalin Marinas, Richard Henderson, Dr. David Alan Gilbert, qemu-devel, linux-arm-kernel, Marc Zyngier, Thomas Gleixner, Will Deacon, kvmarm, Dave Martin On Fri, Oct 02, 2020 at 04:38:11PM +0100, Steven Price wrote: > On 02/10/2020 15:36, Andrew Jones wrote: > > On Fri, Sep 25, 2020 at 10:36:05AM +0100, Steven Price wrote: > > > Version 3 of adding MTE support for KVM guests. See the previous (v2) > > > posting for background: > > > > > > https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com > > > > > > These patches add support to KVM to enable MTE within a guest. They are > > > based on Catalin's v9 MTE user-space support series[1] (currently in > > > next). > > > > > > Changes since v2: > > > > > > * MTE is no longer a VCPU feature, instead it is a VM cap. > > > > > > * Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE). > > > > > > * The cap must be set before any VCPUs are created, preventing any > > > shenanigans where MTE is enabled for the guest after memory accesses > > > have been performed. > > > > > > [1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com > > > > > > Steven Price (2): > > > arm64: kvm: Save/restore MTE registers > > > arm64: kvm: Introduce MTE VCPU feature > > > > > > arch/arm64/include/asm/kvm_emulate.h | 3 +++ > > > arch/arm64/include/asm/kvm_host.h | 7 +++++++ > > > arch/arm64/include/asm/sysreg.h | 3 ++- > > > arch/arm64/kvm/arm.c | 9 +++++++++ > > > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ > > > arch/arm64/kvm/mmu.c | 15 +++++++++++++++ > > > arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++----- > > > include/uapi/linux/kvm.h | 1 + > > > 8 files changed, 66 insertions(+), 6 deletions(-) > > > > > > -- > > > 2.20.1 > > > > > > > > > > Hi Steven, > > > > These patches look fine to me, but I'd prefer we have a working > > implementation in QEMU before we get too excited about the KVM > > bits. kvmtool isn't sufficient since it doesn't support migration > > (at least afaik). In the past we've implemented features in KVM > > that look fine, but then issues have been discovered when trying > > to enable them from QEMU, where we also support migration. This > > feature looks like there's risk of issues with the userspace side. > > Although these two patches would probably stay the same, even if > > userspace requires more support. > > I agree kvmtool isn't a great test because it doesn't support migration. The > support in this series is just the basic support for MTE in a guest and we'd > need to wait for the QEMU implementation before deciding whether we need any > extra support (e.g. kernel interfaces for reading/writing tags as discussed > before). > > However, I don't think there's much danger of the support in this series > changing - so extra support can be added when/if it's needed, but I don't > think we need to block these series on that - QEMU can just probe for > whatever additional support it needs before enabling MTE in a guest. I plan > to rebase/repost after -rc1 when the user space support has been merged. > Fair enough, but it feels like we'll be merging half a feature, leaving the other half for somebody else to pick up later. Thanks, drew _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-02 16:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-25 9:36 [PATCH v3 0/2] MTE support for KVM guest Steven Price 2020-09-25 9:36 ` [PATCH v3 1/2] arm64: kvm: Save/restore MTE registers Steven Price 2020-10-02 14:23 ` Andrew Jones 2020-09-25 9:36 ` [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature Steven Price 2020-10-02 14:30 ` Andrew Jones 2020-10-02 15:30 ` Steven Price 2020-10-02 16:20 ` Andrew Jones 2020-10-02 14:36 ` [PATCH v3 0/2] MTE support for KVM guest Andrew Jones 2020-10-02 15:38 ` Steven Price 2020-10-02 16:23 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).