On Thu, Jun 11, 2026 at 10:29:50AM +0200, Filip Hejsek wrote:
> On Thu, 2026-06-11 at 03:38 -0400, Michael S. Tsirkin wrote:
> > [...]
> > > >
> > > > Wait a second. Why is there this rproc test here?
> > > > Was not in the original code and commit log says nothing about it.
> > > >
> > >
> > > Previously, this code was in config_work_handler(), which was never
> > > called for rproc_serial (it's scheduled from config_intr(), which is
> > > the config_changed handler only for virtio_console).
> > >
> > > Now update_size_from_config() is called unconditionally from
> > > virtcons_probe(), so it will be called for rproc_serial too, which
> > > doesn't have the F_SIZE feature.
> >
> > So why not test it?
>
> The virtio_console driver implements two similar but distinct virtio
> devices: VIRTIO_ID_CONSOLE and VIRTIO_ID_RPROC_SERIAL. Although some of
> the implementation code is shared, the devices are different. In
> particular, rproc_serial doesn't support multiport nor any of the tty
> specific features. This means that the relevant feature bits are not
> valid for this device and must not be tested.
> I have to admit though that I don't quite understand what the
> RPROC_SERIAL device is supposed to be used for. It was added by commit
> 1b6370463e88b0c1c317de16d7b962acc1dab4f2, which describes it as "a
> simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for
> communicating with a remote processor in an asymmetric multi-processing
> configuration". It seems that it was never standardized, as the virtio
> spec only says that its ID is reserved.
>
> > What does "not a valid feature" mean?
>
> I copied the "not a valid feature" comment form other instances in the
> same file where a feature is tested, e.g. in resize_console():
>
> /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
> if (!is_rproc_serial(vdev) &&
> virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> hvc_resize(port->cons.hvc, port->cons.ws);
>
>
> Best regards,
> Filip Hejsek
I get it, it's existing code. It still makes no sense.
rproc has:
static const unsigned int rproc_serial_features[] = {
};
No features.
So testing any feature bit at all always returns 0.
there's no reason to special case anything.
So I'm testing this, but I'm only compiling rproc, so pls holler if it seems
wrong:
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 198b97314168..2261862d4b4c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -340,7 +340,7 @@ static inline bool use_multiport(struct ports_device
*portdev)
*/
if (!portdev->vdev)
return false;
- return __virtio_test_bit(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
+ return virtio_has_feature(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
}
static DEFINE_SPINLOCK(dma_bufs_lock);
@@ -1156,9 +1156,7 @@ static void resize_console(struct port *port)
vdev = port->portdev->vdev;
- /* Don't test F_SIZE at all if we're rproc: not a valid feature! */
- if (!is_rproc_serial(vdev) &&
- virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
+ if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
hvc_resize(port->cons.hvc, port->cons.ws);
}
@@ -1783,11 +1781,8 @@ static void update_size_from_config(struct ports_device
*portdev)
* We'll use this way of resizing only for legacy support.
* For multiport devices, use control messages to indicate
* console size changes so that it can be done per-port.
- *
- * Don't test F_SIZE at all if we're rproc: not a valid feature.
*/
- if (is_rproc_serial(vdev) ||
- use_multiport(portdev) ||
+ if (use_multiport(portdev) ||
!virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
return;
@@ -1994,9 +1989,7 @@ static int virtcons_probe(struct virtio_device *vdev)
multiport = false;
portdev->max_nr_ports = 1;
- /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
- if (!is_rproc_serial(vdev) &&
- virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
+ if (virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
struct virtio_console_config, max_nr_ports,
&portdev->max_nr_ports) == 0) {
if (portdev->max_nr_ports == 0 ||
--
MST
