Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 23 Jun 2021 20:48:28 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     "Tian\, Kevin" <kevin.tian@intel.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        "Dey\, Megha" <megha.dey@intel.com>,
        "Raj\, Ashok" <ashok.raj@intel.com>,
        "Pan\, Jacob jun" <jacob.jun.pan@intel.com>,
        "Jiang\, Dave" <dave.jiang@intel.com>,
        "Liu\, Yi L" <yi.l.liu@intel.com>,
        "Lu\, Baolu" <baolu.lu@intel.com>,
        "Williams\, Dan J" <dan.j.williams@intel.com>,
        "Luck\, Tony" <tony.luck@intel.com>,
        "Kumar\, Sanjay K" <sanjay.k.kumar@intel.com>,
        LKML <linux-kernel@vger.kernel.org>, KVM <kvm@vger.kernel.org>,
        Kirti Wankhede <kwankhede@nvidia.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Marc Zyngier <maz@kernel.org>,
        Bjorn Helgaas <helgaas@kernel.org>
Subject: Re: Virtualizing MSI-X on IMS via VFIO
Message-ID: <20210623204828.2bc7e6dc.alex.williamson@redhat.com>
In-Reply-To: <87mtrgatqo.ffs@nanos.tec.linutronix.de>
References: <20210622131217.76b28f6f.alex.williamson@redhat.com>
        <87o8bxcuxv.ffs@nanos.tec.linutronix.de>
        <20210623091935.3ab3e378.alex.williamson@redhat.com>
        <MWHPR11MB18864420ACE88E060203F7818C079@MWHPR11MB1886.namprd11.prod.outlook.com>
        <87mtrgatqo.ffs@nanos.tec.linutronix.de>
Organization: Red Hat
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 24 Jun 2021 04:20:31 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> Kevin,
> 
> thank you very much for digging into this! You made my day!
> 
> On Thu, Jun 24 2021 at 00:00, Kevin Tian wrote:
> >> From: Alex Williamson <alex.williamson@redhat.com>
> >> To work with what we've got, the vfio API describes the limitation of
> >> the host interfaces via the VFIO_IRQ_INFO_NORESIZE flag.  QEMU then
> >> makes a choice in an attempt to better reflect what we can infer of the
> >> guest programming of the device to incrementally enable vectors.  We  
> >
> > It's a surprise to me that Qemu even doesn't look at this flag today after
> > searching its code...  
> 
> Indeed.
> 
> git clone https://github.com/qemu/qemu.git
> cd qemu
> git log -p | grep NORESIZE
> + * The NORESIZE flag indicates that the interrupt lines within the index
> +#define VFIO_IRQ_INFO_NORESIZE		(1 << 3)
> 
> According to the git history of QEMU this was never used at all and I
> don't care about the magic muck which might be in some RHT repository
> which might make use of that.
> 
> Find below the proper fix for this nonsense which just wasted everyones
> time. I'll post it officialy with a proper changelog tomorrow unless
> Kevin beats me to it who actually unearthed this and surely earns the
> credit.
> 
> Alex, I seriously have to ask what you were trying to tell us about this
> flag and it's great value and the design related to this.
> 
> I'm sure you can submit the corresponding fix to qemu yourself.
> 
> And once you are back from lala land, can you please explain how
> VFIO/PCI/MSIX is supposed to work in reality?

It's part of the spec, there's never been a case of !NORESIZE, assuming
NORESIZE is the safe behavior.  Sorry, there's no smoking gun here, NAK

> ---
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1644,8 +1644,6 @@ static long intel_vgpu_ioctl(struct mdev
>  		if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
>  			info.flags |= (VFIO_IRQ_INFO_MASKABLE |
>  				       VFIO_IRQ_INFO_AUTOMASKED);
> -		else
> -			info.flags |= VFIO_IRQ_INFO_NORESIZE;
>  
>  		return copy_to_user((void __user *)arg, &info, minsz) ?
>  			-EFAULT : 0;
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -1018,8 +1018,6 @@ static long vfio_pci_ioctl(struct vfio_d
>  		if (info.index == VFIO_PCI_INTX_IRQ_INDEX)
>  			info.flags |= (VFIO_IRQ_INFO_MASKABLE |
>  				       VFIO_IRQ_INFO_AUTOMASKED);
> -		else
> -			info.flags |= VFIO_IRQ_INFO_NORESIZE;
>  
>  		return copy_to_user((void __user *)arg, &info, minsz) ?
>  			-EFAULT : 0;
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -693,16 +693,6 @@ struct vfio_region_info_cap_nvlink2_lnks
>   * automatically masked by VFIO and the user needs to unmask the line
>   * to receive new interrupts.  This is primarily intended to distinguish
>   * level triggered interrupts.
> - *
> - * The NORESIZE flag indicates that the interrupt lines within the index
> - * are setup as a set and new subindexes cannot be enabled without first
> - * disabling the entire index.  This is used for interrupts like PCI MSI
> - * and MSI-X where the driver may only use a subset of the available
> - * indexes, but VFIO needs to enable a specific number of vectors
> - * upfront.  In the case of MSI-X, where the user can enable MSI-X and
> - * then add and unmask vectors, it's up to userspace to make the decision
> - * whether to allocate the maximum supported number of vectors or tear
> - * down setup and incrementally increase the vectors as each is enabled.
>   */
>  struct vfio_irq_info {
>  	__u32	argsz;
> @@ -710,7 +700,6 @@ struct vfio_irq_info {
>  #define VFIO_IRQ_INFO_EVENTFD		(1 << 0)
>  #define VFIO_IRQ_INFO_MASKABLE		(1 << 1)
>  #define VFIO_IRQ_INFO_AUTOMASKED	(1 << 2)
> -#define VFIO_IRQ_INFO_NORESIZE		(1 << 3)
>  	__u32	index;		/* IRQ index */
>  	__u32	count;		/* Number of IRQs within this index */
>  };
> --- a/samples/vfio-mdev/mtty.c
> +++ b/samples/vfio-mdev/mtty.c
> @@ -1092,9 +1092,6 @@ static int mtty_get_irq_info(struct mdev
>  	if (irq_info->index == VFIO_PCI_INTX_IRQ_INDEX)
>  		irq_info->flags |= (VFIO_IRQ_INFO_MASKABLE |
>  				VFIO_IRQ_INFO_AUTOMASKED);
> -	else
> -		irq_info->flags |= VFIO_IRQ_INFO_NORESIZE;
> -
>  	return 0;
>  }
>  
>