diff mbox

[Xen-devel,v4,05/16] libxl/arm: Construct ACPI RSDP table

Message ID 1471343113-10652-6-git-send-email-zhaoshenglong@huawei.com
State Superseded
Headers show

Commit Message

Shannon Zhao Aug. 16, 2016, 10:25 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Construct ACPI RSDP table and add a helper to calculate the ACPI table
checksum.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Julien Grall Aug. 29, 2016, 6:03 p.m. UTC | #1
Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Construct ACPI RSDP table and add a helper to calculate the ACPI table
> checksum.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 6be9eb0..9432e44 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>
> +#define ACPI_BUILD_APPNAME6 "XenARM"
> +#define ACPI_BUILD_APPNAME4 "Xen "
> +
>  enum {
>      RSDP,
>      XSDT,
> @@ -112,6 +115,37 @@ out:
>      return rc;
>  }
>
> +static void calculate_checksum(void *table, uint32_t checksum_offset,
> +                               uint32_t length)
> +{
> +    uint8_t *p, sum = 0;
> +
> +    p = table;
> +    p[checksum_offset] = 0;
> +
> +    while ( length-- )
> +        sum = sum + *p++;
> +
> +    p = table;
> +    p[checksum_offset] = -sum;
> +}
> +
> +static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
> +                           struct acpitable acpitables[])
> +{
> +    uint64_t offset = acpitables[RSDP].addr - GUEST_ACPI_BASE;
> +    struct acpi_table_rsdp *rsdp = (void *)dom->acpi_modules[0].data + offset;

Why do you cast to (void *)?

> +
> +    memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
> +    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
> +    rsdp->length = acpitables[RSDP].size;
> +    rsdp->revision = 0x02;
> +    rsdp->xsdt_physical_address = acpitables[XSDT].addr;
> +    calculate_checksum(rsdp,
> +                       offsetof(struct acpi_table_rsdp, extended_checksum),
> +                       acpitables[RSDP].size);
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> @@ -137,6 +171,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>      dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
>
>      rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
> +    if (rc)
> +        goto out;
> +
> +    make_acpi_rsdp(gc, dom, acpitables);
>
>  out:
>      return rc;
>
Julien Grall Aug. 29, 2016, 6:05 p.m. UTC | #2
Hi Shannon,

On 25/08/2016 04:05, Shannon Zhao wrote:
>
>
> On 2016/8/24 20:52, Wei Liu wrote:
>> On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Construct ACPI RSDP table and add a helper to calculate the ACPI table
>>>> checksum.
>>>>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> ---
>>>>  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 38 insertions(+)
>>>>
>>>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>>>> index 6be9eb0..9432e44 100644
>>>> --- a/tools/libxl/libxl_arm_acpi.c
>>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>>> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>>>>  _hidden
>>>>  extern const int dsdt_anycpu_arm_len;
>>>>
>>>> +#define ACPI_BUILD_APPNAME6 "XenARM"
>>>> +#define ACPI_BUILD_APPNAME4 "Xen "
>>>> +
>> Where do these come from? If they are from a spec, could you please add
>> a comment here?
>>
> Not from some spec. Just fake a OEM for these tables like the
> ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.

In this case, why don't we re-use the one from x86?

Regards,
Julien Grall Aug. 30, 2016, 5:11 p.m. UTC | #3
Hi Shannon,

On 30/08/16 02:21, Shannon Zhao wrote:
>
>
> On 2016/8/30 2:05, Julien Grall wrote:
>> Hi Shannon,
>>
>> On 25/08/2016 04:05, Shannon Zhao wrote:
>>>
>>>
>>> On 2016/8/24 20:52, Wei Liu wrote:
>>>> On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
>>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>>
>>>>>> Construct ACPI RSDP table and add a helper to calculate the ACPI table
>>>>>> checksum.
>>>>>>
>>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>> ---
>>>>>>  tools/libxl/libxl_arm_acpi.c | 38
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 38 insertions(+)
>>>>>>
>>>>>> diff --git a/tools/libxl/libxl_arm_acpi.c
>>>>>> b/tools/libxl/libxl_arm_acpi.c
>>>>>> index 6be9eb0..9432e44 100644
>>>>>> --- a/tools/libxl/libxl_arm_acpi.c
>>>>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>>>>> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>>>>>>  _hidden
>>>>>>  extern const int dsdt_anycpu_arm_len;
>>>>>>
>>>>>> +#define ACPI_BUILD_APPNAME6 "XenARM"
>>>>>> +#define ACPI_BUILD_APPNAME4 "Xen "
>>>>>> +
>>>> Where do these come from? If they are from a spec, could you please add
>>>> a comment here?
>>>>
>>> Not from some spec. Just fake a OEM for these tables like the
>>> ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.
>>
>> In this case, why don't we re-use the one from x86?
>>
> While the ACPI_OEM_TABLE_ID and ACPI_CREATOR_ID of x86 are HVM specific,
> I don't think it's proper for ARM.

IIRC we have an OEM ID and Creator ID reserved for Xen. Stefano can you 
confirm?

In any case, it would be better if we re-use the existing one. It does 
not hurt to use a different name. For instance the signature of the MADT 
table is "APIC" with is x86 specific...

Regards,
diff mbox

Patch

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 6be9eb0..9432e44 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -33,6 +33,9 @@  extern const unsigned char dsdt_anycpu_arm[];
 _hidden
 extern const int dsdt_anycpu_arm_len;
 
+#define ACPI_BUILD_APPNAME6 "XenARM"
+#define ACPI_BUILD_APPNAME4 "Xen "
+
 enum {
     RSDP,
     XSDT,
@@ -112,6 +115,37 @@  out:
     return rc;
 }
 
+static void calculate_checksum(void *table, uint32_t checksum_offset,
+                               uint32_t length)
+{
+    uint8_t *p, sum = 0;
+
+    p = table;
+    p[checksum_offset] = 0;
+
+    while ( length-- )
+        sum = sum + *p++;
+
+    p = table;
+    p[checksum_offset] = -sum;
+}
+
+static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
+                           struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[RSDP].addr - GUEST_ACPI_BASE;
+    struct acpi_table_rsdp *rsdp = (void *)dom->acpi_modules[0].data + offset;
+
+    memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
+    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
+    rsdp->length = acpitables[RSDP].size;
+    rsdp->revision = 0x02;
+    rsdp->xsdt_physical_address = acpitables[XSDT].addr;
+    calculate_checksum(rsdp,
+                       offsetof(struct acpi_table_rsdp, extended_checksum),
+                       acpitables[RSDP].size);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -137,6 +171,10 @@  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
     dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
 
     rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
+    if (rc)
+        goto out;
+
+    make_acpi_rsdp(gc, dom, acpitables);
 
 out:
     return rc;