Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback
On Sun, Sep 24, 2023 at 2:14 PM BALATON Zoltan wrote:
> > If you still want a machine audiodev propery then could the device handle
> >> it without needing changes to the machine? Like in via_isa_realize() add
> >>
> >> if (current_machine->audiodev) {
> >> qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
> >> }
> >>
> >> before qdev_realize(DEVICE(>ac97) then no need to change the device
> >> creation in board code.
> >>
> >
> > No, current_machine should not be used at all outside board code.
>
> OK, can you start from pci_bus and walk up the QOM tree then to find the
> machine in vt92686.c so the board code does not have to care about this?
The machine itself should not be used outside board code, neither via
current_machine nor by any other means. There are so few places where
it happens (most of them in fw_cfg) that I'm not really willing to
compromise on this. The board sets properties on the devices, it's not
the devices that fetch settings from outside.
Paolo
Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback
On Sun, 24 Sep 2023, Paolo Bonzini wrote:
Il sab 23 set 2023, 14:23 BALATON Zoltan ha scritto:
On Sat, 23 Sep 2023, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini
---
hw/isa/vt82c686.c | 2 ++
hw/mips/fuloong2e.c | 13 ++---
hw/ppc/pegasos2.c | 10 --
3 files changed, 20 insertions(+), 5 deletions(-)
This looks better but I still wonder if this machine audiodev propery is
needed at all. If there's one -audiodev option specified it's already
picked up by default devices and if there are more one could use -global
to set it. Why isn't that enough?
Mostly because it's less predictable. Ideally all the state of the emulator
would be visible and settable via explicit links.
You were absolutely right that we still need to keep some level of magic in
softmmu/vl.c to make QEMU easier to use for the command line. However, a
while ago there was an idea of making an alternative binary that is
entirely configurable via QMP, and past work in that direction resulted in
*lots* of cleanups that actually made softmmu/vl.c understandable. While I
am not sure this QMP binary is ever going to happen, I would like to make
it possible to avoid the magic.
If you still want a machine audiodev propery then could the device handle
it without needing changes to the machine? Like in via_isa_realize() add
if (current_machine->audiodev) {
qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
}
before qdev_realize(DEVICE(>ac97) then no need to change the device
creation in board code.
No, current_machine should not be used at all outside board code.
OK, can you start from pci_bus and walk up the QOM tree then to find the
machine in vt92686.c so the board code does not have to care about this?
Regards,
BALATON Zoltan
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78c..3ec8e43708a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
object_initialize_child(obj, "uhci2", >uhci[1],
TYPE_VT82C686B_USB_UHCI);
object_initialize_child(obj, "ac97", >ac97, TYPE_VIA_AC97);
object_initialize_child(obj, "mc97", >mc97, TYPE_VIA_MC97);
+
+object_property_add_alias(obj, "audiodev", OBJECT(>ac97),
"audiodev");
}
static const TypeInfo via_isa_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..df2be188257 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -41,6 +41,7 @@
#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "qemu/error-report.h"
+#include "audio/audio.h"
#define ENVP_PADDR 0x2000
#define ENVP_VADDR cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
@@ -295,9 +296,13 @@ static void mips_fuloong2e_init(MachineState
*machine)
pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
/* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
-
PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
- TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(pci_dev), "audiodev",
machine->audiodev);
+}
+pci_realize_and_unref(pci_dev, pci_bus, _abort);
+
object_property_add_alias(OBJECT(machine), "rtc-time",
object_resolve_path_component(OBJECT(pci_dev),
"rtc"),
@@ -337,6 +342,8 @@ static void mips_fuloong2e_machine_init(MachineClass
*mc)
mc->default_ram_size = 256 * MiB;
mc->default_ram_id = "fuloong2e.ram";
mc->minimum_page_bits = 14;
+
+machine_add_audiodev_property(mc);
}
DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..61c302895c9 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -37,6 +37,7 @@
#include "qemu/datadir.h"
#include "sysemu/device_tree.h"
#include "hw/ppc/vof.h"
+#include "audio/audio.h"
#include
@@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
/* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12,
0),
- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0),
TYPE_VT8231_ISA));
+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(via), "audiodev",
machine->audiodev);
+}
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, _abort);
for (i = 0; i < PCI_NUM_PINS; i++) {
pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
}
@@ -564,6 +568,8 @@ static void pegasos2_machine_class_init(ObjectClass
*oc, void *data)
vhc->encode_hpt_for_kvm_pr = vhyp_encode_hpt_for_kvm_pr;
vmc->setprop = pegasos2_setprop;
+
+
Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback
Il sab 23 set 2023, 14:23 BALATON Zoltan ha scritto:
> On Sat, 23 Sep 2023, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini
> > ---
> > hw/isa/vt82c686.c | 2 ++
> > hw/mips/fuloong2e.c | 13 ++---
> > hw/ppc/pegasos2.c | 10 --
> > 3 files changed, 20 insertions(+), 5 deletions(-)
>
> This looks better but I still wonder if this machine audiodev propery is
> needed at all. If there's one -audiodev option specified it's already
> picked up by default devices and if there are more one could use -global
> to set it. Why isn't that enough?
>
Mostly because it's less predictable. Ideally all the state of the emulator
would be visible and settable via explicit links.
You were absolutely right that we still need to keep some level of magic in
softmmu/vl.c to make QEMU easier to use for the command line. However, a
while ago there was an idea of making an alternative binary that is
entirely configurable via QMP, and past work in that direction resulted in
*lots* of cleanups that actually made softmmu/vl.c understandable. While I
am not sure this QMP binary is ever going to happen, I would like to make
it possible to avoid the magic.
If you still want a machine audiodev propery then could the device handle
> it without needing changes to the machine? Like in via_isa_realize() add
>
> if (current_machine->audiodev) {
> qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
> }
>
> before qdev_realize(DEVICE(>ac97) then no need to change the device
> creation in board code.
>
No, current_machine should not be used at all outside board code.
Paolo
> Regards,
> BALATON Zoltan
>
> > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> > index 57bdfb4e78c..3ec8e43708a 100644
> > --- a/hw/isa/vt82c686.c
> > +++ b/hw/isa/vt82c686.c
> > @@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
> > object_initialize_child(obj, "uhci2", >uhci[1],
> TYPE_VT82C686B_USB_UHCI);
> > object_initialize_child(obj, "ac97", >ac97, TYPE_VIA_AC97);
> > object_initialize_child(obj, "mc97", >mc97, TYPE_VIA_MC97);
> > +
> > +object_property_add_alias(obj, "audiodev", OBJECT(>ac97),
> "audiodev");
> > }
> >
> > static const TypeInfo via_isa_info = {
> > diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> > index c827f615f3b..df2be188257 100644
> > --- a/hw/mips/fuloong2e.c
> > +++ b/hw/mips/fuloong2e.c
> > @@ -41,6 +41,7 @@
> > #include "sysemu/reset.h"
> > #include "sysemu/sysemu.h"
> > #include "qemu/error-report.h"
> > +#include "audio/audio.h"
> >
> > #define ENVP_PADDR 0x2000
> > #define ENVP_VADDR cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
> > @@ -295,9 +296,13 @@ static void mips_fuloong2e_init(MachineState
> *machine)
> > pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
> >
> > /* South bridge -> IP5 */
> > -pci_dev = pci_create_simple_multifunction(pci_bus,
> > -
> PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
> > - TYPE_VT82C686B_ISA);
> > +pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
> > +TYPE_VT82C686B_ISA);
> > +if (machine->audiodev) {
> > +qdev_prop_set_string(DEVICE(pci_dev), "audiodev",
> machine->audiodev);
> > +}
> > +pci_realize_and_unref(pci_dev, pci_bus, _abort);
> > +
> > object_property_add_alias(OBJECT(machine), "rtc-time",
> >
> object_resolve_path_component(OBJECT(pci_dev),
> > "rtc"),
> > @@ -337,6 +342,8 @@ static void mips_fuloong2e_machine_init(MachineClass
> *mc)
> > mc->default_ram_size = 256 * MiB;
> > mc->default_ram_id = "fuloong2e.ram";
> > mc->minimum_page_bits = 14;
> > +
> > +machine_add_audiodev_property(mc);
> > }
> >
> > DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
> > diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> > index bd397cf2b5c..61c302895c9 100644
> > --- a/hw/ppc/pegasos2.c
> > +++ b/hw/ppc/pegasos2.c
> > @@ -37,6 +37,7 @@
> > #include "qemu/datadir.h"
> > #include "sysemu/device_tree.h"
> > #include "hw/ppc/vof.h"
> > +#include "audio/audio.h"
> >
> > #include
> >
> > @@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
> > pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
> >
> > /* VIA VT8231 South Bridge (multifunction PCI device) */
> > -via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12,
> 0),
> > - TYPE_VT8231_ISA));
> > +via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0),
> TYPE_VT8231_ISA));
> > +if (machine->audiodev) {
> > +qdev_prop_set_string(DEVICE(via), "audiodev",
> machine->audiodev);
> > +}
> > +pci_realize_and_unref(PCI_DEVICE(via), pci_bus, _abort);
> > for (i = 0; i < PCI_NUM_PINS; i++) {
> > pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
> > }
> > @@ -564,6 +568,8 @@ static void
Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback
On Sat, 23 Sep 2023, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini
---
hw/isa/vt82c686.c | 2 ++
hw/mips/fuloong2e.c | 13 ++---
hw/ppc/pegasos2.c | 10 --
3 files changed, 20 insertions(+), 5 deletions(-)
This looks better but I still wonder if this machine audiodev propery is
needed at all. If there's one -audiodev option specified it's already
picked up by default devices and if there are more one could use -global
to set it. Why isn't that enough?
If you still want a machine audiodev propery then could the device handle
it without needing changes to the machine? Like in via_isa_realize() add
if (current_machine->audiodev) {
qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
}
before qdev_realize(DEVICE(>ac97) then no need to change the device
creation in board code.
Regards,
BALATON Zoltan
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78c..3ec8e43708a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
object_initialize_child(obj, "uhci2", >uhci[1], TYPE_VT82C686B_USB_UHCI);
object_initialize_child(obj, "ac97", >ac97, TYPE_VIA_AC97);
object_initialize_child(obj, "mc97", >mc97, TYPE_VIA_MC97);
+
+object_property_add_alias(obj, "audiodev", OBJECT(>ac97), "audiodev");
}
static const TypeInfo via_isa_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..df2be188257 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -41,6 +41,7 @@
#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "qemu/error-report.h"
+#include "audio/audio.h"
#define ENVP_PADDR 0x2000
#define ENVP_VADDR cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
@@ -295,9 +296,13 @@ static void mips_fuloong2e_init(MachineState *machine)
pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
/* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
- PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
- TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
+}
+pci_realize_and_unref(pci_dev, pci_bus, _abort);
+
object_property_add_alias(OBJECT(machine), "rtc-time",
object_resolve_path_component(OBJECT(pci_dev),
"rtc"),
@@ -337,6 +342,8 @@ static void mips_fuloong2e_machine_init(MachineClass *mc)
mc->default_ram_size = 256 * MiB;
mc->default_ram_id = "fuloong2e.ram";
mc->minimum_page_bits = 14;
+
+machine_add_audiodev_property(mc);
}
DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..61c302895c9 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -37,6 +37,7 @@
#include "qemu/datadir.h"
#include "sysemu/device_tree.h"
#include "hw/ppc/vof.h"
+#include "audio/audio.h"
#include
@@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
/* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA));
+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(via), "audiodev", machine->audiodev);
+}
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, _abort);
for (i = 0; i < PCI_NUM_PINS; i++) {
pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
}
@@ -564,6 +568,8 @@ static void pegasos2_machine_class_init(ObjectClass *oc,
void *data)
vhc->encode_hpt_for_kvm_pr = vhyp_encode_hpt_for_kvm_pr;
vmc->setprop = pegasos2_setprop;
+
+machine_add_audiodev_property(mc);
}
static const TypeInfo pegasos2_machine_info = {
