Re: [PATCH 04/16] KVM: x86/mmu: Add address conversion functions for TDX shared bit of GPA

Huang, Kai
2024-05-17T04:27:13Z

>> E.g,  why we cannot do:
>>
>>         static bool kvm_use_private_root(struct kvm *kvm)
>>         {
>>                 return kvm->arch.vm_type == VM_TYPE_TDX;
>>         }
>>
>> Or,
>>         static bool kvm_use_private_root(struct kvm *kvm)
>>         {
>>                 return kvm->arch.use_private_root;
>>         }
>>
>> Or, assuming we would love to keep the kvm_gfn_shared_mask():
>>
>>         static bool kvm_use_private_root(struct kvm *kvm)
>>         {
>>                 return !!kvm_gfn_shared_mask(kvm);
>>         }
>>
>> And then:
>>
>> In fault handler:
>>
>>         if (fault->is_private && kvm_use_private_root(kvm))
>>                 // use private root
>>         else
>>                 // use shared/normal root
>>
>> When you zap:
>>
>>         bool private_gpa = kvm_mem_is_private(kvm, gfn);
>>
>>         if (private_gpa && kvm_use_private_root(kvm))
>>                 // zap private root
>>         else
>>                 // zap shared/normal root.
>>
>
> I think you are trying to say not to abuse kvm_gfn_shared_mask() as is currently
> done in this logic. But we already agreed on this. So not sure.

To be clear: We agreed on this in general, but not on this
kvm_on_private_root().

It's obvious that you still want to "use kvm_gfn_shared_mask() to
determine whether a GPA is private" for this helper but I don't like it.
In fact I don't see why we even need this helper.

I think I am just too obsessed on avoiding using kvm_gfn_shared_mask()
so I'll stop commenting/replying on this.

[...]

>
> I don't think we can get rid of the shared mask. Even if we relied on
> kvm_mem_is_private() to determine if a GPA is private or shared, at absolute
> minimum we need to add the shared bit when we are zapping a GFN or mapping it.

No we cannot, but we can avoid using it here.

>
> Let's table the discussion until we have some code to look again.

100% agreed.