diff mbox series

[v2,2/2] xen/dm: arm: Introduce inject_msi2 DM op

Message ID e9e7d722eeac100048ddd5a4032e3d7af39f8c3f.1744621166.git.mykyta_poturai@epam.com (mailing list archive)
State New
Headers show
Series Add support for MSI injection on Arm | expand

Commit Message

Mykyta Poturai April 14, 2025, 9:51 a.m. UTC
From: Mykyta Poturai <Mykyta_Poturai@epam.com>

Add the second version of inject_msi DM op, which allows to specify
the source_id of an MSI interrupt. This is needed for correct MSI
injection on ARM.

It would not be safe to include the source_id in the original inject_msi
in the pad field, because we have no way to know if it is set or not.

Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
v1->v2:
* fix warning for ignored source_id
* rework xen_dm_op_inject_msi2 struct
* rework xendevicemodel_inject_msi2 params
---
 tools/include/xendevicemodel.h               | 14 ++++++++++++++
 tools/libs/devicemodel/core.c                | 20 ++++++++++++++++++++
 tools/libs/devicemodel/libxendevicemodel.map |  5 +++++
 xen/arch/arm/dm.c                            | 17 +++++++++++++++++
 xen/arch/x86/hvm/dm.c                        | 18 ++++++++++++++++++
 xen/include/public/hvm/dm_op.h               | 18 ++++++++++++++++++
 6 files changed, 92 insertions(+)

Comments

Jan Beulich April 16, 2025, 3:16 p.m. UTC | #1
On 14.04.2025 11:51, Mykyta Poturai wrote:
> From: Mykyta Poturai <Mykyta_Poturai@epam.com>
> 
> Add the second version of inject_msi DM op, which allows to specify
> the source_id of an MSI interrupt. This is needed for correct MSI
> injection on ARM.

If this is about Arm, why does x86 also gain (incomplete) handling?

