On 16.08.2024 13:12, Sergiy Kibrik wrote: > Do not compile handlers of guest access to AMD-specific MSRs when > CONFIG_AMD=n.
What I'm missing in the description is clarification on how boundaries were drawn. In guest_rdmsr() there is, for example, also handling of MSR_AMD_PATCHLEVEL. Which I'm okay to leave aside for now, as long as it's clear why that is. > --- a/xen/arch/x86/msr.c > +++ b/xen/arch/x86/msr.c > @@ -219,6 +219,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t > *val) > *val = msrs->tsc_aux; > break; > > +#ifdef CONFIG_AMD > case MSR_K8_SYSCFG: > case MSR_K8_TOP_MEM1: > case MSR_K8_TOP_MEM2: > @@ -281,6 +282,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t > *val) > ? 0 : (msr - MSR_AMD64_DR1_ADDRESS_MASK + > 1), > ARRAY_SIZE(msrs->dr_mask))]; > break; > +#endif /* CONFIG_AMD */ > > /* > * TODO: Implement when we have better topology representation. > @@ -552,6 +554,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t > val) > wrmsr_tsc_aux(val); > break; > > +#ifdef CONFIG_AMD > case MSR_VIRT_SPEC_CTRL: > if ( !cp->extd.virt_ssbd ) > goto gp_fault; > @@ -598,6 +601,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t > val) > if ( v == curr && (curr->arch.dr7 & DR7_ACTIVE_MASK) ) > wrmsrl(msr, val); > break; > +#endif /* CONFIG_AMD */ > > default: > return X86EMUL_UNHANDLEABLE; Is just adding #ifdef-s actually correct? That results in different behavior on e.g. Intel hardware, I think, depending on whether AMD=y or AMD=n. In the latter case the function will now return X86EMUL_UNHANDLEABLE, while in the former case it would return X86EMUL_EXCEPTION. Jan
