[V4,21/24] xen/ioreq: Make x86's send_invalidate_req() common

Commit Message

Oleksandr Tyshchenko Jan. 12, 2021, 9:52 p.m. UTC

From: Oleksandr Tyshchenko <[e-mail]>

As the IOREQ is a common feature now and we also need to
invalidate qemu/demu mapcache on Arm when the required condition
occurs this patch moves this function to the common code
(and remames it to ioreq_signal_mapcache_invalidate).
This patch also moves per-domain qemu_mapcache_invalidate
variable out of the arch sub-struct (and drops "qemu" prefix).

We don't put this variable inside the #ifdef CONFIG_IOREQ_SERVER
at the end of struct domain, but in the hole next to the group
of 5 bools further up which is more efficient.

The subsequent patch will add mapcache invalidation handling on Arm.

Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
CC: Julien Grall <[e-mail]>
[On Arm only]
Tested-by: Wei Chen <[e-mail]>

---
Please note, this is a split/cleanup/hardening of Julien's PoC:
"Add support for Guest IO forwarding to a device emulator"

Changes RFC -> V1:
   - move send_invalidate_req() to the common code
   - update patch subject/description
   - move qemu_mapcache_invalidate out of the arch sub-struct,
     update checks
   - remove #if defined(CONFIG_ARM64) from the common code

Changes V1 -> V2:
   - was split into:
     - xen/ioreq: Make x86's send_invalidate_req() common
     - xen/arm: Add mapcache invalidation handling
   - update patch description/subject
   - move Arm bits to a separate patch
   - don't alter the common code, the flag is set by arch code
   - rename send_invalidate_req() to send_invalidate_ioreq()
   - guard qemu_mapcache_invalidate with CONFIG_IOREQ_SERVER
   - use bool instead of bool_t
   - remove blank line blank line between head comment and #include-s

Changes V2 -> V3:
   - update patch description
   - drop "qemu" prefix from the variable name
   - rename send_invalidate_req() to ioreq_signal_mapcache_invalidate()

Changes V3 -> V4:
   - change variable location in struct domain
---
 xen/arch/x86/hvm/hypercall.c     |  9 +++++----
 xen/arch/x86/hvm/io.c            | 14 --------------
 xen/common/ioreq.c               | 14 ++++++++++++++
 xen/include/asm-x86/hvm/domain.h |  1 -
 xen/include/asm-x86/hvm/io.h     |  1 -
 xen/include/xen/ioreq.h          |  1 +
 xen/include/xen/sched.h          |  5 +++++
 7 files changed, 25 insertions(+), 20 deletions(-)

Comments

Paul Durrant Jan. 18, 2021, 10:31 a.m. UTC | #1

> -----Original Message-----
> From: Xen-devel <[e-mail]> On Behalf Of Oleksandr Tyshchenko
> Sent: 12 January 2021 21:52
> To: [e-mail]
> Cc: Oleksandr Tyshchenko <[e-mail]>; Jan Beulich <[e-mail]>; Andrew
> Cooper <[e-mail]>; Roger Pau Monné <[e-mail]>; Wei Liu <[e-mail]>;
> George Dunlap <[e-mail]>; Ian Jackson <[e-mail]>; Julien Grall
> <[e-mail]>; Stefano Stabellini <[e-mail]>; Paul Durrant <[e-mail]>; Julien
> Grall <[e-mail]>
> Subject: [PATCH V4 21/24] xen/ioreq: Make x86's send_invalidate_req() common
> 
> From: Oleksandr Tyshchenko <[e-mail]>
> 
> As the IOREQ is a common feature now and we also need to
> invalidate qemu/demu mapcache on Arm when the required condition
> occurs this patch moves this function to the common code
> (and remames it to ioreq_signal_mapcache_invalidate).
> This patch also moves per-domain qemu_mapcache_invalidate
> variable out of the arch sub-struct (and drops "qemu" prefix).
> 
> We don't put this variable inside the #ifdef CONFIG_IOREQ_SERVER
> at the end of struct domain, but in the hole next to the group
> of 5 bools further up which is more efficient.
> 
> The subsequent patch will add mapcache invalidation handling on Arm.
> 
> Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
> CC: Julien Grall <[e-mail]>
> [On Arm only]
> Tested-by: Wei Chen <[e-mail]>

Reviewed-by: Paul Durrant <[e-mail]>

Jan Beulich Jan. 21, 2021, 2:02 p.m. UTC | #2

On 18.01.2021 11:31, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel <[e-mail]> On Behalf Of Oleksandr Tyshchenko
>> Sent: 12 January 2021 21:52
>> To: [e-mail]
>> Cc: Oleksandr Tyshchenko <[e-mail]>; Jan Beulich <[e-mail]>; Andrew
>> Cooper <[e-mail]>; Roger Pau Monné <[e-mail]>; Wei Liu <[e-mail]>;
>> George Dunlap <[e-mail]>; Ian Jackson <[e-mail]>; Julien Grall
>> <[e-mail]>; Stefano Stabellini <[e-mail]>; Paul Durrant <[e-mail]>; Julien
>> Grall <[e-mail]>
>> Subject: [PATCH V4 21/24] xen/ioreq: Make x86's send_invalidate_req() common
>>
>> From: Oleksandr Tyshchenko <[e-mail]>
>>
>> As the IOREQ is a common feature now and we also need to
>> invalidate qemu/demu mapcache on Arm when the required condition
>> occurs this patch moves this function to the common code
>> (and remames it to ioreq_signal_mapcache_invalidate).
>> This patch also moves per-domain qemu_mapcache_invalidate
>> variable out of the arch sub-struct (and drops "qemu" prefix).
>>
>> We don't put this variable inside the #ifdef CONFIG_IOREQ_SERVER
>> at the end of struct domain, but in the hole next to the group
>> of 5 bools further up which is more efficient.
>>
>> The subsequent patch will add mapcache invalidation handling on Arm.
>>
>> Signed-off-by: Oleksandr Tyshchenko <[e-mail]>
>> CC: Julien Grall <[e-mail]>
>> [On Arm only]
>> Tested-by: Wei Chen <[e-mail]>
> 
> Reviewed-by: Paul Durrant <[e-mail]>

Applicable parts
Acked-by: Jan Beulich <[e-mail]>

Jan

Patch

diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index ac573c8..6d41c56 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -20,6 +20,7 @@ 
  */
 #include <xen/lib.h>
 #include <xen/hypercall.h>
+#include <xen/ioreq.h>
 #include <xen/nospec.h>
 
 #include <asm/hvm/emulate.h>
@@ -47,7 +48,7 @@  static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         rc = compat_memory_op(cmd, arg);
 
     if ( (cmd & MEMOP_CMD_MASK) == XENMEM_decrease_reservation )
-        curr->domain->arch.hvm.qemu_mapcache_invalidate = true;
+        curr->domain->mapcache_invalidate = true;
 
     return rc;
 }
@@ -326,9 +327,9 @@  int hvm_hypercall(struct cpu_user_regs *regs)
 
     HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu -> %lx", eax, regs->rax);
 
-    if ( unlikely(currd->arch.hvm.qemu_mapcache_invalidate) &&
-         test_and_clear_bool(currd->arch.hvm.qemu_mapcache_invalidate) )
-        send_invalidate_req();
+    if ( unlikely(currd->mapcache_invalidate) &&
+         test_and_clear_bool(currd->mapcache_invalidate) )
+        ioreq_signal_mapcache_invalidate();
 
     return curr->hcall_preempted ? HVM_HCALL_preempted : HVM_HCALL_completed;
 }
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 66a37ee..046a8eb 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -64,20 +64,6 @@  void send_timeoffset_req(unsigned long timeoff)
         gprintk(XENLOG_ERR, "Unsuccessful timeoffset update\n");
 }
 
-/* Ask ioemu mapcache to invalidate mappings. */
-void send_invalidate_req(void)
-{
-    ioreq_t p = {
-        .type = IOREQ_TYPE_INVALIDATE,
-        .size = 4,
-        .dir = IOREQ_WRITE,
-        .data = ~0UL, /* flush all */
-    };
-
-    if ( ioreq_broadcast(&p, false) != 0 )
-        gprintk(XENLOG_ERR, "Unsuccessful map-cache invalidate\n");
-}
-
 bool hvm_emulate_one_insn(hvm_emulate_validate_t *validate, const char *descr)
 {
     struct hvm_emulate_ctxt ctxt;
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 59f4990..050891f 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -35,6 +35,20 @@ 
 #include <public/hvm/ioreq.h>
 #include <public/hvm/params.h>
 
+/* Ask ioemu mapcache to invalidate mappings. */
+void ioreq_signal_mapcache_invalidate(void)
+{
+    ioreq_t p = {
+        .type = IOREQ_TYPE_INVALIDATE,
+        .size = 4,
+        .dir = IOREQ_WRITE,
+        .data = ~0UL, /* flush all */
+    };
+
+    if ( ioreq_broadcast(&p, false) != 0 )
+        gprintk(XENLOG_ERR, "Unsuccessful map-cache invalidate\n");
+}
+
 static void set_ioreq_server(struct domain *d, unsigned int id,
                              struct ioreq_server *s)
 {
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index b8be1ad..cf959f6 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -122,7 +122,6 @@  struct hvm_domain {
 
     struct viridian_domain *viridian;
 
-    bool_t                 qemu_mapcache_invalidate;
     bool_t                 is_s3_suspended;
 
     /*
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index fb64294..3da0136 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -97,7 +97,6 @@  bool relocate_portio_handler(
     unsigned int size);
 
 void send_timeoffset_req(unsigned long timeoff);
-void send_invalidate_req(void);
 bool handle_mmio_with_translation(unsigned long gla, unsigned long gpfn,
                                   struct npfec);
 bool handle_pio(uint16_t port, unsigned int size, int dir);
diff --git a/xen/include/xen/ioreq.h b/xen/include/xen/ioreq.h
index f0908af..dc47ec7 100644
--- a/xen/include/xen/ioreq.h
+++ b/xen/include/xen/ioreq.h
@@ -101,6 +101,7 @@  struct ioreq_server *ioreq_server_select(struct domain *d,
 int ioreq_send(struct ioreq_server *s, ioreq_t *proto_p,
                bool buffered);
 unsigned int ioreq_broadcast(ioreq_t *p, bool buffered);
+void ioreq_signal_mapcache_invalidate(void);
 
 void ioreq_domain_init(struct domain *d);
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 7aea2bb..5139b44 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -444,6 +444,11 @@  struct domain
      * unpaused for the first time by the systemcontroller.
      */
     bool             creation_finished;
+    /*
+     * Indicates that mapcache invalidation request should be sent to
+     * the device emulator.
+     */
+    bool             mapcache_invalidate;
 
     /* Which guest this guest has privileges on */
     struct domain   *target;