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

[Michael S. Tsirkin]

On Mon, Jun 08, 2015 at 10:00:49AM -0400, Andrew Jones wrote:
> SPCR is the Serial Port Console Redirection table. See the document
> linked from http://uefi.org/acpi. For serial port types, "Interface
> Type", see the documentation for the Debug Port Table 2 (DBG2).
> 
> Signed-off-by: Andrew Jones <address@hidden>
> ---
>  include/hw/acpi/acpi-defs.h | 72 
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 59cf277434b37..e579d4c509fc8 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -197,6 +197,78 @@ enum {
>  };
>  
>  /*
> + * ACPI Serial Port Console Redirection Table
> + */
> +enum {
> +    ACPI_SERIAL_16550_COMPAT        = 0,
> +    ACPI_SERIAL_16550_SUBSET_COMPAT = 1,
> +    ACPI_SERIAL_ARM_PL011_UART      = 3,
> +};
> +
> +enum {
> +    ACPI_SERIAL_ITYPE_PC            = 1,
> +    ACPI_SERIAL_ITYPE_APIC          = 2,
> +    ACPI_SERIAL_ITYPE_SAPIC         = 4,
> +    ACPI_SERIAL_ITYPE_ARMH_GIC      = 8,
> +};
> +
> +enum {
> +    ACPI_SERIAL_BAUD_9600           = 3,
> +    ACPI_SERIAL_BAUD_19200          = 4,
> +    ACPI_SERIAL_BAUD_57600          = 6,
> +    ACPI_SERIAL_BAUD_115200         = 7,
> +};
> +
> +enum {
> +    ACPI_SERIAL_FLOW_DCD_REQUIRED   = 1,
> +    ACPI_SERIAL_FLOW_HW             = 2,
> +    ACPI_SERIAL_FLOW_SW             = 4,
> +};
> +
> +enum {
> +    ACPI_SERIAL_TERM_VT100          = 0,
> +    ACPI_SERIAL_TERM_VT100_PLUS     = 1,
> +    ACPI_SERIAL_TERM_VT_UTF8        = 2,
> +    ACPI_SERIAL_TERM_ANSI           = 3,
> +};
> +

Please don't do these single-use enums, this more than doubles the
amount of code required and makes it hard to look up things in spec.
Do this instead
        sprc->interface_type = 0x0; /* full 16550 interface */

you should also list earliest spec version which has the data
since spec text changes with time.

> +struct AcpiSerialPortConsoleRedirection
> +{
> +    ACPI_TABLE_HEADER_DEF            /* ACPI common table header */
> +    uint8_t  interface_type;         /* See ACPI_SERIAL_ enums */
> +    uint8_t  reserved1[3];           /* Reserved, must be 0 */
> +    struct AcpiGenericAddress base_address;
> +    uint8_t  interrupt_types;        /* Interrupt type support mask,
> +                                      * see ACPI_SERIAL_ITYPE_ enums */
> +    uint8_t  irq;                    /* PC irq used. 0 if 
> ACPI_SERIAL_ITYPE_PC
> +                                      * is not set, else must be one of
> +                                      * 2-7,9-12,14-15 */
> +    uint32_t gsi;                    /* Not valid if only 
> ACPI_SERIAL_ITYPE_PC
> +                                      * is set in the interrupt_types mask */
> +    uint8_t  baud;                   /* See ACPI_SERIAL_BAUD_ enums */
> +    uint8_t  parity;                 /* 0 = no parity, 1-255 reserved */
> +    uint8_t  stopbits;               /* Must be 1 */
> +    uint8_t  flowctrl;               /* See ACPI_SERIAL_FLOW_ enums */
> +    uint8_t  term_type;              /* See ACPI_SERIAL_TERM_ enums */
> +    uint8_t  reserved2;              /* Reserved, must be 0 */
> +    uint16_t pci_device_id;          /* 0xffff, if not a pci dev */
> +    uint16_t pci_vendor_id;          /* 0xffff, if not a pci dev */
> +    uint8_t  pci_bus;                /* PCI bus number, 0 if not a pci dev */
> +    uint8_t  pci_slot;               /* PCI slot number, 0 if not a pci dev 
> */
> +    uint8_t  pci_func;               /* PCI function num, 0 if not a pci dev 
> */
> +    uint32_t pci_flags;              /* PCI compatibility mask,
> +                                      *   bits 1-31 reserved
> +                                      *   bit0 = 0, if not a pci dev
> +                                      *   bit0 = 1, don't suppress PNP or
> +                                      *             power management
> +                                      */
> +    uint8_t  pci_seg;                /* PCI segment number, 0 for systems
> +                                        with less than 255 PCI busses */
> +    uint32_t reserved3;              /* Reserved, must be 0 */
> +} QEMU_PACKED;
> +typedef struct AcpiSerialPortConsoleRedirection 
> AcpiSerialPortConsoleRedirection;
> +
> +/*
>   * ACPI 1.0 Root System Description Table (RSDT)
>   */
>  struct AcpiRsdtDescriptorRev1
> -- 
> 1.8.3.1