| Message ID | 20230526125323.2386324-1-thuth@redhat.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | hw/ppc/mac_newworld: Check for the availability of pci-ohci before using it | expand |
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); >
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.
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
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);
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(-)