* [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests
@ 2018-05-07 10:00 Andrew Cooper
2018-05-07 10:43 ` Jan Beulich
2018-05-07 16:21 ` Brian Woods
0 siblings, 2 replies; 4+ messages in thread
From: Andrew Cooper @ 2018-05-07 10:00 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Brian Woods, Suravee Suthikulpanit, Jan Beulich
We don't advertise SVM in CPUID so a PV guest shouldn't be under the
impression that it can use SVM functionality, but despite this, it really
shouldn't see SVME set when reading EFER.
On Intel processors, 32bit PV guests don't see, and can't use SYSCALL.
Introduce EFER_KNOWN_MASK to whitelist the features Xen knows about, and use
this to clamp the guests view.
Take the opportunity to reuse the mask to simplify svm_vmcb_isvalid(), and
change "undefined" to "unknown" in the print message, as there is at least
EFER.TCE (Translation Cache Extension) defined but unknown to Xen.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
CC: Brian Woods <brian.woods@amd.com>
Arguably, this wants backporting to the stable trees, so should be considered
for 4.11 at this point.
---
xen/arch/x86/hvm/svm/svmdebug.c | 5 ++---
xen/arch/x86/pv/emul-priv-op.c | 11 +++++++++--
xen/include/asm-x86/msr-index.h | 3 +++
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index 6c215d1..d35e405 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -133,9 +133,8 @@ bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
PRINTF("DR7: bits [63:32] are not zero (%#"PRIx64")\n",
vmcb_get_dr7(vmcb));
- if ( efer & ~(EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | EFER_SVME |
- EFER_LMSLE | EFER_FFXSE) )
- PRINTF("EFER: undefined bits are not zero (%#"PRIx64")\n", efer);
+ if ( efer & ~EFER_KNOWN_MASK )
+ PRINTF("EFER: unknown bits are not zero (%#"PRIx64")\n", efer);
if ( hvm_efer_valid(v, efer, -1) )
PRINTF("EFER: %s (%"PRIx64")\n", hvm_efer_valid(v, efer, -1), efer);
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 15f42b3..ce2ec76 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -867,9 +867,16 @@ static int read_msr(unsigned int reg, uint64_t *val,
return X86EMUL_OKAY;
case MSR_EFER:
- *val = read_efer();
+ /* Hide unknown bits, and unconditionally hide SVME from guests. */
+ *val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
+ /*
+ * Hide the 64-bit features from 32-bit guests. SCE has
+ * vendor-dependent behaviour.
+ */
if ( is_pv_32bit_domain(currd) )
- *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE);
+ *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE |
+ (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
+ ? EFER_SCE : 0));
return X86EMUL_OKAY;
case MSR_K7_FID_VID_CTL:
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index c9f44eb..6d94d65 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -31,6 +31,9 @@
#define EFER_LMSLE (1<<_EFER_LMSLE)
#define EFER_FFXSE (1<<_EFER_FFXSE)
+#define EFER_KNOWN_MASK (EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | \
+ EFER_SVME | EFER_LMSLE | EFER_FFXSE)
+
/* Speculation Controls. */
#define MSR_SPEC_CTRL 0x00000048
#define SPEC_CTRL_IBRS (_AC(1, ULL) << 0)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests
2018-05-07 10:00 [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests Andrew Cooper
@ 2018-05-07 10:43 ` Jan Beulich
2018-05-07 10:49 ` Andrew Cooper
2018-05-07 16:21 ` Brian Woods
1 sibling, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2018-05-07 10:43 UTC (permalink / raw)
To: Andrew Cooper; +Cc: brian.woods, Suravee Suthikulpanit, Xen-devel
>>> On 07.05.18 at 12:00, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -867,9 +867,16 @@ static int read_msr(unsigned int reg, uint64_t *val,
> return X86EMUL_OKAY;
>
> case MSR_EFER:
> - *val = read_efer();
> + /* Hide unknown bits, and unconditionally hide SVME from guests. */
> + *val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
> + /*
> + * Hide the 64-bit features from 32-bit guests. SCE has
> + * vendor-dependent behaviour.
> + */
> if ( is_pv_32bit_domain(currd) )
> - *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE);
> + *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE |
> + (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
> + ? EFER_SCE : 0));
> return X86EMUL_OKAY;
Ideally this would check the domain's x86_vendor, but that would require
wiring up emulation afaict, so fwiw
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests
2018-05-07 10:43 ` Jan Beulich
@ 2018-05-07 10:49 ` Andrew Cooper
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2018-05-07 10:49 UTC (permalink / raw)
To: Jan Beulich; +Cc: brian.woods, Suravee Suthikulpanit, Xen-devel
On 07/05/18 11:43, Jan Beulich wrote:
>>>> On 07.05.18 at 12:00, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/pv/emul-priv-op.c
>> +++ b/xen/arch/x86/pv/emul-priv-op.c
>> @@ -867,9 +867,16 @@ static int read_msr(unsigned int reg, uint64_t *val,
>> return X86EMUL_OKAY;
>>
>> case MSR_EFER:
>> - *val = read_efer();
>> + /* Hide unknown bits, and unconditionally hide SVME from guests. */
>> + *val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
>> + /*
>> + * Hide the 64-bit features from 32-bit guests. SCE has
>> + * vendor-dependent behaviour.
>> + */
>> if ( is_pv_32bit_domain(currd) )
>> - *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE);
>> + *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE |
>> + (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
>> + ? EFER_SCE : 0));
>> return X86EMUL_OKAY;
> Ideally this would check the domain's x86_vendor, but that would require
> wiring up emulation afaict, so fwiw
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Yes - we'd have to hook SYSCALL/SYSRET off the #UD handler, and teach
x86_emulate() to understand PV's idea of guest kernel mode. That is a
complete can of worms.
If someone feels like making cross-vendor PV work then great, but I
don't think we can plausibly claim that it might work atm. (Same for
HVM, despite the code we actually have.)
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests
2018-05-07 10:00 [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests Andrew Cooper
2018-05-07 10:43 ` Jan Beulich
@ 2018-05-07 16:21 ` Brian Woods
1 sibling, 0 replies; 4+ messages in thread
From: Brian Woods @ 2018-05-07 16:21 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Brian Woods, Suravee Suthikulpanit, Jan Beulich, Xen-devel
On Mon, May 07, 2018 at 11:00:23AM +0100, Andrew Cooper wrote:
> We don't advertise SVM in CPUID so a PV guest shouldn't be under the
> impression that it can use SVM functionality, but despite this, it really
> shouldn't see SVME set when reading EFER.
>
> On Intel processors, 32bit PV guests don't see, and can't use SYSCALL.
>
> Introduce EFER_KNOWN_MASK to whitelist the features Xen knows about, and use
> this to clamp the guests view.
>
> Take the opportunity to reuse the mask to simplify svm_vmcb_isvalid(), and
> change "undefined" to "unknown" in the print message, as there is at least
> EFER.TCE (Translation Cache Extension) defined but unknown to Xen.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Release-acked-by: Juergen Gross <jgross@suse.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Brian Woods <brian.woods@amd.com>
>
> Arguably, this wants backporting to the stable trees, so should be considered
> for 4.11 at this point.
> ---
> xen/arch/x86/hvm/svm/svmdebug.c | 5 ++---
> xen/arch/x86/pv/emul-priv-op.c | 11 +++++++++--
> xen/include/asm-x86/msr-index.h | 3 +++
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
> index 6c215d1..d35e405 100644
> --- a/xen/arch/x86/hvm/svm/svmdebug.c
> +++ b/xen/arch/x86/hvm/svm/svmdebug.c
> @@ -133,9 +133,8 @@ bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
> PRINTF("DR7: bits [63:32] are not zero (%#"PRIx64")\n",
> vmcb_get_dr7(vmcb));
>
> - if ( efer & ~(EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | EFER_SVME |
> - EFER_LMSLE | EFER_FFXSE) )
> - PRINTF("EFER: undefined bits are not zero (%#"PRIx64")\n", efer);
> + if ( efer & ~EFER_KNOWN_MASK )
> + PRINTF("EFER: unknown bits are not zero (%#"PRIx64")\n", efer);
>
> if ( hvm_efer_valid(v, efer, -1) )
> PRINTF("EFER: %s (%"PRIx64")\n", hvm_efer_valid(v, efer, -1), efer);
> diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
> index 15f42b3..ce2ec76 100644
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -867,9 +867,16 @@ static int read_msr(unsigned int reg, uint64_t *val,
> return X86EMUL_OKAY;
>
> case MSR_EFER:
> - *val = read_efer();
> + /* Hide unknown bits, and unconditionally hide SVME from guests. */
> + *val = read_efer() & EFER_KNOWN_MASK & ~EFER_SVME;
> + /*
> + * Hide the 64-bit features from 32-bit guests. SCE has
> + * vendor-dependent behaviour.
> + */
> if ( is_pv_32bit_domain(currd) )
> - *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE);
> + *val &= ~(EFER_LME | EFER_LMA | EFER_LMSLE |
> + (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
> + ? EFER_SCE : 0));
> return X86EMUL_OKAY;
>
> case MSR_K7_FID_VID_CTL:
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index c9f44eb..6d94d65 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -31,6 +31,9 @@
> #define EFER_LMSLE (1<<_EFER_LMSLE)
> #define EFER_FFXSE (1<<_EFER_FFXSE)
>
> +#define EFER_KNOWN_MASK (EFER_SCE | EFER_LME | EFER_LMA | EFER_NX | \
> + EFER_SVME | EFER_LMSLE | EFER_FFXSE)
> +
> /* Speculation Controls. */
> #define MSR_SPEC_CTRL 0x00000048
> #define SPEC_CTRL_IBRS (_AC(1, ULL) << 0)
> --
> 2.1.4
>
Reviewed-by: Brian Woods <brian.woods@amd.com>
--
Brian Woods
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-07 16:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 10:00 [PATCH v2 for-4.11] x86/pv: Hide more EFER bits from PV guests Andrew Cooper
2018-05-07 10:43 ` Jan Beulich
2018-05-07 10:49 ` Andrew Cooper
2018-05-07 16:21 ` Brian Woods
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.