On 16/05/2024 1:20 pm, Edgecombe, Rick P wrote:
On Thu, 2024-05-16 at 13:04 +1200, Huang, Kai wrote:
I really don't see difference between ...
is_private_mem(gpa)
... and
is_private_gpa(gpa)
If it confuses me, it can confuses other people.
Again, point taken. I'll try to think of a better name. Please share if you do.
The point is there's really no need to distinguish the two. The GPA is
only meaningful when it refers to the memory that it points to.
So far I am not convinced we need this helper, because such info we can
already get from:
1) fault->is_private;
2) Xarray which records memtype for given GFN.
So we should just get rid of it.
Kai, can you got look through the dev branch a bit more before making the same
point on every patch?
kvm_is_private_gpa() is used to set PFERR_PRIVATE_ACCESS, which in turn sets
fault->is_private. So you are saying we can use these other things that are
dependent on it. Look at the other callers too.
Well, I think I didn't make myself clear.
I don't object to have this helper. If it helps, then we can have it.
My objection is the current implementation of it, because it is
*conceptually* wrong for SEV-SNP.
Btw, I just look at the dev branch.
For the common code, it is used in kvm_tdp_mmu_map() and
kvm_tdp_mmu_fast_pf_get_last_sptep() to get whether a GPA is private.
As said above, I don't see why we need a helper with the "current
implementation" (which consults kvm_shared_gfn_mask()) for them. We can
just use fault->gfn + fault->is_private for such purpose.
It is also used in the TDX code like TDX variant handle_ept_violation()
and tdx_vcpu_init_mem_region(). For them to be honest I don't quite
care whether a helper is used. We can have a helper if we have multiple
callers, but this helper should be in TDX code, but not common MMU code.
