9:38 p.m.
-----Original Message-----
From: Jonathon Jongsma <jjongsma(a)redhat.com>
Sent: Saturday, December 2, 2023 6:30 AM
Subject: Re: [PATCH rfcv3 00/11] LIBVIRT: X86: TDX support
Hello,
Thanks for the submission. A few initial general comments:
On 11/27/23 2:55 AM, Zhenzhong Duan wrote:
> Hi,
>
> This series brings libvirt the x86 TDX support.
>
> * What's TDX?
> TDX stands for Trust Domain Extensions which isolates VMs from
> the virtual-machine manager (VMM)/hypervisor and any other software
on
> the platform.
>
> To support TDX, multiple software components, not only KVM but also
QEMU,
> guest Linux and virtual bios, need to be updated. For more details, please
> check link[1], there are TDX spec links and public repository link at github
> for each software component.
>
> This patchset is another software component to extend libvirt to support
TDX,
> with which one can start a VM from high level rather than running qemu
directly.
>
>
> * Misc
> As QEMU use a software emulated way to reset guest which isn't
supported by TDX
> guest for security reason. We add a new way to emulate the reset for TDX
guest,
> called "hard reboot". We achieve this by killing old qemu and start a new
one.
Can you expand on this a little bit more? What problems do you encounter
when you reboot the normal way? I did not notice any patches related to
a hard reboot in the v2 patchset that was submitted a while ago.
If we use existing "fake reboot" in libvirt, qmp command
"system_reset" isn't
supported for TDX guest, because TDX doesn't support resetting each register
of vcpu for security except in TDX debug mode. TDX guest will be shutdown
instead.
In v2, reboot isn't supported yet, only support shutdown for TDX guest.
What other approaches did you consider to solve this issue? The changes
to virsh adding a reconnect timeout option for the console command in
particular feel hacky to me.
One possible way I can think of is to let qemu do the kill/create job,
i.e. destroy TDX vcpus, create new one. This way we can utilize existing
"fake reboot" interface between qemu and libvirt.
Yes, that patch looks hacky, we can drop it and not to support virsh reconnect.
> Complete code can be found at [1], matching qemu code can be found at
[2].
>
> There are some new properties for tdx-guest object, i.e. `mrconfigid`,
`mrowner`,
> `mrownerconfig` and `debug` which aren't in matching qemu[2] yet. I keep
them
> intentionally as they will be implemented in qemu as extention series of [2].
>
>
> * Test
> start/stop/reboot with virsh
> stop/reboot trigger in guest
> stop with on_poweroff=destroy/restart
> reboot with on_reboot=destroy/restart
>
> * Patch organization
> - patch 1-3: Support query of TDX capabilities.
> - patch 4-6: Add TDX type to launchsecurity framework.
> - patch 7-11: Add hard reboot support to TDX guest
I would expect to see some test cases for tdx launch security as well
(at least in qemuxml2argv). I suppose it is difficult to incorporate
these tests until the qemu changes are merged upstream and we can
regenerate capabilities that include tdx-guest support, etc, though...
Will add test cases in v3.
Thanks
Zhenzhong
> [1] https://github.com/intel/libvirt-tdx/commits/tdx_for_upstream_rfcv3
> [2] https://github.com/intel/qemu-tdx/tree/tdx-qemu-upstream-v3
>
> Thanks
> Zhenzhong
>
>
> Changelog:
> rfcv3:
> - Change to generate qemu cmdline with -bios
> - drop firmware auto match as -bios is used
> - add a hard reboot method to reboot TDX guest
>
> rfcv2:
> - give up using qmp cmd and check TDX directly on host for TDX
capabilities.
> - use launchsecurity framework to support TDX
> - use <os>.<loader> for general loader
> - add auto firmware match feature for TDX
>
> A example TDVF fimware description file 70-edk2-x86_64-tdx.json:
> {
> "description": "UEFI firmware for x86_64, supporting Intel
TDX",
> "interface-types": [
> "uefi"
> ],
> "mapping": {
> "device": "generic",
> "filename": "/usr/share/OVMF/OVMF_CODE-tdx.fd"
> },
> "targets": [
> {
> "architecture": "x86_64",
> "machines": [
> "pc-q35-*"
> ]
> }
> ],
> "features": [
> "intel-tdx",
> "verbose-dynamic"
> ],
> "tags": [
>
> ]
> }
>
> rfcv2:
> https://www.mail-archive.com/libvir-list@redhat.com/msg219378.html
>
> Chenyi Qiang (3):
> qemu: add hard reboot in QEMU driver
> qemu: make hard reboot as the TDX default reboot mode
> virsh: add new option "timekeep" to keep virsh console alive
>
> Zhenzhong Duan (8):
> qemu: Check if INTEL Trust Domain Extention support is enabled
> qemu: Add TDX capability
> conf: expose TDX feature in domain capabilities
> conf: add tdx as launch security type
> qemu: Add command line and validation for TDX type
> qemu: force special parameters enabled for TDX guest
> qemu: Extend hard reboot in Qemu driver
> conf: Add support to keep same domid for hard reboot
>
> docs/formatdomaincaps.rst | 1 +
> include/libvirt/libvirt-domain.h | 2 +
> src/conf/domain_capabilities.c | 1 +
> src/conf/domain_capabilities.h | 1 +
> src/conf/domain_conf.c | 50 ++++++++++++++++
> src/conf/domain_conf.h | 11 ++++
> src/conf/schemas/domaincaps.rng | 9 +++
> src/conf/schemas/domaincommon.rng | 34 +++++++++++
> src/conf/virconftypes.h | 2 +
> src/qemu/qemu_capabilities.c | 38 +++++++++++-
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_command.c | 29 +++++++++
> src/qemu/qemu_domain.c | 18 ++++++
> src/qemu/qemu_domain.h | 4 ++
> src/qemu/qemu_driver.c | 85 ++++++++++++++++++++------
> src/qemu/qemu_firmware.c | 1 +
> src/qemu/qemu_monitor.c | 19 +++++-
> src/qemu/qemu_monitor.h | 2 +-
> src/qemu/qemu_monitor_json.c | 6 +-
> src/qemu/qemu_namespace.c | 1 +
> src/qemu/qemu_process.c | 99
++++++++++++++++++++++++++++++-
> src/qemu/qemu_validate.c | 18 ++++++
> tools/virsh-console.c | 3 +
> tools/virsh-domain.c | 64 +++++++++++++++-----
> tools/virsh.h | 1 +
> 25 files changed, 463 insertions(+), 37 deletions(-)
>