2023-11-19 06:31:01

by Binbin Wu

[permalink] [raw]
Subject: Re: [PATCH v17 020/116] KVM: TDX: create/destroy VM structure



On 11/7/2023 10:55 PM, [email protected] wrote:
> From: Isaku Yamahata <[email protected]>
>
> As the first step to create TDX guest, create/destroy VM struct. Assign
> TDX private Host Key ID (HKID) to the TDX guest for memory encryption and
> allocate extra pages for the TDX guest. On destruction, free allocated
> pages, and HKID.
>
> Before tearing down private page tables, TDX requires some resources of the
> guest TD to be destroyed (i.e. HKID must have been reclaimed, etc). Add
> mmu notifier release callback before tearing down private page tables for
> it.
>
> Add vm_free() of kvm_x86_ops hook at the end of kvm_arch_destroy_vm()
> because some per-VM TDX resources, e.g. TDR, need to be freed after other
> TDX resources, e.g. HKID, were freed.
>
> Co-developed-by: Kai Huang <[email protected]>
> Signed-off-by: Kai Huang <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> Signed-off-by: Isaku Yamahata <[email protected]>
>
> ---
[...]
> +
> +static int tdx_reclaim_page(hpa_t pa)
> +{
> + int r;
> +
> + r = __tdx_reclaim_page(pa);
> + if (!r)
> + tdx_clear_page(pa);
> + return r;
> +}
> +
> +static void tdx_reclaim_td_page(unsigned long td_page_pa)

This function is used to reclaim td control sturcture pages like TDCX,
TDVPX,
TDVPR. Should this function name be more specific?
For me, it is a bit confusing.

Or maybe do "td page" have specific meaning referring to these control
structures
pages in TDX?



> +{
> + WARN_ON_ONCE(!td_page_pa);
> +
> + /*
> + * TDCX are being reclaimed. TDX module maps TDCX with HKID
> + * assigned to the TD. Here the cache associated to the TD
> + * was already flushed by TDH.PHYMEM.CACHE.WB before here, So
> + * cache doesn't need to be flushed again.
> + */
> + if (tdx_reclaim_page(td_page_pa))
> + /*
> + * Leak the page on failure:
> + * tdx_reclaim_page() returns an error if and only if there's an
> + * unexpected, fatal error, e.g. a SEAMCALL with bad params,
> + * incorrect concurrency in KVM, a TDX Module bug, etc.
> + * Retrying at a later point is highly unlikely to be
> + * successful.
> + * No log here as tdx_reclaim_page() already did.
> + */
> + return;
> + free_page((unsigned long)__va(td_page_pa));
> +}
> +
[...]


2023-11-21 19:16:39

by Isaku Yamahata

[permalink] [raw]
Subject: Re: [PATCH v17 020/116] KVM: TDX: create/destroy VM structure

On Sun, Nov 19, 2023 at 02:30:19PM +0800,
Binbin Wu <[email protected]> wrote:

> > +static int tdx_reclaim_page(hpa_t pa)
> > +{
> > + int r;
> > +
> > + r = __tdx_reclaim_page(pa);
> > + if (!r)
> > + tdx_clear_page(pa);
> > + return r;
> > +}
> > +
> > +static void tdx_reclaim_td_page(unsigned long td_page_pa)
>
> This function is used to reclaim td control sturcture pages like TDCX,
> TDVPX,
> TDVPR. Should this function name be more specific?
> For me, it is a bit confusing.
>
> Or maybe do "td page" have specific meaning referring to these control
> structures
> pages in TDX?

As they are control page, how about tdx_reclaim_control_page()?
--
Isaku Yamahata <[email protected]>