Re: [Xen-devel] [PATCH] arm/acpi: hide watchdog timer in GTDT table for dom0

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 28 Nov 2016, Shanker Donthineni wrote:
> Either we have to hide the watchdog timer section in GTDT or emulate
> watchdog timer block for dom0. Otherwise, system gets panic when
> dom0 accesses its MMIO registers. The current XEN doesn't support
> virtualization of watchdog timer, so hide the watchdog timer section
> for dom0.
> 
> Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>

Thanks for the patch, it looks good. Just a couple of questions below.


>  xen/arch/arm/domain_build.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/acpi.h  |  1 +
>  2 files changed, 42 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e8a400c..611c803 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1668,6 +1668,8 @@ static int acpi_create_xsdt(struct domain *d, struct 
> membank tbl_add[])
>                             ACPI_SIG_FADT, tbl_add[TBL_FADT].start);
>      acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
>                             ACPI_SIG_MADT, tbl_add[TBL_MADT].start);
> +    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
> +                           ACPI_SIG_GTDT, tbl_add[TBL_GTDT].start);
>      xsdt->table_offset_entry[entry_count] = tbl_add[TBL_STAO].start;
>  
>      xsdt->header.length = table_size;
> @@ -1718,6 +1720,41 @@ static int acpi_create_stao(struct domain *d, struct 
> membank tbl_add[])
>      return 0;
>  }
>  
> +static int acpi_create_gtdt(struct domain *d, struct membank tbl_add[])
> +{
> +    struct acpi_table_header *table = NULL;
> +    struct acpi_table_gtdt *gtdt = NULL;
> +    u32 table_size = sizeof(struct acpi_table_gtdt);
> +    u32 offset = acpi_get_table_offset(tbl_add, TBL_GTDT);
> +    acpi_status status;
> +    u8 *base_ptr, checksum;
> +
> +    status = acpi_get_table(ACPI_SIG_GTDT, 0, &table);
> +
> +    if ( ACPI_FAILURE(status) )
> +    {
> +        const char *msg = acpi_format_exception(status);
> +
> +        printk("Failed to get GTDT table, %s\n", msg);
> +        return -EINVAL;
> +    }
> +
> +    base_ptr = d->arch.efi_acpi_table + offset;
> +    ACPI_MEMCPY(base_ptr, table, sizeof(struct acpi_table_gtdt));

Use table_size


> +    gtdt = (struct acpi_table_gtdt *)base_ptr;
> +    gtdt->header.length = table_size;
> +    gtdt->platform_timer_count = 0;
> +    gtdt->platform_timer_offset = table_size;

Why table_size instead of 0? Is that the expected values when the array
is empty?


> +    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, gtdt), table_size);
> +    gtdt->header.checksum -= checksum;
> +
> +    tbl_add[TBL_GTDT].start = d->arch.efi_acpi_gpa + offset;
> +    tbl_add[TBL_GTDT].size = table_size;
> +
> +    return 0;
> +}
> +
>  static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
>  {
>      struct acpi_table_header *table = NULL;
> @@ -1909,6 +1946,10 @@ static int prepare_acpi(struct domain *d, struct 
> kernel_info *kinfo)
>      if ( rc != 0 )
>          return rc;
>  
> +    rc = acpi_create_gtdt(d, tbl_add);
> +    if ( rc != 0 )
> +        return rc;
> +
>      rc = acpi_create_xsdt(d, tbl_add);
>      if ( rc != 0 )
>          return rc;
> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
> index 9f954d3..214511c 100644
> --- a/xen/include/asm-arm/acpi.h
> +++ b/xen/include/asm-arm/acpi.h
> @@ -36,6 +36,7 @@ typedef enum {
>      TBL_FADT,
>      TBL_MADT,
>      TBL_STAO,
> +    TBL_GTDT,
>      TBL_XSDT,
>      TBL_RSDP,
>      TBL_EFIT,

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel