Received: by 10.192.165.148 with SMTP id m20csp127158imm; Thu, 19 Apr 2018 17:43:12 -0700 (PDT) X-Google-Smtp-Source: AIpwx493IyGNvGJ/53mrQtA48wTSon5Mgr9ijZ7s9eBiW6rzuk0YQPpaF4GI4vF+mrmWK+krjWvw X-Received: by 2002:a17:902:7d10:: with SMTP id z16-v6mr8030060pll.79.1524184992601; Thu, 19 Apr 2018 17:43:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1524184992; cv=none; d=google.com; s=arc-20160816; b=xZyi215JiTC8KON+TU266NUrmtx4LXtHDdEFE1LAm5TyGKcC66IwJr1rHHHwvbhY1Z XsjJTVhMdU1UVXhhG7GvBrbCOz2lcijpK5Oo4XTFrQdEmbMBdqay33Ps2fuskC09Sp8C 3MdlnqT3aSZSQoQ3GrZsRzRadJA1ezsmOp5PJQQiR/ewOxwIRDGarDFHlEDHsGB8z81o K2z0wx0oz1QFQxQ0aLDlQp7J9cwhsK1iDSCv6Oz296oDEkoXp/Itpl3I+GUZmckFu997 KmPLVxPqd3vjTqt93htcTMG2h6Qio24P6XbZJcRjCYdO2Gy2pEEjIJggmvHoEQ5XEbL7 2h9w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :arc-authentication-results; bh=JCFMO8I4mqTnF68cYO1/IvKy5lEmiCC3l8lirXHLlUU=; b=ZiVEjZYTf8g+hTKClmKsbZdPeJYWgBC/JlJmaR30z8Y7ZiGQrqq/IhGn+iBg/hidJg JN12CFj6jAR5EDkVOyssGfz+puhyBI6/aZLcp7sWbGoMXRA0+o0En6ZXDUSAzdBUzfj0 2+J7IH+nuwMCZSx8y9SflRoljJpLSvp9gvMKCxTadgrMPvanzhlz3PJOlDmqivGknJ1T sak2gHYbagBmaBCs/WqU4ydOHfMgDuiC3bdYgp4ulXR4M9K1ihFA9JxIQM794H+YQ7zj 129thpsriiV/+PcXUJv68pVgk8obxfmPhjy8ABsQfBRfb3/wVuzEW+QJfw1GSthQes7/ bGQA== 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 g7-v6si4687198plt.358.2018.04.19.17.42.45; Thu, 19 Apr 2018 17:43:12 -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 S1754003AbeDTAlB (ORCPT + 99 others); Thu, 19 Apr 2018 20:41:01 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:37124 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753761AbeDTAk7 (ORCPT ); Thu, 19 Apr 2018 20:40:59 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 86EAC722FE; Fri, 20 Apr 2018 00:40:58 +0000 (UTC) Received: from redhat.com (ovpn-120-175.rdu2.redhat.com [10.10.120.175]) by smtp.corp.redhat.com (Postfix) with SMTP id B0EC1A9A1C; Fri, 20 Apr 2018 00:40:55 +0000 (UTC) Date: Fri, 20 Apr 2018 03:40:55 +0300 From: "Michael S. Tsirkin" To: Alexander Duyck Cc: "Daly, Dan" , Bjorn Helgaas , "Duyck, Alexander H" , linux-pci@vger.kernel.org, virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org, Netdev , LKML , linux-nvme@lists.infradead.org, Keith Busch , netanel@amazon.com, Don Dutile , Maximilian Heyne , "Wang, Liang-min" , "Rustad, Mark D" , David Woodhouse , Christoph Hellwig , dwmw@amazon.co.uk Subject: Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices Message-ID: <20180420030640-mutt-send-email-mst@kernel.org> References: <20180315183449.3102.64791.stgit@localhost.localdomain> <20180315184132.3102.90947.stgit@localhost.localdomain> <20180316183042-mutt-send-email-mst@kernel.org> <20180403161151-mutt-send-email-mst@kernel.org> <20180403212503-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 20 Apr 2018 00:40:58 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 20 Apr 2018 00:40:58 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mst@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 03, 2018 at 12:06:03PM -0700, Alexander Duyck wrote: > On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin wrote: > > On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote: > >> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin wrote: > >> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote: > >> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin wrote: > >> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote: > >> >> >> From: Alexander Duyck > >> >> >> > >> >> >> Hardware-realized virtio_pci devices can implement SR-IOV, so this > >> >> >> patch enables its use. The device in question is an upcoming Intel > >> >> >> NIC that implements both a virtio_net PF and virtio_net VFs. These > >> >> >> are hardware realizations of what has been up to now been a software > >> >> >> interface. > >> >> >> > >> >> >> The device in question has the following 4-part PCI IDs: > >> >> >> > >> >> >> PF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 15fe > >> >> >> VF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 05fe > >> >> >> > >> >> >> The patch currently needs no check for device ID, because the callback > >> >> >> will never be made for devices that do not assert the capability or > >> >> >> when run on a platform incapable of SR-IOV. > >> >> >> > >> >> >> One reason for this patch is because the hardware requires the > >> >> >> vendor ID of a VF to be the same as the vendor ID of the PF that > >> >> >> created it. So it seemed logical to simply have a fully-functioning > >> >> >> virtio_net PF create the VFs. This patch makes that possible. > >> >> >> > >> >> >> Reviewed-by: Christoph Hellwig > >> >> >> Signed-off-by: Mark Rustad > >> >> >> Signed-off-by: Alexander Duyck > >> >> > > >> >> > So if and when virtio PFs can manage the VFs, then we can > >> >> > add a feature bit for that? > >> >> > Seems reasonable. > >> >> > >> >> Yes. If nothing else you may not even need a feature bit depending on > >> >> how things go. > >> > > >> > OTOH if the interface is changed in an incompatible way, > >> > and old Linux will attempt to drive the new device > >> > since there is no check. > >> > > >> > I think we should add a feature bit right away. > >> > >> I'm not sure why you would need a feature bit. The capability is > >> controlled via PCI configuration space. If it is present the device > >> has the capability. If it is not then it does not. > >> > >> Basically if the PCI configuration space is not present then the sysfs > >> entries will not be spawned and nothing will attempt to use this > >> function. > >> > >> - ALex > > > > It's about compability with older guests which ignore the > > capability. > > > > The feature is thus helpful so host knows whether guest supports VFs. > > The thing is if the capability is ignored then the feature isn't used. > So for SR-IOV it isn't an uncommon thing for there to be drivers for > the PF floating around that do not support SR-IOV. In such cases > SR-IOV just isn't used while the hardware could support it. Right but how come there are VF drivers but PF driver does not know about these? And are there PF drivers that intentially do not enable SRIOV because it's known to be broken in some way? Case in point I do think virtio want to limit this depending on a feature bit on general principles (the principle being that all extensions have feature bits). There are security implications here - we previously relied on whitelisting after all. Wouldn't it be safer to be a bit more careful and update the actual PF drivers? It's just one line per driver, but it can be done with an ack by driver maintainer. If/once we find out all drivers do have it, we can then change the default. > I would think in the case of virtio it would be the same kind of > thing. Basically if SR-IOV is supported by the host then the > capability would be present. If SR-IOV is supported by the guest then > it would make use of the capability to spawn VFs. If either the > capability isn't present, or the driver doesn't use it then you won't > be able to spawn VFs in the guest. > Maybe I am missing something. Do you support dynamically changing the > PCI configuration space for Virtio devices based on the presence of > feature bits provided by the guest? No. The point is that IMHO at least virtio - in absence of feature bit - to ignore VFs rather than assume they are safe to drive in an unmanaged way. > Also are you saying this patch set should wait on the feature bit to > be added, or are you talking about doing this as some sort of > follow-up? > > - Alex I think for virtio it should include the feature bit, yes. Adding feature bit is very easy - post a patch to the virtio TC mailing list, wait about a week to give people time to respond (two weeks if it is around holidays and such). -- MST