> @@ -539,6 +540,23 @@ int dm_op(const struct dmop_args *op_args)
>          break;
>      }
>  
> +    case XEN_DMOP_inject_msi2:
> +    {
> +        const struct xen_dm_op_inject_msi2 *data = &op.u.inject_msi2;
> +
> +        if ( data->pad || data->flags & ~XEN_DMOP_MSI_SOURCE_ID_VALID )

Nit: If the x86 code is to stay in the first place, parentheses please
around the & expression. Even if not, similar issues appear to exist
in the Arm code.

> +        {
> +            rc = -EINVAL;
> +            break;
> +        }
> +
> +        if ( data->flags & XEN_DMOP_MSI_SOURCE_ID_VALID )
> +            gprintk(XENLOG_WARNING "XEN_DMOP_inject_msi2: source_id is ignored\n");

Does this compile? It looks to me as if there was a missing comma.

> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -444,6 +444,23 @@ struct xen_dm_op_nr_vcpus {
>  };
>  typedef struct xen_dm_op_nr_vcpus xen_dm_op_nr_vcpus_t;
>  
> +#define XEN_DMOP_inject_msi2 21
> +
> +struct xen_dm_op_inject_msi2 {
> +      /* IN - MSI data */
> +      uint32_t data;
> +      /* IN - next two fields form an ID of the device triggering the MSI */
> +      uint16_t segment; /* The segment number */
> +      uint16_t source_id; /* The source ID that is local to segment (PCI BDF) */
> +      /* IN - types of source ID */
> +      uint32_t flags;
> +#define XEN_DMOP_MSI_SOURCE_ID_VALID (1u << 0)

What purpose does this flag serve? IOW what's the value of using this
sub-op when one doesn't want to specify a source ID?

Jan
diff mbox series

Patch

diff --git a/tools/include/xendevicemodel.h b/tools/include/xendevicemodel.h
index 227e7fd810..d0847dfdc8 100644
--- a/tools/include/xendevicemodel.h
+++ b/tools/include/xendevicemodel.h
@@ -236,6 +236,20 @@  int xendevicemodel_inject_msi(
     xendevicemodel_handle *dmod, domid_t domid, uint64_t msi_addr,
     uint32_t msi_data);
 
+/**
+ * This function injects an MSI into a guest.
+ *
+ * @parm dmod a handle to an open devicemodel interface.
+ * @parm domid the domain id to be serviced
+ * @parm msi_addr the MSI address
+ * @parm source_id the PCI SBDF of the source device
+ * @parm msi_data the MSI data
+ * @return 0 on success, -1 on failure.
+*/
+int xendevicemodel_inject_msi2(
+    xendevicemodel_handle *dmod, domid_t domid, uint64_t addr, uint32_t source_id,
+    uint32_t data);
+
 /**
  * This function enables tracking of changes in the VRAM area.
  *
diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
index 8e619eeb0a..92db92e89b 100644
--- a/tools/libs/devicemodel/core.c
+++ b/tools/libs/devicemodel/core.c
@@ -448,6 +448,26 @@  int xendevicemodel_set_irq_level(
     return xendevicemodel_op(dmod, domid, 1, &op, sizeof(op));
 }
 
+int xendevicemodel_inject_msi2(
+    xendevicemodel_handle *dmod, domid_t domid, uint64_t addr, uint32_t source_id,
+    uint32_t data)
+{
+    uint16_t segment = source_id >> 16;
+    uint16_t bdf = source_id & 0xffff;
+    struct xen_dm_op op = {
+        .op = XEN_DMOP_inject_msi2,
+        .u.inject_msi2 = {
+            .addr = addr,
+            .data = data,
+            .segment = segment,
+            .source_id = bdf,
+            .flags = XEN_DMOP_MSI_SOURCE_ID_VALID,
+        },
+    };
+
+    return xendevicemodel_op(dmod, domid, 1, &op, sizeof(op));
+}
+
 int xendevicemodel_set_pci_link_route(
     xendevicemodel_handle *dmod, domid_t domid, uint8_t link, uint8_t irq)
 {
diff --git a/tools/libs/devicemodel/libxendevicemodel.map b/tools/libs/devicemodel/libxendevicemodel.map
index f7f9e3d932..aa05768642 100644
--- a/tools/libs/devicemodel/libxendevicemodel.map
+++ b/tools/libs/devicemodel/libxendevicemodel.map
@@ -44,3 +44,8 @@  VERS_1.4 {
 		xendevicemodel_set_irq_level;
 		xendevicemodel_nr_vcpus;
 } VERS_1.3;
+
+VERS_1.5 {
+	global:
+		xendevicemodel_inject_msi2;
+} VERS_1.4;
diff --git a/xen/arch/arm/dm.c b/xen/arch/arm/dm.c
index 773a0a2592..a1340b45a3 100644
--- a/xen/arch/arm/dm.c
+++ b/xen/arch/arm/dm.c
@@ -27,6 +27,7 @@  int dm_op(const struct dmop_args *op_args)
         [XEN_DMOP_set_ioreq_server_state]           = sizeof(struct xen_dm_op_set_ioreq_server_state),
         [XEN_DMOP_destroy_ioreq_server]             = sizeof(struct xen_dm_op_destroy_ioreq_server),
         [XEN_DMOP_set_irq_level]                    = sizeof(struct xen_dm_op_set_irq_level),
+        [XEN_DMOP_inject_msi2]                      = sizeof(struct xen_dm_op_inject_msi2),
         [XEN_DMOP_nr_vcpus]                         = sizeof(struct xen_dm_op_nr_vcpus),
     };
 
@@ -112,6 +113,22 @@  int dm_op(const struct dmop_args *op_args)
         break;
     }
 
+    case XEN_DMOP_inject_msi2:
+    {
+        const struct xen_dm_op_inject_msi2 *data = &op.u.inject_msi2;
+        uint32_t pci_sbdf = data->segment << 16 | data->source_id;
+
+        if ( !(data->flags & XEN_DMOP_MSI_SOURCE_ID_VALID) || data->pad || 
+             data->flags & ~XEN_DMOP_MSI_SOURCE_ID_VALID )
+        {
+            rc = -EINVAL;
+            break;
+        }
+
+        rc = vgic_its_trigger_msi(d, data->addr, pci_sbdf, data->data);
+        break;
+
+    }
     case XEN_DMOP_nr_vcpus:
     {
         struct xen_dm_op_nr_vcpus *data = &op.u.nr_vcpus;
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 462691f91d..da0a00844b 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -344,6 +344,7 @@  int dm_op(const struct dmop_args *op_args)
         [XEN_DMOP_set_mem_type]                     = sizeof(struct xen_dm_op_set_mem_type),
         [XEN_DMOP_inject_event]                     = sizeof(struct xen_dm_op_inject_event),
         [XEN_DMOP_inject_msi]                       = sizeof(struct xen_dm_op_inject_msi),
+        [XEN_DMOP_inject_msi2]                      = sizeof(struct xen_dm_op_inject_msi2),
         [XEN_DMOP_map_mem_type_to_ioreq_server]     = sizeof(struct xen_dm_op_map_mem_type_to_ioreq_server),
         [XEN_DMOP_remote_shutdown]                  = sizeof(struct xen_dm_op_remote_shutdown),
         [XEN_DMOP_relocate_memory]                  = sizeof(struct xen_dm_op_relocate_memory),
@@ -539,6 +540,23 @@  int dm_op(const struct dmop_args *op_args)
         break;
     }
 
+    case XEN_DMOP_inject_msi2:
+    {
+        const struct xen_dm_op_inject_msi2 *data = &op.u.inject_msi2;
+
+        if ( data->pad || data->flags & ~XEN_DMOP_MSI_SOURCE_ID_VALID )
+        {
+            rc = -EINVAL;
+            break;
+        }
+
+        if ( data->flags & XEN_DMOP_MSI_SOURCE_ID_VALID )
+            gprintk(XENLOG_WARNING "XEN_DMOP_inject_msi2: source_id is ignored\n");
+
+        rc = hvm_inject_msi(d, data->addr, data->data);
+        break;
+    }
+
     case XEN_DMOP_remote_shutdown:
     {
         const struct xen_dm_op_remote_shutdown *data =
diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
index 2bf0fdc1ae..4141af4300 100644
--- a/xen/include/public/hvm/dm_op.h
+++ b/xen/include/public/hvm/dm_op.h
@@ -444,6 +444,23 @@  struct xen_dm_op_nr_vcpus {
 };
 typedef struct xen_dm_op_nr_vcpus xen_dm_op_nr_vcpus_t;
 
+#define XEN_DMOP_inject_msi2 21
+
+struct xen_dm_op_inject_msi2 {
+      /* IN - MSI data */
+      uint32_t data;
+      /* IN - next two fields form an ID of the device triggering the MSI */
+      uint16_t segment; /* The segment number */
+      uint16_t source_id; /* The source ID that is local to segment (PCI BDF) */
+      /* IN - types of source ID */
+      uint32_t flags;
+#define XEN_DMOP_MSI_SOURCE_ID_VALID (1u << 0)
+      uint32_t pad;
+      /* IN - MSI address */
+      uint64_aligned_t addr;
+};
+typedef struct xen_dm_op_inject_msi2 xen_dm_op_inject_msi2_t;
+
 struct xen_dm_op {
     uint32_t op;
     uint32_t pad;
@@ -463,6 +480,7 @@  struct xen_dm_op {
         xen_dm_op_set_mem_type_t set_mem_type;
         xen_dm_op_inject_event_t inject_event;
         xen_dm_op_inject_msi_t inject_msi;
+        xen_dm_op_inject_msi2_t inject_msi2;
         xen_dm_op_map_mem_type_to_ioreq_server_t map_mem_type_to_ioreq_server;
         xen_dm_op_remote_shutdown_t remote_shutdown;
         xen_dm_op_relocate_memory_t relocate_memory;