[PATCH] x86emul: don't allow INVLPG in real mode

[PATCH] x86emul: don't allow INVLPG in real mode

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

As both INVLPG and INVLPGA have basically the same exception rules
(leaving aside that INVLPGA requires SVME enabled, which so far isn't
being taken care of, and that INVLPG requires ModRM.mod != 3), fold
the handling of the two as much as possible alongside achieving the
goal of the patch (at once doing the #UD checks pror to the #GP one,
which ought to be more in line with how hardware does things).

But please note that AMD and Intel disagree on what exceptions INVLPG
may raise, and the more reasonable Intel variant is being followed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3880,13 +3880,14 @@ x86_emulate(
         switch( modrm )
         {
         case 0xdf: /* invlpga */
-            generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
-            generate_exception_if(!mode_ring0(), EXC_GP, 0);
-            fail_if(ops->invlpg == NULL);
-            if ( (rc = ops->invlpg(x86_seg_none, truncate_ea(_regs.eax),
-                                   ctxt)) )
-                goto done;
-            goto no_writeback;
+            ea.mem.off = truncate_ea(_regs.eax);
+            /*
+             * This is questionable: AMD's PM calls the address used here both
+             * virtual and linear. They've informally indicated "linear" is
+             * meant there.
+             */
+            ea.mem.seg = x86_seg_none;
+            goto invlpg;
         case 0xf9: /* rdtscp */ {
             uint64_t tsc_aux;
             fail_if(ops->read_msr == NULL);
@@ -4006,8 +4007,10 @@ x86_emulate(
                 goto done;
             break;
         case 7: /* invlpg */
-            generate_exception_if(!mode_ring0(), EXC_GP, 0);
             generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
+        invlpg:
+            generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
+            generate_exception_if(!mode_ring0(), EXC_GP, 0);
             fail_if(ops->invlpg == NULL);
             if ( (rc = ops->invlpg(ea.mem.seg, ea.mem.off, ctxt)) )
                 goto done;




[-- Attachment #2: x86emul-invlpg.patch --]
[-- Type: text/plain, Size: 2220 bytes --]

x86emul: don't allow INVLPG in real mode

As both INVLPG and INVLPGA have basically the same exception rules
(leaving aside that INVLPGA requires SVME enabled, which so far isn't
being taken care of, and that INVLPG requires ModRM.mod != 3), fold
the handling of the two as much as possible alongside achieving the
goal of the patch (at once doing the #UD checks pror to the #GP one,
which ought to be more in line with how hardware does things).

But please note that AMD and Intel disagree on what exceptions INVLPG
may raise, and the more reasonable Intel variant is being followed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3880,13 +3880,14 @@ x86_emulate(
         switch( modrm )
         {
         case 0xdf: /* invlpga */
-            generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
-            generate_exception_if(!mode_ring0(), EXC_GP, 0);
-            fail_if(ops->invlpg == NULL);
-            if ( (rc = ops->invlpg(x86_seg_none, truncate_ea(_regs.eax),
-                                   ctxt)) )
-                goto done;
-            goto no_writeback;
+            ea.mem.off = truncate_ea(_regs.eax);
+            /*
+             * This is questionable: AMD's PM calls the address used here both
+             * virtual and linear. They've informally indicated "linear" is
+             * meant there.
+             */
+            ea.mem.seg = x86_seg_none;
+            goto invlpg;
         case 0xf9: /* rdtscp */ {
             uint64_t tsc_aux;
             fail_if(ops->read_msr == NULL);
@@ -4006,8 +4007,10 @@ x86_emulate(
                 goto done;
             break;
         case 7: /* invlpg */
-            generate_exception_if(!mode_ring0(), EXC_GP, 0);
             generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
+        invlpg:
+            generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
+            generate_exception_if(!mode_ring0(), EXC_GP, 0);
             fail_if(ops->invlpg == NULL);
             if ( (rc = ops->invlpg(ea.mem.seg, ea.mem.off, ctxt)) )
                 goto done;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86emul: don't allow INVLPG in real mode

[Wei Liu]

On Fri, Apr 22, 2016 at 08:17:35AM -0600, Jan Beulich wrote:
> As both INVLPG and INVLPGA have basically the same exception rules
> (leaving aside that INVLPGA requires SVME enabled, which so far isn't
> being taken care of, and that INVLPG requires ModRM.mod != 3), fold
> the handling of the two as much as possible alongside achieving the
> goal of the patch (at once doing the #UD checks pror to the #GP one,
> which ought to be more in line with how hardware does things).
> 
> But please note that AMD and Intel disagree on what exceptions INVLPG
> may raise, and the more reasonable Intel variant is being followed.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 

So this patch supersedes Andrew's patch to fix invlpg on AMD with shadow
paging?

Ask because I have a list of things to track and not sure if this is
something new.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86emul: don't allow INVLPG in real mode

[Jan Beulich]

>>> On 22.04.16 at 16:40, <wei.liu2@citrix.com> wrote:
> On Fri, Apr 22, 2016 at 08:17:35AM -0600, Jan Beulich wrote:
>> As both INVLPG and INVLPGA have basically the same exception rules
>> (leaving aside that INVLPGA requires SVME enabled, which so far isn't
>> being taken care of, and that INVLPG requires ModRM.mod != 3), fold
>> the handling of the two as much as possible alongside achieving the
>> goal of the patch (at once doing the #UD checks pror to the #GP one,
>> which ought to be more in line with how hardware does things).
>> 
>> But please note that AMD and Intel disagree on what exceptions INVLPG
>> may raise, and the more reasonable Intel variant is being followed.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
> 
> So this patch supersedes Andrew's patch to fix invlpg on AMD with shadow
> paging?

No, it is addressing an issue I found while reviewing Andrew's.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86emul: don't allow INVLPG in real mode

[Andrew Cooper]

On 22/04/16 15:17, Jan Beulich wrote:
> As both INVLPG and INVLPGA have basically the same exception rules
> (leaving aside that INVLPGA requires SVME enabled, which so far isn't
> being taken care of,

We also don't appear to handle the ASID in %ecx correctly either.  Yet
another item on the TODO list for nested virt.

>  and that INVLPG requires ModRM.mod != 3), fold
> the handling of the two as much as possible alongside achieving the
> goal of the patch (at once doing the #UD checks pror to the #GP one,
> which ought to be more in line with how hardware does things).
>
> But please note that AMD and Intel disagree on what exceptions INVLPG
> may raise, and the more reasonable Intel variant is being followed.

Which differences?

You introduce the !in_protmode() check to the `invlpg` path.  However,
both manuals agree that it should work in real mode (subject to the lock
prefix not being in use).  I presume the difference is to whether it is
eligible for use in vm86 mode, where indeed Intel is stricter than AMD.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86emul: don't allow INVLPG in real mode

[Jan Beulich]

>>> On 22.04.16 at 18:08, <andrew.cooper3@citrix.com> wrote:
> On 22/04/16 15:17, Jan Beulich wrote:
>> As both INVLPG and INVLPGA have basically the same exception rules
>> (leaving aside that INVLPGA requires SVME enabled, which so far isn't
>> being taken care of,
> 
> We also don't appear to handle the ASID in %ecx correctly either.  Yet
> another item on the TODO list for nested virt.
> 
>>  and that INVLPG requires ModRM.mod != 3), fold
>> the handling of the two as much as possible alongside achieving the
>> goal of the patch (at once doing the #UD checks pror to the #GP one,
>> which ought to be more in line with how hardware does things).
>>
>> But please note that AMD and Intel disagree on what exceptions INVLPG
>> may raise, and the more reasonable Intel variant is being followed.
> 
> Which differences?
> 
> You introduce the !in_protmode() check to the `invlpg` path.  However,
> both manuals agree that it should work in real mode (subject to the lock
> prefix not being in use).  I presume the difference is to whether it is
> eligible for use in vm86 mode, where indeed Intel is stricter than AMD.

Oh, indeed - I must have mixed things up badly. Withdrawing
the patch.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel