diff mbox series

[XEN,v2,2/5] x86/amd: configurable handling of AMD-specific MSRs access

Message ID 917d2186db56c6f4c820f6b9e26b29fbe93301d6.1723806405.git.Sergiy_Kibrik@epam.com (mailing list archive)
State New
Headers show
Series x86/CPU: optional build of Intel/AMD CPUs support | expand

Commit Message

Sergiy Kibrik Aug. 16, 2024, 11:12 a.m. UTC
Do not compile handlers of guest access to AMD-specific MSRs when CONFIG_AMD=n.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/msr.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jan Beulich Aug. 19, 2024, 12:21 p.m. UTC | #1
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
diff mbox series

Patch

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 289cf10b78..4567de7fc8 100644
--- 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;