On 17.11.2023 10:18, James Dingwall wrote: > On Thu, Nov 16, 2023 at 04:32:47PM +0000, Andrew Cooper wrote: >> On 16/11/2023 4:15 pm, James Dingwall wrote: >>> Hi, >>> >>> Per the msr_relaxed documentation: >>> >>> "If using this option is necessary to fix an issue, please report a bug." >>> >>> After recently upgrading an environment from Xen 4.14.5 to Xen 4.15.5 we >>> started experiencing a BSOD at boot with one of our Windows guests. We >>> found >>> that enabling `msr_relaxed = 1` in the guest configuration has resolved the >>> problem. With a debug build of Xen and `hvm_debug=2048` on the command line >>> the following messages were caught as the BSOD happened: >>> >>> (XEN) [HVM:11.0] <vmx_msr_read_intercept> ecx=0x1a2 >>> (XEN) vmx.c:3298:d11v0 RDMSR 0x000001a2 unimplemented >>> (XEN) d11v0 VIRIDIAN CRASH: 1e ffffffffc0000096 fffff80b8de81eb5 0 0 >>> >>> I found that MSR 0x1a2 is MSR_TEMPERATURE_TARGET and from that this patch >>> series from last month: >>> >>> https://patchwork.kernel.org/project/xen-devel/list/?series=796550 >>> >>> Picking out just a small part of that fixes the problem for us. Although the >>> the patch is against 4.15.5 I think it would be relevant to more recent >>> releases too. >> >> Which version of Windows, and what hardware? >> >> The Viridian Crash isn't about the RDMSR itself - it's presumably >> collateral damage shortly thereafter. >> >> Does filling in 0 for that MSR also resolve the issue? It's model >> specific and we absolutely cannot pass it through from real hardware >> like that. >> > > Hi Andrew, > > Thanks for your response. The guest is running Windows 10 and the crash > happens in a proprietary hardware driver. A little bit of knowledge as > they say was enough to stop the crash but I don't understand the impact > of what I've actually done... > > To rework the patch I'd need a bit of guidance, if I understand your > suggestion I set the MSR to 0 with this change in emul-priv-op.c:
For the purpose of the experiment suggested by Andrew ... > diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c > index ed97b1d6fcc..66f5e417df6 100644 > --- a/xen/arch/x86/pv/emul-priv-op.c > +++ b/xen/arch/x86/pv/emul-priv-op.c > @@ -976,6 +976,10 @@ static int read_msr(unsigned int reg, uint64_t *val, > *val = 0; > return X86EMUL_OKAY; > > + case MSR_TEMPERATURE_TARGET: > + *val = 0; > + return X86EMUL_OKAY; > + > case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR(7): > case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL(3): > case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2: ... you wouldn't need this (affects PV domains only), and ... > and this in vmx.c: > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 54023a92587..bbf37b7f272 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -3259,6 +3259,11 @@ static int vmx_msr_read_intercept(unsigned int msr, > uint64_t *msr_content) > if ( !nvmx_msr_read_intercept(msr, msr_content) ) > goto gp_fault; > break; > + > + case MSR_TEMPERATURE_TARGET: > + *msr_content = 0; > + break; > + > case MSR_IA32_MISC_ENABLE: > rdmsrl(MSR_IA32_MISC_ENABLE, *msr_content); > /* Debug Trace Store is not supported. */ ... indeed this ought to do. An eventual real patch may want to look different, though. Jan
