Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes.

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


* The goal of this RFC patch
The purpose of this post is to get feedback early on high level design issue of
libvirt enhancement for TDX. Referenced much on AMD SEV and S390 PV implemention
at link[2][3]. This 2nd version is rebased on upstream + s390 v4 version as
shown in [3] to utilize the common launchsecurity framework code.


* Patch organization
- patch 1-3: Support query of TDX capabilities.
- patch 4-6: Add TDX type to launchsecurity framework.
- patch   7: Add general loader support for TDX.
- patch   8: Add firmware descriptor support for TDX.


* Misc
Just let you know we have released v2 version of TDX qemu in [1], and the API
for libvirt is keeping stable. Using these patches we have succesfully booted
and tested a guest both with and without TDX enabled.


* Diff to v1:
- 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": [

    ]
}


Links:
[1] https://lists.nongnu.org/archive/html/qemu-devel/2021-07/msg01682.html
[2] https://github.com/codomania/libvirt/commits/v9
[3] https://www.mail-archive.com/libvir-list@redhat.com/msg219144.html

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: Add general loader support
  qemu: Add firmware descriptor support for TDX

 docs/formatdomaincaps.html.in  |  17 ++++++
 docs/schemas/domaincaps.rng    |   9 +++
 docs/schemas/domaincommon.rng  |  18 ++++++
 src/conf/domain_capabilities.c |   1 +
 src/conf/domain_capabilities.h |   1 +
 src/conf/domain_conf.c         |  49 ++++++++++++++++
 src/conf/domain_conf.h         |  11 ++++
 src/conf/virconftypes.h        |   2 +
 src/qemu/qemu_capabilities.c   |  44 ++++++++++++++-
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_command.c        |  38 +++++++++++++
 src/qemu/qemu_firmware.c       | 100 ++++++++++++++++++++++++++++++++-
 src/qemu/qemu_namespace.c      |   2 +
 src/qemu/qemu_process.c        |   1 +
 src/qemu/qemu_validate.c       |  28 +++++++++
 15 files changed, 319 insertions(+), 3 deletions(-)

-- 
2.25.1

On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
> Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2 comes.
> 
> * 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.
> 
> 
> * The goal of this RFC patch
> The purpose of this post is to get feedback early on high level design issue of
> libvirt enhancement for TDX. Referenced much on AMD SEV and S390 PV implemention
> at link[2][3]. This 2nd version is rebased on upstream + s390 v4 version as
> shown in [3] to utilize the common launchsecurity framework code.
> 
> 
> * Patch organization
> - patch 1-3: Support query of TDX capabilities.
> - patch 4-6: Add TDX type to launchsecurity framework.
> - patch   7: Add general loader support for TDX.
> - patch   8: Add firmware descriptor support for TDX.
> 
> 
> * Misc
> Just let you know we have released v2 version of TDX qemu in [1], and the API
> for libvirt is keeping stable. Using these patches we have succesfully booted
> and tested a guest both with and without TDX enabled.

Overall looks good. It's missing documentation and the QEMU patches
are missing documentation as well. I was looking into Intel
specification but I failed to find the necessary info there as well.
What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what
data is supposed to be stored there, what are the limitation and so on.

What I could gather these are exposed in the VM and are used for
measurement but that's it.

Another thing that I've missed in v1, QEMU patches are introducing new
`-machine pic=no` option and for TDX PIC has to be disabled. The libvirt
patches are putting it on the QEMU command line but it is not reflected
in the VM XML, so I would say we need to introduce new hypervisor
feature [1]:

  <features>
    ...
    <pic state='on|off'/>
    ...
  </features>

[1] <https://libvirt.org/formatdomain.html#hypervisor-features>

> * Diff to v1:
> - 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",

I think using 'loader' as that's the actual device in QEMU used with
this firmware will be better. The patches posted to QEMU doesn't extend
`docs/interop/firmware.json` so this example may change once some
specific format is accepted by QEMU community.

You will most likely need to add the firmware descriptor to QEMU project
as well (`pc-bios/descriptors/70-edk2-x86_64-tdx.json`). NOTE: The name
should not use `edk2` if it's not edk2 based firmware.

Pavel

>         "filename": "/usr/share/OVMF/OVMF_CODE-tdx.fd"
>     },
>     "targets": [
>         {
>             "architecture": "x86_64",
>             "machines": [
>                 "pc-q35-*"
>             ]
>         }
>     ],
>     "features": [
>         "intel-tdx",
>         "verbose-dynamic"
>     ],
>     "tags": [
> 
>     ]
> }
> 
> 
> Links:
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2021-07/msg01682.html
> [2] https://github.com/codomania/libvirt/commits/v9
> [3] https://www.mail-archive.com/libvir-list@redhat.com/msg219144.html
> 
> 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: Add general loader support
>   qemu: Add firmware descriptor support for TDX
> 
>  docs/formatdomaincaps.html.in  |  17 ++++++
>  docs/schemas/domaincaps.rng    |   9 +++
>  docs/schemas/domaincommon.rng  |  18 ++++++
>  src/conf/domain_capabilities.c |   1 +
>  src/conf/domain_capabilities.h |   1 +
>  src/conf/domain_conf.c         |  49 ++++++++++++++++
>  src/conf/domain_conf.h         |  11 ++++
>  src/conf/virconftypes.h        |   2 +
>  src/qemu/qemu_capabilities.c   |  44 ++++++++++++++-
>  src/qemu/qemu_capabilities.h   |   1 +
>  src/qemu/qemu_command.c        |  38 +++++++++++++
>  src/qemu/qemu_firmware.c       | 100 ++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_namespace.c      |   2 +
>  src/qemu/qemu_process.c        |   1 +
>  src/qemu/qemu_validate.c       |  28 +++++++++
>  15 files changed, 319 insertions(+), 3 deletions(-)
> 
> -- 
> 2.25.1
> 

> -----Original Message-----
> From: Pavel Hrdina <phrdina@redhat.com>
> Sent: Wednesday, July 21, 2021 10:23 PM
> To: Duan, Zhenzhong <zhenzhong.duan@intel.com>
> Cc: libvir-list@redhat.com; pkrempa@redhat.com; berrange@redhat.com;
> Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J
> <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com>
> Subject: Re: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support
> 
> On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
> > Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2
> comes.
> >
> > * 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.
[...]
> > * Misc
> > Just let you know we have released v2 version of TDX qemu in [1], and
> > the API for libvirt is keeping stable. Using these patches we have
> > succesfully booted and tested a guest both with and without TDX enabled.
> 
> Overall looks good. It's missing documentation and the QEMU patches are
> missing documentation as well. I was looking into Intel specification but I
> failed to find the necessary info there as well.
> What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data
> is supposed to be stored there, what are the limitation and so on.
Oh, yes. Thanks for point out. We will add the doc both for qemu and libvirt.

> 
> What I could gather these are exposed in the VM and are used for
> measurement but that's it.
> 
> Another thing that I've missed in v1, QEMU patches are introducing new `-
> machine pic=no` option and for TDX PIC has to be disabled. The libvirt
> patches are putting it on the QEMU command line but it is not reflected in
> the VM XML, so I would say we need to introduce new hypervisor feature [1]:
> 
>   <features>
>     ...
>     <pic state='on|off'/>
>     ...
>   </features>
> 
> [1] <https://libvirt.org/formatdomain.html#hypervisor-features>
Will add this feature.

> 
> > * Diff to v1:
> > - 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",
> 
> I think using 'loader' as that's the actual device in QEMU used with this
> firmware will be better. The patches posted to QEMU doesn't extend
> `docs/interop/firmware.json` so this example may change once some specific
> format is accepted by QEMU community.
Will do.
> 
> You will most likely need to add the firmware descriptor to QEMU project as
> well (`pc-bios/descriptors/70-edk2-x86_64-tdx.json`). NOTE: The name
> should not use `edk2` if it's not edk2 based firmware.
I see, will do. Thanks very much for your suggestions.

Regards
Zhenzhong

> -----Original Message-----
> From: Pavel Hrdina <phrdina@redhat.com>
> Sent: Wednesday, July 21, 2021 10:23 PM
> To: Duan, Zhenzhong <zhenzhong.duan@intel.com>
> Cc: libvir-list@redhat.com; pkrempa@redhat.com; berrange@redhat.com;
> Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J
> <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com>
> Subject: Re: [RFC PATCH v2 0/8] LIBVIRT: X86: TDX support
> 
> On Fri, Jul 16, 2021 at 11:10:28AM +0800, Zhenzhong Duan wrote:
> > Thanks Peter, Pavel and Daniel's comments on v1 version, now the v2
> comes.
[...]
> > * Misc
> > Just let you know we have released v2 version of TDX qemu in [1], and
> > the API for libvirt is keeping stable. Using these patches we have
> > succesfully booted and tested a guest both with and without TDX enabled.
> 
> Overall looks good. It's missing documentation and the QEMU patches are
> missing documentation as well. I was looking into Intel specification but I
> failed to find the necessary info there as well.
> What are the values `mrconfigid`, `mrowner`, `mrownerconfig` for, what data
> is supposed to be stored there, what are the limitation and so on.
> 
> What I could gather these are exposed in the VM and are used for
> measurement but that's it.
> 
> Another thing that I've missed in v1, QEMU patches are introducing new `-
> machine pic=no` option and for TDX PIC has to be disabled. The libvirt
> patches are putting it on the QEMU command line but it is not reflected in
> the VM XML, so I would say we need to introduce new hypervisor feature [1]:
> 
>   <features>
>     ...
>     <pic state='on|off'/>
>     ...
>   </features>
> 
> [1] <https://libvirt.org/formatdomain.html#hypervisor-features>
> 
> > * Diff to v1:
> > - 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",
> 
> I think using 'loader' as that's the actual device in QEMU used with this
> firmware will be better. The patches posted to QEMU doesn't extend
> `docs/interop/firmware.json` so this example may change once some specific
> format is accepted by QEMU community.
Hi Pavel,

Just want to clarify you want 'generic' changing to 'loader' only in 70-edk2-x86_64-tdx.json
Or also want all the 'generic' and '_GENERIC' string in ('[RFC PATCH v2 8/8] qemu: Add firmware descriptor support for TDX') to be changed?

Thanks
Zhenzhong