[PATCH-for-9.0 00/10] hw/xen: Have most of Xen files become target-agnostic

[PATCH-for-9.0 00/10] hw/xen: Have most of Xen files become target-agnostic

[Philippe Mathieu-Daudé]

Hi,

After discussing with Alex Bennée I realized most Xen code
should be target-agnostic. David Woodhouse confirmed that
last week, so I had a quick look and here is the result.

More work is required to be able to instanciate Xen HW in
an heterogeneous machine, but this doesn't make sense yet
until we can run multiple accelerators concurrently.

Only build-tested.

Regards,

Phil.

Philippe Mathieu-Daudé (10):
  sysemu/xen: Forbid using Xen headers in user emulation
  hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix
  hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'
  hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()
  hw/xen: Use target-agnostic qemu_target_page_bits()
  hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources
  sysemu/xen-mapcache: Check Xen availability with
    CONFIG_XEN_IS_POSSIBLE
  system/physmem: Only include 'hw/xen/xen.h' when Xen is available
  hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'
  hw/xen: Have most of Xen files become target-agnostic

 hw/xen/xen_pt.h                 | 14 --------------
 include/hw/arm/xen_arch_hvm.h   |  9 ---------
 include/hw/i386/xen_arch_hvm.h  | 11 -----------
 include/hw/xen/arch_hvm.h       |  5 -----
 include/hw/xen/xen-hvm-common.h |  8 +++++++-
 include/hw/xen/xen_igd.h        | 23 +++++++++++++++++++++++
 include/sysemu/xen-mapcache.h   |  3 ++-
 include/sysemu/xen.h            |  8 ++++----
 accel/xen/xen-all.c             |  1 +
 hw/arm/xen_arm.c                | 14 +++++++++++---
 hw/i386/pc_piix.c               |  1 +
 hw/i386/xen/xen-hvm.c           | 16 ++++++++++++----
 hw/xen/xen-hvm-common.c         | 16 +++++++---------
 hw/xen/xen_pt.c                 |  3 ++-
 hw/xen/xen_pt_config_init.c     |  3 ++-
 hw/xen/xen_pt_graphics.c        |  3 ++-
 hw/xen/xen_pt_stub.c            |  2 +-
 system/physmem.c                |  5 ++++-
 accel/xen/meson.build           |  2 +-
 hw/block/dataplane/meson.build  |  2 +-
 hw/xen/meson.build              | 13 ++++---------
 21 files changed, 85 insertions(+), 77 deletions(-)
 delete mode 100644 include/hw/arm/xen_arch_hvm.h
 delete mode 100644 include/hw/i386/xen_arch_hvm.h
 delete mode 100644 include/hw/xen/arch_hvm.h
 create mode 100644 include/hw/xen/xen_igd.h

-- 
2.41.0

[PATCH-for-9.0 01/10] sysemu/xen: Forbid using Xen headers in user emulation

[Philippe Mathieu-Daudé]

Xen is a system specific accelerator, it makes no sense
to include its headers in user emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/xen.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
index bc13ad5692..a9f591f26d 100644
--- a/include/sysemu/xen.h
+++ b/include/sysemu/xen.h
@@ -10,6 +10,10 @@
 #ifndef SYSEMU_XEN_H
 #define SYSEMU_XEN_H
 
+#ifdef CONFIG_USER_ONLY
+#error Cannot include sysemu/xen.h from user emulation
+#endif
+
 #include "exec/cpu-common.h"
 
 #ifdef NEED_CPU_H
@@ -26,16 +30,13 @@ extern bool xen_allowed;
 
 #define xen_enabled()           (xen_allowed)
 
-#ifndef CONFIG_USER_ONLY
 void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
                    struct MemoryRegion *mr, Error **errp);
-#endif
 
 #else /* !CONFIG_XEN_IS_POSSIBLE */
 
 #define xen_enabled() 0
-#ifndef CONFIG_USER_ONLY
 static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
 {
     /* nothing */
@@ -45,7 +46,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
 {
     g_assert_not_reached();
 }
-#endif
 
 #endif /* CONFIG_XEN_IS_POSSIBLE */
 
-- 
2.41.0

[PATCH-for-9.0 02/10] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix

[Philippe Mathieu-Daudé]

Use a common 'xen_arch_' prefix for architecture-specific functions.
Rename xen_arch_set_memory() and xen_arch_handle_ioreq().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/xen_arch_hvm.h  | 4 ++--
 include/hw/i386/xen_arch_hvm.h | 4 ++--
 hw/arm/xen_arm.c               | 4 ++--
 hw/i386/xen/xen-hvm.c          | 6 +++---
 hw/xen/xen-hvm-common.c        | 4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
index 8fd645e723..6a974f2020 100644
--- a/include/hw/arm/xen_arch_hvm.h
+++ b/include/hw/arm/xen_arch_hvm.h
@@ -2,8 +2,8 @@
 #define HW_XEN_ARCH_ARM_HVM_H
 
 #include <xen/hvm/ioreq.h>
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void arch_xen_set_memory(XenIOState *state,
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
                          MemoryRegionSection *section,
                          bool add);
 #endif
diff --git a/include/hw/i386/xen_arch_hvm.h b/include/hw/i386/xen_arch_hvm.h
index 1000f8f543..2822304955 100644
--- a/include/hw/i386/xen_arch_hvm.h
+++ b/include/hw/i386/xen_arch_hvm.h
@@ -4,8 +4,8 @@
 #include <xen/hvm/ioreq.h>
 #include "hw/xen/xen-hvm-common.h"
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void arch_xen_set_memory(XenIOState *state,
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
                          MemoryRegionSection *section,
                          bool add);
 #endif
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index a5631529d0..28d790f4ce 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -129,14 +129,14 @@ static void xen_init_ram(MachineState *machine)
     }
 }
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
     hw_error("Invalid ioreq type 0x%x\n", req->type);
 
     return;
 }
 
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
                          bool add)
 {
 }
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f42621e674..ffa95e3c3d 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -659,8 +659,8 @@ void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
     }
 }
 
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
-                                bool add)
+void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
+                         bool add)
 {
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = int128_get64(section->size);
@@ -700,7 +700,7 @@ void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
     }
 }
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
     switch (req->type) {
     case IOREQ_TYPE_VMWARE_PORT:
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 565dc39c8f..1d8bd9aea7 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -65,7 +65,7 @@ static void xen_set_memory(struct MemoryListener *listener,
         }
     }
 
