Re: [PATCH v4 11/17] KVM: TDX: Honor the guest's accept level contained in an EPT violation

[Edgecombe, Rick P]

On Fri, 2026-10-09 at 17:33 +0800, Yan Zhao wrote:
> On Thu, Oct 08, 2026 at 09:05:32AM +0800, Edgecombe, Rick P wrote:
> > On Mon, 2026-09-28 at 17:11 +0800, Yan Zhao wrote:
> > 
> > > +
> > > +	/*
> > > +	 * No TLB flush is required, as the "BLOCK + TRACK + kick off vCPUs"
> > > +	 * sequence required by the TDX-Module includes a TLB flush.
> > > +	 */
> > 
> > I think the TLB flush is actually more obvious than the level + 1/2 bits below.
> > It is worried about inhibiting 1GB mappings, why?
> Hmm, including inhibiting 1GB mappings can make the logic more generic and more
> correct in concept. That is, when the guest accept level is 2MB, 1GB mappings
> should be inhibited conceptually. This help remove the code dependency on the
> assumption that 1GB mappings are impossible with the initial implementation,
> given that inhibiting 1GB mappings as well is simple.

I strongly think the code or comments should not prep for 1GB S-EPT pages or
promotion. Warning if 1GB S-EPT pages is tried is good. But I don't see how
silently handling this particular corner of huge pages helps anything.

If/when we get to 1GB mappings the code could have evolved at that point. So
introducing code or comments now will add constraints on other work that is
done. We don't want to think about it.

> 
> > > +	hugepage_set_guest_inhibit(slot, gfn, level + 1);
> > > +	if (level == PG_LEVEL_4K)
> > > +		hugepage_set_guest_inhibit(slot, gfn, level + 2);
> > > +
> > > +	return 0;
> > > +}
> > > +
> If you have a strong opinion to drop inhibiting 1GB mappings for now, I can add
> the following diff:
> 
> @@ -2183,7 +2172,13 @@ static int tdx_handle_guest_accept_ept_violation(struct kvm_vcpu *vcpu, gfn_t gf
>                 return 0;
> 
>         level = tdx_get_ept_violation_level(vcpu);
> -       if (level > PG_LEVEL_2M)
> +       KVM_BUG_ON(level != PG_LEVEL_2M && level != PG_LEVEL_4K, kvm);
> +
> +       /*
> +        * No need to inhibit 1GB mappings since they are unsupported with
> +        * the initial huge page implementation.

This also implies 1GB is coming...

> +        */
> +       if (level == PG_LEVEL_2M)
>                 return 0;
> 
>         if (hugepage_test_guest_inhibit(slot, gfn, level + 1))
> @@ -2203,8 +2198,6 @@ static int tdx_handle_guest_accept_ept_violation(struct kvm_vcpu *vcpu, gfn_t gf
>          * sequence required by the TDX-Module includes a TLB flush.
>          */
>         hugepage_set_guest_inhibit(slot, gfn, level + 1);
> -       if (level == PG_LEVEL_4K)
> -               hugepage_set_guest_inhibit(slot, gfn, level + 2);
> 
>         return 0;
>  }
> 

There are a lot of possible things that are not handled if a 1GB mapping sneaks
through.

Why not KVM_BUG_ON() if something tries to map 1GB in the appropriate
tdx_sept_set_private_spte() sub functions. tdx_sept_split_leaf_spte() should be
covered already, but not tdx_sept_map_leaf_spte() looks like.