From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 14B2C1FF183 for ; Wed, 8 Oct 2025 12:06:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8C90B3C39; Wed, 8 Oct 2025 12:06:27 +0200 (CEST) Message-ID: Date: Wed, 8 Oct 2025 12:06:22 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Anton Iacobaeus References: <20251001151237.50385-1-anton.iacobaeus@canarybit.eu> <20251001151237.50385-7-anton.iacobaeus@canarybit.eu> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20251001151237.50385-7-anton.iacobaeus@canarybit.eu> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759917952271 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.023 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [intel.com, wikipedia.org] Subject: Re: [pve-devel] [PATCH qemu-server v2 2/3] Add check for TDX support X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Am 04.10.25 um 3:23 PM schrieb Anton Iacobaeus: > From: Philipp Giersfeld > > Check whether TDX is enabled on this machine. Instead of using CPUID > like AMD SEV, Intel TDX enablement can be verified by reading an MSR > (https://cc-enabling.trustedservices.intel.com/intel-tdx-enabling-guide/05/host_os_setup/). > > Signed-off-by: Philipp Giersfeld > Signed-off-by: Anton Iacobaeus > --- > .../query-machine-capabilities.c | 97 ++++++++++++++++--- > src/usr/modules-load.conf | 1 + > 2 files changed, 85 insertions(+), 13 deletions(-) > > diff --git a/src/query-machine-capabilities/query-machine-capabilities.c b/src/query-machine-capabilities/query-machine-capabilities.c > index 0c522afc..5bc39450 100644 > --- a/src/query-machine-capabilities/query-machine-capabilities.c > +++ b/src/query-machine-capabilities/query-machine-capabilities.c > @@ -4,6 +4,8 @@ > #include > #include > #include > +#include > +#include > > #define eprintf(...) fprintf(stderr, __VA_ARGS__) > > @@ -18,17 +20,56 @@ typedef struct { > > uint8_t cbitpos; > uint8_t reduced_phys_bits; > -} cpu_caps_t; > +} cpu_caps_amd_sev_t; > > -void query_cpu_capabilities(cpu_caps_t *res) { > +typedef struct { > + bool tdx_support; Should we also check for Intel TME and SGX as described in the guide you referenced? > +} cpu_caps_intel_tdx_t; > + > +int read_msr(uint32_t msr_index, unsigned int *value) { > + int64_t data; It would be nicer if 'data' and 'value' are the same type (apart from the pointer). Both could be uint64_t, or? For value, it should also be adapted on the call site if going for this. > + char* msr_file_name = "/dev/cpu/0/msr"; > + int fd; > + > + fd = open(msr_file_name, O_RDONLY); > + if (fd < 0) { > + if (errno == ENXIO) { > + eprintf("rdmsr: No CPU 0\n"); > + return -1; > + } else if (errno == EIO) { > + eprintf("rdmsr: CPU doesn't support MSRs\n"); > + return -1; > + } else { > + perror("rdmsr: failed to open MSR"); > + return -1; > + } > + } > + > + if (pread(fd, &data, sizeof data, msr_index) != sizeof data) { Style nit: I'd prefer sizeof() with parentheses, because it eases reading a bit and is more common in my experience > + if (errno == EIO) { > + eprintf("rdmsr: CPU cannot read MSR 0x%08x\n", msr_index); > + return -1; > + } else { > + perror("rdmsr: pread"); > + return -1; > + } > + } > + > + *value = data; > + > + close(fd); > + return 0; > +} > + > +void query_cpu_capabilities_sev(cpu_caps_amd_sev_t *res) { > uint32_t eax, ebx, ecx, edx; > > // query Encrypted Memory Capabilities, see: > // https://en.wikipedia.org/wiki/CPUID#EAX=8000001Fh:_Encrypted_Memory_Capabilities > uint32_t query_function = 0x8000001F; > asm volatile("cpuid" > - : "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx) > - : "0"(query_function) > + : "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx) > + : "0"(query_function) > ); > > res->sev_support = (eax & (1<<1)) != 0; > @@ -39,6 +80,18 @@ void query_cpu_capabilities(cpu_caps_t *res) { > res->reduced_phys_bits = (ebx >> 6) & 0x3f; > } > > +int query_cpu_capabilities_tdx(cpu_caps_intel_tdx_t *res) { > + unsigned int value; > + > + if (read_msr(0x1401, &value) == 0) { > + res->tdx_support = (value & (1<<11)); > + } else { > + eprintf("Error reading MSR, Intel TDX support undetermined\n"); Nit: Since the register is expected to not be there for many CPUs, should we rather say "Cannot read" instead of "Error reading"? Error makes it sound like something is wrong which is not necessarily the case. Just to avoid admins getting nervous ;) Or maybe even just reduce the message to "Intel TDX support undetermined", the actual info what happened is already printed inside read_msr(). > + return -1; > + } > + return 0; > +} > + > int prepare_output_directory() { > // Check that the directory exists and create it if it does not. > struct stat statbuf; > @@ -65,8 +118,8 @@ int main() { > return 1; > } > > - cpu_caps_t caps; > - query_cpu_capabilities(&caps); > + cpu_caps_amd_sev_t caps_sev; > + query_cpu_capabilities_sev(&caps_sev); Pre-existing, but should we try to detect whether the CPU is AMD or Intel right away and then only do the relevant call? Otherwise, we risk the JSON claiming support for the other vendor's feature, because we query data in a way that doesn't make sense for the current vendor. Also to not show the error messages for AMD CPUs on every boot: Oct 08 11:50:21 pve9a1 query-machine-capabilities[1139]: rdmsr: CPU cannot read MSR 0x00001401 Oct 08 11:50:21 pve9a1 query-machine-capabilities[1139]: Error reading MSR, Intel TDX support undetermined > > FILE *file = fopen(OUTPUT_PATH, "w"); > if (file == NULL) { > @@ -82,18 +135,36 @@ int main() { > " \"sev-support\": %s," > " \"sev-support-es\": %s," > " \"sev-support-snp\": %s" > - " }" > - " }\n", > - caps.cbitpos, > - caps.reduced_phys_bits, > - caps.sev_support ? "true" : "false", > - caps.sev_es_support ? "true" : "false", > - caps.sev_snp_support ? "true" : "false" > + " }", > + caps_sev.cbitpos, > + caps_sev.reduced_phys_bits, > + caps_sev.sev_support ? "true" : "false", > + caps_sev.sev_es_support ? "true" : "false", > + caps_sev.sev_snp_support ? "true" : "false" > ); > if (ret < 0) { > eprintf("Error writing to file '" OUTPUT_PATH "': %s\n", strerror(errno)); > } > > + cpu_caps_intel_tdx_t caps_tdx; > + if (query_cpu_capabilities_tdx(&caps_tdx) == 0) { > + ret = fprintf(file, > + "," > + " \"intel-tdx\": {" > + " \"tdx-support\": %s" > + " }", > + caps_tdx.tdx_support ? "true" : "false" > + ); > + if (ret < 0) { > + eprintf("Error writing to file '" OUTPUT_PATH "': %s\n", strerror(errno)); > + } > + } > + > + ret = fprintf(file, " }\n"); > + if (ret < 0) { > + eprintf("Error writing to file '" OUTPUT_PATH "': %s\n", strerror(errno)); > + } > + > ret = fclose(file); > if (ret != 0) { > eprintf("Error closing file '" OUTPUT_PATH "': %s\n", strerror(errno)); > diff --git a/src/usr/modules-load.conf b/src/usr/modules-load.conf > index aee7d42a..f45d256b 100644 > --- a/src/usr/modules-load.conf > +++ b/src/usr/modules-load.conf > @@ -1 +1,2 @@ > vhost_net > +msr _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel