[Qemu-devel] [PATCH v2 0/2] ACPI/arm-virt: add DBG2

[Qemu-devel] [PATCH v2 0/2] ACPI/arm-virt: add DBG2

[Leif Lindholm]

The Debug Port Table 2 (DBG2) is mandated by the ARM Server Base Boot
Requirements specification. Add the DBG2 table definitions, and set up
an entry in the ARM virt machine for the pl011 UART.

Changes since v1:
- Static structure replaced with separate Header/Device structs.
- Missing cpu_to_le*() transforms added in table construction.
- Added missing setting of address_size_offset.
- Commit message modified to mention SPCR spec version bump.

Not changed since v1:
- It's still statically allocated, although the structure definitions
  would now permit a dynamic creation ... I'm just slightly too
  unfamiliar with both ACPI in general and the QEMU aml_* functions to
  quite wrap my head around how to do this dynamically.

Leif Lindholm (2):
  ACPI: Add definitions for the DBG2 table
  hw/arm/virt-acpi-build: Add DBG2 table

 hw/arm/virt-acpi-build.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++-
 include/hw/acpi/acpi-defs.h | 35 ++++++++++++++++++++++++--
 2 files changed, 92 insertions(+), 3 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table

[Leif Lindholm]

The DBG2 table can be considered a "companion" to SPCR - it points out
debug consoles available in the system.

