On 30/01/2024 9:13 am, Roger Pau Monne wrote: > Roger Pau Monne (3): > x86/intel: expose IPRED_CTRL to guests > x86/intel: expose RRSBA_CTRL to guests > x86/intel: expose BHI_CTRL to guests
A couple of things. First, diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c index a04a11858045..382bc07785d0 100644 --- a/xen/arch/x86/msr.c +++ b/xen/arch/x86/msr.c @@ -309,8 +309,8 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val) /* * Caller to confirm that MSR_SPEC_CTRL is available. Intel and AMD have - * separate CPUID features for this functionality, but only set will be - * active. + * separate CPUID features for some of this functionality, but only one set + * will be active on a single host. */ uint64_t msr_spec_ctrl_valid_bits(const struct cpu_policy *cp) { There was a typo (missing the one in "but only one set"), but you're also adding bits now which are Intel-only and very likely to stay that way. IPRED_CTRL finally gives Intel CPUs a control with a similar security property to AMD IBRS; i.e. I doubt AMD are going to gain support for these bits when they already guarantee that property and have done for years already. Next, I can't say that I particularly love that indentation. This seems marginally less bad return (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | (ssbd ? SPEC_CTRL_SSBD : 0) | (psfd ? SPEC_CTRL_PSFD : 0) | (cp->feat.ipred_ctrl ? (SPEC_CTRL_IPRED_DIS_U | SPEC_CTRL_IPRED_DIS_S) : 0) | (cp->feat.rrsba_ctrl ? (SPEC_CTRL_RRSBA_DIS_U | SPEC_CTRL_RRSBA_DIS_S) : 0) | (cp->feat.bhi_ctrl ? SPEC_CTRL_BHI_DIS_S : 0) | 0); insofar as at least it's fewer lines. Given the length of these new constants, I can't think of anything better. Happy to fix both on commit. ~Andrew