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] 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 550a3c31b6..8bf17967ba 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 e1cb2a2d19..3fcdbe0d6f 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 605ad5b3e6..e8a6e1511f 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 } }