Merge ~paelzer/ubuntu/+source/qemu:lp1776189-hpb-cosmic into ubuntu/+source/qemu:ubuntu/devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

This can be tested from PPA [1]
and was Regression tested against the functionally equal (other type names) [2]

Initially implemented for the bug, then adapted for our discussion around naming.
Finally regression tests found an issue in the type aliasing that was fixed as well.

Example without the new type using more than 1TB of memory:
The guest starts up a while, but when initializing memory it crashes and the guest log will show:
KVM internal error. Suberror: 1
emulation failure
RAX=0000000000000000 RBX=0000000000000046 RCX=0000000000000000 RDX=00000000000003f4
...

With the new type set (e.g. via virsh edit) the guest works.

[1]: https://launchpad.net/~canonical-server/+archive/ubuntu/large-virt
[2]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3284

[Christian Ehrhardt  (paelzer)]

Added the planned NEWS file, now ready for review.

[Robie Basak (racb)]

Looks good, thanks. I appreciate the detailed description and rationale included. A few minor inline comments.

The patch itself I'm not confident in reviewing myself in detail as I'm not familiar with qemu internals. It looks reasonable to me though, and doesn't look like it'll impact anything else as it's adding a new type rather than making changes to anything existing. I did try to compare to RH's patch but as that one changes an existing machine type it is rather different.

Maybe get Ryan to review the qemu patch, if time permits?

[Robie Basak (racb)]

review: Approve

[Christian Ehrhardt  (paelzer)]

It was all a nice thought with the "+" sign.
But it is not allowed in all layers of the stack above as machine type.

So changing this to be a - as it was before.

[Christian Ehrhardt  (paelzer)]

MP here updated to have the "-" based diff now, no other changes, so not resetting the former ack.

[Ryan Harper (raharper)]

Couple of nits in-line.

[Christian Ehrhardt  (paelzer)]

Adressed Ryans Nits and pushed again.

[Christian Ehrhardt  (paelzer)]

Agreed on IRC to consider ryan as an ack after Nits were adressed.
This is merged now, setting status accordingly.

