| Message ID | 20230530135854.1517-3-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Automatic IBRS support | expand |
On 30/05/2023 2:58 pm, Alejandro Vallejo wrote: > Expose AutoIBRS to HVM guests. EFER is swapped by VMRUN, so Xen only has to > make sure writes to EFER.AIBRSE are gated on the feature being exposed. > > Also hide EFER.AIBRSE from PV guests as they have no say in the matter. > > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> I've committed this, but made two tweaks to the commit message. First, "x86/hvm" in the subject because it's important context at a glance. Second, I've adjusted the bit about PV guests. The reason why we can't expose it yet is because Xen doesn't currently context switch EFER between PV guests. ~Andrew
On Tue, May 30, 2023 at 06:31:03PM +0100, Andrew Cooper wrote: > I've committed this, but made two tweaks to the commit message. First, > "x86/hvm" in the subject because it's important context at a glance. Sure, that makes sense. > Second, I've adjusted the bit about PV guests. The reason why we can't > expose it yet is because Xen doesn't currently context switch EFER > between PV guests. > > ~Andrew We could of course context switch EFER sensibly, but what would that mean for Automatic IBRS? It can't be trivially used for domain-to-domain isolation because every domain is in a co-equal protection level. Is there a non-obvious edge that exposing some interface to it gives for PV? The only useful case I can think of is PVH, and that seems to be subsumed by HVM. Alejandro
On 31/05/2023 10:01 am, Alejandro Vallejo wrote: > On Tue, May 30, 2023 at 06:31:03PM +0100, Andrew Cooper wrote: >> I've committed this, but made two tweaks to the commit message. First, >> "x86/hvm" in the subject because it's important context at a glance. > Sure, that makes sense. > >> Second, I've adjusted the bit about PV guests. The reason why we can't >> expose it yet is because Xen doesn't currently context switch EFER >> between PV guests. >> >> ~Andrew > We could of course context switch EFER sensibly, but what would that mean > for Automatic IBRS? It can't be trivially used for domain-to-domain > isolation because every domain is in a co-equal protection level. Is there > a non-obvious edge that exposing some interface to it gives for PV? The > only useful case I can think of is PVH, and that seems to be subsumed by > HVM. Hence why it's fine to not worry about PV for now. Right now, when we decide to use IBRS on AMD, we set it unilaterally. This turns out to be better performance than flipping it on privilege changes (whether that's non-Xen <-> Xen, or guest user <-> kernel). PV guests are obscure corner cases these days, and fall outside of anything the hardware vendors care about when it comes to prediction mode. The only sane option is to have Xen explicitly tell the the PV guest what Xen is doing, and let the guest decide if it wants to do anything further in terms of protections. ~Andrew
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index d7d31b5393..2d6e4bb9c6 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -936,6 +936,9 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value, if ( (value & EFER_FFXSE) && !p->extd.ffxsr ) return "FFXSE without feature"; + if ( (value & EFER_AIBRSE) && !p->extd.auto_ibrs ) + return "AutoIBRS without feature"; + return NULL; } diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h index 73d0af2615..49cb334c61 100644 --- a/xen/arch/x86/include/asm/msr-index.h +++ b/xen/arch/x86/include/asm/msr-index.h @@ -178,7 +178,8 @@ #define EFER_AIBRSE (_AC(1, ULL) << 21) /* Automatic IBRS Enable */ #define EFER_KNOWN_MASK \ - (EFER_SCE | EFER_LME | EFER_LMA | EFER_NXE | EFER_SVME | EFER_FFXSE) + (EFER_SCE | EFER_LME | EFER_LMA | EFER_NXE | EFER_SVME | EFER_FFXSE | \ + EFER_AIBRSE) #define MSR_STAR 0xc0000081 /* legacy mode SYSCALL target */ #define MSR_LSTAR 0xc0000082 /* long mode SYSCALL target */ diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c index 8a4ef9c35e..142bc4818c 100644 --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -853,8 +853,8 @@ static uint64_t guest_efer(const struct domain *d) { uint64_t val; - /* Hide unknown bits, and unconditionally hide SVME from guests. */ - val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME; + /* Hide unknown bits, and unconditionally hide SVME and AIBRSE from guests. */ + val = read_efer() & EFER_KNOWN_MASK & ~(EFER_SVME | EFER_AIBRSE); /* * Hide the 64-bit features from 32-bit guests. SCE has * vendor-dependent behaviour. diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index 3ac144100e..51d737a125 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -287,7 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A AVX-IFMA Instructions */ /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */ XEN_CPUFEATURE(LFENCE_DISPATCH, 11*32+ 2) /*A LFENCE always serializing */ XEN_CPUFEATURE(NSCB, 11*32+ 6) /*A Null Selector Clears Base (and limit too) */ -XEN_CPUFEATURE(AUTO_IBRS, 11*32+ 8) /* HW can handle IBRS on its own */ +XEN_CPUFEATURE(AUTO_IBRS, 11*32+ 8) /*S HW can handle IBRS on its own */ XEN_CPUFEATURE(CPUID_USER_DIS, 11*32+17) /* CPUID disable for CPL > 0 software */ /* Intel-defined CPU features, CPUID level 0x00000007:1.ebx, word 12 */