|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |