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

[PATCH v5 10/11] hvm/ioreq: Negotiate extended destination ID support per ioreq server



Extended (15-bit) MSI / IO-APIC destination IDs need Xen to decode the
raw MSI message for every passthrough MSI, which in turn requires every
device model to use XEN_DMOP_bind_pt_msi_irq. Let each ioreq server
advertise that it does so:

- XEN_DMOP_create_ioreq_server gains a flags byte (reusing pad[0]) with
  XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID. arch_ioreq_server_create_check()
  validates the flags (rejecting unknown bits, and the whole flag on
  non-x86) and, once the feature is locked, refuses a server that lacks
  it.

- hvm_ext_dest_id_enabled() is true if at least one server exists and
  all of them opted in. arch_domain_creation_finished() latches the
  result into the tri-state d->arch.hvm.ext_dest_id, unless a migration
  stream already fixed it.

- A new HVM_SAVE_TYPE(EXT_DEST_ID) record (with save/check/load)
  migrates the latched value, so the destination host behaves
  identically regardless of when its device model re-registers ioreq
  servers. A stream from a Xen predating the feature carries no such
  record. hvm_load() then resolves the still-UNSET state to DISABLED so
  arch_domain_creation_finished() does not mistake the restored domain
  for a fresh one and enable the feature behind an unaware guest.

libxendevicemodel's xendevicemodel_create_ioreq_server() grows the flags
argument (0 for existing callers).

Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
---
Changes in v5:
- Commit message rewritten as a bulleted summary.
- As discussed with Jan: The creation_finished latch only fires while it
  is still UNSET, so a value restored from the migration stream wins. v4
  OR-ed hvm_ext_dest_id_enabled() onto a bool, which flipped an unaware
  migrated guest to ENABLED.
- hvm_ext_dest_id_enabled() moved out of the header into common/ioreq.c
  as a function with a locking note. It iterates via
  ARRAY_SIZE(d->ioreq_server.server), not MAX_NR_IOREQ_SERVERS.
- Flags are unsigned int now. Flag-bit validation is arch-specific
  arch_ioreq_server_create_check() rejects unknown bits and the Arm stub
  rejects any non-zero flag. The is_hvm_domain() check is dropped from
  the x86 hook. The post-lock rejection tests EXT_DEST_ID_ENABLED
  explicitly.
- The save record gains a check handler (ext_dest_id_check). Load
  validates the enum range and returns -ENODATA on a truncated record.
- Save-record comment de-x86-ified.
- A stream from a Xen predating the feature carries no EXT_DEST_ID
  record, so hvm_load() resolves the UNSET state to DISABLED at
  end-of-stream.
- Removed the vIO-APIC ioapic_check() extended-bit rejection loop.
- Added parentheses around the bitwise logic.

Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
---
 tools/include/xendevicemodel.h          |  3 +-
 tools/libs/ctrl/xc_devicemodel_compat.c |  2 +-
 tools/libs/devicemodel/core.c           |  3 +-
 xen/arch/arm/ioreq.c                    |  6 +++
 xen/arch/x86/domain.c                   | 13 ++++++
 xen/arch/x86/hvm/ioreq.c                | 57 +++++++++++++++++++++++++
 xen/arch/x86/hvm/save.c                 |  9 ++++
 xen/common/ioreq.c                      | 31 ++++++++++++--
 xen/include/public/arch-x86/hvm/save.h  | 16 ++++++-
 xen/include/public/hvm/dm_op.h          | 13 +++++-
 xen/include/xen/ioreq.h                 | 11 +++++
 11 files changed, 156 insertions(+), 8 deletions(-)

diff --git a/tools/include/xendevicemodel.h b/tools/include/xendevicemodel.h
index 698d719119..72994d1313 100644
--- a/tools/include/xendevicemodel.h
+++ b/tools/include/xendevicemodel.h
@@ -44,12 +44,13 @@ int xendevicemodel_close(xendevicemodel_handle *dmod);
  * @parm domid the domain id to be serviced
  * @parm handle_bufioreq how should the IOREQ Server handle buffered
  *                       requests (HVM_IOREQSRV_BUFIOREQ_*)?
