Re: [Xen-devel] [PATCH 7/7] x86emul: support SYSRET

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/03/2020 11:55, Jan Beulich wrote:
> On 25.03.2020 11:00, Andrew Cooper wrote:
>> On 24/03/2020 16:29, Jan Beulich wrote:
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -5975,6 +5975,60 @@ x86_emulate(
>>>              goto done;
>>>          break;
>>>  
>>> +    case X86EMUL_OPC(0x0f, 0x07): /* sysret */
>>> +        vcpu_must_have(syscall);
>>> +        /* Inject #UD if syscall/sysret are disabled. */
>>> +        fail_if(!ops->read_msr);
>>> +        if ( (rc = ops->read_msr(MSR_EFER, &msr_val, ctxt)) != 
>>> X86EMUL_OKAY )
>>> +            goto done;
>>> +        generate_exception_if((msr_val & EFER_SCE) == 0, EXC_UD);
>> (as with the SYSCALL side), no need for the vcpu_must_have(syscall) as
>> well as this check.
> Upon re-reading I'm now confused - are you suggesting to also drop
> the EFER.SCE check?

No.  The SCE check is critical and needs to remain.

The exact delta I had put together was:

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index c730511ebe..57ce7e00be 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -5883,9 +5883,11 @@ x86_emulate(
 
 #ifdef __XEN__
     case X86EMUL_OPC(0x0f, 0x05): /* syscall */
-        generate_exception_if(!in_protmode(ctxt, ops), EXC_UD);
+        if ( !in_protmode(ctxt, ops) ||
+             ((ctxt->cpuid->x86_vendor & X86_VENDOR_INTEL) &&
!mode_64bit()) )
+            generate_exception(EXC_UD);
 
-        /* Inject #UD if syscall/sysret are disabled. */
+        /* Inject #UD if SCE is disabled.  Subsumes the SYSCALL CPUID
check. */
         fail_if(ops->read_msr == NULL);
         if ( (rc = ops->read_msr(MSR_EFER, &msr_val, ctxt)) !=
X86EMUL_OKAY )
             goto done;


(Looking at the commit date, Mon Dec 19 13:32:11 2016 is quite a long
time ago...)

~Andrew