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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.12.2023 15:33, Andrew Cooper wrote:
> 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.

As long as we simply ignore that field's value, I don't see anything
wrong there (not very different from Arm ignoring the address, as the
intent looks to be). And ...

>>  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.

... I wonder whether I'll still be around when Xen actually gets there.

Jan

> 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
>