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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 17 May 2024 16:26:51 +1200
From: "Huang, Kai" <kai.huang@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
	"seanjc@...gle.com" <seanjc@...gle.com>
CC: "sagis@...gle.com" <sagis@...gle.com>, "dmatlack@...gle.com"
	<dmatlack@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Zhao, Yan Y" <yan.y.zhao@...el.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>, "Aktas, Erdem"
	<erdemaktas@...gle.com>
Subject: Re: [PATCH 04/16] KVM: x86/mmu: Add address conversion functions for
 TDX shared bit of GPA


>> 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.