From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUACB-0000d6-Ah for qemu-devel@nongnu.org; Tue, 25 Aug 2015 05:07:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUAC5-0002s6-7I for qemu-devel@nongnu.org; Tue, 25 Aug 2015 05:07:07 -0400 Received: from mail-wi0-x22a.google.com ([2a00:1450:400c:c05::22a]:33755) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUAC5-0002rJ-0V for qemu-devel@nongnu.org; Tue, 25 Aug 2015 05:07:01 -0400 Received: by wijp15 with SMTP id p15so8826681wij.0 for ; Tue, 25 Aug 2015 02:06:59 -0700 (PDT) References: <1438974931-21128-1-git-send-email-ehabkost@redhat.com> From: Marcel Apfelbaum Message-ID: <55DC3031.7090204@gmail.com> Date: Tue, 25 Aug 2015 12:06:57 +0300 MIME-Version: 1.0 In-Reply-To: <1438974931-21128-1-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.5] piix: Document coreboot-specific RAM size config register Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , "Michael S. Tsirkin" Cc: Ed Swierk , qemu-devel@nongnu.org, Richard Smith On 08/07/2015 10:15 PM, Eduardo Habkost wrote: > The existing i440fx initialization code sets a PCI config register that > isn't documented anywhere in the Intel 440FX datasheet. Register 0x57 is > DRAMC (DRAM Control) and has nothing to do with the RAM size. > > This was implemented in commit ec5f92ce6ac8ec09056be77e03c941be188648fa > because old coreboot code tried to read registers 0x5a-0x5f,0x56,0x57 to > get the RAM size from QEMU, but I couldn't find out why coreboot did > that. I assume it was a mistake, and the original code was supposed to > be reading the DRB[0-7] registers (offsets 0x60-0x67). > > Document that coreboot-specific register offset in a macro and a > comment, for future reference. > > Cc: Ed Swierk > Cc: Richard Smith > Signed-off-by: Eduardo Habkost > --- > References to coreboot commits: > * Original commit adding code reading register offsets > 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, 0x56, 0x57 to Intel 440bx code in > coreboot: > cb8eab482ff09ec256456312ef2d6e7710123551 > * Commit adding the same register offsets to QEMU-specific > coreboot code: > 9cf642bad3fdd2205ffdd83a3222a39855b1ceff > * coreboot commit replacing the weird register offsets with > code that actually reads the DRB7 register, in the I440BX code: > 1a9c892d58c746aef0cb530481c214e63a6a6871 > * coreboot commit replacing the weird register offets with > code reading the CMOS in QEMU-specific code: > 7339f36961917814ed12d5a4e6f1fe19418b8aca > --- > hw/pci-host/piix.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index ad55f99..1cb25f3 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -117,6 +117,11 @@ struct PCII440FXState { > #define I440FX_PAM_SIZE 7 > #define I440FX_SMRAM 0x72 > > +/* Older coreboot versions (4.0 and older) read a config register that doesn't > + * exist in real hardware, to get the RAM size from QEMU. > + */ > +#define I440FX_COREBOOT_RAM_SIZE 0x57 > + > static void piix3_set_irq(void *opaque, int pirq, int level); > static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx); > static void piix3_write_config_xen(PCIDevice *dev, > @@ -394,7 +399,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, > if (ram_size > 255) { > ram_size = 255; > } > - d->config[0x57] = ram_size; > + d->config[I440FX_COREBOOT_RAM_SIZE] = ram_size; Thanks! Another magic number has now an actual meaning. Reviewed-by: Marcel Apfelbaum Thanks, Marcel > > i440fx_update_memory_mappings(f); > >