Re: [Qemu-devel] [PATCH 5/5] spapr: drop DIV_ROUND_UP() from xics_max_server_number()

[David Gibson]

On Wed, Feb 14, 2018 at 08:41:03PM +0100, Greg Kurz wrote:
> XICS needs to know the highest VCPU id that may be presented to the
> guest plus 1. Commit f303f117fec3 "spapr: ensure we have at least one
> XICS server" changed how the maximum is computed from:
> 
>         smp_cpus * kvmppc_smt_threads() / smp_threads
> 
> to:
> 
>         DIV_ROUND_UP(smp_cpus * kvmppc_smt_threads(), smp_threads)
> 
> This was done because at the time we could pass broken CPU topologies
> to the -smp command line options, such as threads=9,cpus=1. On a POWER8
> host this would give:
> 
>         1 * 8 / 9 == 0 servers
> 
> and cause QEMU to crash later during XICS setup.
> 
> The formulat evolved a bit to accomodate CPU hot-plug and VSMT, but
> most important, stricter checks are performed on the CPU topology.
> 
> With -smp threads=9,cpus=1:
> 
> qemu-system-ppc64:
>  cpu topology: sockets (1) * cores (1) * threads (9) > maxcpus (1)
> 
> With -smp threads=9,maxcpus=1:
> 
> qemu-system-ppc64: maxcpus must be equal to or greater than smp
> 
> More generally, machine types with hotplug support (2.7 and up), no
> longer allow to set maxcpus or smp_cpus to a value that isnt't a
> multiple of smp_threads.
> 
> With -smp threads=4,cpus=6:
> 
> qemu-system-ppc64: smp_cpus (6) must be multiple of threads (4)
> 
> With -smp threads=4,maxcpus=6:
> 
> qemu-system-ppc64: max_cpus (6) must be multiple of threads (4)
> 
> This means that the division is perfect and we don't need DIV_ROUND_UP(),
> and we could do a regular division:
> 
>         max_cpus * spapr->vsmt / smp_threads
> 
> So this patch changes xics_max_server_number() to use the spapr_vcpu_id(),
> which works too since max_cpus is a multiple of smp_threads:
> 
>     (max_cpus / smp_threads ) * spapr->vsmt + max_cpus % smp_threads
> 
> It breaks migration of pre-2.7 machine types with unusual CPU topologies,
> but I guess this is an acceptable trade-off.

No, not really.  Weird topologies are still allowed on old machine
types for backwards compatibility, and we shouldn't break that.  I
like the idea of consolidating this calculation, but we can't do it by
just breaking the older machines (at least not until they're formally
deprecated).

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