Re: [Qemu-ppc] [PATCH] spapr: add ibm, chip-id property in device tree

[Alexey Kardashevskiy]

On 04/14/2014 05:41 PM, Alexander Graf wrote:
> 
> On 11.04.14 18:41, Alexey Kardashevskiy wrote:
>> On 04/12/2014 02:29 AM, Alexander Graf wrote:
>>> On 13.03.14 07:29, Alexey Kardashevskiy wrote:
>>>> This adds a "ibm,chip-id" property for CPU nodes which should be the same
>>>> for all cores in the same CPU socket. The recent guest kernels use this
>>>> information to associate threads with sockets.
>>>>
>>>> Refer to the kernel commit 256f2d4b463d3030ebc8d2b54f427543814a2bdc
>>>> for more details.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>> ---
>>>>    hw/ppc/spapr.c | 9 +++++++++
>>>>    1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index bf46c38..6366230 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -308,6 +308,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>>        uint32_t interrupt_server_ranges_prop[] = {0,
>>>> cpu_to_be32(smp_cpus)};
>>>>        int i, smt = kvmppc_smt_threads();
>>>>        unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
>>>> +    QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
>>>> +    unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0)
>>>> : 0;
>>>>          fdt = g_malloc0(FDT_MAX_SIZE);
>>>>        _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
>>>> @@ -465,6 +467,13 @@ static void *spapr_create_fdt_skel(hwaddr
>>>> initrd_base,
>>>>                                   page_sizes_prop,
>>>> page_sizes_prop_size)));
>>>>            }
>>>>    +        if (sockets) {
>>>> +            int cpus_per_socket = smp_cpus / sockets;
>>>> +            uint32_t chip_id = cs->cpu_index / cpus_per_socket;
>>>> +
>>>> +            _FDT((fdt_property_cell(fdt, "ibm,chip-id", chip_id)));
>>>> +        }
>>> Have you verified this works correctly with threads?
>> Sorry, do not follow you.
>> -smp X,sockets=Y,threads=Z - what combination of XYZ is suspicious?
> 
> No, I think it works. Does it work for non-power-of-2 socket numbers?

Yes.
-smp 12,sockets=3,threads=4

address@hidden cpus]# find -iname ibm,chip-id
./PowerPC,address@hidden/ibm,chip-id
./PowerPC,address@hidden/ibm,chip-id
./PowerPC,address@hidden/ibm,chip-id
address@hidden cpus]# find -iname ibm,chip-id -exec hexdump {} \;
0000000 0000 0002
0000004
0000000 0000 0001
0000004
0000000 0000 0000
0000004

address@hidden cpus] cat /proc/cpuinfo
processor       : 0
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 1
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 2
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 3
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 4
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 5
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 6
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 7
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 8
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 9
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 10
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

processor       : 11
cpu             : POWER7 (raw), altivec supported
clock           : 3720.000000MHz
revision        : 2.1 (pvr 003f 0201)

timebase        : 512000000
platform        : pSeries
model           : IBM pSeries (emulated by qemu)
machine         : CHRP IBM pSeries (emulated by qemu)


> 
>>
>>
>>> Also, I don't see why
>>> we should omit the chip-id when we don't define sockets.
>> Why should we pollute device tree...
> 
> What does pHyp do? If we say we don't want to "pollute the device tree" for
> the sockets=1 case then this should be explicit. Right now we put the
> chip-id in when you say sockets=1, but not if you omit it.

It is made up thing which is absent in pHyp.
Ok, I'll enable it always, won't hurt.


> 
> 
> Alex
> 


-- 
Alexey