[Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)

Igor Mammedov posted 5 patches 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1505318697-77161-1-git-send-email-imammedo@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
include/hw/arm/armv7m.h        |  2 +-
include/hw/arm/aspeed_soc.h    |  2 +-
include/hw/arm/stm32f205_soc.h |  2 +-
include/hw/boards.h            |  6 ++++++
include/qom/cpu.h              | 22 +++++++++++++++++++
target/arm/cpu.h               |  3 +++
target/i386/cpu.h              |  9 ++++++++
bsd-user/main.c                |  4 ----
hw/alpha/dp264.c               |  4 ----
hw/arm/armv7m.c                | 40 +++++-----------------------------
hw/arm/aspeed_soc.c            | 13 +++++------
hw/arm/collie.c                | 10 +++------
hw/arm/exynos4210.c            |  6 +-----
hw/arm/gumstix.c               |  5 +++--
hw/arm/highbank.c              | 10 ++++-----
hw/arm/integratorcp.c          | 30 ++------------------------
hw/arm/mainstone.c             |  9 ++++----
hw/arm/mps2.c                  | 17 ++++++---------
hw/arm/musicpal.c              | 11 ++--------
hw/arm/netduino2.c             |  2 +-
hw/arm/nseries.c               |  4 +++-
hw/arm/omap1.c                 | 11 ++--------
hw/arm/omap2.c                 |  8 ++-----
hw/arm/omap_sx1.c              |  5 ++++-
hw/arm/palm.c                  |  5 +++--
hw/arm/pxa2xx.c                | 18 ++++------------
hw/arm/realview.c              | 25 +++++----------------
hw/arm/spitz.c                 | 12 ++++++-----
hw/arm/stellaris.c             | 16 ++++++--------
hw/arm/stm32f205_soc.c         |  4 ++--
hw/arm/strongarm.c             | 15 +++----------
hw/arm/tosa.c                  |  4 ----
hw/arm/versatilepb.c           | 15 +++----------
hw/arm/vexpress.c              | 32 +++++++++------------------
hw/arm/virt.c                  | 46 ++++++++-------------------------------
hw/arm/xilinx_zynq.c           | 10 ++-------
hw/arm/z2.c                    |  9 +++-----
hw/i386/pc.c                   | 41 +++++------------------------------
hw/i386/pc_piix.c              |  4 +---
hw/lm32/lm32_boards.c          |  8 -------
hw/lm32/milkymist.c            |  4 ----
hw/m68k/an5206.c               |  4 ----
hw/m68k/mcf5208.c              |  4 ----
hw/mips/cps.c                  |  4 ----
hw/mips/mips_fulong2e.c        |  4 ----
hw/mips/mips_jazz.c            |  4 ----
hw/mips/mips_malta.c           |  4 ----
hw/mips/mips_mipssim.c         |  4 ----
hw/mips/mips_r4k.c             |  4 ----
hw/moxie/moxiesim.c            |  4 ----
hw/openrisc/openrisc_sim.c     |  4 ----
hw/ppc/e500.c                  |  4 ----
hw/ppc/mac_newworld.c          |  4 ----
hw/ppc/mac_oldworld.c          |  4 ----
hw/ppc/ppc440_bamboo.c         |  4 ----
hw/ppc/ppc4xx_devs.c           |  5 -----
hw/ppc/prep.c                  |  9 --------
hw/ppc/virtex_ml507.c          |  4 ----
hw/sh4/r2d.c                   |  4 ----
hw/sh4/shix.c                  |  4 ----
hw/sparc/leon3.c               |  4 ----
hw/sparc/sun4m.c               |  4 ----
hw/sparc64/sparc64.c           |  4 ----
hw/tricore/tricore_testboard.c |  4 ----
hw/unicore32/puv3.c            |  4 ----
hw/xtensa/sim.c                |  5 -----
hw/xtensa/xtfpga.c             |  5 -----
linux-user/main.c              |  4 ----
qom/cpu.c                      | 49 +++++++++++++++++++++++-------------------
target/arm/cpu.c               |  2 +-
target/i386/cpu.c              |  3 ---
vl.c                           | 10 +++++++++
72 files changed, 194 insertions(+), 489 deletions(-)
[Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Posted by Igor Mammedov 6 years, 7 months ago
Changelog since v1:
 * fix merge conflicts with ignore_memory_transaction_failures
 * fix couple merge conflicts where SoC type string where replaced by type macro
 * keep plain prefix string in: strncmp(cpu_type, "pxa27", 5)
 * s/"%s" ARM_CPU_TYPE_SUFFIX/ARM_CPU_TYPE_NAME("%s")/
 * drop not needed assert
 * instead of checking error/reporting/exiting explicitly
   use error_fatal which will do all of it for us
 * squash in "cpu: rename cpu_parse_features() to cpu_parse_cpu_model()"


Issue 1:                                                                         
Some callers call CPUClass->parse_features manually to convert                   
'-cpu cpufoo,featurestr' string to cpu type and featurestr                       
into a set of global properties and then do controlled                           
cpu creation with setting properties and completing it with realize.             
That's a lot of code duplication as they all are practically                     
reimplement the same parsing logic.                                              
                                                                                 
Some use cpu_generic_init() instead which does the same parsing                  
along with creation/realizing cpu within one wrapper.                            
                                                                                 
And some trying to switch to controlled cpu creation,                            
implement object_new()/set properties/realize steps                              
but forget feature parsing logic witch leads to 'bugs'                           
commit (00909b585 hw/arm/integratorcp: Support specifying features via -cpu)     
                                                                                 
Issue 2:                                                                         
Default cpu model selection logic is spread over  all board's                    
machine_init() fuctions but it's basicallyi hardcodes default                    
cpu model string in init function.                                               
                                                                                 
 if (!cpu_model) {                                                               
     cpu_model = "some cpu model string";                                        
 }                                                                               
                                                                                 
and written in different ways.                                                   
it forces machine_init callbacks to parse cpu_model string                       
either by using cpu_generic_init() or by manually calling                        
cpu_class_by_name()/CPUClass::parse_features to perform                          
name to cpu type translation.

This series moves -cpu option parsing to generic machine code                    
that removes some of code duplication and makes cpus creation                    
process more generic/simple:                                                            
                                                                                 
 * unify default (fallback) cpu type handling by replacing                       
   hardcoded cpu_model strings with cpu type directly in                         
                                                                                 
   machine_foo_class_init() {                                                    
       MachineClass::default_cpu_type = BOARD_DEFAULT_CPU_TYPE                   
   }                                                                             
                                                                                 
   which allows to generalize move cpu model parsing instead of                  
   parsing it in each board.                                                     
                                                                                 
 * make generic machine vl.c parse cpu_model into properties/cpu_type            
   and let boards use cpu_type without any cpu_model prasing.                    
   Generic parsing will kick in only if board advertises its support             
   by setting MachineClass::default_cpu_type to a cpu type.                      
                                                                                 
PS:                                                                              
I intend make tree-wide conversion but as one series it's too many patches,       
so I'm splitting out it into an intial series that implements generic            
part and several patchsets that will do per target conversion.                   
                                                                                 
As part of initial series x86 and ARM targets conversion is included             
to showcase generalization usage. Per target conversions are done
as 1 patch per target, it might be too much for targets that have lots
of boards (ARM) so let me know if you'd like to split it on per board
basis (then I'll respin it as separate series on top of generic patches)

github tree for testing:
https://github.com/imammedo/qemu.git default_machine_cpu_type_PC_ARM_v2


Igor Mammedov (5):
  qom: cpus: split cpu_generic_init() on feature parsing and cpu
    creation parts
  cpu: make cpu_generic_init() abort QEMU on error
  vl.c: convert cpu_model to cpu type and set of global properties
    before machine_init()
  pc: use generic cpu_model parsing
  arm: drop intermediate cpu_model -> cpu type parsing and use cpu type
    directly

 include/hw/arm/armv7m.h        |  2 +-
 include/hw/arm/aspeed_soc.h    |  2 +-
 include/hw/arm/stm32f205_soc.h |  2 +-
 include/hw/boards.h            |  6 ++++++
 include/qom/cpu.h              | 22 +++++++++++++++++++
 target/arm/cpu.h               |  3 +++
 target/i386/cpu.h              |  9 ++++++++
 bsd-user/main.c                |  4 ----
 hw/alpha/dp264.c               |  4 ----
 hw/arm/armv7m.c                | 40 +++++-----------------------------
 hw/arm/aspeed_soc.c            | 13 +++++------
 hw/arm/collie.c                | 10 +++------
 hw/arm/exynos4210.c            |  6 +-----
 hw/arm/gumstix.c               |  5 +++--
 hw/arm/highbank.c              | 10 ++++-----
 hw/arm/integratorcp.c          | 30 ++------------------------
 hw/arm/mainstone.c             |  9 ++++----
 hw/arm/mps2.c                  | 17 ++++++---------
 hw/arm/musicpal.c              | 11 ++--------
 hw/arm/netduino2.c             |  2 +-
 hw/arm/nseries.c               |  4 +++-
 hw/arm/omap1.c                 | 11 ++--------
 hw/arm/omap2.c                 |  8 ++-----
 hw/arm/omap_sx1.c              |  5 ++++-
 hw/arm/palm.c                  |  5 +++--
 hw/arm/pxa2xx.c                | 18 ++++------------
 hw/arm/realview.c              | 25 +++++----------------
 hw/arm/spitz.c                 | 12 ++++++-----
 hw/arm/stellaris.c             | 16 ++++++--------
 hw/arm/stm32f205_soc.c         |  4 ++--
 hw/arm/strongarm.c             | 15 +++----------
 hw/arm/tosa.c                  |  4 ----
 hw/arm/versatilepb.c           | 15 +++----------
 hw/arm/vexpress.c              | 32 +++++++++------------------
 hw/arm/virt.c                  | 46 ++++++++-------------------------------
 hw/arm/xilinx_zynq.c           | 10 ++-------
 hw/arm/z2.c                    |  9 +++-----
 hw/i386/pc.c                   | 41 +++++------------------------------
 hw/i386/pc_piix.c              |  4 +---
 hw/lm32/lm32_boards.c          |  8 -------
 hw/lm32/milkymist.c            |  4 ----
 hw/m68k/an5206.c               |  4 ----
 hw/m68k/mcf5208.c              |  4 ----
 hw/mips/cps.c                  |  4 ----
 hw/mips/mips_fulong2e.c        |  4 ----
 hw/mips/mips_jazz.c            |  4 ----
 hw/mips/mips_malta.c           |  4 ----
 hw/mips/mips_mipssim.c         |  4 ----
 hw/mips/mips_r4k.c             |  4 ----
 hw/moxie/moxiesim.c            |  4 ----
 hw/openrisc/openrisc_sim.c     |  4 ----
 hw/ppc/e500.c                  |  4 ----
 hw/ppc/mac_newworld.c          |  4 ----
 hw/ppc/mac_oldworld.c          |  4 ----
 hw/ppc/ppc440_bamboo.c         |  4 ----
 hw/ppc/ppc4xx_devs.c           |  5 -----
 hw/ppc/prep.c                  |  9 --------
 hw/ppc/virtex_ml507.c          |  4 ----
 hw/sh4/r2d.c                   |  4 ----
 hw/sh4/shix.c                  |  4 ----
 hw/sparc/leon3.c               |  4 ----
 hw/sparc/sun4m.c               |  4 ----
 hw/sparc64/sparc64.c           |  4 ----
 hw/tricore/tricore_testboard.c |  4 ----
 hw/unicore32/puv3.c            |  4 ----
 hw/xtensa/sim.c                |  5 -----
 hw/xtensa/xtfpga.c             |  5 -----
 linux-user/main.c              |  4 ----
 qom/cpu.c                      | 49 +++++++++++++++++++++++-------------------
 target/arm/cpu.c               |  2 +-
 target/i386/cpu.c              |  3 ---
 vl.c                           | 10 +++++++++
 72 files changed, 194 insertions(+), 489 deletions(-)

-- 
2.7.4


Re: [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Posted by Igor Mammedov 6 years, 7 months ago
On Wed, 13 Sep 2017 18:04:52 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> Changelog since v1:
>  * fix merge conflicts with ignore_memory_transaction_failures
>  * fix couple merge conflicts where SoC type string where replaced by type macro
>  * keep plain prefix string in: strncmp(cpu_type, "pxa27", 5)
>  * s/"%s" ARM_CPU_TYPE_SUFFIX/ARM_CPU_TYPE_NAME("%s")/
>  * drop not needed assert
>  * instead of checking error/reporting/exiting explicitly
>    use error_fatal which will do all of it for us
>  * squash in "cpu: rename cpu_parse_features() to cpu_parse_cpu_model()"
> 
> 
> Issue 1:                                                                         
> Some callers call CPUClass->parse_features manually to convert                   
> '-cpu cpufoo,featurestr' string to cpu type and featurestr                       
> into a set of global properties and then do controlled                           
> cpu creation with setting properties and completing it with realize.             
> That's a lot of code duplication as they all are practically                     
> reimplement the same parsing logic.                                              
>                                                                                  
> Some use cpu_generic_init() instead which does the same parsing                  
> along with creation/realizing cpu within one wrapper.                            
>                                                                                  
> And some trying to switch to controlled cpu creation,                            
> implement object_new()/set properties/realize steps                              
> but forget feature parsing logic witch leads to 'bugs'                           
> commit (00909b585 hw/arm/integratorcp: Support specifying features via -cpu)     
>                                                                                  
> Issue 2:                                                                         
> Default cpu model selection logic is spread over  all board's                    
> machine_init() fuctions but it's basicallyi hardcodes default                    
> cpu model string in init function.                                               
>                                                                                  
>  if (!cpu_model) {                                                               
>      cpu_model = "some cpu model string";                                        
>  }                                                                               
>                                                                                  
> and written in different ways.                                                   
> it forces machine_init callbacks to parse cpu_model string                       
> either by using cpu_generic_init() or by manually calling                        
> cpu_class_by_name()/CPUClass::parse_features to perform                          
> name to cpu type translation.
> 
> This series moves -cpu option parsing to generic machine code                    
> that removes some of code duplication and makes cpus creation                    
> process more generic/simple:                                                            
>                                                                                  
>  * unify default (fallback) cpu type handling by replacing                       
>    hardcoded cpu_model strings with cpu type directly in                         
>                                                                                  
>    machine_foo_class_init() {                                                    
>        MachineClass::default_cpu_type = BOARD_DEFAULT_CPU_TYPE                   
>    }                                                                             
>                                                                                  
>    which allows to generalize move cpu model parsing instead of                  
>    parsing it in each board.                                                     
>                                                                                  
>  * make generic machine vl.c parse cpu_model into properties/cpu_type            
>    and let boards use cpu_type without any cpu_model prasing.                    
>    Generic parsing will kick in only if board advertises its support             
>    by setting MachineClass::default_cpu_type to a cpu type.                      
>                                                                                  
> PS:                                                                              
> I intend make tree-wide conversion but as one series it's too many patches,       
> so I'm splitting out it into an intial series that implements generic            
> part and several patchsets that will do per target conversion.                   
>                                                                                  
> As part of initial series x86 and ARM targets conversion is included             
> to showcase generalization usage. Per target conversions are done
> as 1 patch per target, it might be too much for targets that have lots
> of boards (ARM) so let me know if you'd like to split it on per board
> basis (then I'll respin it as separate series on top of generic patches)
> 
> github tree for testing:
> https://github.com/imammedo/qemu.git default_machine_cpu_type_PC_ARM_v2
> 
Hi Eduardo,

Could you merge it via machine tree?

Re: [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Posted by Igor Mammedov 6 years, 7 months ago
On Fri, 15 Sep 2017 17:03:26 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

[...]
> > 
> Hi Eduardo,
> 
> Could you merge it via machine tree?
Forgot to actually CC Eduardo,

Just noticed a patch (1) on list adding a new soc that would break,
due to armv7 dropping cpu-model property in 5/5.
It would be better to merge this series asap so that people
could rebase their work on cpu-type based approach, instead of
continuing copy-past old cpu-model based code.

1)
[Qemu devel v9 PATCH 4/5] msf2: Add Smartfusion2 SoC

CCing Subbaraya so he could amend patch to use cpu-type instead of cpu-model.

Re: [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Posted by Eduardo Habkost 6 years, 7 months ago
On Mon, Sep 18, 2017 at 06:42:13PM +0200, Igor Mammedov wrote:
> On Fri, 15 Sep 2017 17:03:26 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> [...]
> > > 
> > Hi Eduardo,
> > 
> > Could you merge it via machine tree?
> Forgot to actually CC Eduardo,
> 
> Just noticed a patch (1) on list adding a new soc that would break,
> due to armv7 dropping cpu-model property in 5/5.
> It would be better to merge this series asap so that people
> could rebase their work on cpu-type based approach, instead of
> continuing copy-past old cpu-model based code.
> 
> 1)
> [Qemu devel v9 PATCH 4/5] msf2: Add Smartfusion2 SoC
> 
> CCing Subbaraya so he could amend patch to use cpu-type instead of cpu-model.

Queued on machine-next:

 https://github.com/ehabkost/qemu.git machine-next

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Posted by Igor Mammedov 6 years, 7 months ago
On Tue, 19 Sep 2017 09:11:25 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Sep 18, 2017 at 06:42:13PM +0200, Igor Mammedov wrote:
> > On Fri, 15 Sep 2017 17:03:26 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> > 
> > [...]  
> > > >   
> > > Hi Eduardo,
> > > 
> > > Could you merge it via machine tree?  
> > Forgot to actually CC Eduardo,
> > 
> > Just noticed a patch (1) on list adding a new soc that would break,
> > due to armv7 dropping cpu-model property in 5/5.
> > It would be better to merge this series asap so that people
> > could rebase their work on cpu-type based approach, instead of
> > continuing copy-past old cpu-model based code.
> > 
> > 1)
> > [Qemu devel v9 PATCH 4/5] msf2: Add Smartfusion2 SoC
> > 
> > CCing Subbaraya so he could amend patch to use cpu-type instead of cpu-model.  
> 
> Queued on machine-next:
> 
>  https://github.com/ehabkost/qemu.git machine-next
> 

Thanks!

Re: [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago
On 09/19/2017 09:11 AM, Eduardo Habkost wrote:
>> Igor Mammedov <imammedo@redhat.com> wrote:
[...]
>>> Could you merge it via machine tree?
[...]
> 
> Queued on machine-next:

Thanks Eduardo for applying the !fixup :)