+ * @parm flags bitmask of XEN_DMOP_IOREQ_SERVER_* capability flags (0 if none).
  * @parm id pointer to an ioservid_t to receive the IOREQ Server id.
  * @return 0 on success, -1 on failure.
  */
 int xendevicemodel_create_ioreq_server(
     xendevicemodel_handle *dmod, domid_t domid, int handle_bufioreq,
-    ioservid_t *id);
+    uint8_t flags, ioservid_t *id);
 
 /**
  * This function retrieves the necessary information to allow an
diff --git a/tools/libs/ctrl/xc_devicemodel_compat.c 
b/tools/libs/ctrl/xc_devicemodel_compat.c
index a46011cd17..91366e250c 100644
--- a/tools/libs/ctrl/xc_devicemodel_compat.c
+++ b/tools/libs/ctrl/xc_devicemodel_compat.c
@@ -11,7 +11,7 @@ int xc_hvm_create_ioreq_server(
     ioservid_t *id)
 {
     return xendevicemodel_create_ioreq_server(xch->dmod, domid,
-                                              handle_bufioreq, id);
+                                              handle_bufioreq, 0, id);
 }
 
 int xc_hvm_get_ioreq_server_info(
diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
index 274a8eb28b..5b2acb1869 100644
--- a/tools/libs/devicemodel/core.c
+++ b/tools/libs/devicemodel/core.c
@@ -167,7 +167,7 @@ static int xendevicemodel_op(
 
 int xendevicemodel_create_ioreq_server(
     xendevicemodel_handle *dmod, domid_t domid, int handle_bufioreq,
-    ioservid_t *id)
+    uint8_t flags, ioservid_t *id)
 {
     struct xen_dm_op op;
     struct xen_dm_op_create_ioreq_server *data;
@@ -179,6 +179,7 @@ int xendevicemodel_create_ioreq_server(
     data = &op.u.create_ioreq_server;
 
     data->handle_bufioreq = handle_bufioreq;
+    data->flags = flags;
 
     rc = xendevicemodel_op(dmod, domid, 1, &op, sizeof(op));
     if (rc)
diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index b4211f0159..7d26180926 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -201,6 +201,12 @@ void arch_ioreq_domain_init(struct domain *d)
 {
 }
 
+int arch_ioreq_server_create_check(const struct domain *d, unsigned int flags)
+{
+    /* No XEN_DMOP_IOREQ_SERVER_* capability flags are defined for Arm. */
+    return flags ? -EINVAL : 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 996b50af7a..69c9093e06 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -25,6 +25,7 @@
 #include <xen/init.h>
 #include <xen/iocap.h>
 #include <xen/iommu.h>
+#include <xen/ioreq.h>
 #include <xen/irq.h>
 #include <xen/kernel.h>
 #include <xen/lib.h>
@@ -1106,7 +1107,19 @@ int arch_domain_soft_reset(struct domain *d)
 void arch_domain_creation_finished(struct domain *d)
 {
     if ( is_hvm_domain(d) )
+    {
+        /*
+         * Latch the extended destination ID decision now that all boot-time
+         * ioreq servers are registered. A value restored from a migration
+         * stream (EXT_DEST_ID save record) already fixes it and wins.
+         */
+        if ( d->arch.hvm.ext_dest_id == EXT_DEST_ID_UNSET )
+            d->arch.hvm.ext_dest_id = hvm_ext_dest_id_enabled(d)
+                                      ? EXT_DEST_ID_ENABLED
+                                      : EXT_DEST_ID_DISABLED;
+
         hvm_domain_creation_finished(d);
+    }
 }
 
 #ifdef CONFIG_COMPAT
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index a5fa97e149..63d22f6738 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -19,6 +19,7 @@
 
 #include <asm/hvm/emulate.h>
 #include <asm/hvm/hvm.h>
+#include <asm/hvm/support.h>
 #include <asm/hvm/vmx/vmx.h>
 #include <asm/msr.h>
 
@@ -325,6 +326,62 @@ void arch_ioreq_domain_init(struct domain *d)
     register_portio_handler(d, 0xcf8, 4, hvm_access_cf8);
 }
 
