Re: [PATCH v19 057/130] KVM: x86/mmu: Add a new is_private member for union kvm_mmu_page_role

[Edgecombe, Rick P]

On Mon, 2024-02-26 at 00:25 -0800, isaku.yamahata@xxxxxxxxx wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> 
> Because TDX support introduces private mapping, add a new member in
> union
> kvm_mmu_page_role with access functions to check the member.

I guess we should have a role bit for private like in this patch, but
just barely. AFAICT we have a gfn and struct kvm in every place where
it is checked (assuming my proposal in patch 56 holds water). So we
could have
bool is_private = !(gfn & kvm_gfn_shared_mask(kvm));

But there are extra bits available in the role, so we can skip the
extra step. Can you think of any more reasons? I want to try to write a
log for this one. It's very short.

> 
> +static inline bool is_private_sptep(u64 *sptep)
> +{
> +       if (WARN_ON_ONCE(!sptep))
> +               return false;

This is not supposed to be NULL, from the existence of the warning. It
looks like some previous comments were to not let the NULL pointer
deference happen and bail if it's NULL. I think maybe we should just
drop the check and warning completely. The NULL pointer deference will
be plenty loud if it happens.

> +       return is_private_sp(sptep_to_sp(sptep));
> +}
> +