[PATCH] hw/ppc/mac_newworld: Check for the availability of pci-ohci before using it

Thomas Huth posted 1 patch 11 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230526125323.2386324-1-thuth@redhat.com
Maintainers: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
hw/ppc/mac_newworld.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] hw/ppc/mac_newworld: Check for the availability of pci-ohci before using it
Posted by Thomas Huth 11 months, 3 weeks ago
pci-ohci might habe been disabled in the QEMU binary (e.g. when "configure"
has been run with "--without-default-devices"). Thus we should check
for its availability before blindly using it.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/mac_newworld.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 535710314a..c7cca430e1 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -349,7 +349,8 @@ static void ppc_core99_init(MachineState *machine)
                                     sysbus_mmio_get_region(s, 3));
     }
 
-    machine->usb |= defaults_enabled() && !machine->usb_disabled;
+    machine->usb |= defaults_enabled() && !machine->usb_disabled &&
+                    module_object_class_by_name("pci-ohci") != 0;
     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);
-- 
2.31.1
Re: [PATCH] hw/ppc/mac_newworld: Check for the availability of pci-ohci before using it
Posted by BALATON Zoltan 11 months, 3 weeks ago
On Fri, 26 May 2023, Thomas Huth wrote:
> pci-ohci might habe been disabled in the QEMU binary (e.g. when "configure"
> has been run with "--without-default-devices"). Thus we should check
> for its availability before blindly using it.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/ppc/mac_newworld.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 535710314a..c7cca430e1 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -349,7 +349,8 @@ static void ppc_core99_init(MachineState *machine)
>                                     sysbus_mmio_get_region(s, 3));
>     }
>
> -    machine->usb |= defaults_enabled() && !machine->usb_disabled;
> +    machine->usb |= defaults_enabled() && !machine->usb_disabled &&
> +                    module_object_class_by_name("pci-ohci") != 0;

Considering that PowerMacs have an OHCI controller built in soldered to 
the motherboard should this depend on it instead and not rely on pulling 
it in with PCI_DEVICES and --without-default-devices disabling it?

Currently it's not quite emulating a real Mac but I think we should aim 
for going that way rather than to keep emulating random Mac hardware.

Regards,
BALATON Zoltan

>     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);
>
Re: [PATCH] hw/ppc/mac_newworld: Check for the availability of pci-ohci before using it
Posted by Mark Cave-Ayland 11 months, 3 weeks ago
On 26/05/2023 14:30, BALATON Zoltan wrote:

> On Fri, 26 May 2023, Thomas Huth wrote:
>> pci-ohci might habe been disabled in the QEMU binary (e.g. when "configure"
>> has been run with "--without-default-devices"). Thus we should check
>> for its availability before blindly using it.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/ppc/mac_newworld.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
>> index 535710314a..c7cca430e1 100644
>> --- a/hw/ppc/mac_newworld.c
>> +++ b/hw/ppc/mac_newworld.c
>> @@ -349,7 +349,8 @@ static void ppc_core99_init(MachineState *machine)
>>                                     sysbus_mmio_get_region(s, 3));
>>     }
>>
>> -    machine->usb |= defaults_enabled() && !machine->usb_disabled;
>> +    machine->usb |= defaults_enabled() && !machine->usb_disabled &&
>> +                    module_object_class_by_name("pci-ohci") != 0;
> 
> Considering that PowerMacs have an OHCI controller built in soldered to the 
> motherboard should this depend on it instead and not rely on pulling it in with 
> PCI_DEVICES and --without-default-devices disabling it?
> 
> Currently it's not quite emulating a real Mac but I think we should aim for going 
> that way rather than to keep emulating random Mac hardware.

Indeed that's correct: New World Macs should always have USB ports built-in. I guess 
the problem here is that this isn't being indicated correctly via Kconfig and/or the 
machine->usb_disabled logic?

> Regards,
> BALATON Zoltan
> 
>>     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);


ATB,

Mark.


Re: [PATCH] hw/ppc/mac_newworld: Check for the availability of pci-ohci before using it
Posted by Thomas Huth 11 months, 2 weeks ago
On 26/05/2023 19.24, Mark Cave-Ayland wrote:
> On 26/05/2023 14:30, BALATON Zoltan wrote:
> 
>> On Fri, 26 May 2023, Thomas Huth wrote:
>>> pci-ohci might habe been disabled in the QEMU binary (e.g. when "configure"
>>> has been run with "--without-default-devices"). Thus we should check
>>> for its availability before blindly using it.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> hw/ppc/mac_newworld.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
>>> index 535710314a..c7cca430e1 100644
>>> --- a/hw/ppc/mac_newworld.c
>>> +++ b/hw/ppc/mac_newworld.c
>>> @@ -349,7 +349,8 @@ static void ppc_core99_init(MachineState *machine)
>>>                                     sysbus_mmio_get_region(s, 3));
>>>     }
>>>
>>> -    machine->usb |= defaults_enabled() && !machine->usb_disabled;
>>> +    machine->usb |= defaults_enabled() && !machine->usb_disabled &&
>>> +                    module_object_class_by_name("pci-ohci") != 0;
>>
>> Considering that PowerMacs have an OHCI controller built in soldered to 
>> the motherboard should this depend on it instead and not rely on pulling 
>> it in with PCI_DEVICES and --without-default-devices disabling it?
>>
>> Currently it's not quite emulating a real Mac but I think we should aim 
>> for going that way rather than to keep emulating random Mac hardware.
> 
> Indeed that's correct: New World Macs should always have USB ports built-in. 
> I guess the problem here is that this isn't being indicated correctly via 
> Kconfig and/or the machine->usb_disabled logic?

Yes, the other solution to the problem is to add a proper "select" statement 
to the Kconfig file. I can also send a patch for that instead.

The other question is whether the OHCI device should always be instantiated, 
even if QEMU had been started with "--nodefaults"? ... otherwise you could 
not hot-plug USB devices to this machine during runtime...

  Thomas