[PATCH 2/2] x86/pv: Handle #PF correctly when reading the IO permission bitmap

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The switch statement in guest_io_okay() is a very expensive way of
pre-initialising x with ~0, and performing a partial read into it.

However, the logic isn't correct either.

In a real TSS, the CPU always reads two bytes (like here), and any TSS limit
violation turns silently into no-access.  But, in-limit accesses trigger #PF
as usual.  AMD document this property explicitly, and while Intel don't (so
far as I can tell), they do behave consistently with AMD.

Switch from __copy_from_guest_offset() to __copy_from_guest_pv(), like
everything else in this file.  This removes code generation setting up
copy_from_user_hvm() (in the likely path even), and safety LFENCEs from
evaluate_nospec().

Change the logic to raise #PF if __copy_from_guest_pv() fails, rather than
disallowing the IO port access.  This brings the behaviour better in line with
normal x86.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

RFC: Should we make the boundary check be (port + bytes + 8)?  That would be
more correct, but liable to break unsuspecting VMs.  Maybe we should just
comment our way out of it.

This needs to be combined with Jan's "x86/PV: simplify (and thus correct)
guest accessor functions" to function completely correctly.  From XTF testing:

This series on its own:

  --- Xen Test Framework ---
  Environment: PV 64bit (Long mode 4 levels)
  Test pv-emul-cr2
  Error: %cr2 expected 0x3000, got 0x2fff
  Test result: ERROR

This series plus Jan's fix:

  --- Xen Test Framework ---
  Environment: PV 64bit (Long mode 4 levels)
  Test pv-emul-cr2
  Test result: SUCCESS

Interestingly, the test also does an `INW` without an output parameter
straddling that page boundary, and it does reliably get 0x3000 even without
the accessor fix.
---
 xen/arch/x86/pv/emul-priv-op.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 978bd6c0775f..b5d184038fa3 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -169,29 +169,26 @@ static int guest_io_okay(unsigned int port, unsigned int 
bytes,
 
     if ( (port + bytes) <= v->arch.pv.iobmp_limit )
     {
-        union { uint8_t bytes[2]; uint16_t mask; } x;
+        const void *__user addr = v->arch.pv.iobmp.p + (port >> 3);
+        uint16_t mask;
+        int rc;
 
-        /*
-         * Grab permission bytes from guest space. Inaccessible bytes are
-         * read as 0xff (no access allowed).
-         */
+        /* Grab permission bytes from guest space. */
         if ( user_mode )
             toggle_guest_pt(v);
 
-        switch ( __copy_from_guest_offset(x.bytes, v->arch.pv.iobmp,
-                                          port>>3, 2) )
-        {
-        default: x.bytes[0] = ~0;
-            /* fallthrough */
-        case 1:  x.bytes[1] = ~0;
-            /* fallthrough */
-        case 0:  break;
-        }
+        rc = __copy_from_guest_pv(&mask, addr, 2);
 
         if ( user_mode )
             toggle_guest_pt(v);
 
-        if ( (x.mask & (((1 << bytes) - 1) << (port & 7))) == 0 )
+        if ( rc )
+        {
+            x86_emul_pagefault(0, (unsigned long)addr + bytes - rc, ctxt);
+            return X86EMUL_EXCEPTION;
+        }
+
+        if ( (mask & (((1 << bytes) - 1) << (port & 7))) == 0 )
             return X86EMUL_OKAY;
     }
 
-- 
2.39.5