* [PATCH 0/3] KVM: arm64: Fixes for the exposed debug architecture
@ 2021-10-29 0:31 Oliver Upton
2021-10-29 0:32 ` [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context Oliver Upton
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Oliver Upton @ 2021-10-29 0:31 UTC (permalink / raw)
To: kvmarm
Cc: kvm, Marc Zyngier, James Morse, Alexandru Elisei,
Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier,
Ricardo Koller, Reiji Watanabe, Oliver Upton
I had a conversation with Marc about some of the quirks around the debug
architecture on KVM and incorporated some of his suggestions into a
series here. Of course, any glaring mistakes/choices made in this series
is on me :-)
Anyhow:
KVM's implementation of the debug architecture is a bit deviant as it
stands. For one, KVM handles the OS Lock as RAZ/WI, even though the
architecture mandates it. Additionally, KVM advertises more than it can
actually support: FEAT_DoubleLock is exposed as implemented to the
guest, though OSDLR_EL1 is handled as RAZ/WI too.
Only v8.2+ revisions of the debug architecture permit implementations to
omit DoubleLock. Fortunately, the delta between v8.0 and v8.2 is
entirely focused on external debug, a feature that KVM does not support
and likely never will. So, there isn't much of a hurdle to bump KVM's
reported DebugVer to v8.2, thereby allowing KVM to omit DoubleLock from
ID_AA64DFR0_EL1. Of the remaining bits of external debug visible to the
guest, the only additional thing to address is the OSLAR_EL1 issue by
simply context switching the host/guest values.
Patch 1 changes the way KVM backs OSLSR_EL1 in the sys reg table.
Instead of returning a static value from its handler, stash a copy of it
in kvm_cpu_context and return that when read.
Patch 2 makes the material change of allowing a guest to actually toggle
the OSLK bit by redirecting writes to OSLAR_EL1.OSLK to OSLSR_EL1.OSLK.
When saving context, simply stash the value of OSLSR_EL1. On resume,
apply OSLSR_EL1.OSLK to OSLAR_EL1.OSLK.
Finally, Patch 3 raises the KVM debug architecture to v8.2 and exposes
FEAT_DoubleLock as NI to the guest. With the changes to OSLAR_EL1 in
this series, KVM now does what it says on the tin.
This series applies cleanly to 5.15-rc4, and was (lightly) tested by
booting 5.15-rc4 as a kvmtool guest on this kernel.
Oliver Upton (3):
KVM: arm64: Stash OSLSR_EL1 in the cpu context
KVM: arm64: Allow the guest to change the OS Lock status
KVM: arm64: Raise KVM's reported debug architecture to v8.2
arch/arm64/include/asm/kvm_host.h | 1 +
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 5 +++
arch/arm64/kvm/sys_regs.c | 42 ++++++++++++++++------
3 files changed, 37 insertions(+), 11 deletions(-)
--
2.33.0.1079.g6e70778dc9-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context 2021-10-29 0:31 [PATCH 0/3] KVM: arm64: Fixes for the exposed debug architecture Oliver Upton @ 2021-10-29 0:32 ` Oliver Upton 2021-10-29 11:27 ` Marc Zyngier 2021-10-29 0:32 ` [PATCH 2/3] KVM: arm64: Allow the guest to change the OS Lock status Oliver Upton 2021-10-29 0:32 ` [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 Oliver Upton 2 siblings, 1 reply; 9+ messages in thread From: Oliver Upton @ 2021-10-29 0:32 UTC (permalink / raw) To: kvmarm Cc: kvm, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe, Oliver Upton An upcoming change to KVM will context switch the OS Lock status between guest/host. Add OSLSR_EL1 to the cpu context and handle guest reads using the stored value. Signed-off-by: Oliver Upton <oupton@google.com> --- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/sys_regs.c | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index f8be56d5342b..c98f65c4a1f7 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -172,6 +172,7 @@ enum vcpu_sysreg { MDSCR_EL1, /* Monitor Debug System Control Register */ MDCCINT_EL1, /* Monitor Debug Comms Channel Interrupt Enable Reg */ DISR_EL1, /* Deferred Interrupt Status Register */ + OSLSR_EL1, /* OS Lock Status Register */ /* Performance Monitors Registers */ PMCR_EL0, /* Control Register */ diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 1d46e185f31e..0eb03e7508fe 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -291,12 +291,11 @@ static bool trap_oslsr_el1(struct kvm_vcpu *vcpu, struct sys_reg_params *p, const struct sys_reg_desc *r) { - if (p->is_write) { + if (p->is_write) return ignore_write(vcpu, p); - } else { - p->regval = (1 << 3); - return true; - } + + p->regval = vcpu_read_sys_reg(vcpu, r->reg); + return true; } static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, @@ -1441,7 +1440,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_MDRAR_EL1), trap_raz_wi }, { SYS_DESC(SYS_OSLAR_EL1), trap_raz_wi }, - { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1 }, + { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1, reset_val, OSLSR_EL1, 0x00000008 }, { SYS_DESC(SYS_OSDLR_EL1), trap_raz_wi }, { SYS_DESC(SYS_DBGPRCR_EL1), trap_raz_wi }, { SYS_DESC(SYS_DBGCLAIMSET_EL1), trap_raz_wi }, @@ -1916,7 +1915,7 @@ static const struct sys_reg_desc cp14_regs[] = { { Op1( 0), CRn( 1), CRm( 0), Op2( 4), trap_raz_wi }, DBGBXVR(1), /* DBGOSLSR */ - { Op1( 0), CRn( 1), CRm( 1), Op2( 4), trap_oslsr_el1 }, + { Op1( 0), CRn( 1), CRm( 1), Op2( 4), trap_oslsr_el1, NULL, OSLSR_EL1 }, DBGBXVR(2), DBGBXVR(3), /* DBGOSDLR */ -- 2.33.0.1079.g6e70778dc9-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context 2021-10-29 0:32 ` [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context Oliver Upton @ 2021-10-29 11:27 ` Marc Zyngier 0 siblings, 0 replies; 9+ messages in thread From: Marc Zyngier @ 2021-10-29 11:27 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, kvm, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe On Fri, 29 Oct 2021 01:32:00 +0100, Oliver Upton <oupton@google.com> wrote: > > An upcoming change to KVM will context switch the OS Lock status between > guest/host. Add OSLSR_EL1 to the cpu context and handle guest reads > using the stored value. > > Signed-off-by: Oliver Upton <oupton@google.com> > --- > arch/arm64/include/asm/kvm_host.h | 1 + > arch/arm64/kvm/sys_regs.c | 13 ++++++------- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index f8be56d5342b..c98f65c4a1f7 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -172,6 +172,7 @@ enum vcpu_sysreg { > MDSCR_EL1, /* Monitor Debug System Control Register */ > MDCCINT_EL1, /* Monitor Debug Comms Channel Interrupt Enable Reg */ > DISR_EL1, /* Deferred Interrupt Status Register */ > + OSLSR_EL1, /* OS Lock Status Register */ Please move it one line up, next to the rest of the debug stuff (DISR_EL1 is RAS and not debug). > > /* Performance Monitors Registers */ > PMCR_EL0, /* Control Register */ > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 1d46e185f31e..0eb03e7508fe 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -291,12 +291,11 @@ static bool trap_oslsr_el1(struct kvm_vcpu *vcpu, > struct sys_reg_params *p, > const struct sys_reg_desc *r) > { > - if (p->is_write) { > + if (p->is_write) > return ignore_write(vcpu, p); This should be UNDEF (though the HW should catch that, really). > - } else { > - p->regval = (1 << 3); > - return true; > - } > + > + p->regval = vcpu_read_sys_reg(vcpu, r->reg); > + return true; > } > > static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, > @@ -1441,7 +1440,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > { SYS_DESC(SYS_MDRAR_EL1), trap_raz_wi }, > { SYS_DESC(SYS_OSLAR_EL1), trap_raz_wi }, > - { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1 }, > + { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1, reset_val, OSLSR_EL1, 0x00000008 }, > { SYS_DESC(SYS_OSDLR_EL1), trap_raz_wi }, > { SYS_DESC(SYS_DBGPRCR_EL1), trap_raz_wi }, > { SYS_DESC(SYS_DBGCLAIMSET_EL1), trap_raz_wi }, > @@ -1916,7 +1915,7 @@ static const struct sys_reg_desc cp14_regs[] = { > { Op1( 0), CRn( 1), CRm( 0), Op2( 4), trap_raz_wi }, > DBGBXVR(1), > /* DBGOSLSR */ > - { Op1( 0), CRn( 1), CRm( 1), Op2( 4), trap_oslsr_el1 }, > + { Op1( 0), CRn( 1), CRm( 1), Op2( 4), trap_oslsr_el1, NULL, OSLSR_EL1 }, > DBGBXVR(2), > DBGBXVR(3), > /* DBGOSDLR */ Please update tools/testing/selftests/kvm/aarch64/get-reg-list.c before Andrew catches you red-handed! :D M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] KVM: arm64: Allow the guest to change the OS Lock status 2021-10-29 0:31 [PATCH 0/3] KVM: arm64: Fixes for the exposed debug architecture Oliver Upton 2021-10-29 0:32 ` [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context Oliver Upton @ 2021-10-29 0:32 ` Oliver Upton 2021-10-29 11:16 ` Marc Zyngier 2021-10-29 0:32 ` [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 Oliver Upton 2 siblings, 1 reply; 9+ messages in thread From: Oliver Upton @ 2021-10-29 0:32 UTC (permalink / raw) To: kvmarm Cc: kvm, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe, Oliver Upton KVM diverges from the architecture in the way it handles the OSLAR_EL1 register. While the architecture requires that the register be WO and that the OSLK bit is 1 out of reset, KVM implements the register as RAZ/WI. Align KVM with the architecture by permitting writes to OSLAR_EL1. Since the register is WO, stash the OS Lock status bit in OSLSR_EL1 and context switch the status between host/guest. Additionally, change the reset value of the OSLK bit to 1. Suggested-by: Marc Zyngier <maz@kernel.org> Signed-off-by: Oliver Upton <oupton@google.com> --- arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 5 +++++ arch/arm64/kvm/sys_regs.c | 22 +++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h index de7e14c862e6..a65dab34f85b 100644 --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h @@ -65,6 +65,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) ctxt_sys_reg(ctxt, SP_EL1) = read_sysreg(sp_el1); ctxt_sys_reg(ctxt, ELR_EL1) = read_sysreg_el1(SYS_ELR); ctxt_sys_reg(ctxt, SPSR_EL1) = read_sysreg_el1(SYS_SPSR); + + ctxt_sys_reg(ctxt, OSLSR_EL1) = read_sysreg(oslsr_el1); } static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt) @@ -149,6 +151,9 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) write_sysreg(ctxt_sys_reg(ctxt, SP_EL1), sp_el1); write_sysreg_el1(ctxt_sys_reg(ctxt, ELR_EL1), SYS_ELR); write_sysreg_el1(ctxt_sys_reg(ctxt, SPSR_EL1), SYS_SPSR); + + /* restore OSLSR_EL1 by writing the OSLK bit to OSLAR_EL1 */ + write_sysreg((ctxt_sys_reg(ctxt, OSLSR_EL1) >> 1) & 1, oslar_el1); } static inline void __sysreg_restore_el2_return_state(struct kvm_cpu_context *ctxt) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 0eb03e7508fe..0840ae081290 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -298,6 +298,22 @@ static bool trap_oslsr_el1(struct kvm_vcpu *vcpu, return true; } +static bool trap_oslar_el1(struct kvm_vcpu *vcpu, + struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + u64 oslsr; + + if (!p->is_write) + return read_zero(vcpu, p); + + /* preserve all but the OSLK bit */ + oslsr = vcpu_read_sys_reg(vcpu, OSLSR_EL1) & ~0x2ull; + vcpu_write_sys_reg(vcpu, OSLSR_EL1, oslsr | ((p->regval & 1) << 1)); + return true; +} + + static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, struct sys_reg_params *p, const struct sys_reg_desc *r) @@ -1439,8 +1455,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { DBG_BCR_BVR_WCR_WVR_EL1(15), { SYS_DESC(SYS_MDRAR_EL1), trap_raz_wi }, - { SYS_DESC(SYS_OSLAR_EL1), trap_raz_wi }, - { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1, reset_val, OSLSR_EL1, 0x00000008 }, + { SYS_DESC(SYS_OSLAR_EL1), trap_oslar_el1 }, + { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1, reset_val, OSLSR_EL1, 0x0000000A }, { SYS_DESC(SYS_OSDLR_EL1), trap_raz_wi }, { SYS_DESC(SYS_DBGPRCR_EL1), trap_raz_wi }, { SYS_DESC(SYS_DBGCLAIMSET_EL1), trap_raz_wi }, @@ -1912,7 +1928,7 @@ static const struct sys_reg_desc cp14_regs[] = { DBGBXVR(0), /* DBGOSLAR */ - { Op1( 0), CRn( 1), CRm( 0), Op2( 4), trap_raz_wi }, + { Op1( 0), CRn( 1), CRm( 0), Op2( 4), trap_oslar_el1 }, DBGBXVR(1), /* DBGOSLSR */ { Op1( 0), CRn( 1), CRm( 1), Op2( 4), trap_oslsr_el1, NULL, OSLSR_EL1 }, -- 2.33.0.1079.g6e70778dc9-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] KVM: arm64: Allow the guest to change the OS Lock status 2021-10-29 0:32 ` [PATCH 2/3] KVM: arm64: Allow the guest to change the OS Lock status Oliver Upton @ 2021-10-29 11:16 ` Marc Zyngier 0 siblings, 0 replies; 9+ messages in thread From: Marc Zyngier @ 2021-10-29 11:16 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, kvm, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe Hi Oliver, On Fri, 29 Oct 2021 01:32:01 +0100, Oliver Upton <oupton@google.com> wrote: > > KVM diverges from the architecture in the way it handles the OSLAR_EL1 > register. While the architecture requires that the register be WO and > that the OSLK bit is 1 out of reset, KVM implements the register as > RAZ/WI. > > Align KVM with the architecture by permitting writes to OSLAR_EL1. Since > the register is WO, stash the OS Lock status bit in OSLSR_EL1 and > context switch the status between host/guest. Additionally, change the > reset value of the OSLK bit to 1. > > Suggested-by: Marc Zyngier <maz@kernel.org> > Signed-off-by: Oliver Upton <oupton@google.com> > --- > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 5 +++++ > arch/arm64/kvm/sys_regs.c | 22 +++++++++++++++++++--- > 2 files changed, 24 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > index de7e14c862e6..a65dab34f85b 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > @@ -65,6 +65,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) > ctxt_sys_reg(ctxt, SP_EL1) = read_sysreg(sp_el1); > ctxt_sys_reg(ctxt, ELR_EL1) = read_sysreg_el1(SYS_ELR); > ctxt_sys_reg(ctxt, SPSR_EL1) = read_sysreg_el1(SYS_SPSR); > + > + ctxt_sys_reg(ctxt, OSLSR_EL1) = read_sysreg(oslsr_el1); Why do we need to save/restore this outside of the debug context? It seems to me that this is only needed if debug has been enabled by the guest (KVM_ARM64_DEBUG_DIRTY being set), as we will have trapped the OSLAR_EL1 access otherwise. I don't think we need to deal with this register outside of this context, as debug exceptions cannot happen otherwise (BRK excepted, of course). > } > > static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt) > @@ -149,6 +151,9 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) > write_sysreg(ctxt_sys_reg(ctxt, SP_EL1), sp_el1); > write_sysreg_el1(ctxt_sys_reg(ctxt, ELR_EL1), SYS_ELR); > write_sysreg_el1(ctxt_sys_reg(ctxt, SPSR_EL1), SYS_SPSR); > + > + /* restore OSLSR_EL1 by writing the OSLK bit to OSLAR_EL1 */ > + write_sysreg((ctxt_sys_reg(ctxt, OSLSR_EL1) >> 1) & 1, oslar_el1); Please introduce some eye-pleasing bit definitions ("Here, there, and everywhere", to quote someone famous). > } > > static inline void __sysreg_restore_el2_return_state(struct kvm_cpu_context *ctxt) > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 0eb03e7508fe..0840ae081290 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -298,6 +298,22 @@ static bool trap_oslsr_el1(struct kvm_vcpu *vcpu, > return true; > } > > +static bool trap_oslar_el1(struct kvm_vcpu *vcpu, > + struct sys_reg_params *p, > + const struct sys_reg_desc *r) > +{ > + u64 oslsr; > + > + if (!p->is_write) > + return read_zero(vcpu, p); This really should be an UNDEF (and it really should UNDEF in HW, but we are being, maybe pointlessly, cautious). > + > + /* preserve all but the OSLK bit */ > + oslsr = vcpu_read_sys_reg(vcpu, OSLSR_EL1) & ~0x2ull; > + vcpu_write_sys_reg(vcpu, OSLSR_EL1, oslsr | ((p->regval & 1) << 1)); > + return true; > +} > + > + Extra newline. > static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, > struct sys_reg_params *p, > const struct sys_reg_desc *r) > @@ -1439,8 +1455,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { > DBG_BCR_BVR_WCR_WVR_EL1(15), > > { SYS_DESC(SYS_MDRAR_EL1), trap_raz_wi }, > - { SYS_DESC(SYS_OSLAR_EL1), trap_raz_wi }, > - { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1, reset_val, OSLSR_EL1, 0x00000008 }, > + { SYS_DESC(SYS_OSLAR_EL1), trap_oslar_el1 }, > + { SYS_DESC(SYS_OSLSR_EL1), trap_oslsr_el1, reset_val, OSLSR_EL1, 0x0000000A }, > { SYS_DESC(SYS_OSDLR_EL1), trap_raz_wi }, > { SYS_DESC(SYS_DBGPRCR_EL1), trap_raz_wi }, > { SYS_DESC(SYS_DBGCLAIMSET_EL1), trap_raz_wi }, > @@ -1912,7 +1928,7 @@ static const struct sys_reg_desc cp14_regs[] = { > > DBGBXVR(0), > /* DBGOSLAR */ > - { Op1( 0), CRn( 1), CRm( 0), Op2( 4), trap_raz_wi }, > + { Op1( 0), CRn( 1), CRm( 0), Op2( 4), trap_oslar_el1 }, > DBGBXVR(1), > /* DBGOSLSR */ > { Op1( 0), CRn( 1), CRm( 1), Op2( 4), trap_oslsr_el1, NULL, OSLSR_EL1 }, Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 2021-10-29 0:31 [PATCH 0/3] KVM: arm64: Fixes for the exposed debug architecture Oliver Upton 2021-10-29 0:32 ` [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context Oliver Upton 2021-10-29 0:32 ` [PATCH 2/3] KVM: arm64: Allow the guest to change the OS Lock status Oliver Upton @ 2021-10-29 0:32 ` Oliver Upton 2021-10-29 11:31 ` Marc Zyngier 2 siblings, 1 reply; 9+ messages in thread From: Oliver Upton @ 2021-10-29 0:32 UTC (permalink / raw) To: kvmarm Cc: kvm, Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe, Oliver Upton The additions made to the Debug architecture between v8.0 and v8.2 are only applicable to external debug. KVM does not (and likely will never) support external debug, so KVM can proudly report support for v8.2 to its guests. Raise the reported Debug architecture to v8.2. Additionally, v8.2 makes FEAT_DoubleLock optional. Even though KVM never supported it in the first place, report DoubleLock as not implemented now as the architecture permits it for v8.2. Cc: Reiji Watanabe <reijiw@google.com> Cc: Ricardo Koller <ricarkol@google.com> Suggested-by: Marc Zyngier <maz@kernel.org> Signed-off-by: Oliver Upton <oupton@google.com> --- arch/arm64/kvm/sys_regs.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 0840ae081290..f56ee5830d18 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1109,9 +1109,14 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI)); break; case SYS_ID_AA64DFR0_EL1: - /* Limit debug to ARMv8.0 */ + /* Limit debug to ARMv8.2 */ val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER); - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6); + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 8); + + /* Hide DoubleLock from guests */ + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK); + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), 0xf); + /* Limit guests to PMUv3 for ARMv8.4 */ val = cpuid_feature_cap_perfmon_field(val, ID_AA64DFR0_PMUVER_SHIFT, -- 2.33.0.1079.g6e70778dc9-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 2021-10-29 0:32 ` [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 Oliver Upton @ 2021-10-29 11:31 ` Marc Zyngier 2021-10-29 18:18 ` Oliver Upton 0 siblings, 1 reply; 9+ messages in thread From: Marc Zyngier @ 2021-10-29 11:31 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, kvm, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe On Fri, 29 Oct 2021 01:32:02 +0100, Oliver Upton <oupton@google.com> wrote: > > The additions made to the Debug architecture between v8.0 and v8.2 are > only applicable to external debug. KVM does not (and likely will never) > support external debug, so KVM can proudly report support for v8.2 to > its guests. > > Raise the reported Debug architecture to v8.2. Additionally, v8.2 makes > FEAT_DoubleLock optional. Even though KVM never supported it in the > first place, report DoubleLock as not implemented now as the > architecture permits it for v8.2. > > Cc: Reiji Watanabe <reijiw@google.com> > Cc: Ricardo Koller <ricarkol@google.com> > Suggested-by: Marc Zyngier <maz@kernel.org> > Signed-off-by: Oliver Upton <oupton@google.com> > --- > arch/arm64/kvm/sys_regs.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 0840ae081290..f56ee5830d18 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1109,9 +1109,14 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, > ARM64_FEATURE_MASK(ID_AA64ISAR1_GPI)); > break; > case SYS_ID_AA64DFR0_EL1: > - /* Limit debug to ARMv8.0 */ > + /* Limit debug to ARMv8.2 */ > val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER); > - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6); > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 8); > + > + /* Hide DoubleLock from guests */ > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK); > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), 0CF); > + One issue with that is that this will break migration from an older kernel (DFR0 will be different between source and destination). You'll need a set_user handler and deal with it in a similar way to CSV2/CSV3. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 2021-10-29 11:31 ` Marc Zyngier @ 2021-10-29 18:18 ` Oliver Upton 2021-11-01 10:21 ` Marc Zyngier 0 siblings, 1 reply; 9+ messages in thread From: Oliver Upton @ 2021-10-29 18:18 UTC (permalink / raw) To: Marc Zyngier Cc: kvmarm, kvm, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe Hey Marc, On Fri, Oct 29, 2021 at 4:31 AM Marc Zyngier <maz@kernel.org> wrote: > > On Fri, 29 Oct 2021 01:32:02 +0100, > Oliver Upton <oupton@google.com> wrote: [...] > > case SYS_ID_AA64DFR0_EL1: > > - /* Limit debug to ARMv8.0 */ > > + /* Limit debug to ARMv8.2 */ > > val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER); > > - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6); > > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 8); > > + > > + /* Hide DoubleLock from guests */ > > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK); > > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), 0CF); > > + > > One issue with that is that this will break migration from an older > kernel (DFR0 will be different between source and destination). > > You'll need a set_user handler and deal with it in a similar way to > CSV2/CSV3. Yeah, definitely so. In that case, unless we're strongly motivated to expose these changes soon, I'll just punt the ID register changes until Reiji's series [1] lands, as anything I add for a writable DFR0 will invariably be scrapped in favor of his work. I'll post v2 of this series folding in your feedback (thx for quick review, btw), less this patch. [1] https://patchwork.kernel.org/project/kvm/cover/20211012043535.500493-1-reijiw@google.com/ -- Thanks, Oliver ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 2021-10-29 18:18 ` Oliver Upton @ 2021-11-01 10:21 ` Marc Zyngier 0 siblings, 0 replies; 9+ messages in thread From: Marc Zyngier @ 2021-11-01 10:21 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, kvm, James Morse, Alexandru Elisei, Suzuki K Poulose, linux-arm-kernel, Andrew Jones, Peter Shier, Ricardo Koller, Reiji Watanabe On Fri, 29 Oct 2021 19:18:13 +0100, Oliver Upton <oupton@google.com> wrote: > > Hey Marc, > > On Fri, Oct 29, 2021 at 4:31 AM Marc Zyngier <maz@kernel.org> wrote: > > > > On Fri, 29 Oct 2021 01:32:02 +0100, > > Oliver Upton <oupton@google.com> wrote: > [...] > > > case SYS_ID_AA64DFR0_EL1: > > > - /* Limit debug to ARMv8.0 */ > > > + /* Limit debug to ARMv8.2 */ > > > val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER); > > > - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6); > > > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 8); > > > + > > > + /* Hide DoubleLock from guests */ > > > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK); > > > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DOUBLELOCK), 0CF); > > > + > > > > One issue with that is that this will break migration from an older > > kernel (DFR0 will be different between source and destination). > > > > You'll need a set_user handler and deal with it in a similar way to > > CSV2/CSV3. > > Yeah, definitely so. In that case, unless we're strongly motivated to > expose these changes soon, I'll just punt the ID register changes > until Reiji's series [1] lands, as anything I add for a writable DFR0 > will invariably be scrapped in favor of his work. Yeah, I think that's a sensible thing to do. I need to find the bandwidth to look into these patches... > I'll post v2 of this series folding in your feedback (thx for quick > review, btw), less this patch. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-01 10:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-29 0:31 [PATCH 0/3] KVM: arm64: Fixes for the exposed debug architecture Oliver Upton 2021-10-29 0:32 ` [PATCH 1/3] KVM: arm64: Stash OSLSR_EL1 in the cpu context Oliver Upton 2021-10-29 11:27 ` Marc Zyngier 2021-10-29 0:32 ` [PATCH 2/3] KVM: arm64: Allow the guest to change the OS Lock status Oliver Upton 2021-10-29 11:16 ` Marc Zyngier 2021-10-29 0:32 ` [PATCH 3/3] KVM: arm64: Raise KVM's reported debug architecture to v8.2 Oliver Upton 2021-10-29 11:31 ` Marc Zyngier 2021-10-29 18:18 ` Oliver Upton 2021-11-01 10:21 ` Marc Zyngier
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).