Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

runtime: Allow no initrd path for IBM Z Secure Execution #8693

Merged
merged 1 commit into from Jan 11, 2024
Merged

runtime: Allow no initrd path for IBM Z Secure Execution #8693

merged 1 commit into from Jan 11, 2024

Conversation

BbolroC
Copy link
Member

@BbolroC BbolroC commented Dec 18, 2023 •
edited

This is to reintroduce a configuration rule for IBM Z Secure Execution, where no initrd path should be configured. For the TEE of interest, only a kernel image should be specified with confidential_guest=true.

Fixes: #8692

Signed-off-by: Hyounggyu Choi Hyounggyu.Choi@ibm.com

Test result:

$ ps -ef | grep qemu | grep kernel
root      415937  415926  3 14:51 ?        00:00:02 /opt/kata/bin/qemu-system-s390x -name sandbox-217c61c958d255f517cacfe2cbb1cde84eb58a0f6af7f2a04ce98a1035c072c8 -uuid 3d1d57e4-48c4-436e-9350-2fcd2c1466d5 -machine s390-ccw-virtio,accel=kvm,confidential-guest-support=pv0 -cpu host, -qmp unix:fd=3,server=on,wait=off -m 2048M,slots=10,maxmem=15923M -device virtio-serial-ccw,id=serial0,devno=fe.0.0001 -device virtconsole,chardev=charconsole0,id=console0 -chardev socket,id=charconsole0,path=/run/vc/vm/217c61c958d255f517cacfe2cbb1cde84eb58a0f6af7f2a04ce98a1035c072c8/console.sock,server=on,wait=off -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0002 -object s390-pv-guest,id=pv0 -device vhost-vsock-ccw,vhostfd=4,id=vsock-826465042,guest-cid=826465042,devno=fe.0.0003 -chardev socket,id=char-94dc2772c2c7eaec,path=/run/vc/vm/217c61c958d255f517cacfe2cbb1cde84eb58a0f6af7f2a04ce98a1035c072c8/vhost-fs.sock -device vhost-user-fs-ccw,chardev=char-94dc2772c2c7eaec,tag=kataShared,queue-size=1024,devno=fe.0.0004 -netdev tap,id=network-0,fds=5 -device driver=virtio-net-ccw,netdev=network-0,mac=ce:58:f6:92:c5:07,mq=on,devno=fe.0.0005 -rtc base=utc,driftfix=slew,clock=host -global kvm-pit.lost_tick_policy=discard -vga none -no-user-config -nodefaults -nographic --no-reboot -object memory-backend-file,id=dimm1,size=2048M,mem-path=/dev/shm,share=on -machine memory-backend=dimm1 -kernel /opt/kata/share/kata-containers/kata-containers-se.img -append console=ttysclp0 quiet panic=1 nr_cpus=1 selinux=0 scsi_mod.scan=none -pidfile /run/vc/vm/217c61c958d255f517cacfe2cbb1cde84eb58a0f6af7f2a04ce98a1035c072c8/pid -smp 1,cores=1,threads=1,sockets=1,maxcpus=1
$ ps -ef | grep qemu | grep initrd
$ kubectl get po
NAME                                   READY   STATUS    RESTARTS   AGE
php-apache-kata-qemu-5f5fc68bd-9jmbx   1/1     Running   0          37m

For a normal kata configuration with initrd, a qemu argument -initrd should be specified to run a container like:

