[20.04 FEAT] Base KVM setup for secure guests - qemu part

Bug #1835546 reported by bugproxy on 2019-07-05
18
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu on IBM z Systems
High
Canonical Server Team
qemu (Ubuntu)
High
Skipper Bug Screeners

Bug Description

Enable KVM guests to start and control a guest running in secure mode.
With that Customers can securely run sensitive workloads in KVM on premise and in the cloud.
Feature request for contribution to qemu > 4.0

Currently not available.

Git-Commit will be provided once available

Related branches

bugproxy (bugproxy) on 2019-07-05
tags: added: architecture-s39064 bugnameltc-177557 severity-high targetmilestone-inin1910
Changed in ubuntu:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
affects: ubuntu → qemu (Ubuntu)
Frank Heimes (fheimes) wrote :

Waiting and setting to Incomplete until git commits are shared.

Changed in qemu (Ubuntu):
status: New → Incomplete
Changed in ubuntu-z-systems:
status: New → Incomplete
importance: Undecided → High
assignee: nobody → Canonical Server Team (canonical-server)
summary: - [19.10 FEAT] Base KVM setup for secure guests - qemu part
+ [20.04 FEAT] Base KVM setup for secure guests - qemu part

------- Comment From <email address hidden> 2019-07-30 07:24 EDT-------
Moved target to 20.04, will not make it in time for 19.10

tags: added: targetmilestone-inin2004
removed: targetmilestone-inin1910
tags: added: qemu-20.04

Did this make it into qemu 4.2 or at lest git - or do we want/need to punt this to post Ubuntu 20.04 ?

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-01-28 06:19 EDT-------
Not part of qemu 4.2. need to be applied on top.

Ok, send me upstream commits that need to go on top of 4.2 when they are ready.

Reminder: Feature Freeze is the 27th of February - add some test and delay and you have ~2 weeks from now on.

Frank Heimes (fheimes) on 2020-03-10
information type: Private → Public
Viktor Mihajlovski (mihajlov) wrote :

The QEMU review is still ongoing. As a stop-gap measure it would be great to have PPA with a preliminary build of QEMU (master level + patches).
Attached is a full diff on top of 4.2.

Since the attached patch obsoletes some of the debian/patches content, the following series file should suffice:
pv-full.diff
qboot-stop-using-inttypes.patch
qboot-no-jump-tables.diff

# ubuntu patches
ubuntu/expose-vmx_qemu64cpu.patch
ubuntu/enable-svm-by-default.patch
ubuntu/define-ubuntu-machine-types.patch
ubuntu/pre-bionic-256k-ipxe-efi-roms.patch
ubuntu/lp-1857033-i386-Add-new-CPU-model-Cooperlake.patch

It will be necessary to tweak the machine type and the CPU model patch.
Further, I had to apply this change to debian/rules:
--- orig/debian/rules 2020-02-12 09:21:56.000000000 -0500
+++ pv/debian/rules 2020-03-09 06:54:19.000000000 -0400
@@ -103,6 +101,9 @@
 b/configure-stamp: configure
        dh_testdir

+ # Fix up permission
+ chmod +x scripts/kernel-doc
+
        # system build
        rm -rf b/qemu; mkdir -p b/qemu
        cd b/qemu && \

Dimitri John Ledkov (xnox) wrote :

This is a very akward submission. Where is the git repository with backport?

Ideally against the ubuntu's focal git source tree with all patches already applied (aka treat it just like a normal git repo) https://code.launchpad.net/~usd-import-team/ubuntu/+source/qemu/+git/qemu/+ref/applied/ubuntu/focal-devel

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-12 08:14 EDT-------
Before I venture to go there: As I pointed out, we have no proper backport yet and would in fact need a build on top of current QEMU master.
I can of course build a tree on top of the focal branch based on https://github.com/frankjaa/qemu/tree/protvirt, but this will then comprise ~2K commits (equivalent to the attached diff). Is this really what you want?

This even brings in 5.0 types, moves around vl.c and all that - it isn't even good for a quick build on the PPA IMHO (Ack to comment #7).

Really, don't think about the "package" too much just provide a stack of commits for 4.2, I'm fine if that is just a git branch on top of tag: v4.2.0 and will convert it to packaging changes then.

The part for you is the arch-specific backporting onto 4.2
Let making it proper packaging changes then be my task.

As is this isn't really usable - any chance to refresh it in a more consumable format?

For the time being I pushed a best effort inclusion of the patch in comment #6 as "qemu_4.2-3ubuntu2~ugly1" into the PPA that xnox already linked.

=> https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3970

I further hit and needed changes for:
- ERROR: unknown option --disable-bluez
... ongoing ...

I think the question is what do you need to test this properly and land
this things upstream?

