Re: [Qemu-devel] [PATCH v4 2/5] pc: fw_cfg: move ioport base constant to pc.h

From: Laszlo Ersek <lersek@redhat.com>
To: "Gabriel L. Somlo" <somlo@cmu.edu>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, drjones@redhat.com,
	matt.fleming@intel.com, ehabkost@redhat.com, mst@redhat.com,
	zhaoshenglong@huawei.com, leif.lindholm@linaro.org,
	ard.biesheuvel@linaro.org, kevin@koconnor.net, kraxel@redhat.com,
	pbonzini@redhat.com, imammedo@redhat.com, markmb@redhat.com,
	rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH v4 2/5] pc: fw_cfg: move ioport base constant to pc.h
Date: Tue, 29 Sep 2015 12:20:48 +0200	[thread overview]
Message-ID: <560A6600.8000701@redhat.com> (raw)
In-Reply-To: <1443389342-2186-3-git-send-email-somlo@cmu.edu>

On 09/27/15 23:28, Gabriel L. Somlo wrote:
> Move BIOS_CFG_IOPORT define from pc.c to pc.h, and rename
> it to FW_CFG_IO_BASE.
> 
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
>  hw/i386/pc.c         | 5 ++---
>  include/hw/i386/pc.h | 2 ++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 461c128..70a5898 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -86,7 +86,6 @@ void pc_set_legacy_acpi_data_size(void)
>      acpi_data_size = 0x10000;
>  }
>  
> -#define BIOS_CFG_IOPORT 0x510
>  #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
>  #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
>  #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
> @@ -760,7 +759,7 @@ static FWCfgState *bochs_bios_init(void)
>      int i, j;
>      unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
>  
> -    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
> +    fw_cfg = fw_cfg_init_io(FW_CFG_IO_BASE);
>      /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
>       *
>       * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
> @@ -1292,7 +1291,7 @@ FWCfgState *xen_load_linux(PCMachineState *pcms,
>  
>      assert(MACHINE(pcms)->kernel_filename != NULL);
>  
> -    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
> +    fw_cfg = fw_cfg_init_io(FW_CFG_IO_BASE);
>      rom_set_fw(fw_cfg);
>  
>      load_linux(pcms, fw_cfg);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ab5413f..86007e3 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -211,6 +211,8 @@ typedef void (*cpu_set_smm_t)(int smm, void *arg);
>  
>  void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
>  
> +#define FW_CFG_IO_BASE 0x510
> +
>  /* acpi_piix.c */
>  
>  I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> 

I'm torn whether this should be unified with the *other*
BIOS_CFG_IOPORT* macro definitions (and uses), but since those are
independent (justifiedly), it probably shouldn't.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>