Re: [PATCH v2 15/16] hw/char/virtio-serial: Do not expose the 'emergency-write' property

[Philippe Mathieu-Daudé]

On 2/5/25 11:41, Mark Cave-Ayland wrote:

On 01/05/2025 19:36, Philippe Mathieu-Daudé wrote:

The VIRTIO_CONSOLE_F_EMERG_WRITE feature bit was only set
in the hw_compat_2_7[] array, via the 'emergency-write=off'
property. We removed all machines using that array, lets remove
that property. All instances have this feature bit set and
it can not be disabled.

Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
  hw/char/virtio-serial-bus.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index eb79f5258b6..31bd1b7535d 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c

@@ -1019,6 +1019,13 @@ static void virtser_port_device_unrealize(DeviceState *dev)

      }
  }
+static void virtio_serial_instance_init(Object *obj)
+{
+    VirtIOSerial *vser = VIRTIO_SERIAL(obj);
+
+    vser->host_features |= BIT_ULL(VIRTIO_CONSOLE_F_EMERG_WRITE);
+}
+

  static void virtio_serial_device_realize(DeviceState *dev, Error **errp)

  {
      VirtIODevice *vdev = VIRTIO_DEVICE(dev);

@@ -1155,8 +1162,6 @@ static const VMStateDescription vmstate_virtio_console = {

  static const Property virtio_serial_properties[] = {

      DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,

                                                    31),
-    DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
-                      VIRTIO_CONSOLE_F_EMERG_WRITE, true),
  };

  static void virtio_serial_class_init(ObjectClass *klass, const void *data) @@ -1186,6 +1191,7 @@ static void virtio_serial_class_init(ObjectClass *klass, const void *data)

  static const TypeInfo virtio_device_info = {
      .name = TYPE_VIRTIO_SERIAL,
      .parent = TYPE_VIRTIO_DEVICE,
+    .instance_init = virtio_serial_instance_init,
      .instance_size = sizeof(VirtIOSerial),
      .class_init = virtio_serial_class_init,
      .interfaces = (const InterfaceInfo[]) {

You could also consider removing vser->host_features completely, since it is always ORd into the host_features list in get_features(), plus all remaining mentions are of the form:

if (virtio_has_feature(vser->host_features,
         VIRTIO_CONSOLE_F_EMERG_WRITE)) { ... }

Good remark, thanks!