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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/12/2023 2:25 pm, Jan Beulich wrote:
> On 19.12.2023 15:19, Andrew Cooper wrote:
>> 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.
> As said in another reply, the existing structure has a 32-bit padding
> field, which could be used here. In the handler it's properly being
> checked to be zero right now;

It's still not safe to reuse this zero for a source ID semantic behind
the back of older userspace.

>  whether that would want to remain this
> way, or whether we'd expect source ID to also be passed on x86 I don't
> know (yet).

We do need the source ID in x86, as soon as the guest has vIOMMU for any
reason.

It's a design error that it wasn't added originally, but I suppose you
can say the same of x86 platforms in general, having to retrofit an
OS-visible Source ID to HPETs/IO-APICs to make them compatible with IOMMUs.

~Andrew