| Message ID | 20190524063553.5339-11-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine | expand |
Philippe Mathieu-Daudé <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> > Signed-off-by: Philippe Mathieu-Daudé <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? Thanks, Li Qiang > { > 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 > > >
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 > >
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) { 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);
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> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/i386/pc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)