| Message ID | 20240107132237.50553-3-deller@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/hppa qemu v8.2 regression fixes | expand |
On 1/8/24 00:22, deller@kernel.org wrote: > From: Helge Deller <deller@gmx.de> > > Add support for the qemu --nodefaults option, which will disable the > following default devices: > - lsi53c895a SCSI controller, > - artist graphics card, > - LASI 82596 NIC, > - tulip PCI NIC, > - second serial PCI card, > - USB OHCI controller. > > Adding this option is very useful to allow manual testing and > debugging of the other possible devices on the command line. > > Signed-off-by: Helge Deller <deller@gmx.de> > --- > hw/hppa/machine.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c > index b11907617e..8017002a2a 100644 > --- a/hw/hppa/machine.c > +++ b/hw/hppa/machine.c > @@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, > SysBusDevice *s; > > /* SCSI disk setup. */ > - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); > - lsi53c8xx_handle_legacy_cmdline(dev); > + if (defaults_enabled()) { > + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); > + lsi53c8xx_handle_legacy_cmdline(dev); > + } > > /* Graphics setup. */ > - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { > + if (defaults_enabled() && machine->enable_graphics && > + vga_interface_type != VGA_NONE) { > vga_interface_created = true; > dev = qdev_new("artist"); > s = SYS_BUS_DEVICE(dev); > @@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, > } > > /* Network setup. */ > - if (enable_lasi_lan()) { > + if (defaults_enabled() && enable_lasi_lan()) { > lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), > qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); > } > @@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, > pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */ > > /* create a second serial PCI card when running Astro */ > - if (!lasi_dev) { > + if (defaults_enabled() && !lasi_dev) { > pci_dev = pci_new(-1, "pci-serial-4x"); > qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); > qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); > @@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, > } > > /* create USB OHCI controller for USB keyboard & mouse on Astro machines */ > - if (!lasi_dev && machine->enable_graphics) { > + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { > pci_create_simple(pci_bus, -1, "pci-ohci"); > usb_create_simple(usb_bus_find(-1), "usb-kbd"); > usb_create_simple(usb_bus_find(-1), "usb-mouse"); This almost doubles the uses of default_enabled in the entire tree. I wonder if some of them are redundant or should be using a different test. r~
On 1/9/24 10:57, Richard Henderson wrote: > On 1/8/24 00:22, deller@kernel.org wrote: >> From: Helge Deller <deller@gmx.de> >> >> Add support for the qemu --nodefaults option, which will disable the >> following default devices: >> - lsi53c895a SCSI controller, >> - artist graphics card, >> - LASI 82596 NIC, >> - tulip PCI NIC, >> - second serial PCI card, >> - USB OHCI controller. >> >> Adding this option is very useful to allow manual testing and >> debugging of the other possible devices on the command line. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> --- >> hw/hppa/machine.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >> index b11907617e..8017002a2a 100644 >> --- a/hw/hppa/machine.c >> +++ b/hw/hppa/machine.c >> @@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >> SysBusDevice *s; >> /* SCSI disk setup. */ >> - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >> - lsi53c8xx_handle_legacy_cmdline(dev); >> + if (defaults_enabled()) { >> + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >> + lsi53c8xx_handle_legacy_cmdline(dev); >> + } >> /* Graphics setup. */ >> - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { >> + if (defaults_enabled() && machine->enable_graphics && >> + vga_interface_type != VGA_NONE) { >> vga_interface_created = true; >> dev = qdev_new("artist"); >> s = SYS_BUS_DEVICE(dev); >> @@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >> } >> /* Network setup. */ >> - if (enable_lasi_lan()) { >> + if (defaults_enabled() && enable_lasi_lan()) { >> lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), >> qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); >> } >> @@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >> pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */ >> /* create a second serial PCI card when running Astro */ >> - if (!lasi_dev) { >> + if (defaults_enabled() && !lasi_dev) { >> pci_dev = pci_new(-1, "pci-serial-4x"); >> qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); >> qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); >> @@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >> } >> /* create USB OHCI controller for USB keyboard & mouse on Astro machines */ >> - if (!lasi_dev && machine->enable_graphics) { >> + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { >> pci_create_simple(pci_bus, -1, "pci-ohci"); >> usb_create_simple(usb_bus_find(-1), "usb-kbd"); >> usb_create_simple(usb_bus_find(-1), "usb-mouse"); > > This almost doubles the uses of default_enabled in the entire tree. > I wonder if some of them are redundant or should be using a different > test. Any proposal? Maybe introduce a local variable hppa_bare_metal = !defaults_enabled(); and use that instead? Helge
On 1/9/24 22:16, Helge Deller wrote: > On 1/9/24 10:57, Richard Henderson wrote: >> On 1/8/24 00:22, deller@kernel.org wrote: >>> From: Helge Deller <deller@gmx.de> >>> >>> Add support for the qemu --nodefaults option, which will disable the >>> following default devices: >>> - lsi53c895a SCSI controller, >>> - artist graphics card, >>> - LASI 82596 NIC, >>> - tulip PCI NIC, >>> - second serial PCI card, >>> - USB OHCI controller. >>> >>> Adding this option is very useful to allow manual testing and >>> debugging of the other possible devices on the command line. >>> >>> Signed-off-by: Helge Deller <deller@gmx.de> >>> --- >>> hw/hppa/machine.c | 15 +++++++++------ >>> 1 file changed, 9 insertions(+), 6 deletions(-) >>> >>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >>> index b11907617e..8017002a2a 100644 >>> --- a/hw/hppa/machine.c >>> +++ b/hw/hppa/machine.c >>> @@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState *machine, >>> PCIBus *pci_bus, >>> SysBusDevice *s; >>> /* SCSI disk setup. */ >>> - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>> - lsi53c8xx_handle_legacy_cmdline(dev); >>> + if (defaults_enabled()) { >>> + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>> + lsi53c8xx_handle_legacy_cmdline(dev); >>> + } >>> /* Graphics setup. */ >>> - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { >>> + if (defaults_enabled() && machine->enable_graphics && >>> + vga_interface_type != VGA_NONE) { >>> vga_interface_created = true; >>> dev = qdev_new("artist"); >>> s = SYS_BUS_DEVICE(dev); >>> @@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState *machine, >>> PCIBus *pci_bus, >>> } >>> /* Network setup. */ >>> - if (enable_lasi_lan()) { >>> + if (defaults_enabled() && enable_lasi_lan()) { >>> lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), >>> qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); >>> } >>> @@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState *machine, >>> PCIBus *pci_bus, >>> pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */ >>> /* create a second serial PCI card when running Astro */ >>> - if (!lasi_dev) { >>> + if (defaults_enabled() && !lasi_dev) { >>> pci_dev = pci_new(-1, "pci-serial-4x"); >>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); >>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); >>> @@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState *machine, >>> PCIBus *pci_bus, >>> } >>> /* create USB OHCI controller for USB keyboard & mouse on Astro machines */ >>> - if (!lasi_dev && machine->enable_graphics) { >>> + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { >>> pci_create_simple(pci_bus, -1, "pci-ohci"); >>> usb_create_simple(usb_bus_find(-1), "usb-kbd"); >>> usb_create_simple(usb_bus_find(-1), "usb-mouse"); >> >> This almost doubles the uses of default_enabled in the entire tree. >> I wonder if some of them are redundant or should be using a different >> test. > > Any proposal? > Maybe introduce a local variable hppa_bare_metal = !defaults_enabled(); > and use that instead? No, not like that. In casual review I am surprised that !defaults_enabled() does not already imply !enable_graphics, unless the command-line goes on to explicitly add a graphics device. Am I missing something? r~
On 1/9/24 17:01, Richard Henderson wrote: > On 1/9/24 22:16, Helge Deller wrote: >> On 1/9/24 10:57, Richard Henderson wrote: >>> On 1/8/24 00:22, deller@kernel.org wrote: >>>> From: Helge Deller <deller@gmx.de> >>>> >>>> Add support for the qemu --nodefaults option, which will disable the >>>> following default devices: >>>> - lsi53c895a SCSI controller, >>>> - artist graphics card, >>>> - LASI 82596 NIC, >>>> - tulip PCI NIC, >>>> - second serial PCI card, >>>> - USB OHCI controller. >>>> >>>> Adding this option is very useful to allow manual testing and >>>> debugging of the other possible devices on the command line. >>>> >>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>> --- >>>> hw/hppa/machine.c | 15 +++++++++------ >>>> 1 file changed, 9 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >>>> index b11907617e..8017002a2a 100644 >>>> --- a/hw/hppa/machine.c >>>> +++ b/hw/hppa/machine.c >>>> @@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>> SysBusDevice *s; >>>> /* SCSI disk setup. */ >>>> - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>>> - lsi53c8xx_handle_legacy_cmdline(dev); >>>> + if (defaults_enabled()) { >>>> + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>>> + lsi53c8xx_handle_legacy_cmdline(dev); >>>> + } >>>> /* Graphics setup. */ >>>> - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { >>>> + if (defaults_enabled() && machine->enable_graphics && >>>> + vga_interface_type != VGA_NONE) { >>>> vga_interface_created = true; >>>> dev = qdev_new("artist"); >>>> s = SYS_BUS_DEVICE(dev); >>>> @@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>> } >>>> /* Network setup. */ >>>> - if (enable_lasi_lan()) { >>>> + if (defaults_enabled() && enable_lasi_lan()) { >>>> lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), >>>> qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); >>>> } >>>> @@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>> pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */ >>>> /* create a second serial PCI card when running Astro */ >>>> - if (!lasi_dev) { >>>> + if (defaults_enabled() && !lasi_dev) { >>>> pci_dev = pci_new(-1, "pci-serial-4x"); >>>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); >>>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); >>>> @@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>> } >>>> /* create USB OHCI controller for USB keyboard & mouse on Astro machines */ >>>> - if (!lasi_dev && machine->enable_graphics) { >>>> + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { >>>> pci_create_simple(pci_bus, -1, "pci-ohci"); >>>> usb_create_simple(usb_bus_find(-1), "usb-kbd"); >>>> usb_create_simple(usb_bus_find(-1), "usb-mouse"); >>> >>> This almost doubles the uses of default_enabled in the entire tree. >>> I wonder if some of them are redundant or should be using a different >>> test. >> >> Any proposal? >> Maybe introduce a local variable hppa_bare_metal = !defaults_enabled(); >> and use that instead? > > No, not like that. Ok. > In casual review I am surprised that !defaults_enabled() does not > already imply !enable_graphics, unless the command-line goes on to > explicitly add a graphics device. > Am I missing something? Will check that tommorow. If it does I'll remove that additional check. But what other do you suggest in general how I should address your concerns here? Helge
On 11/01/2024 23.28, Helge Deller wrote: > On 1/9/24 17:01, Richard Henderson wrote: >> On 1/9/24 22:16, Helge Deller wrote: >>> On 1/9/24 10:57, Richard Henderson wrote: >>>> On 1/8/24 00:22, deller@kernel.org wrote: >>>>> From: Helge Deller <deller@gmx.de> >>>>> >>>>> Add support for the qemu --nodefaults option, which will disable the >>>>> following default devices: >>>>> - lsi53c895a SCSI controller, >>>>> - artist graphics card, >>>>> - LASI 82596 NIC, >>>>> - tulip PCI NIC, >>>>> - second serial PCI card, >>>>> - USB OHCI controller. >>>>> >>>>> Adding this option is very useful to allow manual testing and >>>>> debugging of the other possible devices on the command line. >>>>> >>>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>>> --- >>>>> hw/hppa/machine.c | 15 +++++++++------ >>>>> 1 file changed, 9 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >>>>> index b11907617e..8017002a2a 100644 >>>>> --- a/hw/hppa/machine.c >>>>> +++ b/hw/hppa/machine.c >>>>> @@ -346,11 +346,14 @@ static void >>>>> machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>> SysBusDevice *s; >>>>> /* SCSI disk setup. */ >>>>> - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>>>> - lsi53c8xx_handle_legacy_cmdline(dev); >>>>> + if (defaults_enabled()) { >>>>> + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>>>> + lsi53c8xx_handle_legacy_cmdline(dev); >>>>> + } >>>>> /* Graphics setup. */ >>>>> - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { >>>>> + if (defaults_enabled() && machine->enable_graphics && >>>>> + vga_interface_type != VGA_NONE) { >>>>> vga_interface_created = true; >>>>> dev = qdev_new("artist"); >>>>> s = SYS_BUS_DEVICE(dev); >>>>> @@ -360,7 +363,7 @@ static void >>>>> machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>> } >>>>> /* Network setup. */ >>>>> - if (enable_lasi_lan()) { >>>>> + if (defaults_enabled() && enable_lasi_lan()) { >>>>> lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), >>>>> qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); >>>>> } >>>>> @@ -385,7 +388,7 @@ static void >>>>> machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>> pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* >>>>> Powerbar */ >>>>> /* create a second serial PCI card when running Astro */ >>>>> - if (!lasi_dev) { >>>>> + if (defaults_enabled() && !lasi_dev) { >>>>> pci_dev = pci_new(-1, "pci-serial-4x"); >>>>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); >>>>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); >>>>> @@ -395,7 +398,7 @@ static void >>>>> machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>> } >>>>> /* create USB OHCI controller for USB keyboard & mouse on Astro >>>>> machines */ >>>>> - if (!lasi_dev && machine->enable_graphics) { >>>>> + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { >>>>> pci_create_simple(pci_bus, -1, "pci-ohci"); >>>>> usb_create_simple(usb_bus_find(-1), "usb-kbd"); >>>>> usb_create_simple(usb_bus_find(-1), "usb-mouse"); >>>> >>>> This almost doubles the uses of default_enabled in the entire tree. >>>> I wonder if some of them are redundant or should be using a different >>>> test. >>> >>> Any proposal? >>> Maybe introduce a local variable hppa_bare_metal = !defaults_enabled(); >>> and use that instead? >> >> No, not like that. > > Ok. > >> In casual review I am surprised that !defaults_enabled() does not >> already imply !enable_graphics, unless the command-line goes on to >> explicitly add a graphics device. >> Am I missing something? > > Will check that tommorow. If it does I'll remove that additional check. > > But what other do you suggest in general how I should address your > concerns here? IIRC enable_graphics is not influenced by --nodefaults, but it should be possible to simply check vga_interface_type only - that should get set to VGA_NONE when the user started QEMU with --nodefaults. For networking, other boards normally check nd_table[0]. And for serial, you can check whether serial_hd(0) returns a non-NULL value. For checking whether you have to create SCSI devices by default, you can check drive_get_max_bus(IF_SCSI), I think. HTH, Thomas
On 1/12/24 06:09, Thomas Huth wrote: > On 11/01/2024 23.28, Helge Deller wrote: >> On 1/9/24 17:01, Richard Henderson wrote: >>> On 1/9/24 22:16, Helge Deller wrote: >>>> On 1/9/24 10:57, Richard Henderson wrote: >>>>> On 1/8/24 00:22, deller@kernel.org wrote: >>>>>> From: Helge Deller <deller@gmx.de> >>>>>> >>>>>> Add support for the qemu --nodefaults option, which will disable the >>>>>> following default devices: >>>>>> - lsi53c895a SCSI controller, >>>>>> - artist graphics card, >>>>>> - LASI 82596 NIC, >>>>>> - tulip PCI NIC, >>>>>> - second serial PCI card, >>>>>> - USB OHCI controller. >>>>>> >>>>>> Adding this option is very useful to allow manual testing and >>>>>> debugging of the other possible devices on the command line. >>>>>> >>>>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>>>> --- >>>>>> hw/hppa/machine.c | 15 +++++++++------ >>>>>> 1 file changed, 9 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >>>>>> index b11907617e..8017002a2a 100644 >>>>>> --- a/hw/hppa/machine.c >>>>>> +++ b/hw/hppa/machine.c >>>>>> @@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>>> SysBusDevice *s; >>>>>> /* SCSI disk setup. */ >>>>>> - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>>>>> - lsi53c8xx_handle_legacy_cmdline(dev); >>>>>> + if (defaults_enabled()) { >>>>>> + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); >>>>>> + lsi53c8xx_handle_legacy_cmdline(dev); >>>>>> + } >>>>>> /* Graphics setup. */ >>>>>> - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { >>>>>> + if (defaults_enabled() && machine->enable_graphics && >>>>>> + vga_interface_type != VGA_NONE) { >>>>>> vga_interface_created = true; >>>>>> dev = qdev_new("artist"); >>>>>> s = SYS_BUS_DEVICE(dev); >>>>>> @@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>>> } >>>>>> /* Network setup. */ >>>>>> - if (enable_lasi_lan()) { >>>>>> + if (defaults_enabled() && enable_lasi_lan()) { >>>>>> lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), >>>>>> qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); >>>>>> } >>>>>> @@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>>> pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */ >>>>>> /* create a second serial PCI card when running Astro */ >>>>>> - if (!lasi_dev) { >>>>>> + if (defaults_enabled() && !lasi_dev) { >>>>>> pci_dev = pci_new(-1, "pci-serial-4x"); >>>>>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); >>>>>> qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); >>>>>> @@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, >>>>>> } >>>>>> /* create USB OHCI controller for USB keyboard & mouse on Astro machines */ >>>>>> - if (!lasi_dev && machine->enable_graphics) { >>>>>> + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { >>>>>> pci_create_simple(pci_bus, -1, "pci-ohci"); >>>>>> usb_create_simple(usb_bus_find(-1), "usb-kbd"); >>>>>> usb_create_simple(usb_bus_find(-1), "usb-mouse"); >>>>> >>>>> This almost doubles the uses of default_enabled in the entire tree. >>>>> I wonder if some of them are redundant or should be using a different >>>>> test. >>>> >>>> Any proposal? >>>> Maybe introduce a local variable hppa_bare_metal = !defaults_enabled(); >>>> and use that instead? >>> >>> No, not like that. >> >> Ok. >> >>> In casual review I am surprised that !defaults_enabled() does not >>> already imply !enable_graphics, unless the command-line goes on to >>> explicitly add a graphics device. >>> Am I missing something? >> >> Will check that tommorow. If it does I'll remove that additional check. >> >> But what other do you suggest in general how I should address your >> concerns here? > > IIRC enable_graphics is not influenced by --nodefaults, but it should be possible to simply check vga_interface_type only - that should get set to VGA_NONE when the user started QEMU with --nodefaults. > > For networking, other boards normally check nd_table[0]. And for serial, you can check whether serial_hd(0) returns a non-NULL value. > > For checking whether you have to create SCSI devices by default, you can check drive_get_max_bus(IF_SCSI), I think. Thanks a lot! Those hints really helped me to avoid usage of defaults_enabled(). Helge
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c index b11907617e..8017002a2a 100644 --- a/hw/hppa/machine.c +++ b/hw/hppa/machine.c @@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, SysBusDevice *s; /* SCSI disk setup. */ - dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); - lsi53c8xx_handle_legacy_cmdline(dev); + if (defaults_enabled()) { + dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a")); + lsi53c8xx_handle_legacy_cmdline(dev); + } /* Graphics setup. */ - if (machine->enable_graphics && vga_interface_type != VGA_NONE) { + if (defaults_enabled() && machine->enable_graphics && + vga_interface_type != VGA_NONE) { vga_interface_created = true; dev = qdev_new("artist"); s = SYS_BUS_DEVICE(dev); @@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, } /* Network setup. */ - if (enable_lasi_lan()) { + if (defaults_enabled() && enable_lasi_lan()) { lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA), qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA)); } @@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */ /* create a second serial PCI card when running Astro */ - if (!lasi_dev) { + if (defaults_enabled() && !lasi_dev) { pci_dev = pci_new(-1, "pci-serial-4x"); qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1)); qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2)); @@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus, } /* create USB OHCI controller for USB keyboard & mouse on Astro machines */ - if (!lasi_dev && machine->enable_graphics) { + if (defaults_enabled() && !lasi_dev && machine->enable_graphics) { pci_create_simple(pci_bus, -1, "pci-ohci"); usb_create_simple(usb_bus_find(-1), "usb-kbd"); usb_create_simple(usb_bus_find(-1), "usb-mouse");