All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm)
Date: Fri, 15 Sep 2017 17:03:26 +0200	[thread overview]
Message-ID: <20170915170326.5278d578@nial.brq.redhat.com> (raw)
In-Reply-To: <1505318697-77161-1-git-send-email-imammedo@redhat.com>

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?

  parent reply	other threads:[~2017-09-15 15:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-13 16:04 [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm) Igor Mammedov
2017-09-13 16:04 ` [Qemu-devel] [PATCH v2 1/5] qom: cpus: split cpu_generic_init() on feature parsing and cpu creation parts Igor Mammedov
2017-09-13 16:04 ` [Qemu-devel] [PATCH v2 2/5] cpu: make cpu_generic_init() abort QEMU on error Igor Mammedov
2017-09-13 16:04 ` [Qemu-devel] [PATCH v2 3/5] vl.c: convert cpu_model to cpu type and set of global properties before machine_init() Igor Mammedov
2017-09-14 18:19   ` Philippe Mathieu-Daudé
2017-09-13 16:04 ` [Qemu-devel] [PATCH v2 4/5] pc: use generic cpu_model parsing Igor Mammedov
2017-09-13 16:04 ` [Qemu-devel] [PATCH v2 5/5] arm: drop intermediate cpu_model -> cpu type parsing and use cpu type directly Igor Mammedov
2017-09-14  3:47   ` Philippe Mathieu-Daudé
2017-09-14  7:50     ` Igor Mammedov
2017-09-14 22:09       ` Alistair Francis
2017-09-14 17:57   ` Alistair Francis
2017-09-18 21:53   ` [Qemu-devel] [PATCH] !fixup arm: missed comment update Philippe Mathieu-Daudé
2017-09-15 15:03 ` Igor Mammedov [this message]
2017-09-18 16:42   ` [Qemu-devel] [PATCH v2 0/5] generalize parsing of cpu_model (x86/arm) Igor Mammedov
2017-09-19 12:11     ` Eduardo Habkost
2017-09-19 12:46       ` Igor Mammedov
2017-09-19 12:56       ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170915170326.5278d578@nial.brq.redhat.com \
    --to=imammedo@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.