-    arch_xen_set_memory(state, section, add);
+    xen_arch_set_memory(state, section, add);
 }
 
 void xen_region_add(MemoryListener *listener,
@@ -452,7 +452,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
             cpu_ioreq_config(state, req);
             break;
         default:
-            arch_handle_ioreq(state, req);
+            xen_arch_handle_ioreq(state, req);
     }
     if (req->dir == IOREQ_READ) {
         trace_handle_ioreq_read(req, req->type, req->df, req->data_is_ptr,
-- 
2.41.0

[PATCH-for-9.0 03/10] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'

[Philippe Mathieu-Daudé]

We don't need a target-specific header for common target-specific
prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
in "hw/xen/xen-hvm-common.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/arm/xen_arch_hvm.h   |  9 ---------
 include/hw/i386/xen_arch_hvm.h  | 11 -----------
 include/hw/xen/arch_hvm.h       |  5 -----
 include/hw/xen/xen-hvm-common.h |  6 ++++++
 hw/arm/xen_arm.c                |  1 -
 hw/i386/xen/xen-hvm.c           |  1 -
 hw/xen/xen-hvm-common.c         |  1 -
 7 files changed, 6 insertions(+), 28 deletions(-)
 delete mode 100644 include/hw/arm/xen_arch_hvm.h
 delete mode 100644 include/hw/i386/xen_arch_hvm.h
 delete mode 100644 include/hw/xen/arch_hvm.h

diff --git a/include/hw/arm/xen_arch_hvm.h b/include/hw/arm/xen_arch_hvm.h
deleted file mode 100644
index 6a974f2020..0000000000
--- a/include/hw/arm/xen_arch_hvm.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef HW_XEN_ARCH_ARM_HVM_H
-#define HW_XEN_ARCH_ARM_HVM_H
-
-#include <xen/hvm/ioreq.h>
-void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
-                         MemoryRegionSection *section,
-                         bool add);
-#endif
diff --git a/include/hw/i386/xen_arch_hvm.h b/include/hw/i386/xen_arch_hvm.h
deleted file mode 100644
index 2822304955..0000000000
--- a/include/hw/i386/xen_arch_hvm.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef HW_XEN_ARCH_I386_HVM_H
-#define HW_XEN_ARCH_I386_HVM_H
-
-#include <xen/hvm/ioreq.h>
-#include "hw/xen/xen-hvm-common.h"
-
-void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
-void xen_arch_set_memory(XenIOState *state,
-                         MemoryRegionSection *section,
-                         bool add);
-#endif
diff --git a/include/hw/xen/arch_hvm.h b/include/hw/xen/arch_hvm.h
deleted file mode 100644
index c7c515220d..0000000000
--- a/include/hw/xen/arch_hvm.h
+++ /dev/null
@@ -1,5 +0,0 @@
-#if defined(TARGET_I386) || defined(TARGET_X86_64)
-#include "hw/i386/xen_arch_hvm.h"
-#elif defined(TARGET_ARM) || defined(TARGET_ARM_64)
-#include "hw/arm/xen_arch_hvm.h"
-#endif
diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 4e9904f1a6..27e938d268 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -96,4 +96,10 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
                         const MemoryListener *xen_memory_listener);
 
 void cpu_ioreq_pio(ioreq_t *req);
+
+void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
+void xen_arch_set_memory(XenIOState *state,
+                         MemoryRegionSection *section,
+                         bool add);
+
 #endif /* HW_XEN_HVM_COMMON_H */
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 28d790f4ce..6a1d7719e9 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -33,7 +33,6 @@
 #include "sysemu/sysemu.h"
 #include "hw/xen/xen-hvm-common.h"
 #include "sysemu/tpm.h"
-#include "hw/xen/arch_hvm.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index ffa95e3c3d..f8a195270a 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -21,7 +21,6 @@
 #include "qemu/range.h"
 
 #include "hw/xen/xen-hvm-common.h"
-#include "hw/xen/arch_hvm.h"
 #include <xen/hvm/e820.h>
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 1d8bd9aea7..c028c1b541 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -7,7 +7,6 @@
 #include "hw/xen/xen-hvm-common.h"
 #include "hw/xen/xen-bus.h"
 #include "hw/boards.h"
-#include "hw/xen/arch_hvm.h"
 
 MemoryRegion ram_memory;
 
-- 
2.41.0

[PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.

In order to compile this file once for all targets, factor the
target-specific code out of handle_ioreq() as a per-target handler
called xen_arch_align_ioreq_data().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Should we have a 'unsigned qemu_target_long_bits();' helper
such qemu_target_page_foo() API and target_words_bigendian()?
---
 include/hw/xen/xen-hvm-common.h | 1 +
 hw/arm/xen_arm.c                | 8 ++++++++
 hw/i386/xen/xen-hvm.c           | 8 ++++++++
 hw/xen/xen-hvm-common.c         | 5 +----
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 27e938d268..734bfa3183 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -97,6 +97,7 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
 
 void cpu_ioreq_pio(ioreq_t *req);
 
+void xen_arch_align_ioreq_data(ioreq_t *req);
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req);
 void xen_arch_set_memory(XenIOState *state,
                          MemoryRegionSection *section,
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 6a1d7719e9..c646fd70d0 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -128,6 +128,14 @@ static void xen_init_ram(MachineState *machine)
     }
 }
 
+void xen_arch_align_ioreq_data(ioreq_t *req)
+{
+    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
+            && (req->size < sizeof(target_ulong))) {
+        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+    }
+}
+
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
     hw_error("Invalid ioreq type 0x%x\n", req->type);
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f8a195270a..aff5c5b81d 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -699,6 +699,14 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
     }
 }
 
+void xen_arch_align_ioreq_data(ioreq_t *req)
+{
+    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
+            && (req->size < sizeof(target_ulong))) {
+        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+    }
+}
+
 void xen_arch_handle_ioreq(XenIOState *state, ioreq_t *req)
 {
     switch (req->type) {
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index c028c1b541..03f9417e7e 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -426,10 +426,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
     trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
                        req->addr, req->data, req->count, req->size);
 
-    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
-            (req->size < sizeof (target_ulong))) {
-        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
-    }
+    xen_arch_align_ioreq_data(req);
 
     if (req->dir == IOREQ_WRITE)
         trace_handle_ioreq_write(req, req->type, req->df, req->data_is_ptr,
-- 
2.41.0

[PATCH-for-9.0 05/10] hw/xen: Use target-agnostic qemu_target_page_bits()

[Philippe Mathieu-Daudé]

Instead of the target-specific TARGET_PAGE_BITS definition,
use qemu_target_page_bits() which is target agnostic.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/xen/xen-hvm-common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 03f9417e7e..35b3b5407d 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qapi/error.h"
+#include "exec/target_page.h"
 #include "trace.h"
 
 #include "hw/pci/pci_host.h"
@@ -13,6 +14,7 @@ MemoryRegion ram_memory;
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
                    Error **errp)
 {
+    unsigned target_page_bits = qemu_target_page_bits();
     unsigned long nr_pfn;
     xen_pfn_t *pfn_list;
     int i;
@@ -31,11 +33,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
 
     trace_xen_ram_alloc(ram_addr, size);
 
-    nr_pfn = size >> TARGET_PAGE_BITS;
+    nr_pfn = size >> target_page_bits;
     pfn_list = g_new(xen_pfn_t, nr_pfn);
 
     for (i = 0; i < nr_pfn; i++) {
-        pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
+        pfn_list[i] = (ram_addr >> target_page_bits) + i;
     }
 
     if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, pfn_list)) {
-- 
2.41.0

[PATCH-for-9.0 06/10] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources

[Philippe Mathieu-Daudé]

We rarely need to include "cpu.h" in headers. Including it
'taint' headers to be target-specific. Here only the i386/arm
implementations requires "cpu.h", so include it there and
remove from the "hw/xen/xen-hvm-common.h" *common* header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/xen/xen-hvm-common.h | 1 -
 hw/arm/xen_arm.c                | 1 +
 hw/i386/xen/xen-hvm.c           | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 734bfa3183..ca941fd3eb 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -4,7 +4,6 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 
-#include "cpu.h"
 #include "hw/pci/pci.h"
 #include "hw/hw.h"
 #include "hw/xen/xen_native.h"
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index c646fd70d0..2c97d6adc8 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -33,6 +33,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/xen/xen-hvm-common.h"
 #include "sysemu/tpm.h"
+#include "cpu.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index aff5c5b81d..369d738b50 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -22,6 +22,7 @@
 
 #include "hw/xen/xen-hvm-common.h"
 #include <xen/hvm/e820.h>
+#include "cpu.h"
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
 static MemoryRegion *framebuffer;
-- 
2.41.0

[PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[Philippe Mathieu-Daudé]

"sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
version of CONFIG_XEN. Use it in order to use "sysemu/xen-mapcache.h"
in target-agnostic files.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/xen-mapcache.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h
index c8e7c2f6cf..10c2e3082a 100644
--- a/include/sysemu/xen-mapcache.h
+++ b/include/sysemu/xen-mapcache.h
@@ -10,10 +10,11 @@
 #define XEN_MAPCACHE_H
 
 #include "exec/cpu-common.h"
+#include "sysemu/xen.h"
 
 typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset,
                                          ram_addr_t size);
-#ifdef CONFIG_XEN
+#ifdef CONFIG_XEN_IS_POSSIBLE
 
 void xen_map_cache_init(phys_offset_to_gaddr_t f,
                         void *opaque);
-- 
2.41.0

[PATCH-for-9.0 08/10] system/physmem: Only include 'hw/xen/xen.h' when Xen is available

[Philippe Mathieu-Daudé]

"hw/xen/xen.h" contains declarations for Xen hardware. There is
no point including it when Xen is not available. When Xen is not
available, we have enough with declarations of "sysemu/xen.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/physmem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index fc2b0fee01..fa667437da 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -35,9 +35,9 @@
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
 #include "hw/boards.h"
-#include "hw/xen/xen.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
+#include "sysemu/xen.h"
 #include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "qemu/config-file.h"
@@ -51,6 +51,9 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/hw_accel.h"
 #include "sysemu/xen-mapcache.h"
+#ifdef CONFIG_XEN
+#include "hw/xen/xen.h"
+#endif
 #include "trace/trace-root.h"
 
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
-- 
2.41.0

[PATCH-for-9.0 09/10] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'

[Philippe Mathieu-Daudé]

"hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target
specific. It also declares IGD methods, which are not target
specific.

Target-agnostic code can use IGD methods. To allow that, extract
these methos into a new "hw/xen/xen_igd.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
What license for the new "hw/xen/xen_igd.h" header?
---
 hw/xen/xen_pt.h             | 14 --------------
 include/hw/xen/xen_igd.h    | 23 +++++++++++++++++++++++
 accel/xen/xen-all.c         |  1 +
 hw/i386/pc_piix.c           |  1 +
 hw/xen/xen_pt.c             |  3 ++-
 hw/xen/xen_pt_config_init.c |  3 ++-
 hw/xen/xen_pt_graphics.c    |  3 ++-
 hw/xen/xen_pt_stub.c        |  2 +-
 8 files changed, 32 insertions(+), 18 deletions(-)
 create mode 100644 include/hw/xen/xen_igd.h

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 31bcfdf705..da5af67638 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -5,9 +5,6 @@
 #include "xen-host-pci-device.h"
 #include "qom/object.h"
 
-bool xen_igd_gfx_pt_enabled(void);
-void xen_igd_gfx_pt_set(bool value, Error **errp);
-
 void xen_pt_log(const PCIDevice *d, const char *f, ...) G_GNUC_PRINTF(2, 3);
 
 #define XEN_PT_ERR(d, _f, _a...) xen_pt_log(d, "%s: Error: "_f, __func__, ##_a)
@@ -52,12 +49,6 @@ typedef struct XenPTDeviceClass {
     XenPTQdevRealize pci_qdev_realize;
 } XenPTDeviceClass;
 
-uint32_t igd_read_opregion(XenPCIPassthroughState *s);
-void xen_igd_reserve_slot(PCIBus *pci_bus);
-void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
-void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
-                                           XenHostPCIDevice *dev);
-
 /* function type for config reg */
 typedef int (*xen_pt_conf_reg_init)
     (XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset,
@@ -343,11 +334,6 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
 void *pci_assign_dev_load_option_rom(PCIDevice *dev, int *size,
                                      unsigned int domain, unsigned int bus,
                                      unsigned int slot, unsigned int function);
-static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
-{
-    return (xen_igd_gfx_pt_enabled()
-            && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
-}
 int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
 int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
 void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
diff --git a/include/hw/xen/xen_igd.h b/include/hw/xen/xen_igd.h
new file mode 100644
index 0000000000..52f1f8244c
--- /dev/null
+++ b/include/hw/xen/xen_igd.h
@@ -0,0 +1,23 @@
+#ifndef XEN_IGD_H
+#define XEN_IGD_H
+
+#include "hw/xen/xen-host-pci-device.h"
+
+typedef struct XenPCIPassthroughState XenPCIPassthroughState;
+
+bool xen_igd_gfx_pt_enabled(void);
+void xen_igd_gfx_pt_set(bool value, Error **errp);
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void xen_igd_reserve_slot(PCIBus *pci_bus);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
+void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+                                           XenHostPCIDevice *dev);
+
+static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
+{
+    return (xen_igd_gfx_pt_enabled()
+            && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+
+#endif
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..0bdefce537 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -15,6 +15,7 @@
 #include "hw/xen/xen_native.h"
 #include "hw/xen/xen-legacy-backend.h"
 #include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "chardev/char.h"
 #include "qemu/accel.h"
 #include "sysemu/cpus.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eace854335..a607dcb56c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -56,6 +56,7 @@
 #ifdef CONFIG_XEN
 #include <xen/hvm/hvm_info_table.h>
 #include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #endif
 #include "hw/xen/xen-x86.h"
 #include "hw/xen/xen.h"
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 36e6f93c37..a8edabdabc 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -59,7 +59,8 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "hw/xen/xen.h"
 #include "hw/xen/xen-legacy-backend.h"
 #include "qemu/range.h"
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 2b8680b112..ba4cd78238 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -15,7 +15,8 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "hw/xen/xen-legacy-backend.h"
 
 #define XEN_PT_MERGE_VALUE(value, data, val_mask) \
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 0aed3bb6fd..6c2e3f4840 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -3,7 +3,8 @@
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "xen-host-pci-device.h"
 
 static unsigned long igd_guest_opregion;
diff --git a/hw/xen/xen_pt_stub.c b/hw/xen/xen_pt_stub.c
index 5c108446a8..72feebeb20 100644
--- a/hw/xen/xen_pt_stub.c
+++ b/hw/xen/xen_pt_stub.c
@@ -6,7 +6,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "qapi/error.h"
 
 bool xen_igd_gfx_pt_enabled(void)
-- 
2.41.0

[PATCH-for-9.0 10/10] hw/xen: Have most of Xen files become target-agnostic

[Philippe Mathieu-Daudé]

Previous commits re-organized the target-specific bits
from Xen files. We can now build the common files once
instead of per-target.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/xen/meson.build          |  2 +-
 hw/block/dataplane/meson.build |  2 +-
 hw/xen/meson.build             | 13 ++++---------
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/accel/xen/meson.build b/accel/xen/meson.build
index 002bdb03c6..455ad5d6be 100644
--- a/accel/xen/meson.build
+++ b/accel/xen/meson.build
@@ -1 +1 @@
-specific_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
+system_ss.add(when: 'CONFIG_XEN', if_true: files('xen-all.c'))
diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
index 025b3b061b..4d8bcb0bb9 100644
--- a/hw/block/dataplane/meson.build
+++ b/hw/block/dataplane/meson.build
@@ -1,2 +1,2 @@
 system_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
-specific_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
+system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
diff --git a/hw/xen/meson.build b/hw/xen/meson.build
index d887fa9ba4..29adfadd1c 100644
--- a/hw/xen/meson.build
+++ b/hw/xen/meson.build
@@ -9,15 +9,12 @@ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files(
 
 system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-operations.c',
+  'xen-hvm-common.c',
+  'xen-mapcache.c',
 ))
 
-xen_specific_ss = ss.source_set()
-xen_specific_ss.add(files(
-  'xen-mapcache.c',
-  'xen-hvm-common.c',
-))
 if have_xen_pci_passthrough
-  xen_specific_ss.add(files(
+  system_ss.add(files(
     'xen-host-pci-device.c',
     'xen_pt.c',
     'xen_pt_config_init.c',
@@ -26,7 +23,5 @@ if have_xen_pci_passthrough
     'xen_pt_msi.c',
   ))
 else
-  xen_specific_ss.add(files('xen_pt_stub.c'))
+  system_ss.add(files('xen_pt_stub.c'))
 endif
-
-specific_ss.add_all(when: ['CONFIG_XEN', xen], if_true: xen_specific_ss)
-- 
2.41.0

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Woodhouse, David]

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



On 13 Nov 2023 10:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.

In order to compile this file once for all targets, factor the
target-specific code out of handle_ioreq() as a per-target handler
called xen_arch_align_ioreq_data().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Should we have a 'unsigned qemu_target_long_bits();' helper
such qemu_target_page_foo() API and target_words_bigendian()?

It can be more fun than that though. What about qemu_target_alignof_uint64() for example, which differs between i386 and x86_64 and causes even structs with *explicitly* sized fields to differ because of padding.

I'd *love* to see this series as a step towards my fantasy of being able to support Xen under TCG. After all, without that what's the point in being target-agnostic?

However, I am mildly concerned that some of these files are accidentally using the host ELF ABI, perhaps with explicit management of 32-bit compatibility, and the target-agnosticity is purely an illusion?

See the "protocol" handling and the three ABIs for the ring in xen-block, for example.

Can we be explicit about what's expected to work here and what's not in scope?




Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.



[-- Attachment #2: Type: text/html, Size: 2480 bytes --]

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

On 13/11/23 16:58, Woodhouse, David wrote:
> On 13 Nov 2023 10:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>     Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
>     function to xen-hvm-common"), handle_ioreq() is expected to be
>     target-agnostic. However it uses 'target_ulong', which is a target
>     specific definition.
> 
>     In order to compile this file once for all targets, factor the
>     target-specific code out of handle_ioreq() as a per-target handler
>     called xen_arch_align_ioreq_data().
> 
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>     ---
>     Should we have a 'unsigned qemu_target_long_bits();' helper
>     such qemu_target_page_foo() API and target_words_bigendian()?
> 
> 
> It can be more fun than that though. What about 
> qemu_target_alignof_uint64() for example, which differs between i386 and 
> x86_64 and causes even structs with *explicitly* sized fields to differ 
> because of padding.
> 
> I'd *love* to see this series as a step towards my fantasy of being able 
> to support Xen under TCG. After all, without that what's the point in 
> being target-agnostic?

Another win is we are building all these files once instead of one for
each i386/x86_64/aarch64 targets, so we save CI time and Amazon trees.

> However, I am mildly concerned that some of these files are accidentally 
> using the host ELF ABI, perhaps with explicit management of 32-bit 
> compatibility, and the target-agnosticity is purely an illusion?
> 
> See the "protocol" handling and the three ABIs for the ring in 
> xen-block, for example.

If so I'd expect build failures or violent runtime assertions.

Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
seem target specific at all IMHO. Otherwise I'd really expect it to
fail compiling. But I don't know much about Xen, so I'll let block &
xen experts to have a look.

> Can we be explicit about what's expected to work here and what's not in 
> scope?

What do you mean? Everything is expected to work like without this
series applied :)

Regards,

Phil.

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[David Woodhouse]

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

On Mon, 2023-11-13 at 17:09 +0100, Philippe Mathieu-Daudé wrote:
> On 13/11/23 16:58, Woodhouse, David wrote:
> > On 13 Nov 2023 10:22, Philippe Mathieu-Daudé <philmd@linaro.org>
> > wrote:
> > 
> >     Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move
> > common
> >     function to xen-hvm-common"), handle_ioreq() is expected to be
> >     target-agnostic. However it uses 'target_ulong', which is a
> > target
> >     specific definition.
> > 
> >     In order to compile this file once for all targets, factor the
> >     target-specific code out of handle_ioreq() as a per-target
> > handler
> >     called xen_arch_align_ioreq_data().
> > 
> >     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >     ---
> >     Should we have a 'unsigned qemu_target_long_bits();' helper
> >     such qemu_target_page_foo() API and target_words_bigendian()?
> > 
> > 
> > It can be more fun than that though. What about 
> > qemu_target_alignof_uint64() for example, which differs between
> > i386 and 
> > x86_64 and causes even structs with *explicitly* sized fields to
> > differ 
> > because of padding.
> > 
> > I'd *love* to see this series as a step towards my fantasy of being
> > able 
> > to support Xen under TCG. After all, without that what's the point
> > in 
> > being target-agnostic?
> 
> Another win is we are building all these files once instead of one
> for
> each i386/x86_64/aarch64 targets, so we save CI time and Amazon
> trees.
> 
> > However, I am mildly concerned that some of these files are
> > accidentally 
> > using the host ELF ABI, perhaps with explicit management of 32-bit 
> > compatibility, and the target-agnosticity is purely an illusion?
> > 
> > See the "protocol" handling and the three ABIs for the ring in 
> > xen-block, for example.
> 
> If so I'd expect build failures or violent runtime assertions.

Heh, mostly the guest just crashes in the cases I've seen so far.

See commit a1c1082908d ("hw/xen: use correct default protocol for xen-
block on x86").

> Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
> seem target specific at all IMHO. Otherwise I'd really expect it to
> fail compiling. But I don't know much about Xen, so I'll let block &
> xen experts to have a look.

Where it checks dataplane->protocol and does different things for
BLKIF_PROTOCOL_NATIVE/BLKIF_PROTOCOL_X86_32/BLKIF_PROTOCOL_X86_64, the
*structures* it uses are intended to be using the correct ABI. I think
the structs for BLKIF_PROTOCOL_NATIVE may actually be *different*
according to the target, in theory?

I don't know that they are *correct* right now, if the host is
different from the target. But that's just a bug (that only matters if
we ever want to support Xen-compatible guests using TCG).

> > Can we be explicit about what's expected to work here and what's
> > not in scope?
> 
> What do you mean? Everything is expected to work like without this
> series applied :)

I think that if we ever do support Xen-compatible guests using TCG,
we'll have to fix that bug and use the right target-specific
structures... and then perhaps we'll want the affected files to
actually become target-specfic again?

I think this series makes it look like target-agnostic support *should*
work... but it doesn't really?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 02/10] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Use a common 'xen_arch_' prefix for architecture-specific functions.
> Rename xen_arch_set_memory() and xen_arch_handle_ioreq().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 03/10] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> We don't need a target-specific header for common target-specific
> prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
> in "hw/xen/xen-hvm-common.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
> function to xen-hvm-common"), handle_ioreq() is expected to be
> target-agnostic. However it uses 'target_ulong', which is a target
> specific definition.
> 
> In order to compile this file once for all targets, factor the
> target-specific code out of handle_ioreq() as a per-target handler
> called xen_arch_align_ioreq_data().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I prefer commits like this to explicitly state 'No function change
intended', and on that basis:

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

But...



> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -699,6 +699,14 @@ void xen_arch_set_memory(XenIOState *state, MemoryRegionSection *section,
>      }
>  }
>  
> +void xen_arch_align_ioreq_data(ioreq_t *req)
> +{
> +    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)
> +            && (req->size < sizeof(target_ulong))) {
> +        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
> +    }
> +}
> +

