Re: [pve-devel] [PATCH qemu-server v2 3/3] Add support for Intel TDX

[Fiona Ebner <f.ebner@proxmox.com>]

Am 04.10.25 um 3:23 PM schrieb Anton Iacobaeus:
> From: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> 
> This commit adds support for setting up an Intel TDX VM. A Intel TDX VM
> can be setup similar to AMD SEV but uses a different firmware image.
> 
> Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
> Signed-off-by: Anton Iacobaeus <anton.iacobaeus@canarybit.eu>

Apart from a few nits, see below:
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> @@ -3965,6 +3978,10 @@ sub config_to_command {
>      if ($conf->{'amd-sev'}) {
>          push @$devices, '-object', get_amd_sev_object($conf->{'amd-sev'}, $conf->{bios});
>          push @$machineFlags, 'confidential-guest-support=sev0';
> +    } elsif ($conf->{'intel-tdx'}) {
> +        push @$devices, '-object', get_intel_tdx_object($conf->{'intel-tdx'}, $conf->{bios});
> +        push @$machineFlags, 'confidential-guest-support=tdx0';
> +        push @$machineFlags, 'kernel_irqchip=split';

Nit: would be nice to have a comment describing the rationale behind the
kernel_irqchip option and/or a sentence in the commit message.

>      }
>  
>      PVE::QemuServer::Virtiofs::config($conf, $vmid, $devices);
> diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> index 65a7b565..bd5540e6 100644
> --- a/src/PVE/QemuServer/CPUConfig.pm
> +++ b/src/PVE/QemuServer/CPUConfig.pm
> @@ -18,6 +18,7 @@ our @EXPORT_OK = qw(
>      get_cpu_bitness
>      is_native_arch
>      get_amd_sev_object
> +    get_intel_tdx_object
>      get_cvm_type
>  );
>  
> @@ -282,6 +283,18 @@ my $sev_fmt = {
>  };
>  PVE::JSONSchema::register_format('pve-qemu-sev-fmt', $sev_fmt);
>  
> +my $tdx_fmt = {
> +    type => {
> +        description => "Enable TDX",
> +        type => 'string',
> +        default_key => 1,
> +        format_description => "tdx-type",
> +        enum => ['tdx'],
> +        maxLength => 3,

Nit: There is an explicit enum already, so this is superfluous and needs
to be dropped/updated as soon as a variant with a longer name appears

> +    },
> +};
> +PVE::JSONSchema::register_format('pve-qemu-tdx-fmt', $tdx_fmt);
> +
>  PVE::JSONSchema::register_format('pve-phys-bits', \&parse_phys_bits);
>  
>  sub parse_phys_bits {
> @@ -887,6 +900,9 @@ sub get_cvm_type {
>      if ($conf->{'amd-sev'}) {
>          my $sev = PVE::JSONSchema::parse_property_string($sev_fmt, $conf->{'amd-sev'});
>          return $sev->{type};
> +    } elsif ($conf->{'intel-tdx'}) {
> +        my $tdx = PVE::JSONSchema::parse_property_string($tdx_fmt, $conf->{'intel-tdx'});
> +        return $tdx->{type};
>      } else {
>          return undef;
>      }
> @@ -945,6 +961,21 @@ sub get_amd_sev_object {
>      return $sev_mem_object;
>  }
>  
> +sub get_intel_tdx_object {
> +    my ($intel_tdx, $bios) = @_;
> +    my $intel_tdx_conf = PVE::JSONSchema::parse_property_string($tdx_fmt, $intel_tdx);
> +    my $tdx_hw_caps = get_hw_capabilities()->{'intel-tdx'};
> +    

Style nit: additional whitespace above here

> +    if (!$tdx_hw_caps->{'tdx-support'}) {
> +	    die "Your CPU does not support Intel TDX.\n";

Style nit: a tab snuck in here, you can use 'make tidy' to have the code
formatted nowadays

> +    }
> +    if (!$bios || $bios ne 'ovmf') {
> +	    die "To use Intel TDX, you need to change the BIOS to OVMF.\n";

Style nit: same here

> +    }
> +    my $tdx_mem_object = 'tdx-guest,id=tdx0';

Style nit: pre-existing with AMD SEV, but I'd not use _mem_ in the
varialbe name here. It could also just be a direct return of the string.

> +    return $tdx_mem_object;
> +}
> +
>  __PACKAGE__->register();
>  __PACKAGE__->init();
>  


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel