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

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(a)linux.ibm.com&gt; > Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; > --- > 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 >

> 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.

> case VIR_DOMAIN_LAUNCH_SECURITY_NONE: > break; > case VIR_DOMAIN_LAUNCH_SECURITY_LAST:

> 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.

> + </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.

Pavel > + </launchSecurity> > +</domain>