Also update SPCR comments to reflect DBG2 is now described in this file,
and update the supported SPCR specification revision (no functional
change).

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 include/hw/acpi/acpi-defs.h | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 2b431e6..a7bd984 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -197,10 +197,41 @@ enum {
 };
 
 /*
- * Serial Port Console Redirection Table (SPCR), Rev. 1.02
+ * Debug Port Table 2 (DBG2)
  *
  * For .interface_type see Debug Port Table 2 (DBG2) serial port
- * subtypes in Table 3, Rev. May 22, 2012
+ * subtypes in Table 3, Rev. Aug 10, 2015
+ *
+ */
+struct AcpiDebugPort2Header {
+    ACPI_TABLE_HEADER_DEF
+    uint32_t devices_offset;
+    uint32_t devices_count;
+} QEMU_PACKED;
+typedef struct AcpiDebugPort2Header
+               AcpiDebugPort2Header;
+
+struct AcpiDebugPort2Device {
+    uint8_t  revision;
+    uint16_t length;
+    uint8_t  address_count;
+    uint16_t namepath_length;
+    uint16_t namepath_offset;
+    uint16_t oem_data_length;
+    uint16_t oem_data_offset;
+    uint16_t port_type;
+    uint16_t port_subtype;
+    uint8_t  reserved1[2];
+    uint16_t base_address_offset;
+    uint16_t address_size_offset;
+} QEMU_PACKED;
+typedef struct AcpiDebugPort2Device
+               AcpiDebugPort2Device;
+
+/*
+ * Serial Port Console Redirection Table (SPCR), Rev. 1.03
+ *
+ * .interface_type format same as for DBG2.
  */
 struct AcpiSerialPortConsoleRedirection {
     ACPI_TABLE_HEADER_DEF
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table

[Andrew Jones]

On Sun, Sep 13, 2015 at 04:06:32PM +0100, Leif Lindholm wrote:
> The DBG2 table can be considered a "companion" to SPCR - it points out
> debug consoles available in the system.
> 
> Also update SPCR comments to reflect DBG2 is now described in this file,
> and update the supported SPCR specification revision (no functional
> change).
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  include/hw/acpi/acpi-defs.h | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 2b431e6..a7bd984 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -197,10 +197,41 @@ enum {
>  };
>  
>  /*
> - * Serial Port Console Redirection Table (SPCR), Rev. 1.02
> + * Debug Port Table 2 (DBG2)
>   *
>   * For .interface_type see Debug Port Table 2 (DBG2) serial port
> - * subtypes in Table 3, Rev. May 22, 2012
> + * subtypes in Table 3, Rev. Aug 10, 2015
> + *
> + */
> +struct AcpiDebugPort2Header {
> +    ACPI_TABLE_HEADER_DEF
> +    uint32_t devices_offset;
> +    uint32_t devices_count;
> +} QEMU_PACKED;
> +typedef struct AcpiDebugPort2Header
> +               AcpiDebugPort2Header;
typedef line could be all on one line

> +
> +struct AcpiDebugPort2Device {
> +    uint8_t  revision;
> +    uint16_t length;
> +    uint8_t  address_count;
> +    uint16_t namepath_length;
> +    uint16_t namepath_offset;
> +    uint16_t oem_data_length;
> +    uint16_t oem_data_offset;
> +    uint16_t port_type;
> +    uint16_t port_subtype;
> +    uint8_t  reserved1[2];
> +    uint16_t base_address_offset;
> +    uint16_t address_size_offset;
> +} QEMU_PACKED;
> +typedef struct AcpiDebugPort2Device
> +               AcpiDebugPort2Device;

also could be on one line

> +
> +/*
> + * Serial Port Console Redirection Table (SPCR), Rev. 1.03
> + *
> + * .interface_type format same as for DBG2.
>   */
>  struct AcpiSerialPortConsoleRedirection {
>      ACPI_TABLE_HEADER_DEF
> -- 
> 2.1.4
> 
> 
Otherwise
Reviewed-by: Andrew Jones <drjones@redhat.com>

[Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table

[Leif Lindholm]

Add a DBG2 table, describing the pl011 UART.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 hw/arm/virt-acpi-build.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9088248..0ea7023 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -352,6 +352,61 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
 }
 
 static void
+build_dbg2(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+{
+    AcpiDebugPort2Header *dbg2;
+    AcpiDebugPort2Device *dev;
+    struct AcpiGenericAddress *addr;
+    uint32_t *addr_size;
+    char *name;
+    const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
+    int table_size, dev_size, namepath_length;
+    const char namepath[] = ".";
+
+    namepath_length = strlen(namepath) + 1;
+    dev_size = sizeof(*dev) + sizeof(*addr) * 1 + sizeof(uint32_t) * 1 +
+        namepath_length;
+    table_size = dev_size + sizeof(AcpiDebugPort2Header);
+
+    dbg2 = acpi_data_push(table_data, table_size);
+    dev = (void *)dbg2 + sizeof(*dbg2);
+    addr = (void *)dev + sizeof(*dev);
+    addr_size = (void *)addr + sizeof(*addr);
+    name = (void *)addr_size + sizeof(*addr_size);
+
+    dbg2->devices_offset = sizeof(*dbg2);
+    dbg2->devices_count = 1;
+
+    /* First (only) debug device */
+    dev->revision = 0;
+    dev->length = cpu_to_le16(dev_size);
+    dev->address_count = 1;
+    dev->namepath_length = cpu_to_le16(namepath_length);
+    dev->namepath_offset = cpu_to_le16((void *)name - (void *)dev);
+    dev->oem_data_length = 0;
+    dev->oem_data_offset = 0;
+    dev->port_type = cpu_to_le16(0x8000);    /* Serial */
+    dev->port_subtype = cpu_to_le16(0x3);    /* ARM PL011 UART */
+    dev->base_address_offset = cpu_to_le16((void *)addr - (void *)dev);
+    dev->address_size_offset = cpu_to_le16((void *)addr_size - (void *)dev);
+
+    /* First (only) address */
+    addr->space_id = AML_SYSTEM_MEMORY;
+    addr->bit_width = 8;
+    addr->bit_offset = 0;
+    addr->access_width = 1;
+    addr->address = cpu_to_le64(uart_memmap->base);
+
+    /* Size of first (only) address */
+    *addr_size = cpu_to_le32(sizeof(*addr));
+
+    /* Namespace String for first (only) device */
+    strcpy(name, namepath);
+
+    build_header(linker, table_data, (void *)dbg2, "DBG2", table_size, 0);
+}
+
+static void
 build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
 {
     AcpiSerialPortConsoleRedirection *spcr;
@@ -577,7 +632,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
     dsdt = tables_blob->len;
     build_dsdt(tables_blob, tables->linker, guest_info);
 
-    /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
+    /* FADT MADT GTDT MCFG DBG2 SPCR pointed to by RSDT */
     acpi_add_table(table_offsets, tables_blob);
     build_fadt(tables_blob, tables->linker, dsdt);
 
@@ -591,6 +646,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
     build_mcfg(tables_blob, tables->linker, guest_info);
 
     acpi_add_table(table_offsets, tables_blob);
+    build_dbg2(tables_blob, tables->linker, guest_info);
+
+    acpi_add_table(table_offsets, tables_blob);
     build_spcr(tables_blob, tables->linker, guest_info);
 
     /* RSDT is pointed to by RSDP */
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table

[Andrew Jones]

On Sun, Sep 13, 2015 at 04:06:33PM +0100, Leif Lindholm wrote:
> Add a DBG2 table, describing the pl011 UART.
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9088248..0ea7023 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -352,6 +352,61 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>  }
>  
>  static void
> +build_dbg2(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> +{
> +    AcpiDebugPort2Header *dbg2;
> +    AcpiDebugPort2Device *dev;
> +    struct AcpiGenericAddress *addr;
> +    uint32_t *addr_size;
> +    char *name;
> +    const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
> +    int table_size, dev_size, namepath_length;
> +    const char namepath[] = ".";
> +
> +    namepath_length = strlen(namepath) + 1;
> +    dev_size = sizeof(*dev) + sizeof(*addr) * 1 + sizeof(uint32_t) * 1 +
> +        namepath_length;
> +    table_size = dev_size + sizeof(AcpiDebugPort2Header);
> +
> +    dbg2 = acpi_data_push(table_data, table_size);
> +    dev = (void *)dbg2 + sizeof(*dbg2);
> +    addr = (void *)dev + sizeof(*dev);
> +    addr_size = (void *)addr + sizeof(*addr);
> +    name = (void *)addr_size + sizeof(*addr_size);
> +
> +    dbg2->devices_offset = sizeof(*dbg2);
> +    dbg2->devices_count = 1;
> +
> +    /* First (only) debug device */
> +    dev->revision = 0;
> +    dev->length = cpu_to_le16(dev_size);
> +    dev->address_count = 1;
> +    dev->namepath_length = cpu_to_le16(namepath_length);
> +    dev->namepath_offset = cpu_to_le16((void *)name - (void *)dev);
> +    dev->oem_data_length = 0;
> +    dev->oem_data_offset = 0;
> +    dev->port_type = cpu_to_le16(0x8000);    /* Serial */
> +    dev->port_subtype = cpu_to_le16(0x3);    /* ARM PL011 UART */
> +    dev->base_address_offset = cpu_to_le16((void *)addr - (void *)dev);
> +    dev->address_size_offset = cpu_to_le16((void *)addr_size - (void *)dev);
> +
> +    /* First (only) address */
> +    addr->space_id = AML_SYSTEM_MEMORY;
> +    addr->bit_width = 8;
> +    addr->bit_offset = 0;
> +    addr->access_width = 1;
> +    addr->address = cpu_to_le64(uart_memmap->base);
> +
> +    /* Size of first (only) address */
> +    *addr_size = cpu_to_le32(sizeof(*addr));
> +
> +    /* Namespace String for first (only) device */
> +    strcpy(name, namepath);
> +
> +    build_header(linker, table_data, (void *)dbg2, "DBG2", table_size, 0);
> +}
> +
> +static void
>  build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>  {
>      AcpiSerialPortConsoleRedirection *spcr;
> @@ -577,7 +632,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>      dsdt = tables_blob->len;
>      build_dsdt(tables_blob, tables->linker, guest_info);
>  
> -    /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
> +    /* FADT MADT GTDT MCFG DBG2 SPCR pointed to by RSDT */
>      acpi_add_table(table_offsets, tables_blob);
>      build_fadt(tables_blob, tables->linker, dsdt);
>  
> @@ -591,6 +646,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>      build_mcfg(tables_blob, tables->linker, guest_info);
>  
>      acpi_add_table(table_offsets, tables_blob);
> +    build_dbg2(tables_blob, tables->linker, guest_info);
> +
> +    acpi_add_table(table_offsets, tables_blob);
>      build_spcr(tables_blob, tables->linker, guest_info);
>  
>      /* RSDT is pointed to by RSDP */
> -- 
> 2.1.4
> 
> 

This looks good to me, since it's pretty unlikely we'll ever want
more than one device, so

Reviewed-by: Andrew Jones <drjones@redhat.com>

But, when I read that the table generation had become dynamic, I was sort
of expecting something like

void dbg2_add_device(...)
{
...
}

void build_dbg2(...)
{
   do_top_level_table_stuff...

   for_each_debug_device
      dbg2_add_device()

   finalize_table...
}

drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table

[Shannon Zhao]

On 2015/9/15 0:35, Andrew Jones wrote:
> On Sun, Sep 13, 2015 at 04:06:33PM +0100, Leif Lindholm wrote:
>> Add a DBG2 table, describing the pl011 UART.
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  hw/arm/virt-acpi-build.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 9088248..0ea7023 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -352,6 +352,61 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>>  }
>>  
>>  static void
>> +build_dbg2(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>> +{
>> +    AcpiDebugPort2Header *dbg2;
>> +    AcpiDebugPort2Device *dev;
>> +    struct AcpiGenericAddress *addr;
>> +    uint32_t *addr_size;
>> +    char *name;
>> +    const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
>> +    int table_size, dev_size, namepath_length;
>> +    const char namepath[] = ".";
>> +
>> +    namepath_length = strlen(namepath) + 1;
>> +    dev_size = sizeof(*dev) + sizeof(*addr) * 1 + sizeof(uint32_t) * 1 +
>> +        namepath_length;
>> +    table_size = dev_size + sizeof(AcpiDebugPort2Header);
>> +
>> +    dbg2 = acpi_data_push(table_data, table_size);
>> +    dev = (void *)dbg2 + sizeof(*dbg2);
>> +    addr = (void *)dev + sizeof(*dev);
>> +    addr_size = (void *)addr + sizeof(*addr);
>> +    name = (void *)addr_size + sizeof(*addr_size);
>> +

This looks hard to read.

>> +    dbg2->devices_offset = sizeof(*dbg2);
>> +    dbg2->devices_count = 1;
>> +
>> +    /* First (only) debug device */
>> +    dev->revision = 0;
>> +    dev->length = cpu_to_le16(dev_size);
>> +    dev->address_count = 1;
>> +    dev->namepath_length = cpu_to_le16(namepath_length);
>> +    dev->namepath_offset = cpu_to_le16((void *)name - (void *)dev);
>> +    dev->oem_data_length = 0;
>> +    dev->oem_data_offset = 0;
>> +    dev->port_type = cpu_to_le16(0x8000);    /* Serial */
>> +    dev->port_subtype = cpu_to_le16(0x3);    /* ARM PL011 UART */
>> +    dev->base_address_offset = cpu_to_le16((void *)addr - (void *)dev);
>> +    dev->address_size_offset = cpu_to_le16((void *)addr_size - (void *)dev);
>> +
>> +    /* First (only) address */
>> +    addr->space_id = AML_SYSTEM_MEMORY;
>> +    addr->bit_width = 8;
>> +    addr->bit_offset = 0;
>> +    addr->access_width = 1;
>> +    addr->address = cpu_to_le64(uart_memmap->base);
>> +
>> +    /* Size of first (only) address */
>> +    *addr_size = cpu_to_le32(sizeof(*addr));
>> +
>> +    /* Namespace String for first (only) device */
>> +    strcpy(name, namepath);
>> +
>> +    build_header(linker, table_data, (void *)dbg2, "DBG2", table_size, 0);
>> +}
>> +
>> +static void
>>  build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>>  {
>>      AcpiSerialPortConsoleRedirection *spcr;
>> @@ -577,7 +632,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>>      dsdt = tables_blob->len;
>>      build_dsdt(tables_blob, tables->linker, guest_info);
>>  
>> -    /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
>> +    /* FADT MADT GTDT MCFG DBG2 SPCR pointed to by RSDT */
>>      acpi_add_table(table_offsets, tables_blob);
>>      build_fadt(tables_blob, tables->linker, dsdt);
>>  
>> @@ -591,6 +646,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>>      build_mcfg(tables_blob, tables->linker, guest_info);
>>  
>>      acpi_add_table(table_offsets, tables_blob);
>> +    build_dbg2(tables_blob, tables->linker, guest_info);
>> +
>> +    acpi_add_table(table_offsets, tables_blob);
>>      build_spcr(tables_blob, tables->linker, guest_info);
>>  
>>      /* RSDT is pointed to by RSDP */
>> -- 
>> 2.1.4
>>
>>
> 
> This looks good to me, since it's pretty unlikely we'll ever want
> more than one device, so
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> 
> But, when I read that the table generation had become dynamic, I was sort
> of expecting something like
> 

Leif, you can have a look at build_madt.

-- 
Shannon

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table

[Leif Lindholm]

On Tue, Sep 15, 2015 at 09:20:40AM +0800, Shannon Zhao wrote:
> 
> 
> On 2015/9/15 0:35, Andrew Jones wrote:
> > On Sun, Sep 13, 2015 at 04:06:33PM +0100, Leif Lindholm wrote:
> >> Add a DBG2 table, describing the pl011 UART.
> >>
> >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> >> ---
> >>  hw/arm/virt-acpi-build.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 59 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index 9088248..0ea7023 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -352,6 +352,61 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> >>  }
> >>  
> >>  static void
> >> +build_dbg2(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >> +{
> >> +    AcpiDebugPort2Header *dbg2;
> >> +    AcpiDebugPort2Device *dev;
> >> +    struct AcpiGenericAddress *addr;
> >> +    uint32_t *addr_size;
> >> +    char *name;
> >> +    const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
> >> +    int table_size, dev_size, namepath_length;
> >> +    const char namepath[] = ".";
> >> +
> >> +    namepath_length = strlen(namepath) + 1;
> >> +    dev_size = sizeof(*dev) + sizeof(*addr) * 1 + sizeof(uint32_t) * 1 +
> >> +        namepath_length;
> >> +    table_size = dev_size + sizeof(AcpiDebugPort2Header);
> >> +
> >> +    dbg2 = acpi_data_push(table_data, table_size);
> >> +    dev = (void *)dbg2 + sizeof(*dbg2);
> >> +    addr = (void *)dev + sizeof(*dev);
> >> +    addr_size = (void *)addr + sizeof(*addr);
> >> +    name = (void *)addr_size + sizeof(*addr_size);
> >> +
> 
> This looks hard to read.
> 
> >> +    dbg2->devices_offset = sizeof(*dbg2);
> >> +    dbg2->devices_count = 1;
> >> +
> >> +    /* First (only) debug device */
> >> +    dev->revision = 0;
> >> +    dev->length = cpu_to_le16(dev_size);
> >> +    dev->address_count = 1;
> >> +    dev->namepath_length = cpu_to_le16(namepath_length);
> >> +    dev->namepath_offset = cpu_to_le16((void *)name - (void *)dev);
> >> +    dev->oem_data_length = 0;
> >> +    dev->oem_data_offset = 0;
> >> +    dev->port_type = cpu_to_le16(0x8000);    /* Serial */
> >> +    dev->port_subtype = cpu_to_le16(0x3);    /* ARM PL011 UART */
> >> +    dev->base_address_offset = cpu_to_le16((void *)addr - (void *)dev);
> >> +    dev->address_size_offset = cpu_to_le16((void *)addr_size - (void *)dev);
> >> +
> >> +    /* First (only) address */
> >> +    addr->space_id = AML_SYSTEM_MEMORY;
> >> +    addr->bit_width = 8;
> >> +    addr->bit_offset = 0;
> >> +    addr->access_width = 1;
> >> +    addr->address = cpu_to_le64(uart_memmap->base);
> >> +
> >> +    /* Size of first (only) address */
> >> +    *addr_size = cpu_to_le32(sizeof(*addr));
> >> +
> >> +    /* Namespace String for first (only) device */
> >> +    strcpy(name, namepath);
> >> +
> >> +    build_header(linker, table_data, (void *)dbg2, "DBG2", table_size, 0);
> >> +}
> >> +
> >> +static void
> >>  build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >>  {
> >>      AcpiSerialPortConsoleRedirection *spcr;
> >> @@ -577,7 +632,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
> >>      dsdt = tables_blob->len;
> >>      build_dsdt(tables_blob, tables->linker, guest_info);
> >>  
> >> -    /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
> >> +    /* FADT MADT GTDT MCFG DBG2 SPCR pointed to by RSDT */
> >>      acpi_add_table(table_offsets, tables_blob);
> >>      build_fadt(tables_blob, tables->linker, dsdt);
> >>  
> >> @@ -591,6 +646,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
> >>      build_mcfg(tables_blob, tables->linker, guest_info);
> >>  
> >>      acpi_add_table(table_offsets, tables_blob);
> >> +    build_dbg2(tables_blob, tables->linker, guest_info);
> >> +
> >> +    acpi_add_table(table_offsets, tables_blob);
> >>      build_spcr(tables_blob, tables->linker, guest_info);
> >>  
> >>      /* RSDT is pointed to by RSDP */
> >> -- 
> >> 2.1.4
> >>
> >>
> > 
> > This looks good to me, since it's pretty unlikely we'll ever want
> > more than one device, so
> > 
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > 
> > But, when I read that the table generation had become dynamic, I was sort
> > of expecting something like
> > 
> 
> Leif, you can have a look at build_madt.

That is actually one of the more confusing functions for me, what with
all the pointers that may silently become invalid during the execution
of the function.

But yeah, comparing that one with the i386 one, and perhaps the brain
somewhat more engaged than during the weekend, I have a version a bit
cleaner than the one I sent out over the weekend, and hopefully not
too likely to trigger spurious failures if edited by others in future.

Coming up.

/
    Leif