From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752385AbeDTQPA (ORCPT ); Fri, 20 Apr 2018 12:15:00 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53680 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750858AbeDTQO4 (ORCPT ); Fri, 20 Apr 2018 12:14:56 -0400 Date: Fri, 20 Apr 2018 19:14:51 +0300 From: "Michael S. Tsirkin" To: Alexander Duyck Cc: "Daly, Dan" , "Rustad, Mark D" , 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" , 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: <20180420191055-mutt-send-email-mst@kernel.org> References: <20180316183042-mutt-send-email-mst@kernel.org> <20180403161151-mutt-send-email-mst@kernel.org> <20180403212503-mutt-send-email-mst@kernel.org> <20180420030640-mutt-send-email-mst@kernel.org> <20180420180839-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 20, 2018 at 09:08:51AM -0700, Alexander Duyck wrote: > On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin wrote: > > On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote: > >> > 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). > >> > >> The problem is we are talking about hardware/FPGA, not software. > >> Adding a feature bit means going back and updating RTL. The software > >> side of things is easy, re-validating things after a hardware/FPGA > >> change not so much. > >> > >> If this is a hard requirement I may just drop the virtio patch, push > >> what I have, and leave it to Mark/Dan to deal with the necessary RTL > >> and code changes needed to support Virtio as I don't expect the > >> turnaround to be as easy as just a patch. > >> > >> Thanks. > >> > >> - Alex > > > > Let's focus on virtio in this thread. > > That is kind of what I was thinking, and why I was thinking it might > make sense to make the virtio specific changes a separate patch set. I > could get the PCI bits taken care of in the meantime since they effect > genetic PCI, NVMe, and the Amazon ENA interfaces. > > > Involving the virtio TC in host/guest interface changes is a > > hard requirement. It's just too easy to create conflicts otherwise. > > > > So you guys should have just sent the proposal to the TC when you > > were doing your RTL and you would have been in the clear. > > Agreed. I believe I brought this up when I was originally asked to > look into the coding for this. > > > Generally adding a feature bit with any extension is a good idea: > > this way you merely reserve a feature bit for your feature through > > the TC and are more or less sure of forward and backward compatibility. > > It's incredibly easy. > > Agreed, though in this case I am not sure it makes sense since this > isn't necessarily something that is a Virtio feature itself. It is > just a side effect of the fact that they are adding SR-IOV support to > a device that happens to emulate Virtio NET and apparently their PF > has to be identical to the VF other than the PCIe extended config > space. I got that. My point is not everyone implementing SR-IOV will want to do it like this. Others might want to have VFs be different from PFs somehow. Feature bits ensure forward not just backward compatibility. > > But maybe it's not needed here. I am not making the decisions myself. > > Not too late: post to the TC list and let's see what the response is. > > Without a feature bit you are making a change affecting all future > > implementations without exception so the bar is a bit higher: you need > > to actually post a spec text proposal not just a patch showing how to > > use the feature, and TC needs to vote on it. Voting takes a week, > > review a week or two depending on change complexity. > > > > Hope this helps, > > > > -- > > MST > > I think I will leave this for Dan and Mark to handle since I am still > not all that familiar with the hardware in use here. Once a decision > has been made him and Mark could look at pushing either the one line > patch or something more complex involving a feature flag. > > Thanks. > > Alex As long as the TC is involved. I know it's a bit of a strange thing to block it at the driver level, the issue is with the device, but it's literally the only handle I have to prevent people from doing out of spec hacks then pushing it all on us to maintain. -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 From: mst@redhat.com (Michael S. Tsirkin) Date: Fri, 20 Apr 2018 19:14:51 +0300 Subject: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices In-Reply-To: References: <20180316183042-mutt-send-email-mst@kernel.org> <20180403161151-mutt-send-email-mst@kernel.org> <20180403212503-mutt-send-email-mst@kernel.org> <20180420030640-mutt-send-email-mst@kernel.org> <20180420180839-mutt-send-email-mst@kernel.org> Message-ID: <20180420191055-mutt-send-email-mst@kernel.org> On Fri, Apr 20, 2018@09:08:51AM -0700, Alexander Duyck wrote: > On Fri, Apr 20, 2018@8:28 AM, Michael S. Tsirkin wrote: > > On Fri, Apr 20, 2018@07:56:14AM -0700, Alexander Duyck wrote: > >> > 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). > >> > >> The problem is we are talking about hardware/FPGA, not software. > >> Adding a feature bit means going back and updating RTL. The software > >> side of things is easy, re-validating things after a hardware/FPGA > >> change not so much. > >> > >> If this is a hard requirement I may just drop the virtio patch, push > >> what I have, and leave it to Mark/Dan to deal with the necessary RTL > >> and code changes needed to support Virtio as I don't expect the > >> turnaround to be as easy as just a patch. > >> > >> Thanks. > >> > >> - Alex > > > > Let's focus on virtio in this thread. > > That is kind of what I was thinking, and why I was thinking it might > make sense to make the virtio specific changes a separate patch set. I > could get the PCI bits taken care of in the meantime since they effect > genetic PCI, NVMe, and the Amazon ENA interfaces. > > > Involving the virtio TC in host/guest interface changes is a > > hard requirement. It's just too easy to create conflicts otherwise. > > > > So you guys should have just sent the proposal to the TC when you > > were doing your RTL and you would have been in the clear. > > Agreed. I believe I brought this up when I was originally asked to > look into the coding for this. > > > Generally adding a feature bit with any extension is a good idea: > > this way you merely reserve a feature bit for your feature through > > the TC and are more or less sure of forward and backward compatibility. > > It's incredibly easy. > > Agreed, though in this case I am not sure it makes sense since this > isn't necessarily something that is a Virtio feature itself. It is > just a side effect of the fact that they are adding SR-IOV support to > a device that happens to emulate Virtio NET and apparently their PF > has to be identical to the VF other than the PCIe extended config > space. I got that. My point is not everyone implementing SR-IOV will want to do it like this. Others might want to have VFs be different from PFs somehow. Feature bits ensure forward not just backward compatibility. > > But maybe it's not needed here. I am not making the decisions myself. > > Not too late: post to the TC list and let's see what the response is. > > Without a feature bit you are making a change affecting all future > > implementations without exception so the bar is a bit higher: you need > > to actually post a spec text proposal not just a patch showing how to > > use the feature, and TC needs to vote on it. Voting takes a week, > > review a week or two depending on change complexity. > > > > Hope this helps, > > > > -- > > MST > > I think I will leave this for Dan and Mark to handle since I am still > not all that familiar with the hardware in use here. Once a decision > has been made him and Mark could look at pushing either the one line > patch or something more complex involving a feature flag. > > Thanks. > > Alex As long as the TC is involved. I know it's a bit of a strange thing to block it at the driver level, the issue is with the device, but it's literally the only handle I have to prevent people from doing out of spec hacks then pushing it all on us to maintain. -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: virtio-dev-return-3929-cohuck=redhat.com@lists.oasis-open.org Sender: List-Post: List-Help: List-Unsubscribe: List-Subscribe: Received: from lists.oasis-open.org (oasis-open.org [66.179.20.138]) by lists.oasis-open.org (Postfix) with ESMTP id BCDAE5818FA1 for ; Fri, 20 Apr 2018 09:15:08 -0700 (PDT) Date: Fri, 20 Apr 2018 19:14:51 +0300 From: "Michael S. Tsirkin" Message-ID: <20180420191055-mutt-send-email-mst@kernel.org> References: <20180316183042-mutt-send-email-mst@kernel.org> <20180403161151-mutt-send-email-mst@kernel.org> <20180403212503-mutt-send-email-mst@kernel.org> <20180420030640-mutt-send-email-mst@kernel.org> <20180420180839-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices To: Alexander Duyck Cc: "Daly, Dan" , "Rustad, Mark D" , 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" , David Woodhouse , Christoph Hellwig , dwmw@amazon.co.uk List-ID: On Fri, Apr 20, 2018 at 09:08:51AM -0700, Alexander Duyck wrote: > On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin wrote: > > On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote: > >> > 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). > >> > >> The problem is we are talking about hardware/FPGA, not software. > >> Adding a feature bit means going back and updating RTL. The software > >> side of things is easy, re-validating things after a hardware/FPGA > >> change not so much. > >> > >> If this is a hard requirement I may just drop the virtio patch, push > >> what I have, and leave it to Mark/Dan to deal with the necessary RTL > >> and code changes needed to support Virtio as I don't expect the > >> turnaround to be as easy as just a patch. > >> > >> Thanks. > >> > >> - Alex > > > > Let's focus on virtio in this thread. > > That is kind of what I was thinking, and why I was thinking it might > make sense to make the virtio specific changes a separate patch set. I > could get the PCI bits taken care of in the meantime since they effect > genetic PCI, NVMe, and the Amazon ENA interfaces. > > > Involving the virtio TC in host/guest interface changes is a > > hard requirement. It's just too easy to create conflicts otherwise. > > > > So you guys should have just sent the proposal to the TC when you > > were doing your RTL and you would have been in the clear. > > Agreed. I believe I brought this up when I was originally asked to > look into the coding for this. > > > Generally adding a feature bit with any extension is a good idea: > > this way you merely reserve a feature bit for your feature through > > the TC and are more or less sure of forward and backward compatibility. > > It's incredibly easy. > > Agreed, though in this case I am not sure it makes sense since this > isn't necessarily something that is a Virtio feature itself. It is > just a side effect of the fact that they are adding SR-IOV support to > a device that happens to emulate Virtio NET and apparently their PF > has to be identical to the VF other than the PCIe extended config > space. I got that. My point is not everyone implementing SR-IOV will want to do it like this. Others might want to have VFs be different from PFs somehow. Feature bits ensure forward not just backward compatibility. > > But maybe it's not needed here. I am not making the decisions myself. > > Not too late: post to the TC list and let's see what the response is. > > Without a feature bit you are making a change affecting all future > > implementations without exception so the bar is a bit higher: you need > > to actually post a spec text proposal not just a patch showing how to > > use the feature, and TC needs to vote on it. Voting takes a week, > > review a week or two depending on change complexity. > > > > Hope this helps, > > > > -- > > MST > > I think I will leave this for Dan and Mark to handle since I am still > not all that familiar with the hardware in use here. Once a decision > has been made him and Mark could look at pushing either the one line > patch or something more complex involving a feature flag. > > Thanks. > > Alex As long as the TC is involved. I know it's a bit of a strange thing to block it at the driver level, the issue is with the device, but it's literally the only handle I have to prevent people from doing out of spec hacks then pushing it all on us to maintain. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org