If a 64-bit Xen host is running a 32-bit guest, what is target_ulong,
and what is the actual alignment? I think we are actually communicating
with the 64-bit Xen and it's 64 bits, although the *guest* is 32?

I guess the only time when this would matter is when using
qemu-system-i386 as the device model on 64-bit Xen? And that's not
going to work for various reasons including this?

(I should clarify that I'm not objecting to your patch series, but I
just to understand just what the situation is, before you make it
*look* saner than it is... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 01/10] sysemu/xen: Forbid using Xen headers in user emulation

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
> Xen is a system specific accelerator, it makes no sense
> to include its headers in user emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/sysemu/xen.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH-for-9.0 02/10] hw/xen/xen_arch_hvm: Rename prototypes using 'xen_arch_' prefix

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
> Use a common 'xen_arch_' prefix for architecture-specific functions.
> Rename xen_arch_set_memory() and xen_arch_handle_ioreq().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/arm/xen_arch_hvm.h  | 4 ++--
>   include/hw/i386/xen_arch_hvm.h | 4 ++--
>   hw/arm/xen_arm.c               | 4 ++--
>   hw/i386/xen/xen-hvm.c          | 6 +++---
>   hw/xen/xen-hvm-common.c        | 4 ++--
>   5 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH-for-9.0 03/10] hw/xen: Merge 'hw/xen/arch_hvm.h' in 'hw/xen/xen-hvm-common.h'

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
> We don't need a target-specific header for common target-specific
> prototypes. Declare xen_arch_handle_ioreq() and xen_arch_set_memory()
> in "hw/xen/xen-hvm-common.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/arm/xen_arch_hvm.h   |  9 ---------
>   include/hw/i386/xen_arch_hvm.h  | 11 -----------
>   include/hw/xen/arch_hvm.h       |  5 -----
>   include/hw/xen/xen-hvm-common.h |  6 ++++++
>   hw/arm/xen_arm.c                |  1 -
>   hw/i386/xen/xen-hvm.c           |  1 -
>   hw/xen/xen-hvm-common.c         |  1 -
>   7 files changed, 6 insertions(+), 28 deletions(-)
>   delete mode 100644 include/hw/arm/xen_arch_hvm.h
>   delete mode 100644 include/hw/i386/xen_arch_hvm.h
>   delete mode 100644 include/hw/xen/arch_hvm.h
> 

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
> diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
> index c028c1b541..03f9417e7e 100644
> --- a/hw/xen/xen-hvm-common.c
> +++ b/hw/xen/xen-hvm-common.c
> @@ -426,10 +426,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
>       trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
>                          req->addr, req->data, req->count, req->size);
>   
> -    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
> -            (req->size < sizeof (target_ulong))) {
> -        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
> -    }


