Re: [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/24/19 5:30 PM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于
> 2019年5月24日周五 下午2:50写道：
> 
>     The boot_cpus is used once. Pass it by argument, this will
>     allow us to remove the PCMachineState argument later.
> 
>     Suggested-by: Samuel Ortiz <sameo@linux.intel.com
>     <mailto:sameo@linux.intel.com>>
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com>>
>     ---
>      hw/i386/pc.c | 6 +++---
>      1 file changed, 3 insertions(+), 3 deletions(-)
> 
>     diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>     index 264074489b..01894b9875 100644
>     --- a/hw/i386/pc.c
>     +++ b/hw/i386/pc.c
>     @@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
>          }
>      }
> 
>     -static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
>     +static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t
>     boot_cpus)
> 
> 
> For the patches 10/11/12, I don't think this is an elegant solution.
> When we add more data like 'boot_cpus'
> we need add more arguments?

This fonction is called once at machine creation, so there is no
performance penalty. To keep the code modularizable (reusable) it is an
acceptable tradeoff :)

> 
>      {
>          FWCfgState *fw_cfg;
>          uint64_t *numa_fw_cfg;
>     @@ -938,7 +938,7 @@ static FWCfgState
>     *x86_create_fw_cfg(PCMachineState *pcms)
> 
>          fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
>                                      &address_space_memory);
>     -    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
>     +    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
> 
>          /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
>           *
>     @@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
>                                              option_rom_mr,
>                                              1);
> 
>     -    fw_cfg = x86_create_fw_cfg(pcms);
>     +    fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
> 
>          rom_set_fw(fw_cfg);
> 
>     -- 
>     2.20.1
> 
>