Code review comment for ~rafaeldtinoco/ubuntu/+source/qemu:lp1828495-bionic-devel-qemu

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Christian,

I believe I have address everything you mentioned. Only caveat would be:

(A)

    - 0005-new-cpu-model-for-icelake.patch (LP: #1828495):
      i386: Add new CPU model Icelake-{Server,Client}
      (upstream: 8a11c62da914, desc: v3.0.0-156-g8a11c62da9)

      1) patch header (d/p/u/*.patch) is wrong (local git repo header)
      2) DEP3 is backport if it applies
      3) description was re-wrapped 1) and 2) items
      4) CPUID_7_0_EBX_INTEL_PT <- dropped 2 times in all patches
      5) CPUID_7_0_EDX_PCONFIG dropped comparing to original (backport)

      upstream patches:

      4c257911dcc7 - i386: remove the 'INTEL_PT' CPUID bit from
      76e5a4d58357 - i386: remove the new CPUID 'PCONFIG'...

      to "backport" (cleanup) only if needed after those 2

From patch 0005 and beyond, you can't cleanly compile the source package because missing INTEL_PT and PCONFIG options removal, done together with:

    - 0013-cascadelake-server.patch (LP: #1828495):
      i386: Add new model of Cascadelake-Server
      (upstream: c7a88b52f62, desc: v3.0.0-1662-gc7a88b52f6)

* - 0014-remove-cpuid-pconfig.patch
   i386: remove the new CPUID 'PCONFIG' from Icelake-Server CPU model
   (upstream: 76e5a4d58357, desc: v3.1.0-1495-g76e5a4d583)

* - 0015-remove-cpuid-intel_pt.patch
      i386: remove the 'INTEL_PT' CPUID bit from named CPU models
      (upstream: 4c257911dcc7, desc: v3.1.0-1496-g4c257911dc)

like you suggested. Code is cleaner and better to be supported this way. Because we are not backporting MPX disablement, I had to make some adjustments in patch 0015

(B)

NOW, regarding MSX disablement... I agree with you! Just wanted to document that:

IF someone is running QEMU on Bionic, in a {Sky,Cascade}Lake CPU, has MPX on by default, a possibly different (than Ubuntu) guest OS (other Linux distro ?) AND is running a object containing instructions of that ABI extension (GCC in between 5 and 9 compiled objects and kernel 3.19 - 4.20) WHILE doing a live migration from this QEMU version to higher one, then.. this person would face a bug :o).

PS: I know its a huge corner case, just wanted to document.

« Back to merge proposal