On Fri, Jul 28, 2017 at 02:28:57PM +0200, Igor Mammedov wrote:
> On Wed, 26 Jul 2017 23:18:01 +0300
> "Michael S. Tsirkin" <m...@redhat.com> wrote:
>
> > On Wed, Jul 26, 2017 at 04:09:37PM +0200, Igor Mammedov wrote:
> > > On Tue, 25 Jul 2017 16:36:06 +0300
> > > "Michael S. Tsirkin" <m...@redhat.com> wrote:
> > >
> > > > On Mon, Jul 24, 2017 at 03:50:20PM +0200, Igor Mammedov wrote:
> > > > > w2k used to boot on QEMU until revision of FADT has
> > > > > been bumped to rev3
> > > > > (commit 77af8a2b hw/i386: Use Rev3 FADT (ACPI 2.0) instead of Rev1 to
> > > > > improve guest OS support.)
> > > > >
> > > > > Keep PC machine at rev1 to remain compatible and Q35
> > > > > at rev3 where w2k isn't supported anyway so OSX could
> > > > > run as well.
> > > > >
> > > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > > > > ---
> > > > > v2:
> > > > > - make pc rev1 and q35 rev3.
> > > > > "Michael S. Tsirkin" <m...@redhat.com>
> > > > >
> > > > > PS:
> > > > > Only compile tested.
> > > > >
> > > > >
> > > > > CC: Programmingkid <programmingk...@gmail.com>
> > > > > CC: Phil Dennis-Jordan <li...@philjordan.eu>
> > > > > CC: "Michael S. Tsirkin" <m...@redhat.com>
> > > > >
> > > > > ---
> > > > > hw/i386/acpi-build.c | 22 ++++++++++++++++++----
> > > > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > > index 6b7bade..b9c245c 100644
> > > > > --- a/hw/i386/acpi-build.c
> > > > > +++ b/hw/i386/acpi-build.c
> > > > > @@ -90,6 +90,7 @@ typedef struct AcpiMcfgInfo {
> > > > > } AcpiMcfgInfo;
> > > > >
> > > > > typedef struct AcpiPmInfo {
> > > > > + bool force_rev1_fadt;
> > > > > bool s3_disabled;
> > > > > bool s4_disabled;
> > > > > bool pcihp_bridge_en;
> > > > > @@ -129,10 +130,13 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> > > > > Object *obj = NULL;
> > > > > QObject *o;
> > > > >
> > > > > + pm->force_rev1_fadt = false;
> > > > > pm->cpu_hp_io_base = 0;
> > > > > pm->pcihp_io_base = 0;
> > > > > pm->pcihp_io_len = 0;
> > > > > if (piix) {
> > > > > + /* w2k requires FADT(rev1) or it won't boot, keep PC
> > > > > compatible */
> > > > > + pm->force_rev1_fadt = true;
> > > > > obj = piix;
> > > > > pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> > > > > pm->pcihp_io_base =
> > > > > @@ -304,6 +308,9 @@ static void fadt_setup(AcpiFadtDescriptorRev3
> > > > > *fadt, AcpiPmInfo *pm)
> > > > > fadt->flags |= cpu_to_le32(1 <<
> > > > > ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL);
> > > > > }
> > > > > fadt->century = RTC_CENTURY;
> > > > > + if (pm->force_rev1_fadt) {
> > > > > + return;
> > > > > + }
> > > > >
> > > > > fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP);
> > > > > fadt->reset_value = 0xf;
> > > > > @@ -342,6 +349,8 @@ build_fadt(GArray *table_data, BIOSLinker
> > > > > *linker, AcpiPmInfo *pm,
> > > > > unsigned fw_ctrl_offset = (char *)&fadt->firmware_ctrl -
> > > > > table_data->data;
> > > > > unsigned dsdt_entry_offset = (char *)&fadt->dsdt -
> > > > > table_data->data;
> > > > > unsigned xdsdt_entry_offset = (char *)&fadt->x_dsdt -
> > > > > table_data->data;
> > > > > + int fadt_size = sizeof(*fadt);
> > > > > + int rev = 3;
> > > > >
> > > > > /* FACS address to be filled by Guest linker */
> > > > > bios_linker_loader_add_pointer(linker,
> > > > > @@ -353,12 +362,17 @@ build_fadt(GArray *table_data, BIOSLinker
> > > > > *linker, AcpiPmInfo *pm,
> > > > > bios_linker_loader_add_pointer(linker,
> > > > > ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt),
> > > > > ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> > > > > - bios_linker_loader_add_pointer(linker,
> > > > > - ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset,
> > > > > sizeof(fadt->x_dsdt),
> > > > > - ACPI_BUILD_TABLE_FILE, dsdt_tbl_offset);
> > > > > + if (pm->force_rev1_fadt) {
> > > > > + rev = 1;
> > > > > + fadt_size = offsetof(typeof(*fadt), reset_register);
> > > >
> > > > I don't really like it that we are using structs to format fields - I
> > > > would prefer build_append_int_noprefix based APIs - but since we do,
> > > > let's add a proper structure for FADT rev 1 instead of this hack.
>
> Michael,
>
> I've tried to switch FADT to build_append_int_noprefix APIs,
> but it turned out into quite a bit of re-factoring including
> clean-ups and adding new APIs to pack all the stuff in FADT.
>
> It's too much for hard freeze, lets not rush it and go with
> this patch for 2.10 to put out fire.
>
> When 2.11 merge window is open I'll rebase/post FADTv1/3
> conversion to build_append_int_noprefix API.
Makes sense.