I suspect this should never have been using target_ulong at all: req->data is uint64_t.


r~

Re: [PATCH-for-9.0 05/10] hw/xen: Use target-agnostic qemu_target_page_bits()

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
> Instead of the target-specific TARGET_PAGE_BITS definition,
> use qemu_target_page_bits() which is target agnostic.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/xen/xen-hvm-common.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH-for-9.0 06/10] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources

[Richard Henderson]

On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
> We rarely need to include "cpu.h" in headers. Including it
> 'taint' headers to be target-specific. Here only the i386/arm
> implementations requires "cpu.h", so include it there and
> remove from the "hw/xen/xen-hvm-common.h" *common* header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/xen/xen-hvm-common.h | 1 -
>   hw/arm/xen_arm.c                | 1 +
>   hw/i386/xen/xen-hvm.c           | 1 +
>   3 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH-for-9.0 05/10] hw/xen: Use target-agnostic qemu_target_page_bits()

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Instead of the target-specific TARGET_PAGE_BITS definition,
> use qemu_target_page_bits() which is target agnostic.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 06/10] hw/xen: Reduce inclusion of 'cpu.h' to target-specific sources

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> We rarely need to include "cpu.h" in headers. Including it
> 'taint' headers to be target-specific. Here only the i386/arm
> implementations requires "cpu.h", so include it there and
> remove from the "hw/xen/xen-hvm-common.h" *common* header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> "sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
> version of CONFIG_XEN. Use it in order to use "sysemu/xen-mapcache.h"
> in target-agnostic files.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Noting that CONFIG_XEN_IS_POSSIBLE is for Xen accelerator support, and
may not be set in all cases when we're hosting Xen-compatible guests,

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 08/10] system/physmem: Only include 'hw/xen/xen.h' when Xen is available

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> "hw/xen/xen.h" contains declarations for Xen hardware. There is
> no point including it when Xen is not available.

