Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] remoteproc: host-firmware protocol mismatch by forcing VIRTIO_F_VERSION_1 --
commit 425d88ad507f29f8586fd7c8c447cf1bcdba6678 Author: Francesco Valla <[email protected]> remoteproc: always report VIRTIO_F_VERSION_1 feature 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. > diff --git a/drivers/remoteproc/remoteproc_virtio.c > b/drivers/remoteproc/remoteproc_virtio.c > index 74e0da970f1d8..cfd66d9d1c9e5 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); [Severity: High] Will this forcefully advertise Virtio 1.0 support to the host virtio core, misleading it into Modern mode? By injecting VIRTIO_F_VERSION_1 into rsc->dfeatures, the host virtio core will preserve the feature since it is a transport feature. This configures virtio_ring as strict Little-Endian and assumes Modern struct layouts. [ ... ] > @@ -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; > > /* > * 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); [Severity: High] Does hiding the VIRTIO_F_VERSION_1 feature from the remote firmware guarantee a feature mismatch between the host and device? Since rproc_virtio_finalize_features() masks the bit out before passing the negotiated features back to the firmware, the firmware remains unaware and operates in Legacy mode using guest-native endianness and Legacy struct sizes. Could this lead to dropped messages or crashes due to differing endianness assumptions, particularly on Big-Endian hosts where vring formatting will break? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
