Re: [Qemu-devel] [PATCHv4 0/5] Clean up compatibility mode handling

[David Gibson]

On Fri, Jun 02, 2017 at 10:15:25AM +0200, Greg Kurz wrote:
> On Fri, 2 Jun 2017 12:00:07 +1000
> David Gibson <address@hidden> wrote:
> 
> > On Thu, Jun 01, 2017 at 03:09:15PM +0200, Greg Kurz wrote:
> > > On Thu, 1 Jun 2017 13:59:14 +0200
> > > Cédric Le Goater <address@hidden> wrote:
> > >   
> > > > On 06/01/2017 08:52 AM, David Gibson wrote:  
> > > > > On Wed, May 31, 2017 at 10:58:57AM +0200, Greg Kurz wrote:    
> > > > >> On Wed, 31 May 2017 12:57:48 +1000
> > > > >> David Gibson <address@hidden> wrote:    
> > > > >>> [...]    
> > > > >>>> All old non-pseries machine types already complain when started 
> > > > >>>> with
> > > > >>>> a POWER7 or newer CPU. Providing the extra error message looks 
> > > > >>>> weird:
> > > > >>>>
> > > > >>>> qemu-system-ppc64 -machine ppce500 \
> > > > >>>>                   -cpu POWER7,compat=power6
> > > > >>>> qemu-system-ppc64: CPU 'compat' property is deprecated and has no 
> > > > >>>> effect;
> > > > >>>>  use max-cpu-compat machine property instead
> > > > >>>> MMU model 983043 not supported by this machine.
> > > > >>>>
> > > > >>>> but I guess it's better than crashing. :)      
> > > > >>>
> > > > >>> Well, sure POWER7 doesn't make sense for an e500 machine for other
> > > > >>> reasons.  But POWER7 or POWER8 _would_ make sense for powernv, where
> > > > >>> compat= doesn't.
> > > > >>>    
> > > > >>
> > > > >> The powernv machine type doesn't even support CPU features at all:
> > > > >>
> > > > >>     chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", 
> > > > >> machine->cpu_model);
> > > > >>     if (!object_class_by_name(chip_typename)) {
> > > > >>         error_report("invalid CPU model '%s' for %s machine",
> > > > >>                      machine->cpu_model, 
> > > > >> MACHINE_GET_CLASS(machine)->name);
> > > > >>         exit(1);
> > > > >>     }    
> > > > > 
> > > > > Ah, well, that's another bug, but not one that's in scope for this
> > > > > series.    
> > > > 
> > > > PowerNV is still work in progress. I would not worry about it too much.
> > > >   
> > > 
> > > Of course and this isn't the purpose of the discussion actually. We were
> > > talking about CPU features being relevant or not depending on the machine
> > > type.
> > > 
> > > But I'm not even sure that CPU features are useful at all for ppc, not to
> > > say very confusing (otherwise this series wouldn't be needed for example).
> > > 
> > > Speaking of PowerNV, just as an example, I guess the fix would be to
> > > forbid machine->cpu_model if it contains features. And probably the same
> > > for all other machine types, except pseries for backward compatibility
> > > reasons.  
> > 
> > I don't think that's correct in principle.  I can imagine CPU
> > properties it might make sense to really set on the cpu, regardless of
> > machine type.  A quick look says we don't have any such at the moment,
> > but I don't think it's something we should prevent as a matter of policy.
> 
> Fair enough. Then maybe all machine should parse CPU features and check which
> one are valid before instantiating the CPUs ?

Well, CPu properties *should* be valid for all machine types.  The
fact that compat= wasn't was a mistake.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature