Message ID | 20181129132428.333-7-sameo@linux.intel.com |
---|---|
State | New |
Headers | show |
Series | hw: acpi: RSDP fixes and refactoring | expand |
On Thu, Nov 29, 2018 at 02:24:26PM +0100, Samuel Ortiz wrote: > We add the ability to build legacy or current RSDP tables, based on the > AcpiRsdpData revision field passed to build_rsdp(). > Although arm/virt only uses RSDP v2, adding that capability to > build_rsdp will allow us to share the RSDP build code between ARM and x86. > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > --- > hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- > 1 file changed, 26 insertions(+), 11 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 4782aea4fe..e1338b6f5a 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) > g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ > build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ > build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ > - build_append_int_noprefix(tbl, 36, 4); /* Length */ > - > - /* XSDT address to be filled by guest linker */ > - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > - 24, 8, > - ACPI_BUILD_TABLE_FILE, > - *rsdp_data->xsdt_tbl_offset); > - > - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > + if (rsdp_data->rsdt_tbl_offset) { I see why a pointer was used now. Using a pointer ensures a zero offset won't fail this test. However the test could be replaced with rsdp_data->revision == 0. > + /* RSDT address to be filled by guest linker */ > + bios_linker_loader_add_pointer(linker, > + ACPI_BUILD_RSDP_FILE, 16, 4, > + ACPI_BUILD_TABLE_FILE, > + *rsdp_data->rsdt_tbl_offset); > + } > > /* Checksum to be filled by guest linker */ > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > 0, 20, /* ACPI rev 1.0 RSDP size */ > 8); > > + if (rsdp_data->revision == 0) { > + /* ACPI 1.0 RSDP, we're done */ > + return; > + } > + > + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ > + g_assert(rsdp_data->xsdt_tbl_offset != NULL); So here's the justification for the pointers. We sanity check the callers. We're missing the (rsdp_data->revision == 0 && rsdp_data->rsdt_tbl_offset) sanity check though. > + > + build_append_int_noprefix(tbl, 36, 4); /* Length */ > + > + /* XSDT address to be filled by guest linker */ > + build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > + bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, 24, 8, > + ACPI_BUILD_TABLE_FILE, > + *rsdp_data->xsdt_tbl_offset); > + > + build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > + build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > + > /* Extended checksum to be filled by Guest linker */ > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > 0, 36, /* ACPI rev 2.0 RSDP size */ > -- > 2.19.2 > > Thanks, drew
On Thu, Nov 29, 2018 at 03:42:43PM +0100, Andrew Jones wrote: > On Thu, Nov 29, 2018 at 02:24:26PM +0100, Samuel Ortiz wrote: > > We add the ability to build legacy or current RSDP tables, based on the > > AcpiRsdpData revision field passed to build_rsdp(). > > Although arm/virt only uses RSDP v2, adding that capability to > > build_rsdp will allow us to share the RSDP build code between ARM and x86. > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > --- > > hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- > > 1 file changed, 26 insertions(+), 11 deletions(-) > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index 4782aea4fe..e1338b6f5a 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) > > g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ > > build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ > > build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ > > - build_append_int_noprefix(tbl, 36, 4); /* Length */ > > - > > - /* XSDT address to be filled by guest linker */ > > - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > > - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > > - 24, 8, > > - ACPI_BUILD_TABLE_FILE, > > - *rsdp_data->xsdt_tbl_offset); > > - > > - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > > - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > > + if (rsdp_data->rsdt_tbl_offset) { > > I see why a pointer was used now. Using a pointer ensures a zero > offset won't fail this test. However the test could be replaced with > rsdp_data->revision == 0. > > > + /* RSDT address to be filled by guest linker */ > > + bios_linker_loader_add_pointer(linker, > > + ACPI_BUILD_RSDP_FILE, 16, 4, > > + ACPI_BUILD_TABLE_FILE, > > + *rsdp_data->rsdt_tbl_offset); > > + } > > > > /* Checksum to be filled by guest linker */ > > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > 0, 20, /* ACPI rev 1.0 RSDP size */ > > 8); > > > > + if (rsdp_data->revision == 0) { > > + /* ACPI 1.0 RSDP, we're done */ > > + return; > > + } > > + > > + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ > > + g_assert(rsdp_data->xsdt_tbl_offset != NULL); > > So here's the justification for the pointers. We sanity check the callers. We could sanity check the callers without pointers as well, I don't think there's a strong advantage for pointers here, except consistence. > We're missing the (rsdp_data->revision == 0 && rsdp_data->rsdt_tbl_offset) > sanity check though. I think there's nothing preventing a caller to include both rsdt and xsdt if it wants to be able to run on both < 2.0 and 2.0+ platforms with the same table. So if rsdt is set we should add it, regardless of the revision. Cheers, Samuel.
On Thu, Nov 29, 2018 at 03:59:42PM +0100, Samuel Ortiz wrote: > On Thu, Nov 29, 2018 at 03:42:43PM +0100, Andrew Jones wrote: > > On Thu, Nov 29, 2018 at 02:24:26PM +0100, Samuel Ortiz wrote: > > > We add the ability to build legacy or current RSDP tables, based on the > > > AcpiRsdpData revision field passed to build_rsdp(). > > > Although arm/virt only uses RSDP v2, adding that capability to > > > build_rsdp will allow us to share the RSDP build code between ARM and x86. > > > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > > --- > > > hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- > > > 1 file changed, 26 insertions(+), 11 deletions(-) > > > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > > index 4782aea4fe..e1338b6f5a 100644 > > > --- a/hw/arm/virt-acpi-build.c > > > +++ b/hw/arm/virt-acpi-build.c > > > @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) > > > g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ > > > build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ > > > build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ > > > - build_append_int_noprefix(tbl, 36, 4); /* Length */ > > > - > > > - /* XSDT address to be filled by guest linker */ > > > - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > > > - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > > > - 24, 8, > > > - ACPI_BUILD_TABLE_FILE, > > > - *rsdp_data->xsdt_tbl_offset); > > > - > > > - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > > > - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > > > + if (rsdp_data->rsdt_tbl_offset) { > > > > I see why a pointer was used now. Using a pointer ensures a zero > > offset won't fail this test. However the test could be replaced with > > rsdp_data->revision == 0. > > > > > + /* RSDT address to be filled by guest linker */ > > > + bios_linker_loader_add_pointer(linker, > > > + ACPI_BUILD_RSDP_FILE, 16, 4, > > > + ACPI_BUILD_TABLE_FILE, > > > + *rsdp_data->rsdt_tbl_offset); > > > + } > > > > > > /* Checksum to be filled by guest linker */ > > > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > 0, 20, /* ACPI rev 1.0 RSDP size */ > > > 8); > > > > > > + if (rsdp_data->revision == 0) { > > > + /* ACPI 1.0 RSDP, we're done */ > > > + return; > > > + } > > > + > > > + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ > > > + g_assert(rsdp_data->xsdt_tbl_offset != NULL); > > > > So here's the justification for the pointers. We sanity check the callers. > We could sanity check the callers without pointers as well, I don't > think there's a strong advantage for pointers here, except consistence. > > > > We're missing the (rsdp_data->revision == 0 && rsdp_data->rsdt_tbl_offset) > > sanity check though. > I think there's nothing preventing a caller to include both rsdt and > xsdt if it wants to be able to run on both < 2.0 and 2.0+ platforms with > the same table. So if rsdt is set we should add it, regardless of the revision. True, but if revision is zero, then we *must* have it. Otherwise we'll silently return from this function with neither an rsdt nor xsdt linked. Thanks, drew > > Cheers, > Samuel.
On Thu, Nov 29, 2018 at 04:09:47PM +0100, Andrew Jones wrote: > On Thu, Nov 29, 2018 at 03:59:42PM +0100, Samuel Ortiz wrote: > > On Thu, Nov 29, 2018 at 03:42:43PM +0100, Andrew Jones wrote: > > > On Thu, Nov 29, 2018 at 02:24:26PM +0100, Samuel Ortiz wrote: > > > > We add the ability to build legacy or current RSDP tables, based on the > > > > AcpiRsdpData revision field passed to build_rsdp(). > > > > Although arm/virt only uses RSDP v2, adding that capability to > > > > build_rsdp will allow us to share the RSDP build code between ARM and x86. > > > > > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > > > --- > > > > hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- > > > > 1 file changed, 26 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > > > index 4782aea4fe..e1338b6f5a 100644 > > > > --- a/hw/arm/virt-acpi-build.c > > > > +++ b/hw/arm/virt-acpi-build.c > > > > @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) > > > > g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ > > > > build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ > > > > build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ > > > > - build_append_int_noprefix(tbl, 36, 4); /* Length */ > > > > - > > > > - /* XSDT address to be filled by guest linker */ > > > > - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > > > > - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > > > > - 24, 8, > > > > - ACPI_BUILD_TABLE_FILE, > > > > - *rsdp_data->xsdt_tbl_offset); > > > > - > > > > - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > > > > - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > > > > + if (rsdp_data->rsdt_tbl_offset) { > > > > > > I see why a pointer was used now. Using a pointer ensures a zero > > > offset won't fail this test. However the test could be replaced with > > > rsdp_data->revision == 0. > > > > > > > + /* RSDT address to be filled by guest linker */ > > > > + bios_linker_loader_add_pointer(linker, > > > > + ACPI_BUILD_RSDP_FILE, 16, 4, > > > > + ACPI_BUILD_TABLE_FILE, > > > > + *rsdp_data->rsdt_tbl_offset); > > > > + } > > > > > > > > /* Checksum to be filled by guest linker */ > > > > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > > 0, 20, /* ACPI rev 1.0 RSDP size */ > > > > 8); > > > > > > > > + if (rsdp_data->revision == 0) { > > > > + /* ACPI 1.0 RSDP, we're done */ > > > > + return; > > > > + } > > > > + > > > > + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ > > > > + g_assert(rsdp_data->xsdt_tbl_offset != NULL); > > > > > > So here's the justification for the pointers. We sanity check the callers. > > We could sanity check the callers without pointers as well, I don't > > think there's a strong advantage for pointers here, except consistence. > > > > > > > We're missing the (rsdp_data->revision == 0 && rsdp_data->rsdt_tbl_offset) > > > sanity check though. > > I think there's nothing preventing a caller to include both rsdt and > > xsdt if it wants to be able to run on both < 2.0 and 2.0+ platforms with > > the same table. So if rsdt is set we should add it, regardless of the revision. > > True, but if revision is zero, then we *must* have it. Otherwise we'll > silently return from this function with neither an rsdt nor xsdt linked. Correct, thanks. With v3 I'm adding a small piece of code at the start of build_rsdp() that checks for valid rsdt and xsdt pointers (for rev 0 and 2, respectively). Cheers, Samuel.
On Thu, 29 Nov 2018 14:24:26 +0100 Samuel Ortiz <sameo@linux.intel.com> wrote: > We add the ability to build legacy or current RSDP tables, based on the > AcpiRsdpData revision field passed to build_rsdp(). > Although arm/virt only uses RSDP v2, adding that capability to > build_rsdp will allow us to share the RSDP build code between ARM and x86. > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- > 1 file changed, 26 insertions(+), 11 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 4782aea4fe..e1338b6f5a 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) > g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ > build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ > build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ > - build_append_int_noprefix(tbl, 36, 4); /* Length */ > - > - /* XSDT address to be filled by guest linker */ > - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > - 24, 8, > - ACPI_BUILD_TABLE_FILE, > - *rsdp_data->xsdt_tbl_offset); > - > - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > + if (rsdp_data->rsdt_tbl_offset) { > + /* RSDT address to be filled by guest linker */ > + bios_linker_loader_add_pointer(linker, > + ACPI_BUILD_RSDP_FILE, 16, 4, > + ACPI_BUILD_TABLE_FILE, > + *rsdp_data->rsdt_tbl_offset); > + } > > /* Checksum to be filled by guest linker */ > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > 0, 20, /* ACPI rev 1.0 RSDP size */ > 8); > > + if (rsdp_data->revision == 0) { > + /* ACPI 1.0 RSDP, we're done */ > + return; > + } > + > + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ > + g_assert(rsdp_data->xsdt_tbl_offset != NULL); > + > + build_append_int_noprefix(tbl, 36, 4); /* Length */ > + > + /* XSDT address to be filled by guest linker */ > + build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > + bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, 24, 8, > + ACPI_BUILD_TABLE_FILE, > + *rsdp_data->xsdt_tbl_offset); > + > + build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > + build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > + > /* Extended checksum to be filled by Guest linker */ > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > 0, 36, /* ACPI rev 2.0 RSDP size */
On Thu, 29 Nov 2018 15:59:42 +0100 Samuel Ortiz <sameo@linux.intel.com> wrote: > On Thu, Nov 29, 2018 at 03:42:43PM +0100, Andrew Jones wrote: > > On Thu, Nov 29, 2018 at 02:24:26PM +0100, Samuel Ortiz wrote: > > > We add the ability to build legacy or current RSDP tables, based on the > > > AcpiRsdpData revision field passed to build_rsdp(). > > > Although arm/virt only uses RSDP v2, adding that capability to > > > build_rsdp will allow us to share the RSDP build code between ARM and x86. > > > > > > Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> > > > --- > > > hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- > > > 1 file changed, 26 insertions(+), 11 deletions(-) > > > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > > index 4782aea4fe..e1338b6f5a 100644 > > > --- a/hw/arm/virt-acpi-build.c > > > +++ b/hw/arm/virt-acpi-build.c > > > @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) > > > g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ > > > build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ > > > build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ > > > - build_append_int_noprefix(tbl, 36, 4); /* Length */ > > > - > > > - /* XSDT address to be filled by guest linker */ > > > - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ > > > - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, > > > - 24, 8, > > > - ACPI_BUILD_TABLE_FILE, > > > - *rsdp_data->xsdt_tbl_offset); > > > - > > > - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ > > > - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ > > > + if (rsdp_data->rsdt_tbl_offset) { > > > > I see why a pointer was used now. Using a pointer ensures a zero > > offset won't fail this test. However the test could be replaced with > > rsdp_data->revision == 0. > > > > > + /* RSDT address to be filled by guest linker */ > > > + bios_linker_loader_add_pointer(linker, > > > + ACPI_BUILD_RSDP_FILE, 16, 4, > > > + ACPI_BUILD_TABLE_FILE, > > > + *rsdp_data->rsdt_tbl_offset); > > > + } > > > > > > /* Checksum to be filled by guest linker */ > > > bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, > > > 0, 20, /* ACPI rev 1.0 RSDP size */ > > > 8); > > > > > > + if (rsdp_data->revision == 0) { > > > + /* ACPI 1.0 RSDP, we're done */ > > > + return; > > > + } > > > + > > > + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ > > > + g_assert(rsdp_data->xsdt_tbl_offset != NULL); > > > > So here's the justification for the pointers. We sanity check the callers. > We could sanity check the callers without pointers as well, I don't > think there's a strong advantage for pointers here, except consistence. I remember now, we need pointer to RSDT offset here so that we could skip bios_linker_loader_add_pointer(rsdt) when RSDT isn't provided (0 is also a valid offset), as for xsdt field it follows rsdt one for consistency and we can do sanity checks on it. assert here seems not really necessary as it guarantied to SIGSEGV on NULL point dereference at bios_linker_loader_add_pointer(*xdst) callsite. But I don't care here, so I'll leave it up to Samuel. > > We're missing the (rsdp_data->revision == 0 && rsdp_data->rsdt_tbl_offset) > > sanity check though. > I think there's nothing preventing a caller to include both rsdt and > xsdt if it wants to be able to run on both < 2.0 and 2.0+ platforms with > the same table. So if rsdt is set we should add it, regardless of the revision. agreed, we don't do this in QEMU but it's correct, there could be both and it's better follow better document spec in this case. > > Cheers, > Samuel.
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 4782aea4fe..e1338b6f5a 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -378,23 +378,38 @@ build_rsdp(GArray *tbl, BIOSLinker *linker, AcpiRsdpData *rsdp_data) g_array_append_vals(tbl, rsdp_data->oem_id, 6); /* OEMID */ build_append_int_noprefix(tbl, rsdp_data->revision, 1); /* Revision */ build_append_int_noprefix(tbl, 0, 4); /* RsdtAddress */ - build_append_int_noprefix(tbl, 36, 4); /* Length */ - - /* XSDT address to be filled by guest linker */ - build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ - bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, - 24, 8, - ACPI_BUILD_TABLE_FILE, - *rsdp_data->xsdt_tbl_offset); - - build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ - build_append_int_noprefix(tbl, 0, 3); /* Reserved */ + if (rsdp_data->rsdt_tbl_offset) { + /* RSDT address to be filled by guest linker */ + bios_linker_loader_add_pointer(linker, + ACPI_BUILD_RSDP_FILE, 16, 4, + ACPI_BUILD_TABLE_FILE, + *rsdp_data->rsdt_tbl_offset); + } /* Checksum to be filled by guest linker */ bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, 0, 20, /* ACPI rev 1.0 RSDP size */ 8); + if (rsdp_data->revision == 0) { + /* ACPI 1.0 RSDP, we're done */ + return; + } + + /* The RSDP revision is 2 and later, we must have an XSDT pointer */ + g_assert(rsdp_data->xsdt_tbl_offset != NULL); + + build_append_int_noprefix(tbl, 36, 4); /* Length */ + + /* XSDT address to be filled by guest linker */ + build_append_int_noprefix(tbl, 0, 8); /* XsdtAddress */ + bios_linker_loader_add_pointer(linker, ACPI_BUILD_RSDP_FILE, 24, 8, + ACPI_BUILD_TABLE_FILE, + *rsdp_data->xsdt_tbl_offset); + + build_append_int_noprefix(tbl, 0, 1); /* Extended Checksum */ + build_append_int_noprefix(tbl, 0, 3); /* Reserved */ + /* Extended checksum to be filled by Guest linker */ bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE, 0, 36, /* ACPI rev 2.0 RSDP size */
We add the ability to build legacy or current RSDP tables, based on the AcpiRsdpData revision field passed to build_rsdp(). Although arm/virt only uses RSDP v2, adding that capability to build_rsdp will allow us to share the RSDP build code between ARM and x86. Signed-off-by: Samuel Ortiz <sameo@linux.intel.com> --- hw/arm/virt-acpi-build.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)