Re: [PATCH RFC 05/12] remoteproc: always report VIRTIO_F_VERSION_1 feature

On Tue, Sep 22, 2026 at 08:32:15AM +0200, Francesco Valla wrote:

On Mon, Sep 21, 2026 at 09:47:54AM -0600, Mathieu Poirier wrote:

quoted

On Wed, Sep 16, 2026 at 11:10:50PM +0200, Francesco Valla wrote:

quoted

There is currently no way to report the VIRTIO_F_VERSION_1 using the
resource table alone, as the per-vdev feature array is limited to 32
bits.

Considering that the VirtIO 1.0 specification is now ~10 years old,
always report the VIRTIO_F_VERSION_1 feature, as some drivers depend on
it.

Signed-off-by: Francesco Valla <redacted>
---
 drivers/remoteproc/remoteproc_virtio.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 74e0da970f1d..cfd66d9d1c9e 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c

@@ -249,7 +249,7 @@ static u64 rproc_virtio_get_features(struct virtio_device *vdev)
 
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 
-	return rsc->dfeatures;
+	return rsc->dfeatures | (1ULL << VIRTIO_F_VERSION_1);

I understand this.

quoted

 }
 
 static void rproc_transport_features(struct virtio_device *vdev)

@@ -275,14 +275,16 @@ static int rproc_virtio_finalize_features(struct virtio_device *vdev)
 	/* Give virtio_rproc a chance to accept features. */
 	rproc_transport_features(vdev);
 
-	/* Make sure we don't have any features > 32 bits! */
-	BUG_ON((u32)vdev->features != vdev->features);
+	/* Make sure we don't have any features > 32 bits except VIRTIO_F_VERSION_1 */
+	if (WARN_ON_ONCE((u32)vdev->features !=
+			 (vdev->features & ~(1ULL << VIRTIO_F_VERSION_1))))
+		return -1;

And this.

quoted

 
 	/*
 	 * Remember the finalized features of our vdev, and provide it
 	 * to the remote processor once it is powered on.
 	 */
-	rsc->gfeatures = vdev->features;
+	rsc->gfeatures = vdev->features & ~(1ULL << VIRTIO_F_VERSION_1);
 

But I'm having trouble with this one.  

First bit 32 of vdev->features gets cleared and then bit 0 to 31 are assigned to
rsc->gfeatures.  Why bother with clearing bit 32?

Bits > 31 gets silently dropped because rsc->gfeatures is a u32; this
was inserted to explicitly drop the flag that has been inserted before
and avoid the "silently". It has no real funtional impact and can be
safely dropped.

We have come to the same conclusion.
 

quoted

quoted

 	return 0;
 }

-- 
2.55.0