| Message ID | 1442156793-30708-3-git-send-email-leif.lindholm@linaro.org |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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 */
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(-)