From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KGyq9-00034I-TH for qemu-devel@nongnu.org; Thu, 10 Jul 2008 12:17:53 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KGyq8-00033d-Vq for qemu-devel@nongnu.org; Thu, 10 Jul 2008 12:17:53 -0400 Received: from [199.232.76.173] (port=48064 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KGyq8-00033T-Sg for qemu-devel@nongnu.org; Thu, 10 Jul 2008 12:17:52 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:50610) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KGyq8-0002om-DL for qemu-devel@nongnu.org; Thu, 10 Jul 2008 12:17:52 -0400 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by e33.co.us.ibm.com (8.13.8/8.13.8) with ESMTP id m6AGHmWX030297 for ; Thu, 10 Jul 2008 12:17:48 -0400 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v9.0) with ESMTP id m6AGHmtg127382 for ; Thu, 10 Jul 2008 10:17:48 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m6AGHl5n008378 for ; Thu, 10 Jul 2008 10:17:48 -0600 Date: Thu, 10 Jul 2008 11:17:40 -0500 From: Ryan Harper Message-ID: <20080710161740.GO4188@us.ibm.com> References: <1215662789.9862.41.camel@beth-ubuntu> <023001c8e2a3$a72a6820$0201a8c0@zeug> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <023001c8e2a3$a72a6820$0201a8c0@zeug> Subject: [Qemu-devel] Re: [PATCH] Add HPET support to BIOS Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sebastian Herbszt Cc: Ryan Harper , Beth Kon , qemu-devel@nongnu.org, kvm@vger.kernel.org * Sebastian Herbszt [2008-07-10 10:46]: Hey Sebastian, Thanks for the review, > Beth Kon wrote: > >This patch, written by Ryan Harper, adds HPET support to BIOS. > > > >Signed-off-by: Beth Kon > > > >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. OK > > >+ Method (_STA, 0, NotSerialized) { > >+ Return(0x00) > > Not present? Was playing around with this when trying to get Linux to see the device in the ACPI tables. AFAICT, Linux doesn't care about this value. Should be 1 here then? > > >+ } > >+ 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? Doubtful, I'll confirm whether or not it is needed. > > >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 That's a fair point though one needs the spec to make any sort of intelligent change anyhow. > > Also add a comment that it should be kept in sync with the emulation > (hpet.c). Yeah, this is an important point as they do need to agree. > > - 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 > > > > > > -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 ryanh@us.ibm.com