[FFe] Please accept patches for secure guest feature

[Frank Heimes]

The secure guest feature (aka protvirt) affects multiple components (kernel, qemu and s390-tools - see below).
While dedicated tickets for the different components exist since quite a while, the code arrived late and/or discussion to get it upstream accepted took longer than expected.
(Even if we as of today didn't reached the kernel freeze, I'm already adding kernel to this FFe.)

Since this is a very important feature the current IBM Z and LinuxONE family, it's requested to be included into focal, the next LTS release, to become exploitable by long running systems.

The code is largely architecture specific.
No brand new packages or new upstream version are requested, only the cherry-pick of commits (or PR) - so far everything is 'cherry-pick'-able.

kernel:
The patch set for the kernel is huge (30+ commits), but has only one common code patch (two files).
The arch specific patches landed in between in linux-next, the arch specific one is expected to land there very soon (hours/days from now). The common-code patch ran through several hands and landed in between in Andrew Morton's mmots tree.
A pre-screening of the code was done by the kernel team and it looked acceptable.
(dedicated kernel ticket: https://bugs.launchpad.net/bugs/1835531)

qemu:
The entire code seems to be arch specific.
Again a pre-screening of the maintainer lead to the fact that it should be acceptable, too.
(dedicated qemu ticket: https://bugs.launchpad.net/bugs/1835546)

s390-tools:
The entire tool only exists for the s390x architecture.
Hence obviously everything is arch specific on that.
(dedicated s390-tools ticket: https://bugs.launchpad.net/bugs/1834534)

Currently work is going on to test this function end to end based on Ubuntu components (means based on our s390-tools, qemu and kernel [focal master-next] trees).
On top I applied the patches to the packages as well and did manual test buids.

With that a potential regression can be considered as low - and even in case of a regression, it will affect s390x only.

The patches are being staged for this feature in: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3970

[Ubuntu Kernel Bot (ubuntu-kernel-bot)]

This bug is missing log files that will aid in diagnosing the problem. While running an Ubuntu kernel (not a mainline or third-party kernel) please enter the following command in a terminal window:

apport-collect 1866866

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

[Steve Langasek (vorlon)]

The kernel side does not need an FFe; and once we reach kernel freeze, further uploads are not accepted until the release. So marking this task as 'confirmed'.

[Steve Langasek (vorlon)]

> With that a potential regression can be considered as low - and even in case of a regression,
> it will affect s390x only.

From a release perspective, I don't think this is an adequate rationale for a feature freeze exception. It only affects one architecture, but that is a release architecture and we expect to deliver a quality product on that architecture, the same as for all the others. What are the characteristics of the update itself that would minimize the risk of regression? Are these new features going to change the existing default behavior of qemu / s390-tools, or are they strictly opt-in as new options?

[Frank Heimes (fheimes)]

I agree on the kernel side - as already mentioned in the bug description - I've added that only for the reason of completeness, since the kernel change is affected (even if not really by the FFe).
For the kernel only one add. commit needs to land - and that is expected soon.
Hence (as of today) it looks like this is possible in time prior to the kernel freeze.

With "...it will affect s390x only" I wanted to mitigate the impact on other architectures.
There were - and there are - of course efforts to stabilize this function (across the affected components) as much as possible. Manual test builds where done by HWE as well as by IBM (based on the Ubuntu code level) and tested.
On top a PPA was requested (see above) that allows to test affected packages as they come out of the Ubuntu build systems (so no manual builds anymore).

And your assumption about the opt-in is correct.
Users are _not_ forced to use this and the defaults will not change.
In fact, it can only be an opt-in, since it requires a certain (hw) level of a system.

[Dimitri John Ledkov (xnox)]

The code-drop has landed:
110 files changed, 7261 insertions(+), 764 deletions(-)

majority of this is in the new tool for PVM, but there are alot of changes to zipl affecting booting across all systems, unrelated bugfixes, documentation improvements, whitespace changes, and z15 hardware enablement.

I guess I will need to be line-by-line / patch-by-path review of this now.

[Dimitri John Ledkov (xnox)]

(that's just the s390-tools portion)

[Dimitri John Ledkov (xnox)]

= Refactor =

 The refactor commits, correct minor runtime/compiler warnings or have
 no effect on the resulting build. I want to take them in, as
 bugfixes, to keep the Ubuntu source matching the upstream as closely
 as possible for future cherrypicks to not conflict.

  11bdab2 include/boot/s390.h: add guard for `struct __vector128`
  b06af60 README.md: remove useless empty line
  2c10642 cpumf_helper: Avoid perl warning from pod2usage function
  6fcf64e lib/util_file.h: fix typo in the macro guard
  87b54fc CHANGELOG: Fix formatting

= HW Enablement =

 This commits fall under HWE SRU exception, these expand cpumf tool to
 display z15 specific counters & correctly report/trace fiber channel
 endpoint security status.

 They do not change behaviour on any existing hardware z13/z14
 platforms. And improve behaviour on z15 hardware.

  1086548 cpumf: Add IBM z15 extended counter defintion file
  5d2871d cpumf/data: Add new deflate counters for IBM z15

  fbf8513 zfcpdbf: print HBA FC Endpoint Security trace records
  67496af zdev: Report FC Endpoint Security of zfcp devices
  16b2799 zdev: Handle special case in if-case
  c063273 zdev: Introduce read-only attributes

Bugfix

 These are pure bugfixes to improve the `dbginfo` debug information
 collection tool. Fix documentation for correct ways to enable/disable
 secureboot (also requested in a separate ticket). And a bugfix to
 correct potential buffer overflow in zipl-libc (i.e. common libc-like
 functions which are used by the bootloader standalone code).

  f742ed7 dbginfo: gather ethtool output for per-queue coalescing
  4fa9656 dbginfo: collect softnet_stat
  d415b8e dbginfo: Removed collection of /var/log/opencryptoki/

  299fd2b zipl: fix zipl.conf man page example for secure boot

  36fed0e zipl/libc: Indicate truncated lines in printf with '...'
  f743002 zipl/libc: Replace sprintf with snprintf
  8874b90 zipl/libc: Fix potential buffer overflow in printf
  6fe9e6c zipl/libc: Introduce vsnprintf

PVM feature

 These are the commits related to the new PVM feature. They consist of
 creating a new userspace tool (genprotimg) as well as creating new
 bootloader stages. To support building the new bootloader stages,
 existing zipl bootloader stages have been refactored a little bit to
 use common headers with defined constants (rather than just
 hardcoding them). At the same time, the zipl-libc code has been
 improve to be more strict with parsing / validating certain things.

 The new zipl bootloader stages are only used for th PVM
 feature. Despite the refactors, there does not appear to be any
 behaviour changes of the existing zipl boot stages as used on
 existing hardware configurations for regular zipl IPL. And regular
 boot testing will be performed as part of the Focal to validate LPAR,
 z/VM, KVM platforms with/without secureboot where applicable.

  65b9fc4 genprotimg: introduce new tool for the creation of PV images
  d2f8f97 genprotimg: add relocator for stage3b
  2d60057 genprotimg: boot: use C pre-processor for linker script generation
  3356d6f genprotimg: boot: initial bootloader support
  67aef9b Consolidate `ALIGN, __ALIGN_MASK, ARRAY_SIZE` macros
  e51663b zipl/libc: ...

[Frank Heimes (fheimes)]

For completeness reasons (and this current FFe) please see the attached debdiff.
But for better readability the following link is probably preferable: https://github.com/borntraeger/qemu/commits/pv42

The debdiff shows the difference between Ubuntu latest qemu 4.2 and pv42, EXCEPT (see LP 1835546/, comment #15):
- 9da000ea0a "rebuild bios" -- not picked
- ae150759a9 "s390/sclp: improve special wait psw logic" -- not needed
- 3c664ea0a6 "vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM" -- already in

With that, all patches/commits are s390x specific, except some header changes:
- 6807f46 "Sync pv"

[Frank Heimes (fheimes)]

@ 'Ubuntu Release Team' would you be so kind having a look at the:
- s390-tools patches/changes in comment #7 (more details in Bug:1835546) and the
- qemu patches/changes in comment #8 (more details in Bug:1834534)
that are asked for in this FFe and let us know if the information is adequate and the changes acceptable -thx.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2020-03-20 05:42 EDT-------
(In reply to comment #8)
> For completeness reasons (and this current FFe) please see the attached
> debdiff.
> But for better readability the following link is probably preferable:
> https://github.com/borntraeger/qemu/commits/pv42
>
> The debdiff shows the difference between Ubuntu latest qemu 4.2 and pv42,
> EXCEPT (see LP 1835546/, comment #15):
> - 9da000ea0a "rebuild bios" -- not picked

ok as you build that yourself

> - ae150759a9 "s390/sclp: improve special wait psw logic" -- not needed

This is needed, but according to ChristianE already in via qemu stable. Correct?

> - 3c664ea0a6 "vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM" -- already in

ok

[Steve Langasek (vorlon)]

FFe granted for s390-tools.

[Launchpad Janitor (janitor)]

This bug was fixed in the package s390-tools - 2.12.0-0ubuntu3

---------------
s390-tools (2.12.0-0ubuntu3) focal; urgency=medium

  * Update patch series to master tip:
    - PVM / genprotimg LP: #1834534, FFe LP: #1866866
    - zipl/libc: Fix potential buffer overflow LP: #1865032
    - zipl: Fix secureboot documentation LP: #1864654
    - Many other smaller bugfixes

 -- Dimitri John Ledkov <email address hidden> Fri, 20 Mar 2020 12:08:13 +0000

[Frank Heimes (fheimes)]

The kernel team picked up the ('secure guest' aka protvirt) kernel patches for focal master-next and with that LP 1835531 was changed to Fix Committed.
Hence I'm updating the kernel entry here to Fix Committed, too.

(Notice that the kernel didn't reached the kernel freeze yet, so it's technically not part of this FFe, but since all the affected components are needed for the 'secure guest' feature it was added here for the reason of completeness.)

[Frank Heimes (fheimes)]

With the FFe granted for s390x-tools in comment #11 (thx for that) and the kernel patches having landed in proposed (5.4.0.20), about 2/3 of the overall FFe is now completed.

Having the beta freeze on April the 2nd in mind, and the good progress on testing the patched qemu version that was made available via PPA (protvirt testing by IBM and regression testing by Canonical) and the fact that Cornelia Huck (s390 qemu maintainer) finally allowed the patches to be queued for s390-next (LP 1835546), I'm wondering if the qemu part of this FFe is now acceptable as well, or if additional effort or more time is needed.

@ 'Ubuntu Release Team' would you mind having now a look at the qemu situation, too?

[Łukasz Zemczak (sil2100)]

Ok, even though Steve was the one initially handling this FFe, seeing that he's not available right now I sat down and took a look at the qemu part - browsing through the diff took a while. I see work and testing is happening all the time via LP: #1835546, so I think this is actually in good hands. Though the changeset is really big and makes me feel a bit weary, having a +1 from the qemu maintainer is a plus.

FFe for qemu granted.

[Frank Heimes (fheimes)]

Hello Łukasz, oh, I wasn't aware that he is not available, so many thanks for taking over and having a look at it and of course for the granting.
The qemu maintainer (cpaelzer) already had a look at this of course and was the one who prepared the PPA package and verbally gave his okay after code screening.
But I'm sure he's happy to leave another quick comment on that here, too.
Thx again!

[Christian Ehrhardt  (paelzer)]

There is no power box free this week, but indeed I'm +1 as well.

Test results look good as well (I've re-run them just to be extra sure) btw:
prep (x86_64) : Pass 20 F/S/N 0/0/0 - RC 0 (15 min 55036 lin)
migrate (x86_64) : Pass 288 F/S/N 0/0/0 - RC 0 (60 min 214809 lin)
cross (x86_64) : Pass 24 F/S/N 0/1/3 - RC 0 (51 min 47738 lin)
misc (x86_64) : Pass 103 F/S/N 0/0/0 - RC 0 (39 min 72305 lin)

prep (s390x) : Pass 20 F/S/N 0/0/0 - RC 0 (14 min 43627 lin)
migrate (s390x) : Pass 268 F/S/N 0/5/0 - RC 0 (66 min 161708 lin)
cross (s390x) : Pass 19 F/S/N 1/1/2 - RC 1 (47 min 45401 lin)
misc (s390x) : Pass 67 F/S/N 0/0/0 - RC 0 (24 min 32119 lin)

@Frank - I guess he meant Cornelia being the s390x maintainer who already acked it upstream.
Furthermore in bug 1835546 we have the ack from the IBM test Team.
I think we are as ready as we can be.

Thanky everyone involved!

[Frank Heimes (fheimes)]

Oh, hehe, since I saw (from LP 1835546) that Cornelia already ACKed it, I thought that Lukasz probably meant you, Christian - apologize.
And yes, two teams at IBM tested it successfully.
I will see if I can get a Power9 box freed up on short notice ...

[Frank Heimes (fheimes)]

Since LP 1835546 is now Fix Released, I'm changing for the reason of completeness the qemu entry of this tickets to Fix Released, too.

[Frank Heimes (fheimes)]

With LP 1835531 now Fix Released, I'm changing the kernel entry of this ticket also to Fix Released - and with that the project entry, too.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2020-04-02 07:18 EDT-------
IBM Buzgilla status -> closed, Fix Released with focal