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

[Andrew Jones]

On Mon, Sep 07, 2015 at 03:23:45PM +0100, Leif Lindholm wrote:
> The DBG2 table can be considered a "companion" to SPCR - it points out
> debug consoles available in the system.
> 
> Signed-off-by: Leif Lindholm <address@hidden>
> ---
>  include/hw/acpi/acpi-defs.h | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 2b431e6..af062a7 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -197,10 +197,43 @@ 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
> + *
> + * The specification permits multiple ports with multiple addresses, but this
> + * implementation is limited to one port with one address.
> + */
> +struct AcpiDebugPort2 {
> +    ACPI_TABLE_HEADER_DEF
> +    uint32_t debug_devices_offset;
> +    uint32_t number_debug_devices;
> +    struct {
> +      uint8_t  revision;
> +      uint16_t length;
> +      uint8_t  number_generic_address_registers;
> +      uint16_t namespace_string_length;
> +      uint16_t namespace_string_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_register_offset;
> +      uint16_t address_size_offset;
> +      struct AcpiGenericAddress base_address[1];

Not sure we want to limit the number of addresses. Non ARM (non PL011)
users of this table may not find one address sufficient.

> +      uint32_t address_size[1];
> +      uint8_t  namespace_string[2];
> +    } QEMU_PACKED debug_devices[1];

I'm guessing it's unlikely we'll ever want more than one debug port. So
can we just drop the debug_devices array, flatting the table? OTOH, this
is generic acpi table generation code here, and maybe x86 will want to
use more than one port? In that case we should pull the debug device
structure definition out, and then properly handle the variable length
array. But this is where Igor and mst will suggest just using aml_appends,
instead of defining these structures at all :-)

> +} QEMU_PACKED;
> +typedef struct AcpiDebugPort2
> +               AcpiDebugPort2;
> +
> +/*
> + * 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
>