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 >
