That original coreboot code certainly looks like a mistake. Thanks for
helping close the decade-long loop.
On Fri, Aug 7, 2015 at 12:15 PM, Eduardo Habkost <ehabk...@redhat.com>
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 <eswi...@skyportsystems.com>
> Cc: Richard Smith <smithb...@gmail.com>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
> 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;
>
> i440fx_update_memory_mappings(f);
>
> --
> 2.1.0
>
>
