Re: [RFC PATCH] xen/dm: arm: Introudce arm_inject_msi DM op

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/12/2023 1:48 pm, Mykyta Poturai wrote:
> This patch adds the ability for the device emulator to inject MSI
> interrupts into guests. This is done by adding a new DM op to the device
> model library.
>
> It is not possible to reuse already existing inject_msi DM op, because
> it does not have a devid parameter, which is required for translation of
> MSIs to interrupt numbers on ARM.

Ok, so the original hypercall is broken.

But the new hypercall isn't ARM specific. It's just better form of
inject_msi, and needed for all architectures.

So, name it DMOP_inject_msi2 and get rid of the ARM infix.

> This approach was successfully tested on a virtio-pci setup with QEMU
> backend emulating block devices with following changes from the mainline
> Xen:
>
> This branch was taken as a base for virtio-pci:
> https://github.com/xen-troops/xen/tree/xen-4.18-xt0.2
>
> With added new VGICv3 from here:
> https://github.com/Deedone/xen/tree/new_vgic_v2
> (although it should also work with the current VGIC)
>
> And patches from this branch to allow for proper ITS emulation in guests:
> https://github.com/stewdk/xen/commits/pcie-passthrough-arm-vpci-v11
>
> The main purpose of this RFC is to get some feedback on the addition of
> the new DM op. Is it the right approach or should we somehow modify the
> existing inject_msi DM op to be compatible with ARM?

The DM_OP ABI does allow you to extend the structure behind
DMOP_inject_msi, as long as 0 is meaningful.

However, the semantics of zero-extending are wrong in this case, because
it would mean that users of DMOP_inject_msi on an updated Xen would be
sending interrupts with an implicit source id of host bridge.

So you need a new DMOP_inject_msi2 that has better semantics.

> diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
> index fa98551914..a7d72e4389 100644
> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -444,6 +444,15 @@ struct xen_dm_op_nr_vcpus {
>  };
>  typedef struct xen_dm_op_nr_vcpus xen_dm_op_nr_vcpus_t;
>  
> +#define XEN_DMOP_arm_inject_msi 21
> +
> +struct xen_dm_op_arm_inject_msi {
> +    uint64_t addr;
> +    uint32_t data;
> +    uint32_t devid;

uint32_t source_id; /* PCI SBDF */

Technically the PCI spec calls it the Requester ID, but Source ID is the
more common name.

It is important not to use "devid" because that implies it's only a 3
bit number, and it isn't.

~Andrew