On Tue, Apr 07, 2026 at 01:33:39PM -0400, Andrew Stellman wrote:
> Code analysis only — not observed in practice. And you're right that on PCI it
> doesn't matter: vp_transport_features() re-sets the bit after
> vring_transport_features() clears it, so PCI never actually loses the feature.
Oh. please do test patches, or note they were not tested.
how were rest of patches here tested?
>
> The gap is for transports that call vring_transport_features() without
> independently re-setting VIRTIO_F_RING_RESET afterward. Whether any current
> transport hits this in practice I'm not sure — it may just be a whitelist
> consistency fix at this point.
>
> Happy to add a Fixes tag and resend, or drop it if you think the PCI-level
> re-set makes it unnecessary.
>
> Fixes: 04ca0b0b16f1 ("virtio_pci: support VIRTIO_F_RING_RESET")
surely not this because pci is fine.
> On Tue, Apr 7, 2026 at 12:22 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Tue, Apr 07, 2026 at 08:39:04AM -0400, Andrew Stellman wrote:
> > vring_transport_features() whitelists known transport feature bits and
> > clears the rest via __virtio_clear_bit(). VIRTIO_F_RING_RESET is
> > missing from the whitelist, so it is unconditionally cleared during
> > feature negotiation. Drivers that depend on ring reset capability
> > silently lose the feature.
>
> Hmm was this observed in practice or just from code analysis?
> And on which transport?
> Because
>
>
> static void vp_transport_features(struct virtio_device *vdev, u64
> features)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> struct pci_dev *pci_dev = vp_dev->pci_dev;
>
> if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> pci_find_ext_capability(pci_dev,
> PCI_EXT_CAP_ID_SRIOV))
> __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
>
> if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> __virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
>
>
> ...
>
>
>
> }
>
>
>
>
> > Add VIRTIO_F_RING_RESET to the switch statement, matching the other
> > transport-level features.
> >
> > Signed-off-by: Andrew Stellman <[email protected]>
> > ---
> > drivers/virtio/virtio_ring.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index fbca7ce..2cb643f 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -3524,6 +3524,8 @@ void vring_transport_features(struct virtio_device
> *vdev)
> > break;
> > case VIRTIO_F_IN_ORDER:
> > break;
> > + case VIRTIO_F_RING_RESET:
> > + break;
> > default:
> > /* We don't understand this bit. */
> > __virtio_clear_bit(vdev, i);
> > --
> > 2.34.1
>
>