... if even when Xen *is* available, AFAICT. Can you just remove the
inclusion of hw/xen/xen.h entirely? I think that still builds, at least
for x86.

>  When Xen is not
> available, we have enough with declarations of "sysemu/xen.h".

... and system/xen-mapcache.h

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 09/10] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> "hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target
> specific. It also declares IGD methods, which are not target
> specific.
> 
> Target-agnostic code can use IGD methods. To allow that, extract
> these methos into a new "hw/xen/xen_igd.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

> ---
> What license for the new "hw/xen/xen_igd.h" header?

The existing xen_pt.h came in with xen_pt.c (GPLv2) in commit
eaab4d60d. I think it has to be GPLv2 (and not later) just like
xen_pt.c?


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 10/10] hw/xen: Have most of Xen files become target-agnostic

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Previous commits re-organized the target-specific bits
> from Xen files. We can now build the common files once
> instead of per-target.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 01/10] sysemu/xen: Forbid using Xen headers in user emulation

[David Woodhouse]

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

On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
> Xen is a system specific accelerator, it makes no sense
> to include its headers in user emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

On 13/11/23 19:16, Richard Henderson wrote:
> On 11/13/23 07:21, Philippe Mathieu-Daudé wrote:
>> diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
>> index c028c1b541..03f9417e7e 100644
>> --- a/hw/xen/xen-hvm-common.c
>> +++ b/hw/xen/xen-hvm-common.c
>> @@ -426,10 +426,7 @@ static void handle_ioreq(XenIOState *state, 
>> ioreq_t *req)
>>       trace_handle_ioreq(req, req->type, req->dir, req->df, 
>> req->data_is_ptr,
>>                          req->addr, req->data, req->count, req->size);
>> -    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
>> -            (req->size < sizeof (target_ulong))) {
>> -        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
>> -    }
> 
> 
> I suspect this should never have been using target_ulong at all: 
> req->data is uint64_t.