We can build a snapshot qemu v5.0 into that PPA, Linux kernel v5.7,
anything. As that PPA is solely to facilitate IBM developing and testing
this. Builds in that PPA aware not suitable for landing in the distro.

For example, Focal will get newer qemu series via cloud archive, thus v5.0
and higher will be made available on Focal in the future. Ditto hwe kernels.

On Thu, 12 Mar 2020, 12:30 bugproxy, <email address hidden> wrote:

> ------- Comment From <email address hidden> 2020-03-12 08:14 EDT-------
> Before I venture to go there: As I pointed out, we have no proper backport
> yet and would in fact need a build on top of current QEMU master.
> I can of course build a tree on top of the focal branch based on
> https://github.com/frankjaa/qemu/tree/protvirt, but this will then
> comprise ~2K commits (equivalent to the attached diff). Is this really what
> you want?
>
> --
> You received this bug notification because you are subscribed to Ubuntu.
> Matching subscriptions: s390x
> https://bugs.launchpad.net/bugs/1835546
>
> Title:
> [20.04 FEAT] Base KVM setup for secure guests - qemu part
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu-z-systems/+bug/1835546/+subscriptions
>

------- Comment From <email address hidden> 2020-03-19 04:40 EDT-------
As the QEMU upstream reviews has reached a state I'd consider as being more or less conclusive, Christian has kindly backported the patch series on top of QEMU 4.2 here: https://github.com/borntraeger/qemu/commits/pv42

Frank Heimes (fheimes) on 2020-03-19
Changed in qemu (Ubuntu):
status: Incomplete → New
Changed in ubuntu-z-systems:
status: Incomplete → Triaged
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-19 05:32 EDT-------
@Canonical: A PPA would be great for our internal testing, before the official integration... Many thanks in advance

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-19 07:06 EDT-------
The branch in https://github.com/borntraeger/qemu/commits/pv42 was refreshed today at 12:00 CET. If possible, use this for the PPA build. Thanks!

Hi,
we will not take the following commit:
commit 9da000ea0ae75fbdf14f6e7dc49ad324ba3fe190
Author: Christian Borntraeger <email address hidden>
Date: Thu Mar 19 07:02:20 2020 -0400
    rebuild bios

As a general Ubuntu (and Debian) policy implies that we build everything from source for serviceability (and to no get any dark secrets without knowing about them). I assume you did the code updates as well (I see e.g. pc-bios: s390x: Save iplb location in lowcore), so that the s390-ccw.img that is generated on build will fulfill your needs?
Otherwise please also add the source-changes you need there to the backport.

While adding that to the packaging I found that the following isn't needed since I did a bunch of backports for patches submitted at qemu-stable, therefore I don't need to add "ae150759a9 s390/sclp: improve special wait psw logic" again.

In a similar fashion "3c664ea0a6 vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM" already was added before via bug 1847361.

The rest looks reasonable, thanks for the link to a git branch!

I have started a PPA build for your pre-verfication at:
https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3985/

Please let me know if anything further changes while waiting for this to be fully accepted upstream and also let me know if the testing uncovers anything unexpected.

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-20 04:42 EDT-------
Thanks for providing the PPA.
A first test run looked good.
We'll use this PPA for internal testing now.

