Re: [PATCH v2 1/3] x86/msi: passthrough all MSI-X vector ctrl writes to device model

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Mar 27, 2023 at 12:12:29PM +0200, Roger Pau Monné wrote:
> On Sat, Mar 25, 2023 at 03:49:22AM +0100, Marek Marczykowski-Górecki wrote:
> > QEMU needs to know whether clearing maskbit of a vector is really
> > clearing, or was already cleared before. Currently Xen sends only
> > clearing that bit to the device model, but not setting it, so QEMU
> > cannot detect it. Because of that, QEMU is working this around by
> > checking via /dev/mem, but that isn't the proper approach.
> > 
> > Give all necessary information to QEMU by passing all ctrl writes,
> > including masking a vector. This does include forwarding also writes
> > that did not change the value, but as tested on both Linux (6.1.12) and
> > Windows (10 pro), they don't do excessive writes of unchanged values
> > (Windows seems to clear maskbit in some cases twice, but not more).
> 
> Since we passthrough all the accesses to the device model, is the
> handling in Xen still required?  It might be worth to also expose any
> interfaces needed to the device model so all the functionality done by
> the msixtbl_mmio_ops hooks could be done by QEMU, since we end up
> passing the accesses anyway.

This was discussed on v1 already. Such QEMU would need to be able to do
the actual write. If it's running in stubdomain, it would hit the exact
issue again (page mapped R/O to it). In fact, that might be an issue for
dom0 too (I haven't checked).
I guess that could use my subpage RO feature I just posted then, but it
would still mean intercepting the write twice (not a performance issue
really here, but rather convoluted handling in total).

> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> > v2:
> >  - passthrough quad writes to emulator too (Jan)
> >  - (ab)use len==0 for write len=4 completion (Jan), but add descriptive
> >    #define for this magic value
> > 
> > This behavior change needs to be surfaced to the device model somehow,
> > so it knows whether it can rely on it. I'm open for suggestions.
> 
> Maybe exposed in XEN_DMOP_get_ioreq_server_info?
> 
> But I wonder whether it shouldn't be the other way arround, the device
> model tells Xen it doesn't need to handle any MSI-X accesses because
> QEMU will take care of it, likely using a new flag in
> XEN_DMOP_create_ioreq_server or maybe in XEN_DOMCTL_bind_pt_irq as
> part of the gflags, but then we would need to assert that the flag is
> passed for all MSI-X interrupts bound from that device to the same
> domain.

Is is safe thing to do? I mean, doesn't Xen need to guard access to
MSI-X configuration to assure its safety, especially if no interrupt
remapping is there? It probably doesn't matter for qemu in dom0 case,
but both with deprivileged qemu and stubdom, it might matter.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature