On 16/05/2024 11:59 am, Edgecombe, Rick P wrote:
On Thu, 2024-05-16 at 11:44 +1200, Huang, Kai wrote:
Sorry, still not clear. We need to strip the bit away, so we need to know
what
bit it is. The proposal is to not remember it on struct kvm, so where do we
get
it?
The TDX specific code can get it when TDX guest is created.
The TDX specific code sets it. It knows GPAW/shared bit location.
Actually, we used to allow it to be selected (via GPAW), but now we could
determine it based on EPT level and MAXPA. So we could possibly recalculate
it
in some helper...
But it seems you are suggesting to do away with the concept of knowing what
the
shared bit is.
What I am suggesting is essentially to replace this
kvm_gfn_shared_mask() with some kvm_x86_ops callback (which can just
return the shared bit), assuming the common code somehow still need it
(e.g., setting up the SPTE for shared mapping, which must include the
shared bit to the GPA).
The advantage of this we can get rid of the concept of 'gfn_shared_mask'
in the MMU common code. All GFNs referenced in the common code is the
actual GFN (w/o the shared bit).
When it is actually being used as the shared bit instead of as a way to check if
a guest is a TD, what is the problem? I think the shared_mask serves a real
(small) purpose, but it is misused for a bunch of other stuff. If we move that
other stuff to new helpers, the shared mask will still be needed for it's
original job.
What is the benefit of the x86_ops over a static inline?
I don't have strong objection if the use of kvm_gfn_shared_mask() is
contained in smaller areas that truly need it. Let's discuss in
relevant patch(es).
However I do think the helpers like below makes no sense (for SEV-SNP):
+static inline bool kvm_is_private_gpa(const struct kvm *kvm, gpa_t gpa)
+{
+ gfn_t mask = kvm_gfn_shared_mask(kvm);
+
+ return mask && !(gpa_to_gfn(gpa) & mask);
+}
