[PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[Philippe Mathieu-Daudé]

Only add default devices checking defaults_enabled().
Remove the unused usb_disabled field in MachineState.

Based-on: <20250526112346.48744-1-philmd@linaro.org>
          "hw/ppc: Fix --without-default-devices build"

Philippe Mathieu-Daudé (3):
  hw/ppc/spapr: Only create default devices when requested
  hw/ppc/mac_newworld: Only create default devices when requested
  hw/boards: Remove MachineState::usb_disabled field

 include/hw/boards.h   | 1 -
 hw/core/machine.c     | 1 -
 hw/ppc/mac_newworld.c | 3 +--
 hw/ppc/spapr.c        | 3 +--
 4 files changed, 2 insertions(+), 6 deletions(-)

-- 
2.47.1

[PATCH 1/3] hw/ppc/spapr: Only create default devices when requested

[Philippe Mathieu-Daudé]

Only create default devices when requested, avoid have the
machine implementation to modify the generic 'usb' boolean
set from command line.

Fixes: 4ee9ced979e ("hw/ppc/spapr: simplify usb controller creation logic")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/spapr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 702f774cda9..bbcb4695d4c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3032,7 +3032,6 @@ static void spapr_machine_init(MachineState *machine)
     has_vga = spapr_vga_init(phb->bus, &error_fatal);
     if (has_vga) {
         spapr->want_stdout_path = !machine->enable_graphics;
-        machine->usb |= defaults_enabled() && !machine->usb_disabled;
     } else {
         spapr->want_stdout_path = true;
     }
@@ -3040,7 +3039,7 @@ static void spapr_machine_init(MachineState *machine)
     if (machine->usb) {
         pci_create_simple(phb->bus, -1, "nec-usb-xhci");
 
-        if (has_vga) {
+        if (has_vga && defaults_enabled()) {
             USBBus *usb_bus;
 
             usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS,
-- 
2.47.1

[PATCH 2/3] hw/ppc/mac_newworld: Only create default devices when requested

[Philippe Mathieu-Daudé]

Only create default devices when requested, avoid have the
machine implementation to modify the generic 'usb' boolean
set from command line.

Fixes: 59a0419856c ("hw/ppc/mac_newworld: simplify usb controller creation logic")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/mac_newworld.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 0b6e096116a..7598d42cbc2 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -346,7 +346,6 @@ static void ppc_core99_init(MachineState *machine)
                                     sysbus_mmio_get_region(s, 3));
     }
 
-    machine->usb |= defaults_enabled() && !machine->usb_disabled;
     has_pmu = (core99_machine->via_config != CORE99_VIA_CONFIG_CUDA);
     has_adb = (core99_machine->via_config == CORE99_VIA_CONFIG_CUDA ||
                core99_machine->via_config == CORE99_VIA_CONFIG_PMU_ADB);
@@ -427,7 +426,7 @@ static void ppc_core99_init(MachineState *machine)
 
         /* U3 needs to use USB for input because Linux doesn't support via-cuda
         on PPC64 */
-        if (!has_adb || machine_arch == ARCH_MAC99_U3) {
+        if ((!has_adb || machine_arch == ARCH_MAC99_U3) && defaults_enabled()) {
             USBBus *usb_bus;
 
             usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS,
-- 
2.47.1

[PATCH 3/3] hw/boards: Remove MachineState::usb_disabled field

[Philippe Mathieu-Daudé]

Commit c6e765035bc ("powerpc: fix -machine usb=no for newworld
and pseries machines") added the MachineState::usb_disabled
field. It is now unused, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/boards.h | 1 -
 hw/core/machine.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index a7b1fcffae3..7d9149242ea 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -406,7 +406,6 @@ struct MachineState {
     bool dump_guest_core;
     bool mem_merge;
     bool usb;
-    bool usb_disabled;
     char *firmware;
     bool iommu;
     bool suppress_vmdesc;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index c3f3a5020d0..896b5067214 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -498,7 +498,6 @@ static void machine_set_usb(Object *obj, bool value, Error **errp)
     MachineState *ms = MACHINE(obj);
 
     ms->usb = value;
-    ms->usb_disabled = !value;
 }
 
 static bool machine_get_graphics(Object *obj, Error **errp)
-- 
2.47.1

Re: [PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[Philippe Mathieu-Daudé]

(forgot to Cc Thomas)

On 26/5/25 15:00, Philippe Mathieu-Daudé wrote:
> Only add default devices checking defaults_enabled().
> Remove the unused usb_disabled field in MachineState.
> 
> Based-on: <20250526112346.48744-1-philmd@linaro.org>
>            "hw/ppc: Fix --without-default-devices build"
> 
> Philippe Mathieu-Daudé (3):
>    hw/ppc/spapr: Only create default devices when requested
>    hw/ppc/mac_newworld: Only create default devices when requested
>    hw/boards: Remove MachineState::usb_disabled field
> 
>   include/hw/boards.h   | 1 -
>   hw/core/machine.c     | 1 -
>   hw/ppc/mac_newworld.c | 3 +--
>   hw/ppc/spapr.c        | 3 +--
>   4 files changed, 2 insertions(+), 6 deletions(-)
> 

Re: [PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

On Mon, 26 May 2025, Philippe Mathieu-Daudé wrote:
> Only add default devices checking defaults_enabled().
> Remove the unused usb_disabled field in MachineState.

At least for Mac machines this may be more complex. I think there is a 
-usb switch to enable/disable USB independently of defaults and due to 
some bugs some MacOS versions may need this to boot so maybe it's used.

Regards,
BALATON Zoltan

> Based-on: <20250526112346.48744-1-philmd@linaro.org>
>          "hw/ppc: Fix --without-default-devices build"
>
> Philippe Mathieu-Daudé (3):
>  hw/ppc/spapr: Only create default devices when requested
>  hw/ppc/mac_newworld: Only create default devices when requested
>  hw/boards: Remove MachineState::usb_disabled field
>
> include/hw/boards.h   | 1 -
> hw/core/machine.c     | 1 -
> hw/ppc/mac_newworld.c | 3 +--
> hw/ppc/spapr.c        | 3 +--
> 4 files changed, 2 insertions(+), 6 deletions(-)
>
>

Re: [PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[Philippe Mathieu-Daudé]

On 26/5/25 15:18, BALATON Zoltan wrote:
> On Mon, 26 May 2025, Philippe Mathieu-Daudé wrote:
>> Only add default devices checking defaults_enabled().
>> Remove the unused usb_disabled field in MachineState.
> 
> At least for Mac machines this may be more complex. I think there is a - 
> usb switch to enable/disable USB independently of defaults and due to 
> some bugs some MacOS versions may need this to boot so maybe it's used.

If the user asks -usb off, we shouldn't re-enable it in the shadow.

If a configuration isn't usable, we have to report a configuration
error, possibly providing hints about what should be fixed.

> 
> Regards,
> BALATON Zoltan
> 
>> Based-on: <20250526112346.48744-1-philmd@linaro.org>
>>          "hw/ppc: Fix --without-default-devices build"
>>
>> Philippe Mathieu-Daudé (3):
>>  hw/ppc/spapr: Only create default devices when requested
>>  hw/ppc/mac_newworld: Only create default devices when requested
>>  hw/boards: Remove MachineState::usb_disabled field
>>
>> include/hw/boards.h   | 1 -
>> hw/core/machine.c     | 1 -
>> hw/ppc/mac_newworld.c | 3 +--
>> hw/ppc/spapr.c        | 3 +--
>> 4 files changed, 2 insertions(+), 6 deletions(-)
>>
>>

Re: [PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[Paolo Bonzini]

On 5/26/25 16:20, Philippe Mathieu-Daudé wrote:
> On 26/5/25 15:18, BALATON Zoltan wrote:
>> On Mon, 26 May 2025, Philippe Mathieu-Daudé wrote:
>>> Only add default devices checking defaults_enabled().
>>> Remove the unused usb_disabled field in MachineState.
>>
>> At least for Mac machines this may be more complex. I think there is a 
>> - usb switch to enable/disable USB independently of defaults and due 
>> to some bugs some MacOS versions may need this to boot so maybe it's 
>> used.
> 
> If the user asks -usb off, we shouldn't re-enable it in the shadow.

And if the user asks -usb on, you shouldn't disable it.  My
understanding is that adding

-        if (!has_adb || machine_arch == ARCH_MAC99_U3) {
+        if ((!has_adb || machine_arch == ARCH_MAC99_U3) && defaults_enabled()) {

disables USB completely when -nodefaults.

If you want to remove usb_disabled, change machine->usb to ON_OFF_AUTO
and query

static inline bool machine_usb_enabled(MachineState *ms)
{
         return (defaults_enabled()
                 ? machine->usb != ON_OFF_AUTO_OFF
                 : machine->usb == ON_OFF_AUTO_ON);
}

instead of machine->usb (even better, change the name of the field so that
it causes a compilation error).

Paolo

Re: [PATCH 0/3] hw/boards: Remove field

[Mark Cave-Ayland]

On 27/05/2025 18:19, Paolo Bonzini wrote:

> On 5/26/25 16:20, Philippe Mathieu-Daudé wrote:
>> On 26/5/25 15:18, BALATON Zoltan wrote:
>>> On Mon, 26 May 2025, Philippe Mathieu-Daudé wrote:
>>>> Only add default devices checking defaults_enabled().
>>>> Remove the unused usb_disabled field in MachineState.
>>>
>>> At least for Mac machines this may be more complex. I think there is a - usb 
>>> switch to enable/disable USB independently of defaults and due to some bugs some 
>>> MacOS versions may need this to boot so maybe it's used.
>>
>> If the user asks -usb off, we shouldn't re-enable it in the shadow.
> 
> And if the user asks -usb on, you shouldn't disable it.  My
> understanding is that adding
> 
> -        if (!has_adb || machine_arch == ARCH_MAC99_U3) {
> +        if ((!has_adb || machine_arch == ARCH_MAC99_U3) && defaults_enabled()) {
> 
> disables USB completely when -nodefaults.
> 
> If you want to remove usb_disabled, change machine->usb to ON_OFF_AUTO
> and query
> 
> static inline bool machine_usb_enabled(MachineState *ms)
> {
>          return (defaults_enabled()
>                  ? machine->usb != ON_OFF_AUTO_OFF
>                  : machine->usb == ON_OFF_AUTO_ON);
> }
> 
> instead of machine->usb (even better, change the name of the field so that
> it causes a compilation error).

I have a feeling that this logic was added a while back to allow a build 
--without-default-devices to work? However disabling USB doesn't actually make sense 
for New World Macs, since the presence of USB ports is one of the ways of 
guaranteeing we have a New World Mac. I'm fairly sure I've seen (BSD?) kernel crashes 
if the USB ports are not present on the mac99 machine.

If MachineState::usb_disabled is removed completely, is it still possible to always 
have the USB ports enabled and the keyboard/mouse plugged into the mac99 machine?


ATB,

Mark.

Re: [PATCH 0/3] hw/boards: Remove MachineState::usb_disabled field

[Paolo Bonzini]

On 5/26/25 16:20, Philippe Mathieu-Daudé wrote:
> On 26/5/25 15:18, BALATON Zoltan wrote:
>> On Mon, 26 May 2025, Philippe Mathieu-Daudé wrote:
>>> Only add default devices checking defaults_enabled().
>>> Remove the unused usb_disabled field in MachineState.
>>
>> At least for Mac machines this may be more complex. I think there is a 
>> - usb switch to enable/disable USB independently of defaults and due 
>> to some bugs some MacOS versions may need this to boot so maybe it's 
>> used.
> 
> If the user asks -usb off, we shouldn't re-enable it in the shadow.

And if the user asks -usb on, you shouldn't disable it.  My
understanding is that adding

-        if (!has_adb || machine_arch == ARCH_MAC99_U3) {
+        if ((!has_adb || machine_arch == ARCH_MAC99_U3) && defaults_enabled()) {

disables USB completely when -nodefaults.

If you want to remove usb_disabled, change machine->usb to ON_OFF_AUTO
and query

static inline bool machine_usb_enabled(MachineState *ms)
{
         return (defaults_enabled()
                 ? machine->usb != ON_OFF_AUTO_OFF
                 : machine->usb == ON_OFF_AUTO_ON);
}

instead of machine->usb (even better, change the name of the field so that
it causes a compilation error).

Paolo