Re: [Qemu-devel] [PATCH 6/6] spapr: fix migration of ICP objects from/to older QEMU

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2970 bytes --]

On Wed, May 17, 2017 at 10:33:44PM +0200, Greg Kurz wrote:
> On Wed, 17 May 2017 14:18:16 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, May 15, 2017 at 06:11:27PM +0200, Cédric Le Goater wrote:
> > > >>> +        int smt = kvmppc_smt_threads();
> > > >>> +        int nr_servers = DIV_ROUND_UP(max_cpus * smt, smp_threads);
> > > >>
> > > >> may be we should reintroduce nr_servers at the machine level ? 
> > > >>  
> > > > 
> > > > I had reintroduced it but then I realized it was only used in this
> > > > function.  
> > > 
> > > nr_servers is also used when the device tree is populated with the 
> > > interrupt controller nodes. No big deal.  
> > 
> > Which is guest visible, so we should really make that stay the same
> > for older machine types.  I'd like to avoid re-introducing nr_servers
> > as a property if we can, but maybe we can't.
> > 
> 
> Yes we can :) or at least maybe, if you can shed light on a guest
> visible change introduced by this commit in 2.8:
> 
> commit 9b9a19080a6e548b91420ce7925f2ac81ef63ae8
> Author: David Gibson <david@gibson.dropbear.id.au>
> Date:   Thu Oct 20 16:07:56 2016 +1100
> 
>     pseries: Move construction of /interrupt-controller fdt node
> 
> 
> It changes the "ibm,interrupt-server-ranges" property in the device
> tree from
> 
>     {0, cpu_to_be32(max_cpus)}
> 
> to
> 
>     {0, cpu_to_be32(xics->nr_servers)}
> 
> ie, {0, cpu_to_be32(DIV_ROUND_UP(max_cpus * smt, smp_threads))}
> 
> And indeed, if I start QEMU with
> 
>  -smp cores=2,threads=4,maxcpus=16 -machine type=pseries-2.7,accel=kvm
> 
> the following is exposed to the guest with 2.7:
> 
> ibm,interrupt-server-ranges
>                  00000000 00000010
> 
> and with 2.8 we get:
> 
> ibm,interrupt-server-ranges
>                  00000000 00000020
> 
> LoPAPR B.6.9.1.1 says that the range (ie, the second number) "shall be the
> number of contiguous server#s supported by the unit (this also corresponds
> to the number of “reg” entries)". I'm inclined to think this maps to max_cpus
> but I may be wrong... any clues ?

So, yes, it's a guest visible change in some configurations, but in
those configurations the previous behaviour was sufficiently broken
that I think that's preferable to not changing it.

Basically the ibm,interrupt-server-ranges property gives the total
range of valid server IDs.  The server IDs assigned to each thread are
based on the dt_id, and are spaced by the number of host threads, not
the number of guest threads.  That means that with 2.7, the last cpus
would have advertised ibm,ppc-interrupt-server#s properties with
values outside the range given by ibm,interrupt-server-ranages, which
is definitely wrong.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]