qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 34/60] i386/tdx: implement tdx_cpu_realizefn()


From: Xiaoyao Li
Subject: Re: [PATCH v6 34/60] i386/tdx: implement tdx_cpu_realizefn()
Date: Tue, 14 Jan 2025 16:52:07 +0800
User-agent: Mozilla Thunderbird

On 12/13/2024 6:04 AM, Ira Weiny wrote:
On Tue, Nov 05, 2024 at 12:53:25PM +0100, Paolo Bonzini wrote:
On 11/5/24 12:38, Xiaoyao Li wrote:
On 11/5/2024 6:06 PM, Paolo Bonzini wrote:
On 11/5/24 07:23, Xiaoyao Li wrote:
+static void tdx_cpu_realizefn(X86ConfidentialGuest *cg, CPUState *cs,
+                              Error **errp)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    uint32_t host_phys_bits = host_cpu_phys_bits();
+
+    if (!cpu->phys_bits) {
+        cpu->phys_bits = host_phys_bits;
+    } else if (cpu->phys_bits != host_phys_bits) {
+        error_setg(errp, "TDX only supports host physical bits (%u)",
+                   host_phys_bits);
+    }
+}

This should be already handled by host_cpu_realizefn(), which is
reached via cpu_exec_realizefn().

Why is it needed earlier, but not as early as instance_init?  If
absolutely needed I would do the assignment in patch 33, but I don't
understand why it's necessary.

It's not called earlier but right after cpu_exec_realizefn().

Patch 33 adds x86_confidenetial_guest_cpu_realizefn() right after
ecpu_exec_realizefn(). This patch implements the callback and gets
called in x86_confidenetial_guest_cpu_realizefn() so it's called after
cpu_exec_realizefn().

The reason why host_cpu_realizefn() cannot satisfy is that for normal
VMs, the check in cpu_exec_realizefn() is just a warning and QEMU does
allow the user to configure the physical address bit other than host's
value, and the configured value will be seen inside guest. i.e., "-cpu
phys-bits=xx" where xx != host_value works for normal VMs.

But for TDX, KVM doesn't allow it and the value seen in TD guest is
always the host value.  i.e., "-cpu phys-bits=xx" where xx != host_value
doesn't work for TDX.

Either way, the check should be in tdx_check_features.

Good idea. I will try to implement it in tdx_check_features()

Is there any reason the TDX code can't just force cpu->host_phys_bits to true?

That doesn't work for all the cases. e.g., when user set "host-phys-bits-limit" to a smaller value. For this case, QEMU still needs to validate the final cpu->phys_bits.

Of course, we can force host_phys_bits to true for TDX, and warn and exit when user set "host-phys-bits-limit" to a smaller value than host value.

But I prefer the current direction to check cpu->phys_bits directly, which is straightforward.


Thanks, and I think there's no need to change cpu->phys_bits, either. So
x86_confidenetial_guest_cpu_realizefn() should not be necessary.

I was going to comment that patch 33 should be squashed here but better to just
drop it.

Ira


Paolo





reply via email to

[Prev in Thread] Current Thread [Next in Thread]