* [Qemu-devel] [PATCH v2 0/2] ACPI/arm-virt: add DBG2
@ 2015-09-13 15:06 Leif Lindholm
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table Leif Lindholm
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add " Leif Lindholm
0 siblings, 2 replies; 7+ messages in thread
From: Leif Lindholm @ 2015-09-13 15:06 UTC (permalink / raw
To: qemu-devel; +Cc: peter.maydell, drjones, shannon.zhao, imammedo, mst
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table
2015-09-13 15:06 [Qemu-devel] [PATCH v2 0/2] ACPI/arm-virt: add DBG2 Leif Lindholm
@ 2015-09-13 15:06 ` Leif Lindholm
2015-09-14 16:30 ` Andrew Jones
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add " Leif Lindholm
1 sibling, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2015-09-13 15:06 UTC (permalink / raw
To: qemu-devel; +Cc: peter.maydell, drjones, shannon.zhao, imammedo, mst
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table
2015-09-13 15:06 [Qemu-devel] [PATCH v2 0/2] ACPI/arm-virt: add DBG2 Leif Lindholm
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table Leif Lindholm
@ 2015-09-13 15:06 ` Leif Lindholm
2015-09-14 16:35 ` Andrew Jones
1 sibling, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2015-09-13 15:06 UTC (permalink / raw
To: qemu-devel; +Cc: peter.maydell, drjones, shannon.zhao, imammedo, mst
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table Leif Lindholm
@ 2015-09-14 16:30 ` Andrew Jones
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2015-09-14 16:30 UTC (permalink / raw
To: Leif Lindholm; +Cc: peter.maydell, imammedo, mst, qemu-devel, shannon.zhao
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add " Leif Lindholm
@ 2015-09-14 16:35 ` Andrew Jones
2015-09-15 1:20 ` Shannon Zhao
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Jones @ 2015-09-14 16:35 UTC (permalink / raw
To: Leif Lindholm; +Cc: peter.maydell, imammedo, mst, qemu-devel, shannon.zhao
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table
2015-09-14 16:35 ` Andrew Jones
@ 2015-09-15 1:20 ` Shannon Zhao
2015-09-15 14:30 ` Leif Lindholm
0 siblings, 1 reply; 7+ messages in thread
From: Shannon Zhao @ 2015-09-15 1:20 UTC (permalink / raw
To: Andrew Jones, Leif Lindholm; +Cc: peter.maydell, mst, qemu-devel, imammedo
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add DBG2 table
2015-09-15 1:20 ` Shannon Zhao
@ 2015-09-15 14:30 ` Leif Lindholm
0 siblings, 0 replies; 7+ messages in thread
From: Leif Lindholm @ 2015-09-15 14:30 UTC (permalink / raw
To: Shannon Zhao; +Cc: peter.maydell, Andrew Jones, mst, qemu-devel, imammedo
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-09-15 14:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-13 15:06 [Qemu-devel] [PATCH v2 0/2] ACPI/arm-virt: add DBG2 Leif Lindholm
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 1/2] ACPI: Add definitions for the DBG2 table Leif Lindholm
2015-09-14 16:30 ` Andrew Jones
2015-09-13 15:06 ` [Qemu-devel] [PATCH v2 2/2] hw/arm/virt-acpi-build: Add " Leif Lindholm
2015-09-14 16:35 ` Andrew Jones
2015-09-15 1:20 ` Shannon Zhao
2015-09-15 14:30 ` Leif Lindholm
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.