Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 05/17/2014 12:16 AM, Alexander Graf wrote:
> 
> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
>> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
>> list. However if the kernel is quite old, it may be missing a definition
>> of the actual CPU. To provide an ability for old kernels to work on modern
>> hardware, a Processor Compatibility Mode has been introduced
>> by the PowerISA specification.
>>
>>  From the hardware prospective, it is supported by the Processor
>> Compatibility Register (PCR) which is defined in PowerISA. The register
>> enables one of the compatibility modes (2.05/2.06/2.07).
>> Since PCR is a hypervisor privileged register and cannot be
>> accessed from the guest, the mode selection is done via
>> ibm,client-architecture-support (CAS) RTAS call using which the guest
>> specifies what "raw" and "architected" CPU versions it supports.
>> QEMU works out the best match, changes a "cpu-version" property of
>> every CPU and notifies the guest about the change by setting these
>> properties in the buffer passed as a response on a custom H_CAS hypercall.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>>   hw/ppc/spapr_hcall.c | 83
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   trace-events         |  5 ++++
>>   3 files changed, 128 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index a2c9106..a0882a1 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -35,6 +35,7 @@
>>   #include "kvm_ppc.h"
>>   #include "mmu-hash64.h"
>>   #include "cpu-models.h"
>> +#include "qom/cpu.h"
>>     #include "hw/boards.h"
>>   #include "hw/ppc/ppc.h"
>> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
>> target_ulong size)
>>   {
>>       void *fdt;
>>       sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>> +    CPUState *cs;
>> +    int smt = kvmppc_smt_threads();
>>         size -= sizeof(hdr);
>>         fdt = g_malloc0(size);
>>       _FDT((fdt_create(fdt, size)));
>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>> +
>> +    CPU_FOREACH(cs) {
>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>> +        int smpt = spapr_get_compat_smp_threads(cpu);
>> +        int i, index = ppc_get_vcpu_dt_id(cpu);
>> +        uint32_t servers_prop[smpt];
>> +        uint32_t gservers_prop[smpt * 2];
>> +        char tmp[32];
>> +
>> +        if ((index % smt) != 0) {
>> +            continue;
>> +        }
>> +
>> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
>> +        trace_spapr_cas_add_subnode(tmp);
>> +
>> +        _FDT((fdt_begin_node(fdt, tmp)));
>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>> +
>> +        /* Build interrupt servers and gservers properties */
>> +        for (i = 0; i < smpt; i++) {
>> +            servers_prop[i] = cpu_to_be32(index + i);
>> +            /* Hack, direct the group queues back to cpu 0 */
>> +            gservers_prop[i*2] = cpu_to_be32(index + i);
>> +            gservers_prop[i*2 + 1] = 0;
>> +        }
>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>> +                           servers_prop, sizeof(servers_prop))));
>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>> +                           gservers_prop, sizeof(gservers_prop))));
>> +
>> +        _FDT((fdt_end_node(fdt)));
>> +    }
>> +
>> +    _FDT((fdt_end_node(fdt)));
> 
> Why is this so much code? Can we only replace full nodes?

It is a diff tree, in only has properties to change.

> Then please
> extract the original CPU node creation into a function and just call it
> from the original generation and from here.
> 
> If we can also replace single properties I'd say we only need to override
> cpu-version.


Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
must not see 8 threads but it can update itself and reboot with the kernel
which is aware of POWER8. You are saying we do not want to support that?




-- 
Alexey