[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8d3ac4ca-055a-5d54-602c-e378643ad9cd@intel.com>
Date: Mon, 21 Nov 2022 13:00:35 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Dexuan Cui <decui@...rosoft.com>, ak@...ux.intel.com,
arnd@...db.de, bp@...en8.de, brijesh.singh@....com,
dan.j.williams@...el.com, dave.hansen@...ux.intel.com,
haiyangz@...rosoft.com, hpa@...or.com, jane.chu@...cle.com,
kirill.shutemov@...ux.intel.com, kys@...rosoft.com,
linux-arch@...r.kernel.org, linux-hyperv@...r.kernel.org,
luto@...nel.org, mingo@...hat.com, peterz@...radead.org,
rostedt@...dmis.org, sathyanarayanan.kuppuswamy@...ux.intel.com,
seanjc@...gle.com, tglx@...utronix.de, tony.luck@...el.com,
wei.liu@...nel.org, x86@...nel.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] x86/tdx: Support vmalloc() for
tdx_enc_status_changed()
On 11/21/22 11:51, Dexuan Cui wrote:
> -static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> +static bool tdx_enc_status_changed_for_contiguous_pages(unsigned long vaddr,
> + int numpages, bool enc)
That naming is unfortunate.
First, it's getting way too long.
Second, you don't need two of these functions because it's contiguous or
not. It's because tdx_enc_status_changed() only works on the direct map.
> {
> phys_addr_t start = __pa(vaddr);
> phys_addr_t end = __pa(vaddr + numpages * PAGE_SIZE);
> @@ -798,6 +800,47 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> return true;
> }
>
> +static bool tdx_enc_status_changed_for_vmalloc(unsigned long vaddr,
> + int numpages, bool enc)
> +{
> + void *start_va = (void *)vaddr;
> + void *end_va = start_va + numpages * PAGE_SIZE;
> + phys_addr_t pa;
> +
> + if (offset_in_page(vaddr) != 0)
> + return false;
> +
> + while (start_va < end_va) {
> + pa = slow_virt_to_phys(start_va);
> + if (!enc)
> + pa |= cc_mkdec(0);
> +
> + if (!tdx_map_gpa(pa, pa + PAGE_SIZE, enc))
> + return false;
> +
> + /*
> + * private->shared conversion requires only MapGPA call.
> + *
> + * For shared->private conversion, accept the page using
> + * TDX_ACCEPT_PAGE TDX module call.
> + */
> + if (enc && !try_accept_one(&pa, PAGE_SIZE, PG_LEVEL_4K))
> + return false;
Don't we support large vmalloc() mappings these days?
> + start_va += PAGE_SIZE;
> + }
> +
> + return true;
> +}
I really don't like the copy-and-paste fork here.
I'd almost just rather have this *one* "vmalloc" copy that does
slow_virt_to_phys() on direct map addresses than have two copies.
Can you please look into making *one* function that works on either kind
of mapping?
> +static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> +{
> + if (is_vmalloc_addr((void *)vaddr))
> + return tdx_enc_status_changed_for_vmalloc(vaddr, numpages, enc);
> +
> + return tdx_enc_status_changed_for_contiguous_pages(vaddr, numpages, enc);
> +}
> +
> void __init tdx_early_init(void)
> {
> u64 cc_mask;
Powered by blists - more mailing lists