Hi Igor, On 22/02/18 13:42, Igor Mammedov wrote: > build_append_foo() API doesn't need explicit endianness > conversions which eliminates a source of errors and > it makes build_fadt() look like declarative definition of > FADT table in ACPI spec, which makes it easy to review. > Also it allows easily extending FADT to support other > revisions which will be used by follow up patches > where build_fadt() will be reused for ARM target. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> Reviewed-by: Eric Auger <eric.au...@redhat.com>
Thanks Eric > --- > hw/i386/acpi-build.c | 147 > +++++++++++++++++++++++++++++---------------------- > 1 file changed, 85 insertions(+), 62 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 706ba35..544a4bc 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -299,87 +299,110 @@ build_facs(GArray *table_data, BIOSLinker *linker) > facs->length = cpu_to_le32(sizeof(*facs)); > } > > -/* Load chipset information in FADT */ > -static void fadt_setup(AcpiFadtDescriptorRev3 *fadt, AcpiFadtData f) > -{ > - fadt->model = f.int_model; > - fadt->reserved1 = 0; > - fadt->sci_int = cpu_to_le16(f.sci_int); > - fadt->smi_cmd = cpu_to_le32(f.smi_cmd); > - fadt->acpi_enable = f.acpi_enable_cmd; > - fadt->acpi_disable = f.acpi_disable_cmd; > - /* EVT, CNT, TMR offset matches hw/acpi/core.c */ > - fadt->pm1a_evt_blk = cpu_to_le32(f.pm1_evt.address); > - fadt->pm1a_cnt_blk = cpu_to_le32(f.pm1_cnt.address); > - fadt->pm_tmr_blk = cpu_to_le32(f.pm_tmr.address); > - fadt->gpe0_blk = cpu_to_le32(f.gpe0_blk.address); > - /* EVT, CNT, TMR length matches hw/acpi/core.c */ > - fadt->pm1_evt_len = f.pm1_evt.bit_width / 8; > - fadt->pm1_cnt_len = f.pm1_cnt.bit_width / 8; > - fadt->pm_tmr_len = f.pm_tmr.bit_width / 8; > - fadt->gpe0_blk_len = f.gpe0_blk.bit_width / 8; > - fadt->plvl2_lat = cpu_to_le16(f.c2_latency); > - fadt->plvl3_lat = cpu_to_le16(f.c3_latency); > - fadt->flags = cpu_to_le32(f.flags); > - fadt->century = f.rtc_century; > - if (f.rev == 1) { > - return; > - } > - > - fadt->reset_value = f.reset_val; > - fadt->reset_register = f.reset_reg; > - fadt->reset_register.address = cpu_to_le64(f.reset_reg.address); > - > - fadt->xpm1a_event_block = f.pm1_evt; > - fadt->xpm1a_event_block.address = cpu_to_le64(f.pm1_evt.address); > - > - fadt->xpm1a_control_block = f.pm1_cnt; > - fadt->xpm1a_control_block.address = cpu_to_le64(f.pm1_cnt.address); > - > - fadt->xpm_timer_block = f.pm_tmr; > - fadt->xpm_timer_block.address = cpu_to_le64(f.pm_tmr.address); > - > - fadt->xgpe0_block = f.gpe0_blk; > - fadt->xgpe0_block.address = cpu_to_le64(f.gpe0_blk.address); > -} > - > - > /* FADT */ > static void > -build_fadt(GArray *table_data, BIOSLinker *linker, AcpiFadtData *f, > +build_fadt(GArray *tbl, BIOSLinker *linker, AcpiFadtData *f, > const char *oem_id, const char *oem_table_id) > { > - AcpiFadtDescriptorRev3 *fadt = acpi_data_push(table_data, sizeof(*fadt)); > - 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 off; > + int fadt_start = tbl->len; > + > + acpi_data_push(tbl, sizeof(AcpiTableHeader)); > > - /* FACS address to be filled by Guest linker */ > + /* FACS address to be filled by Guest linker at runtime */ > + off = tbl->len; > + build_append_int_noprefix(tbl, 0, 4); /* FIRMWARE_CTRL */ > if (f->facs_tbl_offset) { > bios_linker_loader_add_pointer(linker, > - ACPI_BUILD_TABLE_FILE, fw_ctrl_offset, > sizeof(fadt->firmware_ctrl), > + ACPI_BUILD_TABLE_FILE, off, 4, > ACPI_BUILD_TABLE_FILE, *f->facs_tbl_offset); > } > > - /* DSDT address to be filled by Guest linker */ > - fadt_setup(fadt, *f); > + /* DSDT address to be filled by Guest linker at runtime */ > + off = tbl->len; > + build_append_int_noprefix(tbl, 0, 4); /* DSDT */ > if (f->dsdt_tbl_offset) { > bios_linker_loader_add_pointer(linker, > - ACPI_BUILD_TABLE_FILE, dsdt_entry_offset, sizeof(fadt->dsdt), > + ACPI_BUILD_TABLE_FILE, off, 4, > ACPI_BUILD_TABLE_FILE, *f->dsdt_tbl_offset); > } > + > + /* ACPI1.0: INT_MODEL, ACPI2.0+: Reserved */ > + build_append_int_noprefix(tbl, f->int_model /* Multiple APIC */, 1); > + /* Preferred_PM_Profile */ > + build_append_int_noprefix(tbl, 0 /* Unspecified */, 1); > + build_append_int_noprefix(tbl, f->sci_int, 2); /* SCI_INT */ > + build_append_int_noprefix(tbl, f->smi_cmd, 4); /* SMI_CMD */ > + build_append_int_noprefix(tbl, f->acpi_enable_cmd, 1); /* ACPI_ENABLE */ > + build_append_int_noprefix(tbl, f->acpi_disable_cmd, 1); /* ACPI_DISABLE > */ > + build_append_int_noprefix(tbl, 0 /* not supported */, 1); /* S4BIOS_REQ > */ > + /* ACPI1.0: Reserved, ACPI2.0+: PSTATE_CNT */ > + build_append_int_noprefix(tbl, 0, 1); > + build_append_int_noprefix(tbl, f->pm1_evt.address, 4); /* PM1a_EVT_BLK */ > + build_append_int_noprefix(tbl, 0, 4); /* PM1b_EVT_BLK */ > + build_append_int_noprefix(tbl, f->pm1_cnt.address, 4); /* PM1a_CNT_BLK */ > + build_append_int_noprefix(tbl, 0, 4); /* PM1b_CNT_BLK */ > + build_append_int_noprefix(tbl, 0, 4); /* PM2_CNT_BLK */ > + build_append_int_noprefix(tbl, f->pm_tmr.address, 4); /* PM_TMR_BLK */ > + build_append_int_noprefix(tbl, f->gpe0_blk.address, 4); /* GPE0_BLK */ > + build_append_int_noprefix(tbl, 0, 4); /* GPE1_BLK */ > + /* PM1_EVT_LEN */ > + build_append_int_noprefix(tbl, f->pm1_evt.bit_width / 8, 1); > + /* PM1_CNT_LEN */ > + build_append_int_noprefix(tbl, f->pm1_cnt.bit_width / 8, 1); > + build_append_int_noprefix(tbl, 0, 1); /* PM2_CNT_LEN */ > + build_append_int_noprefix(tbl, f->pm_tmr.bit_width / 8, 1); /* > PM_TMR_LEN */ > + /* GPE0_BLK_LEN */ > + build_append_int_noprefix(tbl, f->gpe0_blk.bit_width / 8, 1); > + build_append_int_noprefix(tbl, 0, 1); /* GPE1_BLK_LEN */ > + build_append_int_noprefix(tbl, 0, 1); /* GPE1_BASE */ > + build_append_int_noprefix(tbl, 0, 1); /* CST_CNT */ > + build_append_int_noprefix(tbl, f->c2_latency, 2); /* P_LVL2_LAT */ > + build_append_int_noprefix(tbl, f->c3_latency, 2); /* P_LVL3_LAT */ > + build_append_int_noprefix(tbl, 0, 2); /* FLUSH_SIZE */ > + build_append_int_noprefix(tbl, 0, 2); /* FLUSH_STRIDE */ > + build_append_int_noprefix(tbl, 0, 1); /* DUTY_OFFSET */ > + build_append_int_noprefix(tbl, 0, 1); /* DUTY_WIDTH */ > + build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */ > + build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */ > + build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */ > + build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */ > + build_append_int_noprefix(tbl, 0, 1); /* Reserved */ > + build_append_int_noprefix(tbl, f->flags, 4); /* Flags */ > + > if (f->rev == 1) { > - fadt_size = offsetof(typeof(*fadt), reset_register); > - } else if (f->xdsdt_tbl_offset) { > + goto build_hdr; > + } > + > + build_append_gas_from_struct(tbl, &f->reset_reg); /* RESET_REG */ > + build_append_int_noprefix(tbl, f->reset_val, 1); /* RESET_VALUE */ > + build_append_int_noprefix(tbl, 0, 3); /* Reserved, ACPI 3.0 */ > + build_append_int_noprefix(tbl, 0, 8); /* X_FIRMWARE_CTRL */ > + > + /* XDSDT address to be filled by Guest linker at runtime */ > + off = tbl->len; > + build_append_int_noprefix(tbl, 0, 8); /* X_DSDT */ > + if (f->xdsdt_tbl_offset) { > bios_linker_loader_add_pointer(linker, > - ACPI_BUILD_TABLE_FILE, xdsdt_entry_offset, sizeof(fadt->x_dsdt), > + ACPI_BUILD_TABLE_FILE, off, 8, > ACPI_BUILD_TABLE_FILE, *f->xdsdt_tbl_offset); > } > > - build_header(linker, table_data, > - (void *)fadt, "FACP", fadt_size, f->rev, > - oem_id, oem_table_id); > + build_append_gas_from_struct(tbl, &f->pm1_evt); /* X_PM1a_EVT_BLK */ > + /* X_PM1b_EVT_BLK */ > + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); > + build_append_gas_from_struct(tbl, &f->pm1_cnt); /* X_PM1a_CNT_BLK */ > + /* X_PM1b_CNT_BLK */ > + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); > + /* X_PM2_CNT_BLK */ > + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); > + build_append_gas_from_struct(tbl, &f->pm_tmr); /* X_PM_TMR_BLK */ > + build_append_gas_from_struct(tbl, &f->gpe0_blk); /* X_GPE0_BLK */ > + build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0); /* X_GPE1_BLK > */ > + > +build_hdr: > + build_header(linker, tbl, (void *)(tbl->data + fadt_start), > + "FACP", tbl->len - fadt_start, f->rev, oem_id, > oem_table_id); > } > > void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, >