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

[Philippe Mathieu-Daudé]

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 <address@hidden> 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 <address@hidden>
> > > ---
> > >   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);

---