[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/3] hw/arm/virt_acpi_build: Generate DBG2 table
From: |
Andrew Jones |
Subject: |
Re: [PATCH v3 2/3] hw/arm/virt_acpi_build: Generate DBG2 table |
Date: |
Wed, 6 Oct 2021 11:15:23 +0200 |
On Mon, Sep 27, 2021 at 03:17:31PM +0200, Eric Auger wrote:
> ARM SBBR specification mandates DBG2 table (Debug Port Table 2)
> since v1.0 (ARM DEN0044F 8.3.1.7 DBG2).
>
> The DBG2 table allows to describe one or more debug ports.
>
> Generate an DBG2 table featuring a single debug port, the PL011.
>
> The DBG2 specification can be found at
> "Microsoft Debug Port Table 2 (DBG2)"
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table?redirectedfrom=MSDN
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v2 -> v3:
> Took into account all comments from Igor on v2:
> mostly style adjustment, revision references
>
> v1 -> v2:
> - rebased on Igor's refactoring
> ---
> hw/arm/virt-acpi-build.c | 62 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6cec97352b..257d0fee17 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -616,6 +616,63 @@ build_gtdt(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> acpi_table_end(linker, &table);
> }
>
> +/* Debug Port Table 2 (DBG2) */
> +static void
> +build_dbg2(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
nit: I didn't think QEMU liked this style, i.e. QEMU prefers
static void build_dbg2(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
Eh, I see that hw/arm/virt-acpi-build.c is full of the format you have
here, so never mind.
> +{
> + AcpiTable table = { .sig = "DBG2", .rev = 3, .oem_id = vms->oem_id,
Can you explain where the .rev = 3 comes from? The spec says "For this
version of the specification, this value is 0."
> + .oem_table_id = vms->oem_table_id };
> + int dbg2devicelength;
> + const char name[] = "COM0";
> + const int namespace_length = sizeof(name);
> +
> + acpi_table_begin(&table, table_data);
> +
> + dbg2devicelength = 22 /* BaseAddressRegister[] offset */ +
> + 12 /* BaseAddressRegister[] */ +
> + 4 /* AddressSize[] */ +
I'd personally prefer the '+' before the comment, but maybe Igor has a
special ACPI code format preference here.
> + namespace_length /* NamespaceString[] */;
> +
> + /* OffsetDbgDeviceInfo */
> + build_append_int_noprefix(table_data, 44, 4);
> + /* NumberDbgDeviceInfo */
> + build_append_int_noprefix(table_data, 1, 4);
> +
> + /* Table 2. Debug Device Information structure format */
> + build_append_int_noprefix(table_data, 0, 1); /* Revision */
> + build_append_int_noprefix(table_data, dbg2devicelength, 2); /* Length */
> + /* NumberofGenericAddressRegisters */
> + build_append_int_noprefix(table_data, 1, 1);
> + /* NameSpaceStringLength */
> + build_append_int_noprefix(table_data, namespace_length, 2);
> + build_append_int_noprefix(table_data, 38, 2); /* NameSpaceStringOffset */
> + build_append_int_noprefix(table_data, 0, 2); /* OemDataLength */
> + /* OemDataOffset (0 means no OEM data) */
> + build_append_int_noprefix(table_data, 0, 2);
> +
> + /* Port Type */
> + build_append_int_noprefix(table_data, 0x8000 /* Serial */, 2);
> + /* Port Subtype */
> + build_append_int_noprefix(table_data, 0x3 /* ARM PL011 UART */, 2);
> + build_append_int_noprefix(table_data, 0, 2); /* Reserved */
> + /* BaseAddressRegisterOffset */
> + build_append_int_noprefix(table_data, 22, 2);
> + /* AddressSizeOffset */
> + build_append_int_noprefix(table_data, 34, 2);
> +
> + /* BaseAddressRegister[] */
> + build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 8, 0, 1,
> + vms->memmap[VIRT_UART].base);
> +
> + /* AddressSize[] */
> + build_append_int_noprefix(table_data, 0x1000 /* Register Space */, 4);
> +
> + /* NamespaceString[] */
> + g_array_append_vals(table_data, name, namespace_length);
> +
> + acpi_table_end(linker, &table);
> +};
> +
> /*
> * ACPI spec, Revision 5.1 Errata A
> * 5.2.12 Multiple APIC Description Table (MADT)
> @@ -875,7 +932,7 @@ void virt_acpi_build(VirtMachineState *vms,
> AcpiBuildTables *tables)
> dsdt = tables_blob->len;
> build_dsdt(tables_blob, tables->linker, vms);
>
> - /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
> + /* FADT MADT GTDT MCFG SPCR DBG2 pointed to by RSDT */
> acpi_add_table(table_offsets, tables_blob);
> build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
>
> @@ -898,6 +955,9 @@ void virt_acpi_build(VirtMachineState *vms,
> AcpiBuildTables *tables)
> acpi_add_table(table_offsets, tables_blob);
> build_spcr(tables_blob, tables->linker, vms);
>
> + acpi_add_table(table_offsets, tables_blob);
> + build_dbg2(tables_blob, tables->linker, vms);
> +
> if (vms->ras) {
> build_ghes_error_table(tables->hardware_errors, tables->linker);
> acpi_add_table(table_offsets, tables_blob);
> --
> 2.26.3
>
Thanks,
drew