Re: [PATCH] hw/s390x: Emit a warning if user tried to enable USB

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 10/17/19 4:40 PM, Thomas Huth wrote:
> On 17/10/2019 16.34, Cornelia Huck wrote:
>> On Thu, 17 Oct 2019 16:21:23 +0200
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>>> There is no USB on s390x, so running qemu-system-s390x with
>>> "-machine ...,usb=on" is certainly wrong. Emit a warning to make
>>> the users aware of their misconfiguration.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   After a year or two, we could finally turn this into a hard error,
>>>   but I think we should give the users some time to fix their command
>>>   lines first, so I'm initially only emitting a warning here.
>>>
>>>   hw/s390x/s390-virtio-ccw.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index d3edeef0ad..af8c4c0daf 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -243,6 +243,10 @@ static void ccw_init(MachineState *machine)
>>>       VirtualCssBus *css_bus;
>>>       DeviceState *dev;
>>>   
>>> +    if (machine->usb) {
>>> +        warn_report("This machine does not support USB");
>>
>> I'm wondering if this is the only machine type not supporting usb...
>> if not, how are others handling it?
> 
> I think most machines are silently ignoring it, like we did on s390x
> until now, too.
> 
>> The usb parsing code in machine.c does not care if usb is even
>> configured (CONFIG_USB).
> 
> machine.c is common code, so you can not use CONFIG_USB there.

We already have:

bool target_words_bigendian(void)
{
#if defined(TARGET_WORDS_BIGENDIAN)
     return true;
#else
     return false;
#endif
}

What about something such:

-- >8 --
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1689ad3bf8..0c45ab042b 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -986,7 +986,7 @@ static void machine_finalize(Object *obj)

  bool machine_usb(MachineState *machine)
  {
-    return machine->usb;
+    return machine_has_usb() && machine->usb;
  }

  bool machine_kernel_irqchip_allowed(MachineState *machine)
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 303ac084a0..ac545cdd2e 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -1,6 +1,7 @@
  # usb subsystem core
  common-obj-y += core.o combined-packet.o bus.o libhw.o
  common-obj-$(CONFIG_USB) += desc.o desc-msos.o
+obj-y += machine.o

  # usb host adapters
  common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o
diff --git a/hw/usb/machine.c b/hw/usb/machine.c
new file mode 100644
index 0000000000..5381928479
--- /dev/null
+++ b/hw/usb/machine.c
@@ -0,0 +1,12 @@
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+#include "config-devices.h"
+
+bool machine_has_usb(void)
+{
+#if defined(CONFIG_USB)
+    return true;
+#else
+    return false;
+#endif
+}
diff --git a/include/hw/boards.h b/include/hw/boards.h
index be18a5c032..e4518b73b1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -63,6 +63,7 @@ extern MachineState *current_machine;

  void machine_run_board_init(MachineState *machine);
  bool machine_usb(MachineState *machine);
+bool machine_has_usb(void); /* or target_has_usb()? */
  bool machine_kernel_irqchip_allowed(MachineState *machine);
  bool machine_kernel_irqchip_required(MachineState *machine);
  bool machine_kernel_irqchip_split(MachineState *machine);

---