[libvirt] [PATCH 3/4] PowerPC cpu : Handle only high order 16 bits for PVR

[Prerna Saxena]

On Friday 10 October 2014 04:38 PM, Daniel P. Berrange wrote:
> On Fri, Oct 10, 2014 at 04:27:07PM +0530, Prerna Saxena wrote:
>> On Friday 10 October 2014 03:55 PM, Daniel P. Berrange wrote:
>>> On Fri, Oct 10, 2014 at 03:51:22PM +0530, Prerna Saxena wrote:
>>>> >From 1bcba78ed387a876208fec1ef60b81d65c8474e5 Mon Sep 17 00:00:00 2001
>>>> From: "Pradipta Kr. Banerjee" <bpradip at in.ibm.com>
>>>> Date: Tue, 7 Oct 2014 11:12:36 +0530
>>>>
>>>> IBM Power processors differ uniquely across generations (such as power6,
>>>> power7, power8). Each generation signifies a new PowerISA version
>>>> that exhibits features unique to that generation.
>>>> The higher 16 bits of PVR for IBM Power processors encode the CPU generation,
>>>> while the CPU chip (sub)version is encoded in lower 16 bits.
>>>>
>>>> For all practical purposes of launching a VM, we care about the
>>>> generation which the vCPU will belong to, and not specifically the chip
>>>> version. This patch updates the libvirt PVR check to reflect this
>>>> relationship.
>>>> It also contains changes to cpu_map.xml since processor generations
>>>> as understood by QEMU compat mode go as "power6", "power7" or "power8"
>>>> [Reference : QEMU commit 8dfa3a5e85 ]
>>>>
>>>> Signed-off-by: Pradipta Kr. Banerjee <bpradip at in.ibm.com>
>>>> Signed-off-by: Prerna Saxena <prerna at linux.vnet.ibm.com>
>>>> ---
>>>>  src/cpu/cpu_map.xml   | 22 ++++++++--------------
>>>>  src/cpu/cpu_powerpc.c |  7 ++++++-
>>>>  2 files changed, 14 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
>>>> index 18c7b0d..f2f13fb 100644
>>>> --- a/src/cpu/cpu_map.xml
>>>> +++ b/src/cpu/cpu_map.xml
>>>> @@ -602,30 +602,24 @@
>>>>      <vendor name='IBM'/>
>>>>
>>>>      <!-- IBM-based CPU models -->
>>>> -    <model name='POWER7'>
>>>> +    <model name='power6'>
>>>>        <vendor name='IBM'/>
>>>> -      <pvr value='0x003f0200'/>
>>>> +      <pvr value='0x003e0000'/>
>>>>      </model>
>>>>
>>>> -    <model name='POWER7_v2.1'>
>>>> +    <model name='power7'>
>>>>        <vendor name='IBM'/>
>>>> -      <pvr value='0x003f0201'/>
>>>> +      <pvr value='0x003f0000'/>
>>>>      </model>
>>>>
>>>> -    <model name='POWER7_v2.3'>
>>>> +    <model name='power7+'>
>>>>        <vendor name='IBM'/>
>>>> -      <pvr value='0x003f0203'/>
>>>> +      <pvr value='0x004a0000'/>
>>>>      </model>
>>>>
>>>> -    <model name='POWER7+_v2.1'>
>>>> +    <model name='power8'>
>>>>        <vendor name='IBM'/>
>>>> -      <pvr value='0x004a0201'/>
>>>> +      <pvr value='0x004b0000'/>
>>>>      </model>
>>>> -
>>>> -    <model name='POWER8_v1.0'>
>>>> -      <vendor name='IBM'/>
>>>> -      <pvr value='0x004b0100'/>
>>>> -    </model>
>>>> -
>>>>    </arch>
>>>>  </cpus>
>>> This looks like it is going to break upgrades for any existing deployed
>>> guests on PPC using named CPU models, which is not something we can do.
>>>
>> Hmm, if I let this be, it will break compatibility mode support
>> introduced in QEMU commit 8dfa3a5e85.
>> Would it make sense to add a new entry for each PowerPC CPU generation in
>> cpu_map.xml ?
>> Each generation will be distinguished from regular models by PVR-- each
>> of such entries will have lower 16 bits zero'ed.
>>
>> Something like :
>> <model name='power8'>
>>     <vendor name='IBM'/>
>>     <pvr value='0x004b0000'/>
>> </model>
>>
>> Does that look reasonable ?
> I don't really know what the PVF values mean. From the POV of libvirt
> though the key requirement is that existing deployed guests must not
> see *any* change in the features exposed in their CPU if their XML
> does not change, and we must retain compat with existing XML configs.
>
> So you can only add more CPUs, you cannot change existing CPUs.
>
> Regards,
> Daniel
Thanks for the tip!
I understand your perspective. I'll send out a v2 which will respect this requirement.

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India