+int arch_ioreq_server_create_check(const struct domain *d, unsigned int flags)
+{
+    if ( flags & ~XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID )
+        return -EINVAL;
+
+    /*
+     * Once the domain is running with extended destination IDs advertised,
+     * every ioreq server it gains must be able to bind MSIs the new way.
+     * (Before that point d->arch.hvm.ext_dest_id is still UNSET and servers
+     * are levelled at arch_domain_creation_finished().)
+     */
+    if ( d->arch.hvm.ext_dest_id == EXT_DEST_ID_ENABLED &&
+         !(flags & XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID) )
+        return -EPERM;
+
+    return 0;
+}
+
+static int cf_check ext_dest_id_save(struct vcpu *v, hvm_domain_context_t *h)
+{
+    struct hvm_hw_ext_dest_id s = {
+        .enabled = v->domain->arch.hvm.ext_dest_id,
+    };
+
+    return hvm_save_entry(EXT_DEST_ID, 0, h, &s);
+}
+
+static int cf_check ext_dest_id_check(const struct domain *d,
+                                      hvm_domain_context_t *h)
+{
+    const struct hvm_hw_ext_dest_id *s = hvm_get_entry(EXT_DEST_ID, h);
+
+    if ( !s )
+        return -ENODATA;
+
+    return s->enabled > EXT_DEST_ID_ENABLED ? -EINVAL : 0;
+}
+
+static int cf_check ext_dest_id_load(struct domain *d, hvm_domain_context_t *h)
+{
+    struct hvm_hw_ext_dest_id s;
+
+    if ( hvm_load_entry(EXT_DEST_ID, h, &s) )
+        return -ENODATA;
+
+    if ( s.enabled > EXT_DEST_ID_ENABLED )
+        return -EINVAL;
+
+    d->arch.hvm.ext_dest_id = s.enabled;
+
+    return 0;
+}
+
+HVM_REGISTER_SAVE_RESTORE(EXT_DEST_ID, ext_dest_id_save, ext_dest_id_check,
+                          ext_dest_id_load, 1, HVMSR_PER_DOM);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
index 8ab6405706..aac8498049 100644
--- a/xen/arch/x86/hvm/save.c
+++ b/xen/arch/x86/hvm/save.c
@@ -341,6 +341,15 @@ int hvm_load(struct domain *d, bool real, 
hvm_domain_context_t *h)
             /* Reset cursor for hvm_load(, true, ). */
             if ( !real )
                 h->cur = 0;
+
+            /*
+             * A migration stream from a Xen predating extended destination IDs
+             * carries no EXT_DEST_ID record, leaving ext_dest_id UNSET here.
+             * Set it to DISABLED then, because the domain has no knowledge
+             * about EXT_DEST_IDs.
+             */
+            else if ( d->arch.hvm.ext_dest_id == EXT_DEST_ID_UNSET )
+                d->arch.hvm.ext_dest_id = EXT_DEST_ID_DISABLED;
             return 0;
         }
 
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index f5fd30ce12..53003286c4 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -79,6 +79,25 @@ static struct ioreq_server *get_ioreq_server(const struct 
domain *d,
     return GET_IOREQ_SERVER(d, id);
 }
 
+bool hvm_ext_dest_id_enabled(const struct domain *d)
+{
+    unsigned int id;
+    bool any = false;
+
+    for ( id = 0; id < ARRAY_SIZE(d->ioreq_server.server); id++ )
+    {
+        const struct ioreq_server *s = GET_IOREQ_SERVER(d, id);
+
+        if ( !s )
+            continue;
+        if ( !s->ext_dest_id )
+            return false;
+        any = true;
+    }
+
+    return any;
+}
+
 /*
  * Iterate over all possible ioreq servers.
  *
@@ -641,7 +660,7 @@ static void ioreq_server_deinit(struct ioreq_server *s)
 }
 
 static int ioreq_server_create(struct domain *d, int bufioreq_handling,
-                               ioservid_t *id)
+                               unsigned int flags, ioservid_t *id)
 {
     struct ioreq_server *s;
     unsigned int i;
@@ -683,6 +702,8 @@ static int ioreq_server_create(struct domain *d, int 
bufioreq_handling,
         goto fail;
     }
 
+    s->ext_dest_id = flags & XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID;
+
     if ( id )
         *id = i;
 
@@ -1350,10 +1371,14 @@ int ioreq_server_dm_op(struct xen_dm_op *op, struct 
domain *d, bool *const_op)
         *const_op = false;
 
         rc = -EINVAL;
-        if ( data->pad[0] || data->pad[1] || data->pad[2] )
+        if ( data->pad[0] || data->pad[1] )
+            break;
+
+        rc = arch_ioreq_server_create_check(d, data->flags);
+        if ( rc )
             break;
 
-        rc = ioreq_server_create(d, data->handle_bufioreq,
+        rc = ioreq_server_create(d, data->handle_bufioreq, data->flags,
                                  &data->id);
         break;
     }
diff --git a/xen/include/public/arch-x86/hvm/save.h 
b/xen/include/public/arch-x86/hvm/save.h
index ff73b83d65..58d301512e 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -629,12 +629,26 @@ struct hvm_msr {
 
 #define CPU_MSR_CODE  20
 
+/*
+ * Extended (15-bit) MSI / IO-APIC destination ID support, as negotiated with
+ * the domain's ioreq servers and latched when the domain starts running.
+ *
+ * 'enabled' takes the values of enum in struct hvm_domain: 0 = undecided,
+ * 1 = off, 2 = on.
+ */
+struct hvm_hw_ext_dest_id {
+    uint8_t enabled;
+    uint8_t pad[7];
+};
+
+DECLARE_HVM_SAVE_TYPE(EXT_DEST_ID, 21, struct hvm_hw_ext_dest_id);
+
 /* Range 22 - 34 (inclusive) reserved for Amazon */
 
 /*
  * Largest type-code in use
  */