Frank Heimes (fheimes) wrote :

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

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-20 05:45 EDT-------
(In reply to comment #23)
> As a general Ubuntu (and Debian) policy implies that we build everything
> from source for serviceability (and to no get any dark secrets without
> knowing about them). I assume you did the code updates as well (I see e.g.
> pc-bios: s390x: Save iplb location in lowcore), so that the s390-ccw.img
> that is generated on build will fulfill your needs?
> Otherwise please also add the source-changes you need there to the backport.

Yes, the bios only needs this on top
"pc-bios: s390x: Save iplb location in lowcore"

> While adding that to the packaging I found that the following isn't needed
> since I did a bunch of backports for patches submitted at qemu-stable,
> therefore I don't need to add "ae150759a9 s390/sclp: improve special wait
> psw logic" again.

if its already in, even better.

The attachment "focal_qemu_content.diff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-23 05:29 EDT-------
There was an issue found, building on x86 32-bit, which required a fix. The backport branch has been updated:
https://github.com/borntraeger/qemu/commits/pv42

Can you please update the package to sync with the upstream state of affairs?

NB: You can verify the delta by diffing
https://github.com/borntraeger/qemu/commits/pv42_v11 (used for your backport)
and
https://github.com/borntraeger/qemu/commits/pv42_v12

which is

index 1d04cd5..c343cfb 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -656,7 +656,7 @@ int s390_ipl_prepare_pv_header(void)
cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
ipib_pv->pv_header_len);
- rc = s390_pv_set_sec_parms((uint64_t)hdr,
+ rc = s390_pv_set_sec_parms((uintptr_t)hdr,
ipib_pv->pv_header_len);
g_free(hdr);
return rc;

Thanks for the extended testing!

Ack that is the only change:
$ git range-diff v4.2.0..cborntra/pv42_v11 v4.2.0..cborntra/pv42_v12
...
13: 3c664ea0a6 = 13: 3c664ea0a6 vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM
14: 5081c651c9 = 14: 5081c651c9 Sync pv
15: db0a53ee22 ! 15: 295b91aa9d s390x: protvirt: Support unpack facility
    @@ hw/s390x/ipl.c: static void s390_ipl_prepare_qipl(S390CPU *cpu)
     +
     + cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
     + ipib_pv->pv_header_len);
    -+ rc = s390_pv_set_sec_parms((uint64_t)hdr,
    ++ rc = s390_pv_set_sec_parms((uintptr_t)hdr,
     + ipib_pv->pv_header_len);
     + g_free(hdr);
     + return rc;
16: 617d3f7be6 = 16: cdfe6c35aa s390x: protvirt: Add migration blocker
...

I integrated that into the PPA build (which does not include i386 btw as we mostly dropped that arch).

=> 4.2-3ubuntu4~ppa2 once PPA build is complete

FYI For testing I have put this *also* into a different PPA
=> https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3986/

This shall help (me) to test it together with some intended libvirt changes.

For now the qemu content in regard to s390x-protvirt is the same in the old 3985 and the new 3986 PPAs. That will help you to test "just" protvirt and not be influenced by the other incoming changes out of the 3985 PPA.

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-25 05:56 EDT-------
I wanted to point out that Conny, as the s390 QEMU maintainer has queued the patches for s390-next. With the exception of the kernel sync, no more changes will be needed.

See https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07063.html

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-25 10:52 EDT-------
3985 was tested and was good for our test scenarios so far.

3985 will be tested next.

------- Comment From <email address hidden> 2020-03-25 10:57 EDT-------
The commits are now in the maintainer's s390-next tree: https://github.com/cohuck/qemu/commits/s390-next

e599aad s390x: Add unpack facility feature to GA1
e830bd1 docs: system: Add protvirt docs
bfa7b5d s390x: protvirt: Handle SIGP store status correctly
187c84e s390x: protvirt: Move IO control structures over SIDA
57b794e s390x: protvirt: Disable address checks for PV guest IO emulation
0171269 s390x: protvirt: Move diag 308 data over SIDA
88987f0 s390x: protvirt: Set guest IPL PSW
8490488 s390x: protvirt: SCLP interpretation
bc24ee8 s390x: protvirt: Move STSI data over SIDAD
c68b81c s390x: Add SIDA memory ops
0ec597e s390x: protvirt: KVM intercept changes
dc83ca2 s390x: protvirt: Inhibit balloon when switching to protected mode
520935e s390x: protvirt: Add migration blocker
4d226de s390x: protvirt: Support unpack facility
db5bc33 Sync pv
a183ee9 s390x: Move diagnose 308 subcodes and rcs into ipl.h
7722837 s390/ipl: fix off-by-one in update_machine_ipl_properties()
f58f084 Documentation: create/move s390x documentation

To be clear, if you want to continue testing without the coming libvirt it is fine to just stay on 3985 and go on as-is.

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-25 12:27 EDT-------
Hi paelzer,

so QEMU form 3985 is good to go. It looks like it was not clear in my initial response.

And due to your comment we will skip 3986.

Thanks!

Mathew Hodson (mhodson) on 2020-03-26
Changed in qemu (Ubuntu):
importance: Undecided → High

FYI - Added [1] to 4.2-3ubuntu4~ppa7 in PPA 3985

[1]: https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg07969.html

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-26 11:56 EDT-------
> Add https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg07969.html I
> guess?
>
> FYI - Added [1] to 4.2-3ubuntu4~ppa7 in PPA 3985
>
> [1]: https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg07969.html

Yes. It is only an error case but this should be fixed nevertheless.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qemu - 1:4.2-3ubuntu4

---------------
qemu (1:4.2-3ubuntu4) focal; urgency=medium

  * d/p/ubuntu/lp-1835546-*: backport the s390x protvirt feature (LP: #1835546)
  * remove d/p/ubuntu/expose-vmx_qemu64cpu.patch: Stop adding VMX to qemu64
    to avoid broken nesting (LP: #1868692)

 -- Christian Ehrhardt <email address hidden> Fri, 20 Mar 2020 08:02:16 +0100

Changed in qemu (Ubuntu):
status: New → Fix Released
Frank Heimes (fheimes) on 2020-03-27
Changed in ubuntu-z-systems:
status: Triaged → Fix Released
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2020-03-30 03:14 EDT-------
IBM Bugzilla status-> closed, Fix Released with focal

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers