On Thu, Sep 5, 2024 at 1:48 AM Dragos Tatulea <dtatulea@xxxxxxxxxx> wrote: > > > > On 04.09.24 08:34, Jason Wang wrote: > > On Wed, Sep 4, 2024 at 1:59 PM Dragos Tatulea <dtatulea@xxxxxxxxxx> wrote: > >> > >> > >> > >> On 04.09.24 05:38, Jason Wang wrote: > >>> On Wed, Sep 4, 2024 at 1:15 AM Carlos Bilbao > >>> <carlos.bilbao.osdev@xxxxxxxxx> wrote: > >>>> > >>>> From: Carlos Bilbao <cbilbao@xxxxxxxxxxxxxxxx> > >>>> > >>>> 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@xxxxxxxxxxxxxxxx> > >>> > >>> 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 > >>> > >> > > >
