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

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(a)in.ibm.com&gt; >>> 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(a)in.ibm.com&gt; >>> Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com&gt; >>> --- >>> 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