Date:   Fri, 20 Apr 2018 09:08:51 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>,
        "Daly, Dan" <dan.daly@...el.com>,
        "Rustad, Mark D" <mark.d.rustad@...el.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        "Duyck, Alexander H" <alexander.h.duyck@...el.com>,
        linux-pci@...r.kernel.org, virtio-dev@...ts.oasis-open.org,
        kvm@...r.kernel.org, Netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-nvme@...ts.infradead.org,
        Keith Busch <keith.busch@...el.com>, netanel@...zon.com,
        Don Dutile <ddutile@...hat.com>,
        Maximilian Heyne <mheyne@...zon.de>,
        "Wang, Liang-min" <liang-min.wang@...el.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Christoph Hellwig <hch@....de>, dwmw@...zon.co.uk
Subject: Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for
 unmanaged SR-IOV on virtio_pci devices

On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin <mst@...hat.com> 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.

> 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

Powered by blists - more mailing lists