Re: [PATCH v3 5/6] conf: add s390-pv as launch security type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/25/21 12:00 PM, Pavel Hrdina wrote:
On Tue, Jun 22, 2021 at 03:10:48PM +0200, Boris Fiuczynski wrote:
Add launch security type 's390-pv' as well as some tests.

Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
  docs/schemas/domaincommon.rng                 |  1 +
  src/conf/domain_conf.c                        |  8 +++++
  src/conf/domain_conf.h                        |  1 +
  src/qemu/qemu_command.c                       | 26 ++++++++++++++
  src/qemu/qemu_firmware.c                      |  1 +
  src/qemu/qemu_namespace.c                     |  1 +
  src/qemu/qemu_process.c                       |  1 +
  src/qemu/qemu_validate.c                      |  9 +++++
  .../launch-security-s390-pv-ignore-policy.xml | 24 +++++++++++++
  .../launch-security-s390-pv.xml               | 18 ++++++++++
  .../launch-security-s390-pv-ignore-policy.xml |  1 +
  tests/genericxml2xmltest.c                    |  2 ++
  ...ty-s390-pv-ignore-policy.s390x-latest.args | 35 +++++++++++++++++++
  .../launch-security-s390-pv-ignore-policy.xml | 33 +++++++++++++++++
  .../launch-security-s390-pv.s390x-latest.args | 35 +++++++++++++++++++
  .../launch-security-s390-pv.xml               | 30 ++++++++++++++++
  tests/qemuxml2argvtest.c                      |  3 ++
  17 files changed, 229 insertions(+)
  create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml
  create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv.xml
  create mode 120000 tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml
  create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args
  create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml
  create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args
  create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.xml


<snip>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4135a8444a..3ab803f7ce 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6975,6 +6975,9 @@ qemuBuildMachineCommandLine(virCommand *cmd,
                  virBufferAddLit(&buf, ",memory-encryption=sev0");
              }
              break;
+        case VIR_DOMAIN_LAUNCH_SECURITY_PV:
+            virBufferAddLit(&buf, ",confidential-guest-support=pv0");
+            break;

This could be possible shared for all launchSecurity types as well but
it can be done as followup. That would mean using for example lsec0
instead of sev0, pv0, somethingelse0 and so on. It's just an id which
can be anything.


I will add an follow-up patch which changes the id to a common id 'lsec0'.


          case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
              break;
          case VIR_DOMAIN_LAUNCH_SECURITY_LAST:

<snip/>

diff --git a/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml b/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml
new file mode 100644
index 0000000000..0c398cced8
--- /dev/null
+++ b/tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml
@@ -0,0 +1,24 @@
+<domain type='kvm'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+  </devices>
+  <launchSecurity type='s390-pv'>
+    <cbitpos>47</cbitpos>
+    <reducedPhysBits>1</reducedPhysBits>
+    <policy>0x0001</policy>
+    <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert>
+    <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session>

I thing we should not ignore invalid XML bits and error out instead.


This would result in s390-pv checking for the existence of any child elements and if there is a child fail. If other launchSecurity types come along with new or shared child elements checking for SEV and the new types would have to added/changed.
Wouldn't that get messy quickly?



+  </launchSecurity>
+</domain>
diff --git a/tests/genericxml2xmlindata/launch-security-s390-pv.xml b/tests/genericxml2xmlindata/launch-security-s390-pv.xml
new file mode 100644
index 0000000000..29c7fc152d
--- /dev/null
+++ b/tests/genericxml2xmlindata/launch-security-s390-pv.xml
@@ -0,0 +1,18 @@
+<domain type='kvm'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+  </devices>
+  <launchSecurity type='s390-pv'/>
+</domain>
diff --git a/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml b/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml
new file mode 120000
index 0000000000..075c72603d
--- /dev/null
+++ b/tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml
@@ -0,0 +1 @@
+../genericxml2xmlindata/launch-security-s390-pv.xml
\ No newline at end of file
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index ac89422a32..eb15f66c3c 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -233,6 +233,8 @@ mymain(void)
      DO_TEST("tseg");
DO_TEST("launch-security-sev");
+    DO_TEST("launch-security-s390-pv");
+    DO_TEST_DIFFERENT("launch-security-s390-pv-ignore-policy");
DO_TEST_DIFFERENT("cputune");
      DO_TEST("device-backenddomain");
diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args
new file mode 100644
index 0000000000..c9d9b84dd3
--- /dev/null
+++ b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-s390x \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \
+-machine s390-ccw-virtio,accel=kvm,usb=off,dump-guest-core=off,confidential-guest-support=pv0,memory-backend=s390.ram \
+-cpu gen15a-base,aen=on,cmmnt=on,vxpdeh=on,aefsi=on,diag318=on,csske=on,mepoch=on,msa9=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on,msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,deflate=on,edat2=on,etoken=on,vx=on,ipter=on,mepochptff=on,ap=on,vxeh=on,vxpd=on,esop=on,msa9_pckmo=on,vxeh2=on,esort=on,apqi=on,apft=on,els=on,iep=on,apqci=on,cte=on,ais=on,bpb=on,gs=on,ppa15=on,zpci=on,sea_esop2=on,te=on,cmm=on \
+-m 214 \
+-object '{"qom-type":"memory-backend-ram","id":"s390.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
+-device virtio-blk-ccw,devno=fe.0.0000,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \
+-audiodev id=audio1,driver=none \
+-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 \
+-object '{"qom-type":"s390-pv-guest","id":"pv0"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml
new file mode 100644
index 0000000000..052d96dedb
--- /dev/null
+++ b/tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml
@@ -0,0 +1,33 @@
+<domain type='kvm'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-s390x</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='virtio'/>
+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
+    </disk>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='virtio'>
+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
+    </memballoon>
+    <panic model='s390'/>
+  </devices>
+  <launchSecurity type='s390-pv'>
+    <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert>
+    <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session>

This doesn't look correct, we should not format dhCert or session with
s390-pv because based on the patches they are not used at all.

Isn't this the same issue as before about unsued elements being ignored?


Pavel

+  </launchSecurity>
+</domain>

<snip/>


--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux