Re: [libvirt] [PowerPC Patch v3] Libvirt CPU enhancements for Power KVM

On 26.10.2014 11:56, Prerna Saxena wrote: > This patch series is a collection of enhancements for PowerPC CPUs on PowerKVM. > > Series Summary: > ========== > Patch 1/3 : Introduce a new architecture 'ppc64le' for libvirt. > Patch 2/3 : Add libvirt support for VMs running in 'compat' mode on Power KVM. > Patch 3/3 : Optimize PVR comparison for PowerPC CPUs. > > Detail: > ==== > * PowerPC has traditionally been a Big-endian architecture. However, with PowerPC ISA version 2.07, it can run in Little-endian mode as well. IBM Power8 processors, compliant with ISA 2.07 allow > launching VMs in little-endian mode. This is signified by 'ppc64le' architecture. Patch 1 adds this support to libvirt, to allow running VMs based on ppc64le architecture. > > * Patch 2,3 tweak libvirt to correctly model PowerPC CPUs based on recent PowerKVM implementation. > > PowerKVM permits VMs with vcpus in the following allowed modes : > i) Host native mode: > where the vcpu seen in the VM belongs to the same processor generation as the host. > Example: A POWER7 host, conforming to PowerISA version 2.06, will run VMs with "power7" vcpus. > ii) Binary Compatibility ("compat") mode: > PowerISA allows processors to run VMs in binary compatibility ("compat") mode supporting an older version of ISA. > As an example: In compatibility mode, a POWER7 host can run a "power6" VM, conforming to power ISA v2.05. > Similarly, a POWER8 host can run a "power7" VM conforming to PowerISA v2.06. > > QEMU has recently added support to explicitly denote a VM running in compatibility mode through commits 6d9412ea & 8dfa3a5e85. Henceforth, VMs of type (i) will be invoked with the QEMU invocation > "-cpu host", while VMs of type (ii) will be invoked using "-cpu host, compat=power6". > Now, an explicit cpu selection using "-cpu POWER6" is not valid. Instead, the recommended practice is to use the matching compat mode, if the requested cpu type differs from the host. > Patches 2-3 address various aspects of this change. > > * Patch 2 : Adds support for generating the correct command line for QEMU. New xml semantics are introduced to signify this type. > > * Patch 3 : PowerKVM vCPUs 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 order 16 bits of PVR denote the processor generation and the lower order 16 bits denote the cpu chip (sub)version. > For all practical purposes of launching a VM, we care about the generation the vCPU will belong to, and not specifically the chip version. In fact, PowerKVM does not seek out specification of a > unique chip version(such as POWER7_v2.3) for running a vCPU. This patch updates the libvirt PVR check to reflect this relationship. > > > Changelog: > ========= > v1 : https://www.redhat.com/archives/libvir-list/2014-June/msg01338.html > v2 : http://www.redhat.com/archives/libvir-list/2014-October/msg00351.html > > Changes since v2: > i) CPU compat mode is now introduced under CPU mode 'host-model' as against the original proposal of using cpu mode'custom'. (Thanks for the tip, Dan.) > ii) Patch 4/4 that renamed models in cpu_map.xml is dropped. Instead, CPU generations are added to cpu_map.xml as new listings. The PVR check in patch 3 of this series takes care of making the > right match happen for compat mode. > > Regards, > So, I went through the patches and they look good to me from code POV. However, I couldn't ACK 2/3 due to missing documentation. I'm not sure if you are okay with posting v4 and possibly slipping this release (depending when DV freezes the upstream) or just post a follow up patch that would be squashed into 2/3 prior to pushing.