* [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements
@ 2019-12-05 18:06 Mark Rutland
2019-12-05 18:06 ` [PATCH 1/2] kvm/arm64: sanely ratelimit sysreg messages Mark Rutland
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mark Rutland @ 2019-12-05 18:06 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: maz, kvmarm
While testing some other patches, I realised that KVM's logging of
trapped sysreg accesses can log inconsistent information, and this is
arguably unnecessary for IMPLEMENTATION DEFINED system registers.
This patches fix that up, ensureing that logged information is
consistent, and avoiding logging for IMPLEMENTATION DEFINED registers.
Mark.
Mark Rutland (2):
kvm/arm64: sanely ratelimit sysreg messages
kvm/arm64: don't log IMP DEF sysreg traps
arch/arm64/kvm/sys_regs.c | 20 ++++++++++++++------
arch/arm64/kvm/sys_regs.h | 17 +++++++++++++++--
2 files changed, 29 insertions(+), 8 deletions(-)
--
2.11.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] kvm/arm64: sanely ratelimit sysreg messages 2019-12-05 18:06 [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements Mark Rutland @ 2019-12-05 18:06 ` Mark Rutland 2019-12-05 18:06 ` [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps Mark Rutland 2019-12-06 11:44 ` [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements Marc Zyngier 2 siblings, 0 replies; 6+ messages in thread From: Mark Rutland @ 2019-12-05 18:06 UTC (permalink / raw) To: linux-arm-kernel; +Cc: maz, kvmarm Currently kvm_pr_unimpl() is ratelimited, so print_sys_reg_instr() won't spam the console. However, someof its callers try to print some contextual information with kvm_err(), which is not ratelimited. This means that in some cases the context may be printed without the sysreg encoding, which isn't all that useful. Let's ensure that both are consistently printed together and ratelimited, by refactoring print_sys_reg_instr() so that some callers can provide it with an arbitrary format string. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Alexandru Elisei <alexandru.elisei@arm.com> Cc: James Morse <james.morse@arm.com> Cc: Julien Thierry <julien.thierry.kdev@gmail.com> Cc: Marc Zyngier <maz@kernel.org> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: kvmarm@lists.cs.columbia.edu --- arch/arm64/kvm/sys_regs.c | 12 ++++++------ arch/arm64/kvm/sys_regs.h | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 46822afc57e0..d128abd38656 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2098,9 +2098,9 @@ static void unhandled_cp_access(struct kvm_vcpu *vcpu, WARN_ON(1); } - kvm_err("Unsupported guest CP%d access at: %08lx [%08lx]\n", - cp, *vcpu_pc(vcpu), *vcpu_cpsr(vcpu)); - print_sys_reg_instr(params); + print_sys_reg_msg(params, + "Unsupported guest CP%d access at: %08lx [%08lx]\n", + cp, *vcpu_pc(vcpu), *vcpu_cpsr(vcpu)); kvm_inject_undefined(vcpu); } @@ -2249,9 +2249,9 @@ static int emulate_sys_reg(struct kvm_vcpu *vcpu, if (likely(r)) { perform_access(vcpu, params, r); } else { - kvm_err("Unsupported guest sys_reg access at: %lx [%08lx]\n", - *vcpu_pc(vcpu), *vcpu_cpsr(vcpu)); - print_sys_reg_instr(params); + print_sys_reg_msg(params, + "Unsupported guest sys_reg access at: %lx [%08lx]\n", + *vcpu_pc(vcpu), *vcpu_cpsr(vcpu)); kvm_inject_undefined(vcpu); } return 1; diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h index 9bca0312d798..5a6fc30f5989 100644 --- a/arch/arm64/kvm/sys_regs.h +++ b/arch/arm64/kvm/sys_regs.h @@ -62,11 +62,24 @@ struct sys_reg_desc { #define REG_HIDDEN_USER (1 << 0) /* hidden from userspace ioctls */ #define REG_HIDDEN_GUEST (1 << 1) /* hidden from guest */ -static inline void print_sys_reg_instr(const struct sys_reg_params *p) +static __printf(2, 3) +inline void print_sys_reg_msg(const struct sys_reg_params *p, + char *fmt, ...) { + va_list va; + + va_start(va, fmt); /* Look, we even formatted it for you to paste into the table! */ - kvm_pr_unimpl(" { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u), func_%s },\n", + kvm_pr_unimpl("%pV { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u), func_%s },\n", + &(struct va_format){ fmt, &va }, p->Op0, p->Op1, p->CRn, p->CRm, p->Op2, p->is_write ? "write" : "read"); + va_end(va); +} + +static inline void print_sys_reg_instr(const struct sys_reg_params *p) +{ + /* GCC warns on an empty format string */ + print_sys_reg_msg(p, "%s", ""); } static inline bool ignore_write(struct kvm_vcpu *vcpu, -- 2.11.0 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps 2019-12-05 18:06 [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements Mark Rutland 2019-12-05 18:06 ` [PATCH 1/2] kvm/arm64: sanely ratelimit sysreg messages Mark Rutland @ 2019-12-05 18:06 ` Mark Rutland 2019-12-06 19:35 ` Marc Zyngier 2019-12-06 11:44 ` [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements Marc Zyngier 2 siblings, 1 reply; 6+ messages in thread From: Mark Rutland @ 2019-12-05 18:06 UTC (permalink / raw) To: linux-arm-kernel; +Cc: maz, kvmarm We don't intend to support IMPLEMENATION DEFINED system registers, but have to trap them (and emulate them as UNDEFINED). These traps aren't interesting to the system administrator or to the KVM developers, so let's not bother logging when we do so. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Alexandru Elisei <alexandru.elisei@arm.com> Cc: James Morse <james.morse@arm.com> Cc: Julien Thierry <julien.thierry.kdev@gmail.com> Cc: Marc Zyngier <maz@kernel.org> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: kvmarm@lists.cs.columbia.edu --- arch/arm64/kvm/sys_regs.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index d128abd38656..61f019104841 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2233,6 +2233,12 @@ int kvm_handle_cp14_32(struct kvm_vcpu *vcpu, struct kvm_run *run) NULL, 0); } +static bool is_imp_def_sys_reg(struct sys_reg_params *params) +{ + // See ARM DDI 0487E.a, section D12.3.2 + return params->Op0 == 3 && (params->CRn & 0b1011) == 0b1011; +} + static int emulate_sys_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *params) { @@ -2248,6 +2254,8 @@ static int emulate_sys_reg(struct kvm_vcpu *vcpu, if (likely(r)) { perform_access(vcpu, params, r); + } else if (is_imp_def_sysreg(params)) { + kvm_inject_undefined(vcpu); } else { print_sys_reg_msg(params, "Unsupported guest sys_reg access at: %lx [%08lx]\n", -- 2.11.0 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps 2019-12-05 18:06 ` [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps Mark Rutland @ 2019-12-06 19:35 ` Marc Zyngier 2019-12-09 10:32 ` Mark Rutland 0 siblings, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2019-12-06 19:35 UTC (permalink / raw) To: Mark Rutland; +Cc: linux-arm-kernel, kvmarm On Thu, 05 Dec 2019 18:06:52 +0000, Mark Rutland <mark.rutland@arm.com> wrote: > > We don't intend to support IMPLEMENATION DEFINED system registers, but > have to trap them (and emulate them as UNDEFINED). These traps aren't > interesting to the system administrator or to the KVM developers, so > let's not bother logging when we do so. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Cc: Alexandru Elisei <alexandru.elisei@arm.com> > Cc: James Morse <james.morse@arm.com> > Cc: Julien Thierry <julien.thierry.kdev@gmail.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: kvmarm@lists.cs.columbia.edu > --- > arch/arm64/kvm/sys_regs.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index d128abd38656..61f019104841 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2233,6 +2233,12 @@ int kvm_handle_cp14_32(struct kvm_vcpu *vcpu, struct kvm_run *run) > NULL, 0); > } > > +static bool is_imp_def_sys_reg(struct sys_reg_params *params) > +{ > + // See ARM DDI 0487E.a, section D12.3.2 > + return params->Op0 == 3 && (params->CRn & 0b1011) == 0b1011; > +} > + > static int emulate_sys_reg(struct kvm_vcpu *vcpu, > struct sys_reg_params *params) > { > @@ -2248,6 +2254,8 @@ static int emulate_sys_reg(struct kvm_vcpu *vcpu, > > if (likely(r)) { > perform_access(vcpu, params, r); > + } else if (is_imp_def_sysreg(params)) { Meh. Doesn't compile... :-( Fixing it locally. M. -- Jazz is not dead, it just smells funny. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps 2019-12-06 19:35 ` Marc Zyngier @ 2019-12-09 10:32 ` Mark Rutland 0 siblings, 0 replies; 6+ messages in thread From: Mark Rutland @ 2019-12-09 10:32 UTC (permalink / raw) To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm On Fri, Dec 06, 2019 at 07:35:56PM +0000, Marc Zyngier wrote: > On Thu, 05 Dec 2019 18:06:52 +0000, > Mark Rutland <mark.rutland@arm.com> wrote: > > > > We don't intend to support IMPLEMENATION DEFINED system registers, but > > have to trap them (and emulate them as UNDEFINED). These traps aren't > > interesting to the system administrator or to the KVM developers, so > > let's not bother logging when we do so. > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Cc: Alexandru Elisei <alexandru.elisei@arm.com> > > Cc: James Morse <james.morse@arm.com> > > Cc: Julien Thierry <julien.thierry.kdev@gmail.com> > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > Cc: kvmarm@lists.cs.columbia.edu > > --- > > arch/arm64/kvm/sys_regs.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index d128abd38656..61f019104841 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -2233,6 +2233,12 @@ int kvm_handle_cp14_32(struct kvm_vcpu *vcpu, struct kvm_run *run) > > NULL, 0); > > } > > > > +static bool is_imp_def_sys_reg(struct sys_reg_params *params) > > +{ > > + // See ARM DDI 0487E.a, section D12.3.2 > > + return params->Op0 == 3 && (params->CRn & 0b1011) == 0b1011; > > +} > > + > > static int emulate_sys_reg(struct kvm_vcpu *vcpu, > > struct sys_reg_params *params) > > { > > @@ -2248,6 +2254,8 @@ static int emulate_sys_reg(struct kvm_vcpu *vcpu, > > > > if (likely(r)) { > > perform_access(vcpu, params, r); > > + } else if (is_imp_def_sysreg(params)) { > > Meh. Doesn't compile... :-( > Fixing it locally. Whoops, sorry about that. I "fixed" this at the last moment to match emulate_sys_reg(), but evidently failed to rebuild. I had tested the patch before the rename on my machine, at least. Mark. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements 2019-12-05 18:06 [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements Mark Rutland 2019-12-05 18:06 ` [PATCH 1/2] kvm/arm64: sanely ratelimit sysreg messages Mark Rutland 2019-12-05 18:06 ` [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps Mark Rutland @ 2019-12-06 11:44 ` Marc Zyngier 2 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2019-12-06 11:44 UTC (permalink / raw) To: Mark Rutland; +Cc: linux-arm-kernel, kvmarm On 2019-12-05 18:06, Mark Rutland wrote: > While testing some other patches, I realised that KVM's logging of > trapped sysreg accesses can log inconsistent information, and this is > arguably unnecessary for IMPLEMENTATION DEFINED system registers. > > This patches fix that up, ensureing that logged information is > consistent, and avoiding logging for IMPLEMENTATION DEFINED > registers. > > Mark. > > Mark Rutland (2): > kvm/arm64: sanely ratelimit sysreg messages > kvm/arm64: don't log IMP DEF sysreg traps > > arch/arm64/kvm/sys_regs.c | 20 ++++++++++++++------ > arch/arm64/kvm/sys_regs.h | 17 +++++++++++++++-- > 2 files changed, 29 insertions(+), 8 deletions(-) Applied, thanks. M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-09 10:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-05 18:06 [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements Mark Rutland 2019-12-05 18:06 ` [PATCH 1/2] kvm/arm64: sanely ratelimit sysreg messages Mark Rutland 2019-12-05 18:06 ` [PATCH 2/2] kvm/arm64: don't log IMP DEF sysreg traps Mark Rutland 2019-12-06 19:35 ` Marc Zyngier 2019-12-09 10:32 ` Mark Rutland 2019-12-06 11:44 ` [PATCH 0/2] kvm/arm64: unimplemented sysreg improvements 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).