| Message ID | 20221028103419.93398-4-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/i386: Cleanup AML generation for north and south bridges | expand |
On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > > The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > AML generation has been moved into the south bridges and since the > machines define themselves primarily through their north bridges, let's > switch to resolving the north bridges for AML generation instead. This > also allows for easier experimentation with different south bridges in > the "pc" machine, e.g. with PIIX4 and VT82xx. Unfortunately this patch does not apply on the latest master. Also the code seems to be off. Can you rebase and rework the patch? > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/i386/acpi-build.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 73d8a59737..d9eaa5fc4d 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -60,6 +60,7 @@ > #include "hw/i386/fw_cfg.h" > #include "hw/i386/ich9.h" > #include "hw/pci/pci_bus.h" > +#include "hw/pci-host/i440fx.h" > #include "hw/pci-host/q35.h" > #include "hw/i386/x86-iommu.h" > > @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiPmInfo *pm, AcpiMiscInfo *misc, > Range *pci_hole, Range *pci_hole64, MachineState *machine) > { > - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > CrsRangeEntry *entry; > Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > CrsRangeSet crs_range_set; > @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > .oem_table_id = x86ms->oem_table_id }; > > - assert(!!piix != !!lpc); > + assert(!!i440fx != !!q35); > > acpi_table_begin(&table, table_data); > dsdt = init_aml_allocator(); > > build_dbg_aml(dsdt); > - if (piix) { > + if (i440fx) { > sb_scope = aml_scope("_SB"); > dev = aml_device("PCI0"); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > } > build_piix4_pci0_int(dsdt); > - } else if (lpc) { > + } else if (q35) { > sb_scope = aml_scope("_SB"); > dev = aml_device("PCI0"); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > -- > 2.38.1 >
Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: >> >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more >> AML generation has been moved into the south bridges and since the >> machines define themselves primarily through their north bridges, let's >> switch to resolving the north bridges for AML generation instead. This >> also allows for easier experimentation with different south bridges in >> the "pc" machine, e.g. with PIIX4 and VT82xx. > >Unfortunately this patch does not apply on the latest master. Also the >code seems to be off. Can you rebase and rework the patch? I've rebased onto Igor's series to avoid merge conflicts, that's why it doesn't apply onto master. It applies fine there [1]. The first two patches of this series apply fine on both branches, so could possibly be pulled already if Igor's series doesn't make it for 7.2. Best regards, Bernhard [1] https://github.com/patchew-project/qemu/commits/patchew/20221028103419.93398-1-shentey%40gmail.com > >> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> >> --- >> hw/i386/acpi-build.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index 73d8a59737..d9eaa5fc4d 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -60,6 +60,7 @@ >> #include "hw/i386/fw_cfg.h" >> #include "hw/i386/ich9.h" >> #include "hw/pci/pci_bus.h" >> +#include "hw/pci-host/i440fx.h" >> #include "hw/pci-host/q35.h" >> #include "hw/i386/x86-iommu.h" >> >> @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> AcpiPmInfo *pm, AcpiMiscInfo *misc, >> Range *pci_hole, Range *pci_hole64, MachineState *machine) >> { >> - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); >> - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); >> + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); >> + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); >> CrsRangeEntry *entry; >> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; >> CrsRangeSet crs_range_set; >> @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, >> .oem_table_id = x86ms->oem_table_id }; >> >> - assert(!!piix != !!lpc); >> + assert(!!i440fx != !!q35); >> >> acpi_table_begin(&table, table_data); >> dsdt = init_aml_allocator(); >> >> build_dbg_aml(dsdt); >> - if (piix) { >> + if (i440fx) { >> sb_scope = aml_scope("_SB"); >> dev = aml_device("PCI0"); >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); >> @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); >> } >> build_piix4_pci0_int(dsdt); >> - } else if (lpc) { >> + } else if (q35) { >> sb_scope = aml_scope("_SB"); >> dev = aml_device("PCI0"); >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); >> -- >> 2.38.1 >>
On Fri, Oct 28, 2022 at 9:45 PM B <shentey@gmail.com> wrote: > > > > Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: > >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > >> > >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > >> AML generation has been moved into the south bridges and since the > >> machines define themselves primarily through their north bridges, let's > >> switch to resolving the north bridges for AML generation instead. This > >> also allows for easier experimentation with different south bridges in > >> the "pc" machine, e.g. with PIIX4 and VT82xx. > > > >Unfortunately this patch does not apply on the latest master. Also the > >code seems to be off. Can you rebase and rework the patch? > > I've rebased onto Igor's series to avoid merge conflicts, Ok I will let Igor deal with this then since I have not followed his patchset. > that's why it doesn't apply onto master. It applies fine there [1]. > > The first two patches of this series apply fine on both branches, so could possibly be pulled already if Igor's series doesn't make it for 7.2. > > Best regards, > Bernhard > > [1] https://github.com/patchew-project/qemu/commits/patchew/20221028103419.93398-1-shentey%40gmail.com > > > >> > >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> > >> --- > >> hw/i386/acpi-build.c | 11 ++++++----- > >> 1 file changed, 6 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >> index 73d8a59737..d9eaa5fc4d 100644 > >> --- a/hw/i386/acpi-build.c > >> +++ b/hw/i386/acpi-build.c > >> @@ -60,6 +60,7 @@ > >> #include "hw/i386/fw_cfg.h" > >> #include "hw/i386/ich9.h" > >> #include "hw/pci/pci_bus.h" > >> +#include "hw/pci-host/i440fx.h" > >> #include "hw/pci-host/q35.h" > >> #include "hw/i386/x86-iommu.h" > >> > >> @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> AcpiPmInfo *pm, AcpiMiscInfo *misc, > >> Range *pci_hole, Range *pci_hole64, MachineState *machine) > >> { > >> - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > >> - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > >> + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > >> + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > >> CrsRangeEntry *entry; > >> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > >> CrsRangeSet crs_range_set; > >> @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > >> .oem_table_id = x86ms->oem_table_id }; > >> > >> - assert(!!piix != !!lpc); > >> + assert(!!i440fx != !!q35); > >> > >> acpi_table_begin(&table, table_data); > >> dsdt = init_aml_allocator(); > >> > >> build_dbg_aml(dsdt); > >> - if (piix) { > >> + if (i440fx) { > >> sb_scope = aml_scope("_SB"); > >> dev = aml_device("PCI0"); > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > >> @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > >> } > >> build_piix4_pci0_int(dsdt); > >> - } else if (lpc) { > >> + } else if (q35) { > >> sb_scope = aml_scope("_SB"); > >> dev = aml_device("PCI0"); > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > >> -- > >> 2.38.1 > >>
On Fri, Oct 28, 2022 at 10:18:43PM +0530, Ani Sinha wrote: > On Fri, Oct 28, 2022 at 9:45 PM B <shentey@gmail.com> wrote: > > > > > > > > Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: > > >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > > >> > > >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > > >> AML generation has been moved into the south bridges and since the > > >> machines define themselves primarily through their north bridges, let's > > >> switch to resolving the north bridges for AML generation instead. This > > >> also allows for easier experimentation with different south bridges in > > >> the "pc" machine, e.g. with PIIX4 and VT82xx. > > > > > >Unfortunately this patch does not apply on the latest master. Also the > > >code seems to be off. Can you rebase and rework the patch? > > > > I've rebased onto Igor's series to avoid merge conflicts, > > Ok I will let Igor deal with this then since I have not followed his patchset. should you want to review this, it's all in my tree right now. > > that's why it doesn't apply onto master. It applies fine there [1]. > > > > The first two patches of this series apply fine on both branches, so could possibly be pulled already if Igor's series doesn't make it for 7.2. > > > > Best regards, > > Bernhard > > > > [1] https://github.com/patchew-project/qemu/commits/patchew/20221028103419.93398-1-shentey%40gmail.com > > > > > >> > > >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> > > >> --- > > >> hw/i386/acpi-build.c | 11 ++++++----- > > >> 1 file changed, 6 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > >> index 73d8a59737..d9eaa5fc4d 100644 > > >> --- a/hw/i386/acpi-build.c > > >> +++ b/hw/i386/acpi-build.c > > >> @@ -60,6 +60,7 @@ > > >> #include "hw/i386/fw_cfg.h" > > >> #include "hw/i386/ich9.h" > > >> #include "hw/pci/pci_bus.h" > > >> +#include "hw/pci-host/i440fx.h" > > >> #include "hw/pci-host/q35.h" > > >> #include "hw/i386/x86-iommu.h" > > >> > > >> @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > >> AcpiPmInfo *pm, AcpiMiscInfo *misc, > > >> Range *pci_hole, Range *pci_hole64, MachineState *machine) > > >> { > > >> - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > > >> - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > > >> + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > > >> + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > > >> CrsRangeEntry *entry; > > >> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > > >> CrsRangeSet crs_range_set; > > >> @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > >> AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > > >> .oem_table_id = x86ms->oem_table_id }; > > >> > > >> - assert(!!piix != !!lpc); > > >> + assert(!!i440fx != !!q35); > > >> > > >> acpi_table_begin(&table, table_data); > > >> dsdt = init_aml_allocator(); > > >> > > >> build_dbg_aml(dsdt); > > >> - if (piix) { > > >> + if (i440fx) { > > >> sb_scope = aml_scope("_SB"); > > >> dev = aml_device("PCI0"); > > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > >> @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > >> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > > >> } > > >> build_piix4_pci0_int(dsdt); > > >> - } else if (lpc) { > > >> + } else if (q35) { > > >> sb_scope = aml_scope("_SB"); > > >> dev = aml_device("PCI0"); > > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > >> -- > > >> 2.38.1 > > >>
On Sat, 29 Oct 2022, Michael S. Tsirkin wrote: > On Fri, Oct 28, 2022 at 10:18:43PM +0530, Ani Sinha wrote: > > On Fri, Oct 28, 2022 at 9:45 PM B <shentey@gmail.com> wrote: > > > > > > > > > > > > Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: > > > >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > > > >> > > > >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > > > >> AML generation has been moved into the south bridges and since the > > > >> machines define themselves primarily through their north bridges, let's > > > >> switch to resolving the north bridges for AML generation instead. This > > > >> also allows for easier experimentation with different south bridges in > > > >> the "pc" machine, e.g. with PIIX4 and VT82xx. > > > > > > > >Unfortunately this patch does not apply on the latest master. Also the > > > >code seems to be off. Can you rebase and rework the patch? > > > > > > I've rebased onto Igor's series to avoid merge conflicts, > > > > Ok I will let Igor deal with this then since I have not followed his patchset. > > should you want to review this, it's all in my tree right now. I tried your "next" branch from git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git and it does not apply there either. On another note, seems you have picked up all the bits patches except the one that adds the documentation. I wonder why. > > > > that's why it doesn't apply onto master. It applies fine there [1]. > > > > > > The first two patches of this series apply fine on both branches, so could possibly be pulled already if Igor's series doesn't make it for 7.2. > > > > > > Best regards, > > > Bernhard > > > > > > [1] https://github.com/patchew-project/qemu/commits/patchew/20221028103419.93398-1-shentey%40gmail.com > > > > > > > >> > > > >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> > > > >> --- > > > >> hw/i386/acpi-build.c | 11 ++++++----- > > > >> 1 file changed, 6 insertions(+), 5 deletions(-) > > > >> > > > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > >> index 73d8a59737..d9eaa5fc4d 100644 > > > >> --- a/hw/i386/acpi-build.c > > > >> +++ b/hw/i386/acpi-build.c > > > >> @@ -60,6 +60,7 @@ > > > >> #include "hw/i386/fw_cfg.h" > > > >> #include "hw/i386/ich9.h" > > > >> #include "hw/pci/pci_bus.h" > > > >> +#include "hw/pci-host/i440fx.h" > > > >> #include "hw/pci-host/q35.h" > > > >> #include "hw/i386/x86-iommu.h" > > > >> > > > >> @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > >> AcpiPmInfo *pm, AcpiMiscInfo *misc, > > > >> Range *pci_hole, Range *pci_hole64, MachineState *machine) > > > >> { > > > >> - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > > > >> - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > > > >> + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > > > >> + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > > > >> CrsRangeEntry *entry; > > > >> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > > > >> CrsRangeSet crs_range_set; > > > >> @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > >> AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > > > >> .oem_table_id = x86ms->oem_table_id }; > > > >> > > > >> - assert(!!piix != !!lpc); > > > >> + assert(!!i440fx != !!q35); > > > >> > > > >> acpi_table_begin(&table, table_data); > > > >> dsdt = init_aml_allocator(); > > > >> > > > >> build_dbg_aml(dsdt); > > > >> - if (piix) { > > > >> + if (i440fx) { > > > >> sb_scope = aml_scope("_SB"); > > > >> dev = aml_device("PCI0"); > > > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > > >> @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > >> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > > > >> } > > > >> build_piix4_pci0_int(dsdt); > > > >> - } else if (lpc) { > > > >> + } else if (q35) { > > > >> sb_scope = aml_scope("_SB"); > > > >> dev = aml_device("PCI0"); > > > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > > >> -- > > > >> 2.38.1 > > > >> > >
On Sun, Oct 30, 2022 at 09:15:44PM +0530, Ani Sinha wrote: > > > On Sat, 29 Oct 2022, Michael S. Tsirkin wrote: > > > On Fri, Oct 28, 2022 at 10:18:43PM +0530, Ani Sinha wrote: > > > On Fri, Oct 28, 2022 at 9:45 PM B <shentey@gmail.com> wrote: > > > > > > > > > > > > > > > > Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: > > > > >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > > > > >> > > > > >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > > > > >> AML generation has been moved into the south bridges and since the > > > > >> machines define themselves primarily through their north bridges, let's > > > > >> switch to resolving the north bridges for AML generation instead. This > > > > >> also allows for easier experimentation with different south bridges in > > > > >> the "pc" machine, e.g. with PIIX4 and VT82xx. > > > > > > > > > >Unfortunately this patch does not apply on the latest master. Also the > > > > >code seems to be off. Can you rebase and rework the patch? > > > > > > > > I've rebased onto Igor's series to avoid merge conflicts, > > > > > > Ok I will let Igor deal with this then since I have not followed his patchset. > > > > should you want to review this, it's all in my tree right now. > > I tried your "next" branch from > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git > > and it does not apply there either. commit 87bbbe87c259414864a02e8385a0c8becd269ea5 It is already applied there. > On another note, seems you have picked up all the bits patches except > the one that adds the documentation. I wonder why. Not sure, I tried to pick them all up. Will check. > > > > > > that's why it doesn't apply onto master. It applies fine there [1]. > > > > > > > > The first two patches of this series apply fine on both branches, so could possibly be pulled already if Igor's series doesn't make it for 7.2. > > > > > > > > Best regards, > > > > Bernhard > > > > > > > > [1] https://github.com/patchew-project/qemu/commits/patchew/20221028103419.93398-1-shentey%40gmail.com > > > > > > > > > >> > > > > >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> > > > > >> --- > > > > >> hw/i386/acpi-build.c | 11 ++++++----- > > > > >> 1 file changed, 6 insertions(+), 5 deletions(-) > > > > >> > > > > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > >> index 73d8a59737..d9eaa5fc4d 100644 > > > > >> --- a/hw/i386/acpi-build.c > > > > >> +++ b/hw/i386/acpi-build.c > > > > >> @@ -60,6 +60,7 @@ > > > > >> #include "hw/i386/fw_cfg.h" > > > > >> #include "hw/i386/ich9.h" > > > > >> #include "hw/pci/pci_bus.h" > > > > >> +#include "hw/pci-host/i440fx.h" > > > > >> #include "hw/pci-host/q35.h" > > > > >> #include "hw/i386/x86-iommu.h" > > > > >> > > > > >> @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > >> AcpiPmInfo *pm, AcpiMiscInfo *misc, > > > > >> Range *pci_hole, Range *pci_hole64, MachineState *machine) > > > > >> { > > > > >> - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > > > > >> - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > > > > >> + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > > > > >> + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > > > > >> CrsRangeEntry *entry; > > > > >> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > > > > >> CrsRangeSet crs_range_set; > > > > >> @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > >> AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > > > > >> .oem_table_id = x86ms->oem_table_id }; > > > > >> > > > > >> - assert(!!piix != !!lpc); > > > > >> + assert(!!i440fx != !!q35); > > > > >> > > > > >> acpi_table_begin(&table, table_data); > > > > >> dsdt = init_aml_allocator(); > > > > >> > > > > >> build_dbg_aml(dsdt); > > > > >> - if (piix) { > > > > >> + if (i440fx) { > > > > >> sb_scope = aml_scope("_SB"); > > > > >> dev = aml_device("PCI0"); > > > > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > > > > >> @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > > > > >> build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > > > > >> } > > > > >> build_piix4_pci0_int(dsdt); > > > > >> - } else if (lpc) { > > > > >> + } else if (q35) { > > > > >> sb_scope = aml_scope("_SB"); > > > > >> dev = aml_device("PCI0"); > > > > >> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > > > > >> -- > > > > >> 2.38.1 > > > > >> > > > >
On Sun, 30 Oct 2022, Michael S. Tsirkin wrote: > On Sun, Oct 30, 2022 at 09:15:44PM +0530, Ani Sinha wrote: > > > > > > On Sat, 29 Oct 2022, Michael S. Tsirkin wrote: > > > > > On Fri, Oct 28, 2022 at 10:18:43PM +0530, Ani Sinha wrote: > > > > On Fri, Oct 28, 2022 at 9:45 PM B <shentey@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: > > > > > >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > > > > > >> > > > > > >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > > > > > >> AML generation has been moved into the south bridges and since the > > > > > >> machines define themselves primarily through their north bridges, let's > > > > > >> switch to resolving the north bridges for AML generation instead. This > > > > > >> also allows for easier experimentation with different south bridges in > > > > > >> the "pc" machine, e.g. with PIIX4 and VT82xx. > > > > > > > > > > > >Unfortunately this patch does not apply on the latest master. Also the > > > > > >code seems to be off. Can you rebase and rework the patch? > > > > > > > > > > I've rebased onto Igor's series to avoid merge conflicts, > > > > > > > > Ok I will let Igor deal with this then since I have not followed his patchset. > > > > > > should you want to review this, it's all in my tree right now. > > > > I tried your "next" branch from > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git > > > > and it does not apply there either. > > > commit 87bbbe87c259414864a02e8385a0c8becd269ea5 > It is already applied there. Hmm, I am not seeing it : ani@ani-ubuntu:~/workspace/qemu-mst$ git show 87bbbe87c259414864a02e8385a0c8becd269ea5 fatal: bad object 87bbbe87c259414864a02e8385a0c8becd269ea5 ani@ani-ubuntu:~/workspace/qemu-mst$ git branch -vv master 7457fe9541 [origin/master] Update version for v1.7.0-rc2 release * next e336a0d550 [origin/next] ack! hw/ide/piix: Ignore writes of hardwired PCI command register bits
On Sun, Oct 30, 2022 at 09:48:08PM +0530, Ani Sinha wrote: > > > On Sun, 30 Oct 2022, Michael S. Tsirkin wrote: > > > On Sun, Oct 30, 2022 at 09:15:44PM +0530, Ani Sinha wrote: > > > > > > > > > On Sat, 29 Oct 2022, Michael S. Tsirkin wrote: > > > > > > > On Fri, Oct 28, 2022 at 10:18:43PM +0530, Ani Sinha wrote: > > > > > On Fri, Oct 28, 2022 at 9:45 PM B <shentey@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Am 28. Oktober 2022 10:58:07 UTC schrieb Ani Sinha <ani@anisinha.ca>: > > > > > > >On Fri, Oct 28, 2022 at 4:05 PM Bernhard Beschow <shentey@gmail.com> wrote: > > > > > > >> > > > > > > >> The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > > > > > > >> AML generation has been moved into the south bridges and since the > > > > > > >> machines define themselves primarily through their north bridges, let's > > > > > > >> switch to resolving the north bridges for AML generation instead. This > > > > > > >> also allows for easier experimentation with different south bridges in > > > > > > >> the "pc" machine, e.g. with PIIX4 and VT82xx. > > > > > > > > > > > > > >Unfortunately this patch does not apply on the latest master. Also the > > > > > > >code seems to be off. Can you rebase and rework the patch? > > > > > > > > > > > > I've rebased onto Igor's series to avoid merge conflicts, > > > > > > > > > > Ok I will let Igor deal with this then since I have not followed his patchset. > > > > > > > > should you want to review this, it's all in my tree right now. > > > > > > I tried your "next" branch from > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git > > > > > > and it does not apply there either. > > > > > > commit 87bbbe87c259414864a02e8385a0c8becd269ea5 > > It is already applied there. > > Hmm, I am not seeing it : > > ani@ani-ubuntu:~/workspace/qemu-mst$ git show > 87bbbe87c259414864a02e8385a0c8becd269ea5 > fatal: bad object 87bbbe87c259414864a02e8385a0c8becd269ea5 > ani@ani-ubuntu:~/workspace/qemu-mst$ git branch -vv > master 7457fe9541 [origin/master] Update version for v1.7.0-rc2 release > * next e336a0d550 [origin/next] ack! hw/ide/piix: Ignore writes of hardwired PCI command register bits oh right. pushed now.
On Fri, 28 Oct 2022, Bernhard Beschow wrote: > The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > AML generation has been moved into the south bridges and since the > machines define themselves primarily through their north bridges, let's > switch to resolving the north bridges for AML generation instead. This > also allows for easier experimentation with different south bridges in > the "pc" machine, e.g. with PIIX4 and VT82xx. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> Reviewed-by: Ani Sinha <ani@anisinha.ca> > --- > hw/i386/acpi-build.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 73d8a59737..d9eaa5fc4d 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -60,6 +60,7 @@ > #include "hw/i386/fw_cfg.h" > #include "hw/i386/ich9.h" > #include "hw/pci/pci_bus.h" > +#include "hw/pci-host/i440fx.h" > #include "hw/pci-host/q35.h" > #include "hw/i386/x86-iommu.h" > > @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiPmInfo *pm, AcpiMiscInfo *misc, > Range *pci_hole, Range *pci_hole64, MachineState *machine) > { > - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > CrsRangeEntry *entry; > Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > CrsRangeSet crs_range_set; > @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > .oem_table_id = x86ms->oem_table_id }; > > - assert(!!piix != !!lpc); > + assert(!!i440fx != !!q35); > > acpi_table_begin(&table, table_data); > dsdt = init_aml_allocator(); > > build_dbg_aml(dsdt); > - if (piix) { > + if (i440fx) { > sb_scope = aml_scope("_SB"); > dev = aml_device("PCI0"); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > } > build_piix4_pci0_int(dsdt); > - } else if (lpc) { > + } else if (q35) { > sb_scope = aml_scope("_SB"); > dev = aml_device("PCI0"); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); > -- > 2.38.1 > >
On Fri, 28 Oct 2022 12:34:19 +0200 Bernhard Beschow <shentey@gmail.com> wrote: > The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more > AML generation has been moved into the south bridges and since the > machines define themselves primarily through their north bridges, let's > switch to resolving the north bridges for AML generation instead. This > also allows for easier experimentation with different south bridges in > the "pc" machine, e.g. with PIIX4 and VT82xx. Patch looks fine to me in a sense that either would work. But the commit message lacks clear answer to 'why' and what issues it resolves or would resolve/make our easier life down to road. > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/i386/acpi-build.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 73d8a59737..d9eaa5fc4d 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -60,6 +60,7 @@ > #include "hw/i386/fw_cfg.h" > #include "hw/i386/ich9.h" > #include "hw/pci/pci_bus.h" > +#include "hw/pci-host/i440fx.h" > #include "hw/pci-host/q35.h" > #include "hw/i386/x86-iommu.h" > > @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiPmInfo *pm, AcpiMiscInfo *misc, > Range *pci_hole, Range *pci_hole64, MachineState *machine) > { > - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); > - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); > + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); > + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); > CrsRangeEntry *entry; > Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; > CrsRangeSet crs_range_set; > @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, > .oem_table_id = x86ms->oem_table_id }; > > - assert(!!piix != !!lpc); > + assert(!!i440fx != !!q35); > > acpi_table_begin(&table, table_data); > dsdt = init_aml_allocator(); > > build_dbg_aml(dsdt); > - if (piix) { > + if (i440fx) { > sb_scope = aml_scope("_SB"); > dev = aml_device("PCI0"); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); > @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); > } > build_piix4_pci0_int(dsdt); > - } else if (lpc) { > + } else if (q35) { > sb_scope = aml_scope("_SB"); > dev = aml_device("PCI0"); > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 73d8a59737..d9eaa5fc4d 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -60,6 +60,7 @@ #include "hw/i386/fw_cfg.h" #include "hw/i386/ich9.h" #include "hw/pci/pci_bus.h" +#include "hw/pci-host/i440fx.h" #include "hw/pci-host/q35.h" #include "hw/i386/x86-iommu.h" @@ -1322,8 +1323,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, AcpiPmInfo *pm, AcpiMiscInfo *misc, Range *pci_hole, Range *pci_hole64, MachineState *machine) { - Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); - Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); + Object *i440fx = object_resolve_type_unambiguous(TYPE_I440FX_PCI_HOST_BRIDGE); + Object *q35 = object_resolve_type_unambiguous(TYPE_Q35_HOST_DEVICE); CrsRangeEntry *entry; Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs; CrsRangeSet crs_range_set; @@ -1344,13 +1345,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, .oem_table_id = x86ms->oem_table_id }; - assert(!!piix != !!lpc); + assert(!!i440fx != !!q35); acpi_table_begin(&table, table_data); dsdt = init_aml_allocator(); build_dbg_aml(dsdt); - if (piix) { + if (i440fx) { sb_scope = aml_scope("_SB"); dev = aml_device("PCI0"); aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); @@ -1363,7 +1364,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); } build_piix4_pci0_int(dsdt); - } else if (lpc) { + } else if (q35) { sb_scope = aml_scope("_SB"); dev = aml_device("PCI0"); aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
The code currently assumes Q35 iff ICH9 and i440fx iff PIIX. Now that more AML generation has been moved into the south bridges and since the machines define themselves primarily through their north bridges, let's switch to resolving the north bridges for AML generation instead. This also allows for easier experimentation with different south bridges in the "pc" machine, e.g. with PIIX4 and VT82xx. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/i386/acpi-build.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)