From fe5adae5d9b5380015c5c4f8f200daf4343c76f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 12 Jan 2024 12:20:35 +0100 Subject: [PATCH 1/6] qemu-tdx: Update to v8.1.0 + TDX patches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's update the QEMU to the one that's officially maintained by Intel till all the TDX patches make their way upstream. We've had to also update python to explicitly use python3 and add python3-venv as part of the dependencies. Fixes: #8810 Signed-off-by: Fabiano Fidêncio --- .../tdx-qemu-next-2023.9.21-v8.1.0/no_patches.txt | 0 tools/packaging/static-build/qemu/Dockerfile | 4 ++-- versions.yaml | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) create mode 100644 tools/packaging/qemu/patches/tag_patches/tdx-qemu-next-2023.9.21-v8.1.0/no_patches.txt diff --git a/tools/packaging/qemu/patches/tag_patches/tdx-qemu-next-2023.9.21-v8.1.0/no_patches.txt b/tools/packaging/qemu/patches/tag_patches/tdx-qemu-next-2023.9.21-v8.1.0/no_patches.txt new file mode 100644 index 000000000..e69de29bb diff --git a/tools/packaging/static-build/qemu/Dockerfile b/tools/packaging/static-build/qemu/Dockerfile index 7ed1b8995..1c4fd45d1 100644 --- a/tools/packaging/static-build/qemu/Dockerfile +++ b/tools/packaging/static-build/qemu/Dockerfile @@ -58,8 +58,8 @@ RUN apt-get update && apt-get upgrade -y && \ libseccomp-dev${DPKG_ARCH} \ libseccomp2${DPKG_ARCH} \ patch \ - python \ - python-dev \ + python3 \ + python3-dev \ python3-venv \ rsync \ zlib1g-dev${DPKG_ARCH} && \ diff --git a/versions.yaml b/versions.yaml index 36b817bdd..bc3a77bc1 100644 --- a/versions.yaml +++ b/versions.yaml @@ -100,10 +100,9 @@ assets: .*/v?(\d\S+)\.tar\.gz qemu-tdx-experimental: - # yamllint disable-line rule:line-length - description: "QEMU with TDX support - based on https://github.com/intel/tdx-tools/releases/tag/2023ww15" - url: "https://github.com/kata-containers/qemu" - tag: "b67b00e6b4c7831a3f5bc684bc0df7a9bfd1bd56-plus-TDX-v1.10" + description: ¨QEMU with TDX support" + url: "https://github.com/intel/qemu-tdx" + tag: "tdx-qemu-next-2023.9.21-v8.1.0" qemu-snp-experimental: description: "QEMU with SNP support" From 582b5b6b1928ba61e48692ce9c015a28c22e8301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 12 Jan 2024 17:36:33 +0100 Subject: [PATCH 2/6] govmm: tdx: Expose the private=on|off knob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The private=on|off knob is required in order to properly lauunch a TDX guest VM. This is a brand new property that is part of the still in-flight patches adding TDX support on QEMU. Please, see: https://github.com/intel/qemu-tdx/commit/3fdd8072da466d8d6c3da3840269bd7a3ef09293 Signed-off-by: Fabiano Fidêncio --- src/runtime/pkg/govmm/qemu/qemu.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/runtime/pkg/govmm/qemu/qemu.go b/src/runtime/pkg/govmm/qemu/qemu.go index f121696c9..550a3c31b 100644 --- a/src/runtime/pkg/govmm/qemu/qemu.go +++ b/src/runtime/pkg/govmm/qemu/qemu.go @@ -2674,6 +2674,10 @@ type Knobs struct { // IOMMUPlatform will enable IOMMU for supported devices IOMMUPlatform bool + + // Whether private memory should be used or not + // This is required by TDX, at least. + Private bool } // IOThread allows IO to be performed on a separate thread. From 6b4cc5ea6af8eee88a0a774558eef57a40b11cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 12 Jan 2024 15:25:27 +0100 Subject: [PATCH 3/6] Revert "qemu: tdx: Workaround SMP issue with TDX 1.5" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit d1b54ede290e95762099fff4e0bcdad10f816126. Conflicts: src/runtime/virtcontainers/qemu.go This commit was a hack that was needed in order to get QEMU + TDX to work atop of the stack our CI was running on. As we're moving to "the officially supported by distros" host OS, we need to get rid of this. Signed-off-by: Fabiano Fidêncio --- src/runtime/virtcontainers/qemu.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 96b004533..605ad5b3e 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -629,13 +629,6 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi // headaches in the future. knobs.FileBackedMem = false knobs.MemShared = false - - // SMP is currently broken with TDX 1.5, and - // we must ensure we use something like: - // `...,sockets=1,cores=numvcpus,threads=1,...` - smp.Sockets = 1 - smp.Cores = q.config.NumVCPUs() - smp.Threads = 1 } } From b7cccfa019f6bb49bad5b72d4a23fe5ef4523df0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Fri, 12 Jan 2024 15:27:46 +0100 Subject: [PATCH 4/6] qemu: tdx: Adapt command line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit is a mess, but I'm not exactly sure what's the best way to make it less messy, as we're getting QEMU TDX to work while partially reverting 1e34220c41c813a3a585da226e8624f3551a77df. With that said, let me cover the content of this commit. Firstly, we're reverting all the changes related to "memory-backend-memfd-private", as that's what was used with the previous host stack, but it seems it didn't fly upstream. Secondly, in order to get QEMU to properly work with TDX, we need to enforce the 'private=on' knob and use the "memory-backend-ram", and we're doing so, and also making sure to test the `private=on` newly added knob. I'm sorry for the confusion, I understand this is not optimal, I just don't see an easy path to do changes without leaving the code broken during those changes. Signed-off-by: Fabiano Fidêncio --- src/runtime/pkg/govmm/qemu/qemu.go | 22 +++------- src/runtime/pkg/govmm/qemu/qemu_test.go | 54 ++++++++++++++----------- src/runtime/virtcontainers/qemu.go | 10 +---- 3 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/runtime/pkg/govmm/qemu/qemu.go b/src/runtime/pkg/govmm/qemu/qemu.go index 550a3c31b..8bf17967b 100644 --- a/src/runtime/pkg/govmm/qemu/qemu.go +++ b/src/runtime/pkg/govmm/qemu/qemu.go @@ -141,16 +141,9 @@ const ( func isDimmSupported(config *Config) bool { switch runtime.GOARCH { case "amd64", "386", "ppc64le", "arm64": - if config != nil { - if config.Machine.Type == MachineTypeMicrovm { - // microvm does not support NUMA - return false - } - if config.Knobs.MemFDPrivate { - // TDX guests rely on MemFD Private, which - // does not have NUMA support yet - return false - } + if config != nil && config.Machine.Type == MachineTypeMicrovm { + // microvm does not support NUMA + return false } return true default: @@ -2649,9 +2642,6 @@ type Knobs struct { // MemPrealloc will allocate all the RAM upfront MemPrealloc bool - // Private Memory FD meant for private memory map/unmap. - MemFDPrivate bool - // FileBackedMem requires Memory.Size and Memory.Path of the VM to // be set. FileBackedMem bool @@ -3021,13 +3011,10 @@ func (config *Config) appendMemoryKnobs() { return } var objMemParam, numaMemParam string - dimmName := "dimm1" if config.Knobs.HugePages { objMemParam = "memory-backend-file,id=" + dimmName + ",size=" + config.Memory.Size + ",mem-path=/dev/hugepages" numaMemParam = "node,memdev=" + dimmName - } else if config.Knobs.MemFDPrivate { - objMemParam = "memory-backend-memfd-private,id=" + dimmName + ",size=" + config.Memory.Size } else if config.Knobs.FileBackedMem && config.Memory.Path != "" { objMemParam = "memory-backend-file,id=" + dimmName + ",size=" + config.Memory.Size + ",mem-path=" + config.Memory.Path numaMemParam = "node,memdev=" + dimmName @@ -3036,6 +3023,9 @@ func (config *Config) appendMemoryKnobs() { numaMemParam = "node,memdev=" + dimmName } + if config.Knobs.Private { + objMemParam += ",private=on" + } if config.Knobs.MemShared { objMemParam += ",share=on" } diff --git a/src/runtime/pkg/govmm/qemu/qemu_test.go b/src/runtime/pkg/govmm/qemu/qemu_test.go index e1cb2a2d1..3fcdbe0d6 100644 --- a/src/runtime/pkg/govmm/qemu/qemu_test.go +++ b/src/runtime/pkg/govmm/qemu/qemu_test.go @@ -586,6 +586,7 @@ func TestAppendMemoryFileBackedMem(t *testing.T) { knobs := Knobs{ FileBackedMem: true, MemShared: false, + Private: false, } objMemString := "-object memory-backend-file,id=dimm1,size=1G,mem-path=foobar" numaMemString := "-numa node,memdev=dimm1" @@ -599,6 +600,36 @@ func TestAppendMemoryFileBackedMem(t *testing.T) { } testConfigAppend(conf, knobs, memString+" "+knobsString, t) + + // Reset the conf and memString values + conf = &Config{ + Memory: Memory{ + Size: "1G", + Slots: 8, + MaxMem: "3G", + Path: "foobar", + }, + } + memString = "-m 1G,slots=8,maxmem=3G" + testConfigAppend(conf, conf.Memory, memString, t) + + knobs = Knobs{ + FileBackedMem: true, + MemShared: false, + Private: true, + } + objMemString = "-object memory-backend-file,id=dimm1,size=1G,mem-path=foobar,private=on" + numaMemString = "-numa node,memdev=dimm1" + memBackendString = "-machine memory-backend=dimm1" + + knobsString = objMemString + " " + if isDimmSupported(nil) { + knobsString += numaMemString + } else { + knobsString += memBackendString + } + + testConfigAppend(conf, knobs, memString+" "+knobsString, t) } func TestAppendMemoryFileBackedMemPrealloc(t *testing.T) { @@ -632,29 +663,6 @@ func TestAppendMemoryFileBackedMemPrealloc(t *testing.T) { testConfigAppend(conf, knobs, memString+" "+knobsString, t) } -func TestAppendMemoryBackedMemFdPrivate(t *testing.T) { - conf := &Config{ - Memory: Memory{ - Size: "1G", - Slots: 8, - }, - } - memString := "-m 1G,slots=8" - testConfigAppend(conf, conf.Memory, memString, t) - - knobs := Knobs{ - MemFDPrivate: true, - MemShared: false, - } - objMemString := "-object memory-backend-memfd-private,id=dimm1,size=1G" - memBackendString := "-machine memory-backend=dimm1" - - knobsString := objMemString + " " - knobsString += memBackendString - - testConfigAppend(conf, knobs, memString+" "+knobsString, t) -} - func TestNoRebootKnob(t *testing.T) { conf := &Config{} diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 605ad5b3e..e8a6e1511 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -620,15 +620,9 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi // on the hypervisor specific code, as availableGuestProtection() // has been called earlier and we know we have the value stored. if q.arch.getProtection() == tdxProtection { - knobs.MemFDPrivate = true - // In case Nydus or VirtioFS is used, which may become a reality - // in the future, whenever we get those hardened for TDX, those - // knobs below would be automatically set. Let's make sure we - // pre-emptively disable them, and with that we can avoid some - // headaches in the future. - knobs.FileBackedMem = false - knobs.MemShared = false + // TDX relies on ",private=on" passed to the memory object. + knobs.Private = true } } From 2ee03b5dc3d48d5d9dbd527b32f180e82373b266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Wed, 17 Jan 2024 21:54:05 +0100 Subject: [PATCH 5/6] tdvf: Adapt the build command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is done in order to match the example from: https://github.com/intel/tdx-linux/wiki/Instruction-to-set-up-TDX-host-and-guest#build-tdvf-image Signed-off-by: Fabiano Fidêncio --- tools/packaging/static-build/ovmf/build-ovmf.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/packaging/static-build/ovmf/build-ovmf.sh b/tools/packaging/static-build/ovmf/build-ovmf.sh index c0e7d26e2..b87307458 100755 --- a/tools/packaging/static-build/ovmf/build-ovmf.sh +++ b/tools/packaging/static-build/ovmf/build-ovmf.sh @@ -56,7 +56,7 @@ fi info "Building ovmf" build_cmd="build -b ${build_target} -t ${toolchain} -a ${architecture} -p ${ovmf_package}" if [ "${ovmf_build}" == "tdx" ]; then - build_cmd+=" -D DEBUG_ON_SERIAL_PORT=FALSE -D TDX_MEM_PARTIAL_ACCEPT=512 -D TDX_EMULATION_ENABLE=FALSE -D SECURE_BOOT_ENABLE=TRUE -D TDX_ACCEPT_PAGE_SIZE=2M" + build_cmd+=" -D SECURE_BOOT_ENABLE=TRUE" fi eval "${build_cmd}" From cdb8531302e35f6ae898d63ff4646c53ccda8a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Thu, 18 Jan 2024 14:23:14 +0100 Subject: [PATCH 6/6] hypervisor: Simplify TDX protection detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's rely on the kvm module 'tdx' parameter to do so. This aligns with both OSVs (Canonical, Red Hat, SUSE) and the TDX adoption (https://github.com/intel/tdx-linux) stacks. Signed-off-by: Fabiano Fidêncio --- .../virtcontainers/hypervisor_linux_amd64.go | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/src/runtime/virtcontainers/hypervisor_linux_amd64.go b/src/runtime/virtcontainers/hypervisor_linux_amd64.go index 043b36c9f..ac8454016 100644 --- a/src/runtime/virtcontainers/hypervisor_linux_amd64.go +++ b/src/runtime/virtcontainers/hypervisor_linux_amd64.go @@ -8,39 +8,21 @@ package virtcontainers import "os" const ( - tdxSeamSysFirmwareDir = "/sys/firmware/tdx_seam/" - - tdxSysFirmwareDir = "/sys/firmware/tdx/" + tdxKvmParameterPath = "/sys/module/kvm_intel/parameters/tdx" sevKvmParameterPath = "/sys/module/kvm_amd/parameters/sev" snpKvmParameterPath = "/sys/module/kvm_amd/parameters/sev_snp" ) -// TDX is supported and properly loaded when the firmware directory (either tdx or tdx_seam) exists or `tdx` is part of the CPU flag -func checkTdxGuestProtection(flags map[string]bool) bool { - if d, err := os.Stat(tdxSysFirmwareDir); err == nil && d.IsDir() { - return true - } - - if d, err := os.Stat(tdxSeamSysFirmwareDir); err == nil && d.IsDir() { - return true - } - - return false -} - // Implementation of this function is architecture specific func availableGuestProtection() (guestProtection, error) { - flags, err := CPUFlags(procCPUInfo) - if err != nil { - return noneProtection, err + // TDX is supported and enabled when the kvm module 'tdx' parameter is set to 'Y' + if _, err := os.Stat(tdxKvmParameterPath); err == nil { + if c, err := os.ReadFile(tdxKvmParameterPath); err == nil && len(c) > 0 && (c[0] == 'Y') { + return tdxProtection, nil + } } - - if checkTdxGuestProtection(flags) { - return tdxProtection, nil - } - // SEV-SNP is supported and enabled when the kvm module `sev_snp` parameter is set to `Y` // SEV-SNP support infers SEV (-ES) support if _, err := os.Stat(snpKvmParameterPath); err == nil {