netdev - Re: [PATCH 2/2] vDPA: conditionally read fields in virtio-net dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEumKfktMUJOTUYL_JYkFbw8qH331gGARPB2bTH=7wKWPg@mail.gmail.com>
Date:   Fri, 19 Aug 2022 08:42:32 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Si-Wei Liu <si-wei.liu@...cle.com>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        "Zhu, Lingshan" <lingshan.zhu@...el.com>,
        virtualization <virtualization@...ts.linux-foundation.org>,
        netdev <netdev@...r.kernel.org>, kvm <kvm@...r.kernel.org>,
        Parav Pandit <parav@...dia.com>,
        Yongji Xie <xieyongji@...edance.com>,
        "Dawar, Gautam" <gautam.dawar@....com>
Subject: Re: [PATCH 2/2] vDPA: conditionally read fields in virtio-net dev

On Fri, Aug 19, 2022 at 7:20 AM Si-Wei Liu <si-wei.liu@...cle.com> wrote:
>
>
>
> On 8/17/2022 9:15 PM, Jason Wang wrote:
> >
> > 在 2022/8/17 18:37, Michael S. Tsirkin 写道:
> >> On Wed, Aug 17, 2022 at 05:43:22PM +0800, Zhu, Lingshan wrote:
> >>>
> >>> On 8/17/2022 5:39 PM, Michael S. Tsirkin wrote:
> >>>> On Wed, Aug 17, 2022 at 05:13:59PM +0800, Zhu, Lingshan wrote:
> >>>>> On 8/17/2022 4:55 PM, Michael S. Tsirkin wrote:
> >>>>>> On Wed, Aug 17, 2022 at 10:14:26AM +0800, Zhu, Lingshan wrote:
> >>>>>>> Yes it is a little messy, and we can not check _F_VERSION_1
> >>>>>>> because of
> >>>>>>> transitional devices, so maybe this is the best we can do for now
> >>>>>> I think vhost generally needs an API to declare config space
> >>>>>> endian-ness
> >>>>>> to kernel. vdpa can reuse that too then.
> >>>>> Yes, I remember you have mentioned some IOCTL to set the endian-ness,
> >>>>> for vDPA, I think only the vendor driver knows the endian,
> >>>>> so we may need a new function vdpa_ops->get_endian().
> >>>>> In the last thread, we say maybe it's better to add a comment for
> >>>>> now.
> >>>>> But if you think we should add a vdpa_ops->get_endian(), I can work
> >>>>> on it for sure!
> >>>>>
> >>>>> Thanks
> >>>>> Zhu Lingshan
> >>>> I think QEMU has to set endian-ness. No one else knows.
> >>> Yes, for SW based vhost it is true. But for HW vDPA, only
> >>> the device & driver knows the endian, I think we can not
> >>> "set" a hardware's endian.
> >> QEMU knows the guest endian-ness and it knows that
> >> device is accessed through the legacy interface.
> >> It can accordingly send endian-ness to the kernel and
> >> kernel can propagate it to the driver.
> >
> >
> > I wonder if we can simply force LE and then Qemu can do the endian
> > conversion?
> convert from LE for config space fields only, or QEMU has to forcefully
> mediate and covert endianness for all device memory access including
> even the datapath (fields in descriptor and avail/used rings)?

Former. Actually, I want to force modern devices for vDPA when
developing the vDPA framework. But then we see requirements for
transitional or even legacy (e.g the Ali ENI parent). So it
complicates things a lot.

I think several ideas has been proposed:

1) Your proposal of having a vDPA specific way for
modern/transitional/legacy awareness. This seems very clean since each
transport should have the ability to do that but it still requires
some kind of mediation for the case e.g running BE legacy guest on LE
host.

2) Michael suggests using VHOST_SET_VRING_ENDIAN where it means we
need a new config ops for vDPA bus, but it doesn't solve the issue for
config space (at least from its name). We probably need a new ioctl
for both vring and config space.

or

3) revisit the idea of forcing modern only device which may simplify
things a lot

which way should we go?

> I hope
> it's not the latter, otherwise it loses the point to use vDPA for
> datapath acceleration.
>
> Even if its the former, it's a little weird for vendor device to
> implement a LE config space with BE ring layout, although still possible...

Right.

Thanks

>
> -Siwei
> >
> > Thanks
> >
> >
> >>
> >>> So if you think we should add a vdpa_ops->get_endian(),
> >>> I will drop these comments in the next version of
> >>> series, and work on a new patch for get_endian().
> >>>
> >>> Thanks,
> >>> Zhu Lingshan
> >> Guests don't get endian-ness from devices so this seems pointless.
> >>
> >
>