| Message ID | 20241008083901.72850-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] x86/msr: add log messages to MSR state load error paths | expand |
On 08.10.2024 10:39, Roger Pau Monne wrote: > Some error paths in the MSR state loading logic don't contain error messages, > which makes debugging them quite hard without adding extra patches to print the > information. > > Add two new log messages to the MSR state load path that print information > about the entry that failed to load, for both PV and HVM. > > While there also adjust XEN_DOMCTL_set_vcpu_msrs to return -ENXIO in case the > MSR is unhandled or can't be loaded, so it matches the error code used by HVM > MSR loading (and it's less ambiguous than -EINVAL). > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> > --- > Changes since v1: > - Add messages to PV MSR loading. > - Return -ENXIO from PV loading path also. I'm not fully convinced of the need to change the error code, but I also don't mind it. Jan
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 96d816cf1a7d..f76de5be9437 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -1166,6 +1166,7 @@ long arch_do_domctl( if ( msr.reserved ) break; + ret = -ENXIO; switch ( msr.index ) { case MSR_SPEC_CTRL: @@ -1174,9 +1175,16 @@ long arch_do_domctl( case MSR_AMD64_DR0_ADDRESS_MASK: case MSR_AMD64_DR1_ADDRESS_MASK ... MSR_AMD64_DR3_ADDRESS_MASK: if ( guest_wrmsr(v, msr.index, msr.value) != X86EMUL_OKAY ) + { + printk(XENLOG_G_ERR + "%pv load MSR %#x with value %#lx failed\n", + v, msr.index, msr.value); break; + } continue; } + printk(XENLOG_G_ERR "%pv attempted load of unhandled MSR %#x\n", + v, msr.index); break; } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 69a25571db8d..200f0a414138 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -1598,10 +1598,17 @@ static int cf_check hvm_load_cpu_msrs(struct domain *d, hvm_domain_context_t *h) rc = guest_wrmsr(v, ctxt->msr[i].index, ctxt->msr[i].val); if ( rc != X86EMUL_OKAY ) + { + printk(XENLOG_G_ERR + "HVM %pv load MSR %#x with value %#lx failed\n", + v, ctxt->msr[i].index, ctxt->msr[i].val); return -ENXIO; + } break; default: + printk(XENLOG_G_ERR "HVM %pv attempted load of unhandled MSR %#x\n", + v, ctxt->msr[i].index); return -ENXIO; } }
Some error paths in the MSR state loading logic don't contain error messages, which makes debugging them quite hard without adding extra patches to print the information. Add two new log messages to the MSR state load path that print information about the entry that failed to load, for both PV and HVM. While there also adjust XEN_DOMCTL_set_vcpu_msrs to return -ENXIO in case the MSR is unhandled or can't be loaded, so it matches the error code used by HVM MSR loading (and it's less ambiguous than -EINVAL). Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - Add messages to PV MSR loading. - Return -ENXIO from PV loading path also. - Do not print the return code from guest_wrmsr(), it's not specially helpful given the current logic, as it will always be X86EMUL_EXCEPTION in case of failure. --- xen/arch/x86/domctl.c | 8 ++++++++ xen/arch/x86/hvm/hvm.c | 7 +++++++ 2 files changed, 15 insertions(+)