[Qemu-devel] Re: [PATCH] Add HPET support to BIOS

["Sebastian Herbszt" <herbszt@gmx.de>]

Beth Kon wrote:
> This patch, written by Ryan Harper, adds HPET support to BIOS.
> 
> Signed-off-by: Beth Kon <bkon@us.ibm.com>
> 
> diff --git a/bios/Makefile b/bios/Makefile
> index 48022ea..3e73fb5 100644
> --- a/bios/Makefile
> +++ b/bios/Makefile
> @@ -40,7 +40,7 @@ LIBS =  -lm
> RANLIB = ranlib
> 
> BCC = bcc
> -GCC = gcc -m32
> +GCC = gcc -m32 -fno-stack-protector
> HOST_CC = gcc
> AS86 = as86
> 
> diff --git a/bios/acpi-dsdt.dsl b/bios/acpi-dsdt.dsl
> index d1bfa2c..1548c86 100755
> --- a/bios/acpi-dsdt.dsl
> +++ b/bios/acpi-dsdt.dsl
> @@ -262,6 +262,24 @@ DefinitionBlock (
>                 Return (MEMP)
>             }
>         }
> +        Device(HPET) {
> +            Name(_HID,  EISAID("PNP0103"))
> +            Name(_UID, 0)

_UID is optional if only one timer block is present.

> +            Method (_STA, 0, NotSerialized) {
> +                    Return(0x00)

Not present?

> +            }
> +            Name(_CRS, ResourceTemplate() {
> +                DWordMemory(
> +                    ResourceConsumer, PosDecode, MinFixed, MaxFixed,
> +                    NonCacheable, ReadWrite,
> +                    0x00000000,
> +                    0xFED00000,
> +                    0xFED003FF,
> +                    0x00000000,
> +                    0x00000400 /* 1K memory: FED00000 - FED003FF */
> +                )
> +            })
> +        }
>     }
> 
>     Scope(\_SB.PCI0) {
> @@ -628,7 +646,7 @@ DefinitionBlock (
>                 {
>                     Or (PRQ3, 0x80, PRQ3)
>                 }
> -                Method (_CRS, 0, NotSerialized)
> +                Method (_CRS, 1, NotSerialized)
>                 {
>                     Name (PRR0, ResourceTemplate ()
>                     {

Is this change related?

> diff --git a/bios/rombios32.c b/bios/rombios32.c
> index 2dc1d25..c1ec015 100755
> --- a/bios/rombios32.c
> +++ b/bios/rombios32.c
> @@ -1182,7 +1182,7 @@ struct rsdp_descriptor         /* Root System
> Descriptor Pointer */
> struct rsdt_descriptor_rev1
> {
>  ACPI_TABLE_HEADER_DEF                           /* ACPI common table
> header */
> - uint32_t                             table_offset_entry [2]; /* Array
> of pointers to other */
> + uint32_t                             table_offset_entry [3]; /* Array
> of pointers to other */
>  /* ACPI tables */
> };
> 
> @@ -1322,6 +1322,30 @@ struct madt_processor_apic
> #endif
> };
> 
> +/*
> + * ACPI 2.0 Generic Address Space definition.
> + */
> +struct acpi_20_generic_address {
> +    uint8_t  address_space_id;
> +    uint8_t  register_bit_width;
> +    uint8_t  register_bit_offset;
> +    uint8_t  reserved;
> +    uint64_t address;
> +};
> +
> +/*
> + * HPET Description Table
> + */
> +struct acpi_20_hpet {
> +    ACPI_TABLE_HEADER_DEF                           /* ACPI common
> table header */
> +    uint32_t           timer_block_id;
> +    struct acpi_20_generic_address addr;
> +    uint8_t            hpet_number;
> +    uint16_t           min_tick;
> +    uint8_t            page_protect;
> +};
> +#define ACPI_HPET_ADDRESS 0xFED00000UL
> +
> struct madt_io_apic
> {
>  APIC_HEADER_DEF
> @@ -1393,8 +1417,9 @@ void acpi_bios_init(void)
>     struct fadt_descriptor_rev1 *fadt;
>     struct facs_descriptor_rev1 *facs;
>     struct multiple_apic_table *madt;
> +    struct acpi_20_hpet *hpet;
>     uint8_t *dsdt;
> -    uint32_t base_addr, rsdt_addr, fadt_addr, addr, facs_addr,
> dsdt_addr;
> +    uint32_t base_addr, rsdt_addr, fadt_addr, addr, facs_addr,
> dsdt_addr, hpet_addr;
>     uint32_t acpi_tables_size, madt_addr, madt_size;
>     int i;
> 
> @@ -1436,6 +1461,11 @@ void acpi_bios_init(void)
>     madt = (void *)(addr);
>     addr += madt_size;
> 
> +    addr = (addr + 7) & ~7;
> +    hpet_addr = addr;
> +    hpet = (void *)(addr);
> +    addr += sizeof(*hpet);
> +
>     acpi_tables_size = addr - base_addr;
> 
>     BX_INFO("ACPI tables: RSDP addr=0x%08lx ACPI DATA addr=0x%08lx
> size=0x%x\n",
> @@ -1457,6 +1487,7 @@ void acpi_bios_init(void)
>     memset(rsdt, 0, sizeof(*rsdt));
>     rsdt->table_offset_entry[0] = cpu_to_le32(fadt_addr);
>     rsdt->table_offset_entry[1] = cpu_to_le32(madt_addr);
> +    rsdt->table_offset_entry[2] = cpu_to_le32(hpet_addr);
>     acpi_build_table_header((struct acpi_table_header *)rsdt,
>                             "RSDT", sizeof(*rsdt), 1);
> 
> @@ -1540,6 +1571,15 @@ void acpi_bios_init(void)
>         acpi_build_table_header((struct acpi_table_header *)madt,
>                                 "APIC", madt_size, 1);
>     }
> +
> +    /* HPET */
> +    memset(hpet, 0, sizeof(*hpet));
> +    hpet->timer_block_id = cpu_to_le32(0x8086a201);
> +   // hpet->timer_block_id = cpu_to_le32(0x80862201);

This "magic value" could need some explanation so people don't have to look it up.
Something like:
8086 = pci vendor id
a201 = 1010001000000001
1                                  LegacyReplacement IRQ Routing Capable
  0                                reserved
   1                               COUNT_SIZE_CAP counter size
    00010                     Number of Comparators
             00000001      Hardwave revision id

Also add a comment that it should be kept in sync with the emulation (hpet.c).

- Sebastian

> +    hpet->addr.address = cpu_to_le32(ACPI_HPET_ADDRESS);
> +    acpi_build_table_header((struct  acpi_table_header *)hpet,
> +                             "HPET", sizeof(*hpet), 1);
> +
> }
> 
> /* SMBIOS entry point -- must be written to a 16-bit aligned address
> 
> 
>