Re: [PATCH kvmtool] vfio: fix multi-MSI vector handling

[Marc Zyngier <maz@kernel.org>]

Hi Lorenzo,

On 2020-04-24 14:40, Lorenzo Pieralisi wrote:
> A PCI device with a MSI capability enabling Multiple MSI messages
> (through the Multiple Message Enable field in the Message Control
> register[6:4]) is expected to drive the Message Data lower bits (number
> determined by the number of selected vectors) to generate the
> corresponding MSI messages writes on the PCI bus.
> 
> Therefore, KVM expects the MSI data lower bits (a number of
> bits that depend on bits [6:4] of the Message Control
> register - which in turn control the number of vectors
> allocated) to be set-up by kvmtool while programming the
> MSI IRQ routing entries to make sure the MSI entries can
> actually be demultiplexed by KVM and IRQ routes set-up
> accordingly so that when an actual HW fires KVM can
> route it to the correct entry in the interrupt controller
> (and set-up a correct passthrough route for directly
> injected interrupt).
> 
> Current kvmtool code does not set-up Message data entries
> correctly for multi-MSI vectors - the data field is left
> as programmed in the MSI capability by the guest for all
> vector entries, triggering IRQs misrouting.
> 
> Fix it.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
> ---
>  vfio/pci.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/vfio/pci.c b/vfio/pci.c
> index 76e24c1..b43e522 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -434,6 +434,12 @@ static void vfio_pci_msi_cap_write(struct kvm
> *kvm, struct vfio_device *vdev,
> 
>  	for (i = 0; i < nr_vectors; i++) {
>  		entry = &pdev->msi.entries[i];
> +
> +		if (nr_vectors > 1) {
> +			msg.data &= ~(nr_vectors - 1);
> +			msg.data |= i;
> +		}
> +

This matches my own understanding of how MultiMSI works.
Small nit: you don't need to check the condition for the number
of vectors, as this expression is valid for any nr_vectors
that is a power of 2 (as required by the spec).

>  		entry->config.msg = msg;
>  		vfio_pci_update_msi_entry(kvm, vdev, entry);
>  	}

FWIW:

Acked-by: Marc Zyngier <maz@kernel.org>

         M.
-- 
Jazz is not dead. It just smells funny...