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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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é <philmd@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!