On 14.04.2025 11:51, Mykyta Poturai wrote:
> From: Mykyta Poturai <mykyta_potu...@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
