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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Mar 27, 2023 at 12:26:05PM +0200, Marek Marczykowski-Górecki wrote:
> 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).

Oh, sorry, likely missed that discussion, as I don't recall this.

Maybe we need an hypercall for QEMU to notify the masking/unmasking to
Xen?  As any change on the other fields is already handled by QEMU.

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

Yes, that does seem way too convoluted.

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

Right - QEMU shouldn't write directly to the MSI-X table using
/dev/mem, but instead use an hypercall to notify Xen of the
{un,}masking of the MSI-X table entry.  I think that would allow us to
safely get rid of the extra logic in Xen to deal with MSI-X table
accesses.

Thanks, Roger.