This could replace it:

-- >8 --
-    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&
-            (req->size < sizeof (target_ulong))) {
-        req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+    if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)) {
+        req->data = extract64(req->data, 0, BITS_PER_BYTE * req->size);
      }
---

Some notes while looking at this.

Per xen/include/public/hvm/ioreq.h header:

#define IOREQ_TYPE_PIO          0 /* pio */
#define IOREQ_TYPE_COPY         1 /* mmio ops */
#define IOREQ_TYPE_PCI_CONFIG   2
#define IOREQ_TYPE_VMWARE_PORT  3
#define IOREQ_TYPE_TIMEOFFSET   7
#define IOREQ_TYPE_INVALIDATE   8 /* mapcache */

   struct ioreq {
     uint64_t addr;          /* physical address */
     uint64_t data;          /* data (or paddr of data) */
     uint32_t count;         /* for rep prefixes */
     uint32_t size;          /* size in bytes */
     uint32_t vp_eport;      /* evtchn for notifications to/from device 
model */
     uint16_t _pad0;
     uint8_t state:4;
     uint8_t data_is_ptr:1;  /* if 1, data above is the guest paddr
                              * of the real data to use. */
     uint8_t dir:1;          /* 1=read, 0=write */
     uint8_t df:1;
     uint8_t _pad1:1;
     uint8_t type;           /* I/O type */
   };
   typedef struct ioreq ioreq_t;

If 'data' is not a pointer, it is a u64.

- In PIO / VMWARE_PORT modes, only 32-bit are used.

- In MMIO COPY mode, memory is accessed by chunks of 64-bit

- In PCI_CONFIG mode, access is u8 or u16 or u32.

- None of TIMEOFFSET / INVALIDATE use 'req'.

- Fallback is only used in x86 for VMWARE_PORT.

--

Regards,

Phil.

Re: [PATCH-for-9.0 08/10] system/physmem: Only include 'hw/xen/xen.h' when Xen is available

[Philippe Mathieu-Daudé]

On 13/11/23 21:03, David Woodhouse wrote:
> On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
>> "hw/xen/xen.h" contains declarations for Xen hardware. There is
>> no point including it when Xen is not available.
> 
> ... if even when Xen *is* available, AFAICT. Can you just remove the
> inclusion of hw/xen/xen.h entirely? I think that still builds, at least
> for x86.

Yep, also on aarch64, thanks!

>>   When Xen is not
>> available, we have enough with declarations of "sysemu/xen.h".
> 
> ... and system/xen-mapcache.h
> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[Philippe Mathieu-Daudé]

On 13/11/23 18:11, David Woodhouse wrote:
> On Mon, 2023-11-13 at 17:09 +0100, Philippe Mathieu-Daudé wrote:
>> On 13/11/23 16:58, Woodhouse, David wrote:
>>> On 13 Nov 2023 10:22, Philippe Mathieu-Daudé <philmd@linaro.org>
>>> wrote:
>>>
>>>      Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move
>>> common
>>>      function to xen-hvm-common"), handle_ioreq() is expected to be
>>>      target-agnostic. However it uses 'target_ulong', which is a
>>> target
>>>      specific definition.
>>>
>>>      In order to compile this file once for all targets, factor the
>>>      target-specific code out of handle_ioreq() as a per-target
>>> handler
>>>      called xen_arch_align_ioreq_data().
>>>
>>>      Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>      ---
>>>      Should we have a 'unsigned qemu_target_long_bits();' helper
>>>      such qemu_target_page_foo() API and target_words_bigendian()?
>>>
>>>
>>> It can be more fun than that though. What about
>>> qemu_target_alignof_uint64() for example, which differs between
>>> i386 and
>>> x86_64 and causes even structs with *explicitly* sized fields to
>>> differ
>>> because of padding.
>>>
>>> I'd *love* to see this series as a step towards my fantasy of being
>>> able
>>> to support Xen under TCG. After all, without that what's the point
>>> in
>>> being target-agnostic?
>>
>> Another win is we are building all these files once instead of one
>> for
>> each i386/x86_64/aarch64 targets, so we save CI time and Amazon
>> trees.
>>
>>> However, I am mildly concerned that some of these files are
>>> accidentally
>>> using the host ELF ABI, perhaps with explicit management of 32-bit
>>> compatibility, and the target-agnosticity is purely an illusion?
>>>
>>> See the "protocol" handling and the three ABIs for the ring in
>>> xen-block, for example.
>>
>> If so I'd expect build failures or violent runtime assertions.
> 
> Heh, mostly the guest just crashes in the cases I've seen so far.
> 
> See commit a1c1082908d ("hw/xen: use correct default protocol for xen-
> block on x86").
> 
>> Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
>> seem target specific at all IMHO. Otherwise I'd really expect it to
>> fail compiling. But I don't know much about Xen, so I'll let block &
>> xen experts to have a look.
> 
> Where it checks dataplane->protocol and does different things for
> BLKIF_PROTOCOL_NATIVE/BLKIF_PROTOCOL_X86_32/BLKIF_PROTOCOL_X86_64, the
> *structures* it uses are intended to be using the correct ABI. I think
> the structs for BLKIF_PROTOCOL_NATIVE may actually be *different*
> according to the target, in theory?

OK I see what you mean, blkif_back_rings_t union in hw/block/xen_blkif.h

These structures shouldn't differ between targets, this is the point of
an ABI :) And if they were, they wouldn't compile as target agnostic.

> I don't know that they are *correct* right now, if the host is
> different from the target. But that's just a bug (that only matters if
> we ever want to support Xen-compatible guests using TCG).
> 
>>> Can we be explicit about what's expected to work here and what's
>>> not in scope?
>>
>> What do you mean? Everything is expected to work like without this
>> series applied :)
> 
> I think that if we ever do support Xen-compatible guests using TCG,
> we'll have to fix that bug and use the right target-specific
> structures... and then perhaps we'll want the affected files to
> actually become target-specfic again?
> 
> I think this series makes it look like target-agnostic support *should*
> work... but it doesn't really?

