Re: [virtio-comment] Re: [PATCH 09/11] transport-pci: Describe PCI MMR dev config registers

[Jason Wang <jasowang@redhat.com>]

On Fri, Apr 14, 2023 at 1:24âAM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> On 4/13/2023 1:14 AM, Jason Wang wrote:
>
> >> For LM BAR length and number should be same between two PCI VFs. But its
> >> orthogonal to this point. Such checks will be done anyway.
> >
> > Quoted the wrong sections, I think it should be:
> >
> > "
> > length MAY include padding, or fields unused by the driver, or future
> > extensions. Note: For example, a future device might present a large
> > structure size of several MBytes. As current devices never utilize
> > structures larger than 4KBytes in size, driver MAY limit the mapped
> > structure size to e.g. 4KBytes (thus ignoring parts of structure after
> > the first 4KBytes) to allow forward compatibility with such devices
> > without loss of functionality and without wasting resources.
> > "
> yes. This is the one.
>
> > If it's a transitional device but not placed at BAR0, it might have
> > side effects for Linux drivers which assumes BAR0 for legacy.
> >
> True. Transitional can be at BAR0.
>
> > I don't see how easy it could be a non transitional device:
> >
> > "
> > Devices or drivers with no legacy compatibility are referred to as
> > non-transitional devices and drivers, respectively.
> > "
> Michael has suggested rewording of the text.
> It is anyway new text so lets park it aside for now.
> It is mostly tweaking the text.
>
> >
> >> device can expose this optional capability and its attached MMIO region.
> >>
> >> Spec changes are similar to #2.
> >>> - non trivial spec changes which ends up of the tricky cases that
> >>> tries to workaround legacy to fit for a hardware implementation
> >>> - work only for the case of virtualization with the help of
> >>> meditation, can't work for bare metal
> >> For bare-metal PFs usually thin hypervisors are used that does very
> >> minimal setup. But I agree that bare-metal is relatively less important.
> >
> > This is not what I understand. I know several vendors that are using
> > virtio devices for bare metal.
> >
> I was saying the case for legacy bare metal is less of a problem because
> PCIe does not limit functionality, perf is still limited due to IOBAR.
>
> >>
> >>> - only work for some specific archs without SVQ
> >>>
> >> That is the legacy limitation that we don't worry about.
> >>
> >>> 2) allow BAR0 to be MMIO for transitional device
> >>>
> >>> Pros:
> >>> - very minor change for the spec
> >> Spec changes wise they are similar to #1.
> >
> > This is different since the changes for this are trivial.
> >
> >>> - work for virtualization (and it work even without dedicated
> >>> mediation for some setups)
> >> I am not aware where can it work without mediation. Do you know any
> >> specific kernel version where it actually works?
> >
> > E.g current Linux driver did:
> >
> > rc = pci_request_region(pci_dev, 0, "virtio-pci-legacy");
> >
> > It doesn't differ from I/O with memory. It means if you had a
> > "transitional" device with legacy MMIO BAR0, it just works.
> >
>
> Thanks to the abstract PCI API in Linux.

And this (legacy MMIO bar) has been supported by DPDK as well for a while.

>
> >>> - work for bare metal for some setups (without mediation)
> >>> Cons:
> >>> - only work for some specific archs without SVQ
> >>> - BAR0 is required
> >>>
> >> Both are not limitation as they are mainly coming from the legacy side
> >> of things.
> >>
> >>> 3) modern device mediation for legacy
> >>>
> >>> Pros:
> >>> - no changes in the spec
> >>> Cons:
> >>> - require mediation layer in order to work in bare metal
> >>> - require datapath mediation like SVQ to work for virtualization
> >>>
> >> Spec change is still require for net and blk because modern device do
> >> not understand legacy, even with mediation layer.
> >
> > That's fine and easy since we work on top of modern devices.
> >
> >> FEATURE_1, RW cap via CVQ which is not really owned by the hypervisor.
> >
> > Hypervisors can trap if they wish.
> >
> Trapping non legacy accessing for 1.x doesn't make sense.

Actually not, I think I've mentioned the reason for several times:

It's a must for ABI and migration compatibility:

1) The offset is not necessarily a page boundary
2) The length is not necessarily times of a PAGE_SIZE
3) PAGE_SIZE varies among different archs
4) Two vendors may have two different layout for the structure

Thanks