diff --git a/debian/changelog b/debian/changelog
index c5f3553..6922757 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+qemu (1:2.11+dfsg-1ubuntu11) cosmic; urgency=medium
+
+  * d/p/ubuntu/machine-type-hpb.patch: add -hpb machine type
+    for host-phys-bits=true (LP: #1776189)
+    - add an info about this change in debian/qemu-system-x86.NEWS
+
+ -- Christian Ehrhardt <[e-mail]>  Tue, 12 Jun 2018 09:01:00 +0200
+
 qemu (1:2.11+dfsg-1ubuntu10) cosmic; urgency=medium
 
   * SECURITY UPDATE: Speculative Store Bypass
diff --git a/debian/patches/series b/debian/patches/series
index 02b853f..b43c1d8 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -37,3 +37,4 @@ ubuntu/CVE-2018-7858.patch
 ubuntu/CVE-2018-3639/0001-i386-define-the-ssbd-CPUID-feature-bit-CVE-2018-3639.patch
 ubuntu/CVE-2018-3639/0002-i386-define-the-AMD-virt-ssbd-CPUID-feature-bit-CVE-.patch
 ubuntu/CVE-2018-3639/0003-i386-Define-the-Virt-SSBD-MSR-and-handling-of-it-CVE.patch
+ubuntu/machine-type-hpb.patch
diff --git a/debian/patches/ubuntu/machine-type-hpb.patch b/debian/patches/ubuntu/machine-type-hpb.patch
 new file mode 100644
index 0000000..45316ce
--- /dev/null
+++ b/debian/patches/ubuntu/machine-type-hpb.patch
@@ -0,0 +1,88 @@
+Description: Add a -hpb Ubuntu specific machine type suffix
+
+This works already fine on commandline, but Libvirt and other stacks above
+have no exploitation yet. Using a machine type has the benefit of being already
+controllable by most upper layer software like Libvirt (type= in os tag) but
+even up to Openstack (nova.conf or per image metadata on hw_machine_type).
+
+This is based on a discussion:
+   https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1769053
+
+A similar change is in CentOS/RH (there the default is switched, without
+even a way to go back.
+But since this can cause issues e.g. when migrating
+across hosts with different characteristics, it is not set as the default
+in Ubuntu with this change.
+
+Further we want to avoid "machine type proliferation", so we certainly won't
+add a type for every feature. But using a huge guest is more common and
+otherwise not yet achievable.
+
+This can be dropped when:
+ - libvirt exposes phys-bits/host-phys-bits natively
+ - at least the important stacks above exploit that config
+As an alternative we might decide at some point to make it the default without
+a way to switch back in following releases, but for now we don't want to do so.
+
+Forwarded: not-needed
+Forward-info: downstream decision
+Author: Christian Ehrhardt <[e-mail]>
+Origin: http://mirrors.ibiblio.org/ovirt/pub/ovirt-4.0/src/qemu-kvm-ev/kvm-target-i386-Enable-host-phys-bits-on-RHEL.patch
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1776189
+Last-Update: 2018-06-06
+
+--- a/hw/i386/pc_piix.c
++++ b/hw/i386/pc_piix.c
+@@ -1181,6 +1181,16 @@ static void pc_bionic_machine_options(Ma
+ DEFINE_I440FX_MACHINE(bionic, "pc-i440fx-bionic", NULL,
+                       pc_bionic_machine_options);
+ 
++static void pc_bionic_hpb_machine_options(MachineClass *m)
++{
++    pc_i440fx_2_11_machine_options(m);
++    m->desc = "Ubuntu 18.04 PC (i440FX + PIIX, +host-phys-bits=true, 1996)";
++    m->alias = NULL;
++    SET_MACHINE_COMPAT(m, PC_HOST_PHYS_BITS_TRUE);
++}
++DEFINE_I440FX_MACHINE(bionic_hpb, "pc-i440fx-bionic-hpb", NULL,
++                      pc_bionic_hpb_machine_options);
++
+ /*
+  * Due to bug 1621042 we have to consider the broken old wily machine
+  * type as valid xenial type to ensure older VMs that got created prio
+--- a/hw/i386/pc_q35.c
++++ b/hw/i386/pc_q35.c
+@@ -432,3 +432,14 @@ static void pc_q35_bionic_machine_option
+ }
+ DEFINE_Q35_MACHINE(bionic, "pc-q35-bionic", NULL,
+                    pc_q35_bionic_machine_options);
++
++static void pc_q35_bionic_hpb_machine_options(MachineClass *m)
++{
++    pc_q35_2_11_machine_options(m);
++    m->desc = "Ubuntu 18.04 PC (Q35 + ICH9, +host-phys-bits=true, 2009)";
++    /* The ubuntu alias and default is on the i440fx type */
++    m->alias = NULL;
++    SET_MACHINE_COMPAT(m, PC_HOST_PHYS_BITS_TRUE);
++}
++DEFINE_Q35_MACHINE(bionic_hpb, "pc-q35-bionic-hpb", NULL,
++                   pc_q35_bionic_hpb_machine_options);
+--- a/include/hw/i386/pc.h
++++ b/include/hw/i386/pc.h
+@@ -1002,5 +1002,16 @@ bool e820_get_entry(int, uint32_t, uint6
+     } \
+     type_init(pc_machine_init_##suffix)
+ 
++/* This switches the host-phys-bits property default to true which will
++ * allow to run rather huge guests at the price of reduced migratability
++ * between rather different hosts.
++ */
++#define PC_HOST_PHYS_BITS_TRUE \
++        { \
++            .driver = TYPE_X86_CPU,\
++            .property = "host-phys-bits",\
++            .value = "on",\
++        },
++
+ extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
+ #endif
diff --git a/debian/qemu-system-x86.NEWS b/debian/qemu-system-x86.NEWS
index 0f1e44b..c158a3b 100644
--- a/debian/qemu-system-x86.NEWS
+++ b/debian/qemu-system-x86.NEWS
@@ -1,3 +1,52 @@
+qemu (1:2.11+dfsg-1ubuntu11) cosmic; urgency=medium
+
+  Summary:
+  Adding new Ubuntu machine types with -hpb suffix to allow users to run
+  guests >1TB using the qemu host-phys-bits setting. If a cpu provides more
+  physical adressing bits than the default virtual 40 one can drive larger
+  guests by setting host-phys-bits. Using a machine type to do so allows to
+  control this through libvirt and higher virt stack components as of today.
+
+  Details:
+  Currently the virtualization stack has the feature to run guests bigger than
+  one Terabyte, but lacks the means to express and configure that easily.
+
+  Qemu provides phys-bits and host-phys-bits attributes on the -cpu parameter.
+  But due to the fact that higher layers do not expose any configuration for it
+  this feature is so far restricted to qemu commandline users or manual tweaks.
+
+  Long term we want to see libvirt exposing configuration for that and higher
+  layers to exploit it, see https://bugs.launchpad.net/bugs/1769053
+
+  But Ubuntu users ask for a way to configure guests like that right now.
+  To do so we provide a new Ubuntu specific machine type that matches the
+  usual Ubuntu machine type but with host-phys-bits switched on.
+
+  To express in their short names that they are like the base type but plus
+  HostPhysBits turned on they have a -hpb suffix on the usual shortname.
+    - pc-i440fx-bionic-hpb
+    - pc-q35-bionic-hpb
+  And they also list "+host-phys-bits=true" in their description.
+
+  The drawback using this type in an uncontrolled environment, is that you
+  might run into trouble migrating between systems of different hardware
+  characteristics (if the target CPU is not able to handle that many
+  phys-bits). This also is the main reason why we didn't want to make it the
+  default for everyone just yet.
+
+  Since machine type is rather old higher stacks often expose a configuration
+  for it, here for example links in regard to OpenStack:
+  1. Global via nova config:
+    https://docs.openstack.org/nova/pike/configuration/config.html
+  2. Per image via metadata:
+    https://docs.openstack.org/image-guide/image-metadata.html
+
+  The intention is to provide such kind of types until we either decide that
+  it is safe enough to switch it on by default (no extra type) or once libvirt
+  and higher stacks can control (host-)phys-bits directly.
+
+ -- Christian Ehrhardt <[e-mail]>  Tue, 12 Jun 2018 09:28:17 +0200
+
 qemu (1:2.8+dfsg-2ubuntu1) zesty; urgency=low
 
   The ubuntu specific machine types for Trusty, Utopic and Vivid had a bug in