For testing we have:

aarch64: tests/avocado/boot_xen.py
x86_64: tests/avocado/kvm_xen_guest.py

No combination with i386 is tested,
Xen within aarch64 KVM is not tested (not sure it works).

Re: [PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[Philippe Mathieu-Daudé]

On 13/11/23 20:52, David Woodhouse wrote:
> On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
>> "sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
>> version of CONFIG_XEN. Use it in order to use "sysemu/xen-mapcache.h"
>> in target-agnostic files.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Noting that CONFIG_XEN_IS_POSSIBLE is for Xen accelerator support, and
> may not be set in all cases when we're hosting Xen-compatible guests,

As is CONFIG_XEN.

Maybe be worth renaming CONFIG_ACCEL_XEN if you think we need
guest hw specific CONFIG_foo_XEN variables.

> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>

Thanks!

Re: [PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[David Woodhouse]

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

On Tue, 2023-11-14 at 13:25 +0100, Philippe Mathieu-Daudé wrote:
> 
> As is CONFIG_XEN.
> 
> Maybe be worth renaming CONFIG_ACCEL_XEN if you think we need
> guest hw specific CONFIG_foo_XEN variables.

I don't think so. We have CONFIG_XEN_BUS and CONFIG_XEN_EMU (from
commit 820c1aba519b) which I think are all we need.

Thanks.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 04/10] hw/xen: Factor xen_arch_align_ioreq_data() out of handle_ioreq()

[David Woodhouse]

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

On Tue, 2023-11-14 at 08:58 +0100, Philippe Mathieu-Daudé wrote:
> > > Reviewing quickly hw/block/dataplane/xen-block.c, this code doesn't
> > > seem target specific at all IMHO. Otherwise I'd really expect it to
> > > fail compiling. But I don't know much about Xen, so I'll let block &
> > > xen experts to have a look.
> > 
> > Where it checks dataplane->protocol and does different things for
> > BLKIF_PROTOCOL_NATIVE/BLKIF_PROTOCOL_X86_32/BLKIF_PROTOCOL_X86_64, the
> > *structures* it uses are intended to be using the correct ABI. I think
> > the structs for BLKIF_PROTOCOL_NATIVE may actually be *different*
> > according to the target, in theory?
> 
> OK I see what you mean, blkif_back_rings_t union in hw/block/xen_blkif.h
> 
> These structures shouldn't differ between targets, this is the point of
> an ABI :) 

Structures like that *shouldn't* differ between targets, but the Xen
struct blkif_request does:

typedef blkif_vdev_t uint16_t;
struct blkif_request {
    uint8_t        operation;    /* BLKIF_OP_???                         */
    uint8_t        nr_segments;  /* number of segments                   */
    blkif_vdev_t   handle;       /* only for read/write requests         */
    uint64_t       id;           /* private guest value, echoed in resp  */
    blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
    struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
};

This is why we end up with explicit versions for x86-32 and x86-64,
with the 'id' field aligned explicitly to 4 or 8 bytes. The 'native'
version when we build it in qemu will just use the *host* ABI to decide
how to align it.

> And if they were, they wouldn't compile as target agnostic.

The words "wouldn't compile" gives me fantasies of a compiler that
literally errors out, saying "you can't use that struct in arch-
agnostic code because the padding is different on different
architectures".

It isn't so. You're just a tease.

With the exception of the x86-{32,64} special case, the
BLKIF_PROTOCOL_NATIVE support ends up using the *host* ABI, regardless
of which target it was aimed at.

Which might even be OK; are there any other targets which align
uint64_t to 4 bytes, like i386 does? And certainly isn't *your* problem
anyway.

What I was thinking was that if we *do* need to do something to make
BLKIF_PROTOCOL_NATIVE actually differ between targets, maybe that would
mean we really *do* want to build this code separately for each target
rather than just once? 

I suppose *if* we fix it, we can fix it in a way that doesn't require
specific compilation. Like we already did for x86. I literally use
object_dynamic_cast(qdev_get_machine(), "x86-machine") there.

> 
> > I think this series makes it look like target-agnostic support *should*
> > work... but it doesn't really?
> 
> For testing we have:
> 
> aarch64: tests/avocado/boot_xen.py
> x86_64: tests/avocado/kvm_xen_guest.py
> 
> No combination with i386 is tested,
> Xen within aarch64 KVM is not tested (not sure it works).

No, there is support in the *kernel* for Xen guests, which hasn't been
added to AArch64 KVM.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH-for-9.0 07/10] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

[Philippe Mathieu-Daudé]

Cc'ing Paolo

On 14/11/23 13:25, Philippe Mathieu-Daudé wrote:
> On 13/11/23 20:52, David Woodhouse wrote:
>> On Mon, 2023-11-13 at 16:21 +0100, Philippe Mathieu-Daudé wrote:
>>> "sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
>>> version of CONFIG_XEN. Use it in order to use "sysemu/xen-mapcache.h"
>>> in target-agnostic files.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Noting that CONFIG_XEN_IS_POSSIBLE is for Xen accelerator support, and
>> may not be set in all cases when we're hosting Xen-compatible guests,
> 
> As is CONFIG_XEN.

Actually we have:

- host Xen support
   . CONFIG_XEN_BACKEND

- QEMU Xen accelerator
   . CONFIG_XEN

- HW models
   . CONFIG_XEN (old / generic models)
   . CONFIG_XEN_BUS
   . XEN_IGD_PASSTHROUGH (PCI stuff)
   . CONFIG_XEN_EMU (Xen on KVM)

Paolo, David, is that correct?

When can we have CONFIG_XEN without CONFIG_XEN_BACKEND
(and vice-et-versa)?

So for clarity CONFIG_XEN could be split as:
  - CONFIG_ACCEL_XEN
  - CONFIG_XEN_MACHINE_FV (Fully-virtualized)
  - CONFIG_XEN_MACHINE_PV (Para-virtualized)

> Maybe be worth renaming CONFIG_ACCEL_XEN if you think we need
> guest hw specific CONFIG_foo_XEN variables.
> 
>> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> Thanks!