[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v6 34/44] x86/hvm: clear the vCPU's copy of an emulated I/O request once consumed



hvmemul_do_io() keeps the request it is emulating in the vCPU's struct
vcpu_io: the data written, or the response once a device model has
supplied it, with the address and repeat count.  Once the access is done
only the state is set back to STATE_IOREQ_NONE, so the operand stays in
struct vcpu, a xenheap page mapped in every context, until the next
emulated I/O overwrites it.

Drop the copy wherever it is done with: when the response has been taken
into hvmemul_do_io()'s local copy on the instruction's re-execution,
when Xen's own handlers have completed the access, when the request is
retried, suspended or not sent, and in hvmemul_cancel().  A request sent
to a device model stays while it is in flight: the re-execution checks
it, and the port-I/O completion reads its address and size before
re-issuing it.  One which needs no completion -- a port write, or a
request whose data is a guest buffer's address -- loses its address,
data and count as soon as it is sent: the device model has its own copy,
and the completion path reads only the state, type, direction and
data_is_ptr.  Nothing reads the copy after STATE_IOREQ_NONE.

Only the x86 emulator changes.  struct vcpu_io is common code, and Arm's
MMIO completion reads the response from it.  Moving the in-flight copy
out of struct vcpu belongs with narrowing the ioreq server pages, which
carry the same operand while a device model handles it.

Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
---
Changes in v5:
- New in this version.
---
 xen/arch/x86/hvm/emulate.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index e2cf982c25..da7c08b5d9 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -167,6 +167,16 @@ static const struct hvm_io_handler ioreq_server_handler = {
     .ops = &ioreq_server_ops
 };
 
+/*
+ * The vCPU's copy of an emulated I/O request is done with: nothing reads it
+ * before the next request overwrites it.  Clear it all, so that the operand
+ * and the address do not stay in struct vcpu.
+ */
+static void ioreq_done(struct vcpu_io *vio)
+{
+    vio->req = (ioreq_t){ .state = STATE_IOREQ_NONE };
+}
+
 /*
  * Drop all records of in-flight emulation. This is needed whenever a vCPU's
  * register state may have changed behind the emulator's back.
@@ -175,7 +185,7 @@ void hvmemul_cancel(struct vcpu *v)
 {
     struct hvm_vcpu_io *hvio = &v->arch.hvm.hvm_io;
 
-    v->io.req.state = STATE_IOREQ_NONE;
+    ioreq_done(&v->io);
     v->io.completion = VIO_no_completion;
     hvio->mmio_cache_count = 0;
     hvio->mmio_insn_bytes = 0;
@@ -265,8 +275,9 @@ static int hvmemul_do_io(
     case STATE_IOREQ_NONE:
         break;
     case STATE_IORESP_READY:
-        vio->req.state = STATE_IOREQ_NONE;
+        /* The local copy carries the response from here. */
         p = vio->req;
+        ioreq_done(vio);
 
         /* Verify the emulation request has been correctly re-issued */
         if ( (p.type != (is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO)) ||
@@ -330,7 +341,7 @@ static int hvmemul_do_io(
     {
     case X86EMUL_OKAY:
     case X86EMUL_RETRY:
-        vio->req.state = STATE_IOREQ_NONE;
+        ioreq_done(vio);
         break;
     case X86EMUL_UNHANDLEABLE:
     {
@@ -383,7 +394,7 @@ static int hvmemul_do_io(
                 if ( s == NULL )
                 {
                     rc = X86EMUL_RETRY;
-                    vio->req.state = STATE_IOREQ_NONE;
+                    ioreq_done(vio);
                     break;
                 }
 
@@ -394,7 +405,7 @@ static int hvmemul_do_io(
                 if ( dir == IOREQ_READ )
                 {
                     rc = hvm_process_io_intercept(&ioreq_server_handler, &p);
-                    vio->req.state = STATE_IOREQ_NONE;
+                    ioreq_done(vio);
                     break;
                 }
             }
@@ -440,7 +451,7 @@ static int hvmemul_do_io(
                                     paddr_to_pfn(addr));
 
                         rc = X86EMUL_RETRY;
-                        vio->req.state = STATE_IOREQ_NONE;
+                        ioreq_done(vio);
                         break;
                     }
 
@@ -455,15 +466,24 @@ static int hvmemul_do_io(
                              dir ? "read from" : "write to", addr, size);
             }
             rc = hvm_process_io_intercept(&null_handler, &p);
-            vio->req.state = STATE_IOREQ_NONE;
+            ioreq_done(vio);
         }
         else
         {
             rc = ioreq_send(s, &p, 0);
             if ( rc != X86EMUL_RETRY || vio->suspended )
-                vio->req.state = STATE_IOREQ_NONE;
+                ioreq_done(vio);
             else if ( !ioreq_needs_completion(&vio->req) )
+            {
+                /*
+                 * Nothing comes back, and the device model has its own copy
+                 * of the request: keep only what completion checks.
+                 */
+                vio->req.addr = 0;
+                vio->req.data = 0;
+                vio->req.count = 0;
                 rc = X86EMUL_OKAY;
+            }
         }
         break;
     }
-- 
2.56.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.