Re: [PATCH v3 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages

["Huang, Kai" <kai.huang@intel.com>]

> +/*
> + * Simple structure for pre-allocating Dynamic
> + * PAMT pages outside of locks.
> + */
> +struct tdx_prealloc {
> +	struct list_head page_list;
> +	int cnt;
> +};
> +
> +static inline struct page *get_tdx_prealloc_page(struct tdx_prealloc *prealloc)
> +{
> +	struct page *page;
> +
> +	page = list_first_entry_or_null(&prealloc->page_list, struct page, lru);
> +	if (page) {
> +		list_del(&page->lru);
> +		prealloc->cnt--;
> +	}
> +
> +	return page;
> +}
> +
> +static inline int topup_tdx_prealloc_page(struct tdx_prealloc *prealloc, unsigned int min_size)
> +{
> +	while (prealloc->cnt < min_size) {
> +		struct page *page = alloc_page(GFP_KERNEL);
> +
> +		if (!page)
> +			return -ENOMEM;
> +
> +		list_add(&page->lru, &prealloc->page_list);
> +		prealloc->cnt++;
> +	}
> +
> +	return 0;
> +}

Since 'struct tdx_prealloc' replaces the KVM standard 'struct
kvm_mmu_memory_cache' for external page table, and it is allowed to fail
in "topup" operation, why not just call tdx_alloc_page() to "topup" page
for external page table here?

I don't think we need to keep all "DPAMT pages" in the pool, right?

If tdx_alloc_page() succeeds, then the "DPAMT pages" are also "topup"ed,
and PAMT entries for the 2M range of the SEPT page is ready too.

This at least avoids having to export tdx_dpamt_entry_pages(), which is
not nice IMHO.  And I think it should yield simpler code.

One more thinking:

I also have been thinking whether we can continue to use the KVM standard
'struct kvm_mmu_memory_cache' for S-EPT pages.  Below is one more idea for
your reference.

In the previous discussion I think we concluded the 'kmem_cache' doesn't
work nicely with DPAMT (due to the ctor() cannot fail etc).  And when we
don't use 'kmem_cache', KVM just call __get_free_page() to topup objects.
But we need tdx_alloc_page() for allocation here, so this is the problem.

If we add two callbacks for object allocation/free to 'struct
kvm_mmu_memory_cache', then we can have place to hook tdx_alloc_page().

Something like the below:

diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 827ecc0b7e10..5dbd80773689 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -88,6 +88,8 @@ struct kvm_mmu_memory_cache {
        gfp_t gfp_custom;
        u64 init_value;
        struct kmem_cache *kmem_cache;
+       void*(*obj_alloc)(gfp_t gfp);
+       void (*obj_free)(void *);
        int capacity;
        int nobjs;
        void **objects;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6c07dd423458..df2c2100d656 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -355,7 +355,10 @@ static inline void *mmu_memory_cache_alloc_obj(struct
kvm_mmu_memory_cache *mc,
        if (mc->kmem_cache)
                return kmem_cache_alloc(mc->kmem_cache, gfp_flags);
 
-       page = (void *)__get_free_page(gfp_flags);
+       if (mc->obj_alloc)
+               page = mc->obj_alloc(gfp_flags);
+       else
+               page = (void *)__get_free_page(gfp_flags);
        if (page && mc->init_value)
                memset64(page, mc->init_value, PAGE_SIZE / sizeof(u64));
        return page;
@@ -415,6 +418,8 @@ void kvm_mmu_free_memory_cache(struct
kvm_mmu_memory_cache *mc)
        while (mc->nobjs) {
                if (mc->kmem_cache)
                        kmem_cache_free(mc->kmem_cache, mc->objects[--mc-
>nobjs]);
+               else if (mc->obj_free)
+                       mc->obj_free(mc->objects[--mc->nobjs]);
                else
                        free_page((unsigned long)mc->objects[--mc-
>nobjs]);
        }