-#define HVM_SAVE_CODE_MAX 20
+#define HVM_SAVE_CODE_MAX 21
 
 #endif /* __XEN_PUBLIC_HVM_SAVE_X86_H__ */
 
diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
index d76777f71f..0e1774b1d3 100644
--- a/xen/include/public/hvm/dm_op.h
+++ b/xen/include/public/hvm/dm_op.h
@@ -44,13 +44,24 @@ typedef uint16_t ioservid_t;
  * hvm_op.h. If the value is HVM_IOREQSRV_BUFIOREQ_OFF then  the buffered
  * ioreq ring will not be allocated and hence all emulation requests to
  * this server will be synchronous.
+ *
+ * x86 HVM only: a server that sets XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID in
+ * <flags> promises to bind every passthrough MSI via XEN_DMOP_bind_pt_msi_irq
+ * (which carries the raw MSI message). Extended (15-bit) MSI / IO-APIC
+ * destination IDs are enabled for the domain, and advertised via
+ * XEN_HVM_CPUID_EXT_DEST_ID, only if *every* ioreq server registered before
+ * the domain starts running sets this flag. Once the feature is locked in,
+ * later servers that lack the flag are rejected.
  */
 #define XEN_DMOP_create_ioreq_server 1
 
 struct xen_dm_op_create_ioreq_server {
     /* IN - should server handle buffered ioreqs */
     uint8_t handle_bufioreq;
-    uint8_t pad[3];
+    /* IN - XEN_DMOP_IOREQ_SERVER_* */
+    uint8_t flags;
+#define XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID (1u << 0)
+    uint8_t pad[2];
     /* OUT - server id */
     ioservid_t id;
 };
diff --git a/xen/include/xen/ioreq.h b/xen/include/xen/ioreq.h
index e86f0869fa..a75bd04b97 100644
--- a/xen/include/xen/ioreq.h
+++ b/xen/include/xen/ioreq.h
@@ -54,9 +54,19 @@ struct ioreq_server {
     evtchn_port_t          bufioreq_evtchn;
     struct rangeset        *range[NR_IO_RANGE_TYPES];
     bool                   enabled;
+    bool                   ext_dest_id;
     uint8_t                bufioreq_handling;
 };
 
+/*
+ * True if at least one ioreq server is registered and every registered server
+ * set XEN_DMOP_IOREQ_SERVER_EXT_DEST_ID. Only meaningful before the domain
+ * starts running (called from arch_domain_creation_finished(), when no
+ * concurrent ioreq server (de)registration can occur). The result is latched
+ * into d->arch.hvm.ext_dest_id there.
+ */
+bool hvm_ext_dest_id_enabled(const struct domain *d);
+
 static inline paddr_t ioreq_mmio_first_byte(const ioreq_t *p)
 {
     return unlikely(p->df) ?
@@ -137,6 +147,7 @@ bool arch_ioreq_server_destroy_all(struct domain *d);
 bool arch_ioreq_server_get_type_addr(const struct domain *d, const ioreq_t *p,
                                      uint8_t *type, uint64_t *addr);
 void arch_ioreq_domain_init(struct domain *d);
+int arch_ioreq_server_create_check(const struct domain *d, unsigned int flags);
 
 #endif /* __XEN_IOREQ_H__ */
 
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

 


Rackspace

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