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

[Xiaoyao Li]

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()

> Paolo
>
> >   static int tdx_validate_attributes(TdxGuest *tdx, Error **errp)
> >   {
> >       if ((tdx->attributes & ~tdx_caps->supported_attrs)) {
> > @@ -733,4 +749,5 @@ static void tdx_guest_class_init(ObjectClass *oc, 
> > void *data)
> >       klass->kvm_init = tdx_kvm_init;
> >       x86_klass->kvm_type = tdx_kvm_type;
> >       x86_klass->cpu_instance_init = tdx_cpu_instance_init;
> > +    x86_klass->cpu_realizefn = tdx_cpu_realizefn;
> >   }