linux-kernel - Re: [PATCH v2 2/2] vdpa: Remove ioctl VHOST_VDPA_SET

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] [day] [month] [year] [list]

Message-ID: <CACGkMEvfdUYLjx-Z+oB11XW-54ErJsQMKcnu2p=dsj5N_BiEKw@mail.gmail.com>
Date: Thu, 5 Sep 2024 10:23:43 +0800
From: Jason Wang <jasowang@...hat.com>
To: Dragos Tatulea <dtatulea@...dia.com>
Cc: Carlos Bilbao <carlos.bilbao.osdev@...il.com>, mst@...hat.com, shannon.nelson@....com, 
	sashal@...nel.org, alvaro.karsz@...id-run.com, christophe.jaillet@...adoo.fr, 
	steven.sistare@...cle.com, bilbao@...edu, xuanzhuo@...ux.alibaba.com, 
	johnah.palmer@...cle.com, eperezma@...hat.com, cratiu@...dia.com, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	Carlos Bilbao <cbilbao@...italocean.com>
Subject: Re: [PATCH v2 2/2] vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance

On Thu, Sep 5, 2024 at 1:48 AM Dragos Tatulea <dtatulea@...dia.com> wrote:
>
>
>
> On 04.09.24 08:34, Jason Wang wrote:
> > On Wed, Sep 4, 2024 at 1:59 PM Dragos Tatulea <dtatulea@...dia.com> wrote:
> >>
> >>
> >>
> >> On 04.09.24 05:38, Jason Wang wrote:
> >>> On Wed, Sep 4, 2024 at 1:15 AM Carlos Bilbao
> >>> <carlos.bilbao.osdev@...il.com> wrote:
> >>>>
> >>>> From: Carlos Bilbao <cbilbao@...italocean.com>
> >>>>
> >>>> Remove invalid ioctl VHOST_VDPA_SET_CONFIG and all its implementations
> >>>> with vdpa_config_ops->set_config(). This is needed per virtio spec
> >>>> requirements; virtio-spec v3.1 Sec 5.1.4 states that "All of the device
> >>>> configuration fields are read-only for the driver."
> >>>>
> >>>> Signed-off-by: Carlos Bilbao <cbilbao@...italocean.com>
> >>>
> >>> Note that only the config space of the modern device is read only. So
> >>> it should be fine to remove vp_vdpa which only works for modern
> >>> devices.
> >> Just out of curiosity: how will this work for devices that are not
> >> v1.3 compliant but are v1.2 compliant?
> >
> > Devices don't know the version of the spec, it works with features.
> > For example, most devices mandate ACCESS_PLATFORM which implies a
> > mandatory VERSION_1. So they are modern devices.
> >
> And modern devices should not write to the device config space.

It depends on the type of the device.

For example, for blocking device, write_back could be modified by
guest if VIRTIO_BLK_F_CONFIG_WCE is neogitated:

"""
If the VIRTIO_BLK_F_CONFIG_WCE feature is negotiated, the cache mode
can be read or set through the writeback field. 0 corresponds to a
writethrough cache, 1 to a writeback cache13. The cache mode after
reset can be either writeback or writethrough. The actual mode can be
determined by reading writeback after feature negotiation.
"""

> This
> was discouraged in v1.x until v1.3 which now prohibits it. Did I get
> this right?

It really depends on the semantics of the field. My understanding is
that, from 1.0 to 1.3 there's no writable fields defined in the spec.
But it doesn't mean we can't have one in the future.

Thanks

>
> Thanks,
> Dragos
>
> >> Or is this true of all devices
> >> except eni?
> >
> > ENI depends on the virtio-pci legacy library, so we know it's a legacy
> > device implementation which allows mac address setting via config
> > space.
> >
> > Thanks
> >
> >>
> >> Thanks,
> >> Dragos
> >>>
> >>> And for eni, it is a legacy only device, so we should not move the
> >>> set_config there.
> >>>
> >>> For the rest, we need the acks for those maintainers.
> >>>
> >>> Thanks
> >>>
> >>
> >
>