On 2016/6/7 19:19, Julien Grall wrote: > Hello Shannon, > > On 31/05/16 06:02, Shannon Zhao wrote: >> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c >> index 9e99159..0fb4f69 100644 >> --- a/tools/libxl/libxl_arm.c >> +++ b/tools/libxl/libxl_arm.c >> @@ -3,6 +3,7 @@ >> #include "libxl_libfdt_compat.h" >> >> #include <xc_dom.h> >> +#include <acpi_defs.h> >> #include <stdbool.h> >> #include <libfdt.h> >> #include <assert.h> >> @@ -880,13 +881,85 @@ out: >> return rc; >> } >> >> +static void make_acpi_header(struct acpi_table_header *h, const char >> *sig, >> + int len, uint8_t rev) >> +{ >> + memcpy(&h->signature, sig, 4); >> + h->length = len; >> + h->revision = rev; >> + memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6); >> + memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4); >> + memcpy(h->oem_table_id + 4, sig, 4); >> + h->oem_revision = 1; >> + memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4); >> + h->asl_compiler_revision = 1; >> + h->checksum = 0; > > Should not we compute the checksum here? > Sure. >> +} >> + >> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom) >> +{ >> + struct acpi_gtdt_descriptor *gtdt; >> + >> + gtdt = libxl__zalloc(gc, sizeof(*gtdt)); >> + >> + gtdt->secure_el1_interrupt = GUEST_TIMER_PHYS_S_PPI; >> + gtdt->secure_el1_flags = (ACPI_LEVEL_SENSITIVE << >> ACPI_GTDT_INTERRUPT_MODE) >> + |(ACPI_ACTIVE_LOW << >> ACPI_GTDT_INTERRUPT_POLARITY); > > There is no secure EL1 for guest, so this should be 0. > So why does DT add secure EL1 timer in make_timer_node()?
>> + gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI; >> + gtdt->non_secure_el1_flags = >> + (ACPI_LEVEL_SENSITIVE << >> ACPI_GTDT_INTERRUPT_MODE) >> + |(ACPI_ACTIVE_LOW << >> ACPI_GTDT_INTERRUPT_POLARITY); >> + gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI; >> + gtdt->virtual_timer_flags = >> + (ACPI_LEVEL_SENSITIVE << >> ACPI_GTDT_INTERRUPT_MODE) >> + |(ACPI_ACTIVE_LOW << >> ACPI_GTDT_INTERRUPT_POLARITY); >> + >> + make_acpi_header(>dt->header, "GTDT", sizeof(*gtdt), 2); >> + >> + dom->acpitable_blob->gtdt.table = (void *)gtdt; >> + /* Align to 64bit. */ > > I am not sure what the comment is for. > Oops, I planed to make the length of these tables aligned to 64bit, but it turns out it's unnecessary. And I forgot to delete this. >> + dom->acpitable_blob->gtdt.size = sizeof(*gtdt); >> + dom->acpitable_size += dom->acpitable_blob->gtdt.size; >> +} > > Regards, > -- Shannon _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel