Re: [PATCH v2 2/3] x86: Expose Automatic IBRS to guests
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
Re: [PATCH v2 2/3] x86: Expose Automatic IBRS to guests
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
Re: [PATCH v2 2/3] x86: Expose Automatic IBRS to guests
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 > Reviewed-by: Andrew Cooper 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
[PATCH v2 2/3] x86: Expose Automatic IBRS to guests
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
Reviewed-by: Andrew Cooper
---
v2:
* Moved to patch2 from v1/patch3
---
xen/arch/x86/hvm/hvm.c | 3 +++
xen/arch/x86/include/asm/msr-index.h| 3 ++-
xen/arch/x86/pv/emul-priv-op.c | 4 ++--
xen/include/public/arch-x86/cpufeatureset.h | 2 +-
4 files changed, 8 insertions(+), 4 deletions(-)
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_STAR0xc081 /* legacy mode SYSCALL
target */
#define MSR_LSTAR 0xc082 /* 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 0x8021.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 0x0007:1.ebx, word 12 */
--
2.34.1
