Received: by 10.192.165.148 with SMTP id m20csp3623336imm; Mon, 23 Apr 2018 09:30:56 -0700 (PDT) X-Google-Smtp-Source: AIpwx49c0CAAWrNn4IUHuZFlKv0WmibqId3GosBSWsXnzRCbxvyAW9rAzrxdNNz0T8aQeL7HKN0U X-Received: by 10.98.161.10 with SMTP id b10mr20628178pff.214.1524501056512; Mon, 23 Apr 2018 09:30:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1524501056; cv=none; d=google.com; s=arc-20160816; b=W6AqtF7uyZ+3VyR7mPoDH/QT9qr7442f07nIni4ovfTL2g/aFhqk0/mEctWsT5gzvS EeSaa/Bbdu8cubYqSuOCjVeSq5fjq3OUiOyiHKsWbJM8QhoC6metCowDuVY/23s3A344 ASZypImGatCXkkjkwBIicdbF2l5Hc8UM/ZlLptLulpcAs3rAsDErY7f6sP845uPmADYl x/ilExyb9BUBc4XIgSqaxUl6pjsv2fk0kd+XxgytAO3Rpo8XyARaxC54tLrAjlu4qRpI K4+Q8EbtzYaZ7ISD9IGPHfUZh9ew5XE3zvOUpbt0jczsUFnNgawNpFBE07I1jU9XGj91 LWhg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:arc-authentication-results; bh=3O99hqmlabOini7mRonKd3T4XY4OXcqr5BXj743AwrY=; b=jk99m29VIf73nDE6PoCuTDTxGbTTNrlv3q3bdu5pOm9LLfypn1mlB7uY/sbB4vHCRW 4aXFMiYykdCYtAXgZnve098GMdsH645A7L0hwB2mydSgmCAYaAHdylrBmpKui/xQqSp0 LNSwC+DbiADT/iUIqKBkpRF48xZtTg8l5DYcScH4N32Tmc7wiEV2DB6rg+ywAzhU3GDK U7gGyPPRaqFhp/56tQ0rsoBAb09PwICpRROtsN3CL44LLRCmVx7tL1VGQMa9Y1F3iJVh 1JOmWbsrAAImaRUO5ICnAPfZ+qqLc5ZtG2Q5BKUfDXAiRqhbOO6Dgg01YR1+Dz/pmCIh wDuw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id s22-v6si12809960plp.216.2018.04.23.09.30.42; Mon, 23 Apr 2018 09:30:56 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755874AbeDWPV3 (ORCPT + 99 others); Mon, 23 Apr 2018 11:21:29 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:33158 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755635AbeDWPV0 (ORCPT ); Mon, 23 Apr 2018 11:21:26 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DB55AF0803; Mon, 23 Apr 2018 15:21:25 +0000 (UTC) Received: from [10.18.17.89] (dhcp-17-89.bos.redhat.com [10.18.17.89]) by smtp.corp.redhat.com (Postfix) with ESMTP id 68C5B21449DA; Mon, 23 Apr 2018 15:21:24 +0000 (UTC) Subject: Re: [pci PATCH v8 0/4] Add support for unmanaged SR-IOV To: Bjorn Helgaas , Alexander Duyck Cc: bhelgaas@google.com, linux-pci@vger.kernel.org, virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org, netdev@vger.kernel.org, dan.daly@intel.com, linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org, keith.busch@intel.com, netanel@amazon.com, mheyne@amazon.de, liang-min.wang@intel.com, mark.d.rustad@intel.com, dwmw2@infradead.org, hch@lst.de, dwmw@amazon.co.uk References: <20180420162633.46077.49012.stgit@ahduyck-green-test.jf.intel.com> <20180421203437.GW28657@bhelgaas-glaptop.roam.corp.google.com> From: Don Dutile Message-ID: <84adb7c2-bcf7-5e17-a9d0-482416e2d26c@redhat.com> Date: Mon, 23 Apr 2018 11:21:24 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180421203437.GW28657@bhelgaas-glaptop.roam.corp.google.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 23 Apr 2018 15:21:25 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 23 Apr 2018 15:21:25 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'ddutile@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/21/2018 04:34 PM, Bjorn Helgaas wrote: > On Fri, Apr 20, 2018 at 12:28:08PM -0400, Alexander Duyck wrote: >> This series is meant to add support for SR-IOV on devices when the VFs are >> not managed by the kernel. Examples of recent patches attempting to do this >> include: >> virto - https://patchwork.kernel.org/patch/10241225/ >> pci-stub - https://patchwork.kernel.org/patch/10109935/ >> vfio - https://patchwork.kernel.org/patch/10103353/ >> uio - https://patchwork.kernel.org/patch/9974031/ >> >> Since this is quickly blowing up into a multi-driver problem it is probably >> best to implement this solution as generically as possible. >> >> This series is an attempt to do that. What we do with this patch set is >> provide a generic framework to enable SR-IOV in the case that the PF driver >> doesn't support managing the VFs itself. >> >> I based my patch set originally on the patch by Mark Rustad but there isn't >> much left after going through and cleaning out the bits that were no longer >> needed, and after incorporating the feedback from David Miller. At this point >> the only items to be fully reused was his patch description which is now >> present in patch 3 of the set. >> >> This solution is limited in scope to just adding support for devices that >> provide no functionality for SR-IOV other than allocating the VFs by >> calling pci_enable_sriov. Previous sets had included patches for VFIO, but >> for now I am dropping that as the scope of that work is larger then I >> think I can take on at this time. >> >> v2: Reduced scope back to just virtio_pci and vfio-pci >> Broke into 3 patch set from single patch >> Changed autoprobe behavior to always set when num_vfs is set non-zero >> v3: Updated Documentation to clarify when sriov_unmanaged_autoprobe is used >> Wrapped vfio_pci_sriov_configure to fix build errors w/o SR-IOV in kernel >> v4: Dropped vfio-pci patch >> Added ena and nvme to drivers now using pci_sriov_configure_unmanaged >> Dropped pci_disable_sriov call in virtio_pci to be consistent with ena >> v5: Dropped sriov_unmanaged_autoprobe and pci_sriov_conifgure_unmanaged >> Added new patch that enables pci_sriov_configure_simple >> Updated drivers to use pci_sriov_configure_simple >> v6: Defined pci_sriov_configure_simple as NULL when SR-IOV is not enabled >> Updated drivers to drop "#ifdef" checks for IOV >> Added pci-pf-stub as place for PF-only drivers to add support >> v7: Dropped pci_id table explanation from pci-pf-stub driver >> Updated pci_sriov_configure_simple to drop need for err value >> Fixed comment explaining why pci_sriov_configure_simple is NULL >> v8: Dropped virtio from the set, support to be added later after TC approval >> >> Cc: Mark Rustad >> Cc: Maximilian Heyne >> Cc: Liang-Min Wang >> Cc: David Woodhouse >> >> --- >> >> Alexander Duyck (4): >> pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources >> ena: Migrate over to unmanaged SR-IOV support >> nvme: Migrate over to unmanaged SR-IOV support >> pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs >> >> >> drivers/net/ethernet/amazon/ena/ena_netdev.c | 28 ------------- >> drivers/nvme/host/pci.c | 20 ---------- >> drivers/pci/Kconfig | 12 ++++++ >> drivers/pci/Makefile | 2 + >> drivers/pci/iov.c | 31 +++++++++++++++ >> drivers/pci/pci-pf-stub.c | 54 ++++++++++++++++++++++++++ >> include/linux/pci.h | 3 + >> include/linux/pci_ids.h | 2 + >> 8 files changed, 106 insertions(+), 46 deletions(-) >> create mode 100644 drivers/pci/pci-pf-stub.c > > I tentatively applied these to pci/virtualization-review. > > The code changes look fine, but I want to flesh out the changelogs a > little bit before merging them. > > For example, I'm not sure what you mean by "devices where the PF is > not capable of managing VF resources." > I agree w/Bjorn's assessment of the changelog. The VF's are (minimally) assigned via the pf-stub driver, so they are 'managed by the kernel'. The security model is the same as the existing one, which was the issue we resolved in the previous set(s) of patches. I am hoping that something like vfio will be used to deal with the VF ownership and the reset mechanisms during assignement & de-assignment to 'guests' (qemu-kvm, DPDK, or whatever user-process), so the known, existing security model(s) is(are) maintained as well. If so, it'd be good to add such verbage somewhere (as 0/n is not kept in anything but possibly Bjorn's patchwork, or whatever patch mgmt tool he uses, and future reference would be good to have) say, an update to Documentation/PCI/pci-iov-howto.txt. So... the 'unmanaged SR-IOV' Subject, IMO, is not a valid Subject for the patch series any longer. No objections to the patch series, as Bjorn noted, just the commit log(s)/nomenclature of what is really being done. The expectation of VF enablement via the PF was born out of the fairly complicated, and unique PF vs VF drivers of the first implementations, which AlexD knows so well. This "VFs act just like PFs without SRIOV capabilities" support is what this patch set enables with a much lighter configuration mechanism. So, maybe the patch set ought to be 'lightweight SRIOV enablement'. --dd > It *sounds* like you're saying the hardware works differently on some > devices, but I don't think that's what you mean. I think you're > saying something about which drivers are used for the PF and the VF. > > I think a trivial example of how this will be used might help. I > assume this involves a virtualization scenario where the host uses the > PF to enable several VFs, but the host doesn't use the PF for much > else. Then you assign the VFs to guests, and drivers in the guest > OSes use the VFs. > > Since .sriov_configure() is only used by sriov_numvfs_store(), I > assume the usage model involves writing to the sysfs sriov_numvfs > attribute to enable the VFs, then assigning them to guests? > > Bjorn >