$ ps -ef | grep qemu | grep initrd
root      435056  435046  4 16:44 ?        00:00:00 /opt/kata/bin/qemu-system-s390x -name sandbox-73f231f408a3cf15b3b6b9182ad4e8ebc5751857907436fe3c6d5039ecb7eecf -uuid 3132e80b-ba75-4f2d-a50b-e8883280c2cd -machine s390-ccw-virtio,accel=kvm -cpu host, -qmp unix:fd=3,server=on,wait=off -m 2048M,slots=10,maxmem=15923M -device virtio-serial-ccw,id=serial0,devno=fe.0.0001 -device virtconsole,chardev=charconsole0,id=console0 -chardev socket,id=charconsole0,path=/run/vc/vm/73f231f408a3cf15b3b6b9182ad4e8ebc5751857907436fe3c6d5039ecb7eecf/console.sock,server=on,wait=off -device virtio-scsi-ccw,id=scsi0,devno=fe.0.0002 -device vhost-vsock-ccw,vhostfd=4,id=vsock-3232324942,guest-cid=3232324942,devno=fe.0.0003 -chardev socket,id=char-baa771172eab7fe6,path=/run/vc/vm/73f231f408a3cf15b3b6b9182ad4e8ebc5751857907436fe3c6d5039ecb7eecf/vhost-fs.sock -device vhost-user-fs-ccw,chardev=char-baa771172eab7fe6,tag=kataShared,queue-size=1024,devno=fe.0.0004 -netdev tap,id=network-0,fds=5 -device driver=virtio-net-ccw,netdev=network-0,mac=62:00:92:d6:bf:7a,mq=on,devno=fe.0.0005 -rtc base=utc,driftfix=slew,clock=host -global kvm-pit.lost_tick_policy=discard -vga none -no-user-config -nodefaults -nographic --no-reboot -object memory-backend-file,id=dimm1,size=2048M,mem-path=/dev/shm,share=on -machine memory-backend=dimm1 -kernel /opt/kata/share/kata-containers/vmlinux-6.1.52-116 -initrd /opt/kata/share/kata-containers/kata-ubuntu-20.04.initrd -append console=ttysclp0 quiet panic=1 nr_cpus=16 selinux=0 scsi_mod.scan=none -pidfile /run/vc/vm/73f231f408a3cf15b3b6b9182ad4e8ebc5751857907436fe3c6d5039ecb7eecf/pid -smp 1,cores=1,threads=1,sockets=16,maxcpus=16

@katacontainersbot katacontainersbot added the size/small Small and simple task label Dec 18, 2023
@BbolroC BbolroC added no-backport-needed area/runtime Issues that impact the runtime (including shimv2) and removed size/small Small and simple task labels Dec 18, 2023
@katacontainersbot katacontainersbot added the size/small Small and simple task label Dec 18, 2023
@BbolroC BbolroC force-pushed the ibm-se-config-validation-fix branch 2 times, most recently from e1dc739 to b9c2c00 Compare December 18, 2023 16:50
amshinde
amshinde reviewed Dec 18, 2023
View reviewed changes
src/runtime/pkg/govmm/qemu/qemu.go Outdated
if config.Kernel.Path != "" {
config.qemuParams = append(config.qemuParams, "-kernel")
config.qemuParams = append(config.qemuParams, config.Kernel.Path)

if config.Kernel.InitrdPath != "" {
config.qemuParams = append(config.qemuParams, "-initrd")
config.qemuParams = append(config.qemuParams, config.Kernel.InitrdPath)
} else {
if logger != nil {
logger.Infof("initrd path is empty, assuming IBM Z Secure Execution")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be logged even on non IBM z architectures right? I think this needs to go in the architecture specific qemu implementaion file. Similar comment for the code below as well.

Copy link
Member Author

@BbolroC BbolroC Dec 19, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there would be no such a case for other architectures due to conf.HypervisorMachineType == QemuCCWVirtio below. This conditional is already s390x specific (s390-ccw-virtio). Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. now I got your point. this will be logged for where kernel and image are set. I will change the code as you suggested. But others wouldn't be reached out because the conditional is located in where image and initrd are not set and a machine type is s390 specific. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: I've removed the log for appendKernel() because it should be architecture-agnostic and an error due to the misconfiguration of the parameters is unlikely to happen thanks to the validation code at a hypervisor level.

@BbolroC
runtime: Allow no initrd path for IBM Z Secure Execution
540a2a7 
This is to reintroduce a configuration rule for IBM Z Secure Execution,
where no initrd path should be configured. For the TEE of interest,
only a kernel image should be specified with `confidential_guest=true`.

Fixes: kata-containers#8692

Signed-off-by: Hyounggyu Choi <Hyounggyu.Choi@ibm.com>
@BbolroC
Copy link
Member Author

BbolroC commented Jan 10, 2024

/test

@BbolroC
Copy link
Member Author

BbolroC commented Jan 10, 2024

@amshinde I am looking forward to your follow-up feedback after my action to your comment. Thanks!

stevenhorsman
stevenhorsman approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay to me. Thanks!

@BbolroC BbolroC merged commit f62ec0a into kata-containers:main Jan 11, 2024
172 of 180 checks passed
@BbolroC BbolroC deleted the ibm-se-config-validation-fix branch January 11, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amshinde amshinde amshinde left review comments

@bergwolf bergwolf bergwolf approved these changes

@stevenhorsman stevenhorsman stevenhorsman approved these changes

Assignees
No one assigned
Labels
area/runtime Issues that impact the runtime (including shimv2) ok-to-test size/small Small and simple task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

runtime: Empty string for initrd not allowed for IBM Z Secure Execution
5 participants
@BbolroC @bergwolf @amshinde @stevenhorsman @katacontainersbot