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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.11.2022 20:20, 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.

Except for qword writes as it looks. Furthermore even clearing
requests aren't sent if address/data are unchanged. If you agree,
please add this here in some form for having a complete picture.

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

Can we perhaps still avoid sending dword writes which don't change
the mask bit?

> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -271,7 +271,8 @@ out:
>  }
>  
>  static int msixtbl_write(struct vcpu *v, unsigned long address,
> -                         unsigned int len, unsigned long val)
> +                         unsigned int len, unsigned long val,
> +                         bool completion)
>  {

I'd like to propose an alternative approach without an extra parameter:
Have msix_write_completion() pass 0 for "len" and move the initial
check

    if ( (len != 4 && len != 8) || (address & (len - 1)) )
        return r;

into _msixtbl_write(). Then ...

> @@ -343,7 +344,7 @@ static int msixtbl_write(struct vcpu *v, unsigned long 
> address,
>  
>  unlock:
>      spin_unlock_irqrestore(&desc->lock, flags);
> -    if ( len == 4 )
> +    if ( len == 4 && completion )
>          r = X86EMUL_OKAY;

... this could simply be "if ( !len )", seeing that even with your
approach it could simply be "if ( completion )".

Jan