Re: [PATCH 00/15] hw/nvme: SR-IOV with Virtualization Enhancements

From: Klaus Jensen <its@irrelevant.dk>
To: Lukasz Maniak <lukasz.maniak@linux.intel.com>
Cc: "Łukasz Gieryk" <lukasz.gieryk@linux.intel.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH 00/15] hw/nvme: SR-IOV with Virtualization Enhancements
Date: Tue, 26 Oct 2021 20:20:12 +0200	[thread overview]
Message-ID: <YXhG3L+brG0q6o/2@apples.localdomain> (raw)
In-Reply-To: <20211007162406.1920374-1-lukasz.maniak@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

On Oct  7 18:23, Lukasz Maniak wrote:
> Hi,
> 
> This series of patches is an attempt to add support for the following
> sections of NVMe specification revision 1.4:
> 
> 8.5 Virtualization Enhancements (Optional)
>     8.5.1 VQ Resource Definition
>     8.5.2 VI Resource Definition
>     8.5.3 Secondary Controller States and Resource Configuration
>     8.5.4 Single Root I/O Virtualization and Sharing (SR-IOV)
> 
> The NVMe controller's Single Root I/O Virtualization and Sharing
> implementation is based on patches introducing SR-IOV support for PCI
> Express proposed by Knut Omang:
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg05155.html
> 
> However, based on what I was able to find historically, Knut's patches
> have not yet been pulled into QEMU due to no example of a working device
> up to this point:
> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg02722.html
> 
> In terms of design, the Physical Function controller and the Virtual
> Function controllers are almost independent, with few exceptions:
> PF handles flexible resource allocation for all its children (VFs have
> read-only access to this data), and reset (PF explicitly calls it on VFs).
> Since the MMIO access is serialized, no extra precautions are required
> to handle concurrent resets, as well as the secondary controller state
> access doesn't need to be atomic.
> 
> A controller with full SR-IOV support must be capable of handling the
> Namespace Management command. As there is a pending review with this
> functionality, this patch list is not duplicating efforts.
> Yet, NS management patches are not required to test the SR-IOV support.
> 
> We tested the patches on Ubuntu 20.04.3 LTS with kernel 5.4.0. We have
> hit various issues with NVMe CLI (list and virt-mgmt commands) between
> releases from version 1.09 to master, thus we chose this golden NVMe CLI
> hash for testing: a50a0c1.
> 
> The implementation is not 100% finished and certainly not bug free,
> since we are already aware of some issues e.g. interaction with
> namespaces related to AER, or unexpected (?) kernel behavior in more
> complex reset scenarios. However, our SR-IOV implementation is already
> able to support typical SR-IOV use cases, so we believe the patches are
> ready to share with the community.
> 
> Hope you find some time to review the work we did, and share your
> thoughts.
> 
> Kind regards,
> Lukasz

Hi Lukasz,

If possible, can you share a brief guide on testing this? I keep hitting
an assert

  qemu-system-x86_64: ../hw/pci/pci.c:1215: pci_register_bar: Assertion `!pci_is_vf(pci_dev)' failed.

when I try to modify sriov_numvfs. This should be fixed anyway, but I
might be doing something wrong in the first place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]