linux-kernel - RE: [PATCH 3/6] x86/tdx: Support vmalloc() for tdx_enc_status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 23 Nov 2022 04:01:32 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Dave Hansen <dave.hansen@...el.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "arnd@...db.de" <arnd@...db.de>, "bp@...en8.de" <bp@...en8.de>,
        "brijesh.singh@....com" <brijesh.singh@....com>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "jane.chu@...cle.com" <jane.chu@...cle.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        KY Srinivasan <kys@...rosoft.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "x86@...nel.org" <x86@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 3/6] x86/tdx: Support vmalloc() for
 tdx_enc_status_changed()

> From: Dave Hansen <dave.hansen@...el.com>
> Sent: Monday, November 21, 2022 1:01 PM
> [...]
> 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.

Will try to make one function with better naming.

> > +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?

I just noticed Nicholas Piggin's huge vmalloc mapping patches that were
merged in April 2021. I'll take a look and see what I can use here.

> > +		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.

It looks like typically set_memory_{de/en}crypted() are not invoked
frequently when Linux is running, e.g. the swiotlb bounce buffers are
only initialzed once with set_memory_decrypt(). A driver runs in a guest
on a hypervisor typically also only initializes the buffers (which need to
be shared with the hypervisor) with set_memory_decrypt() once when
the driver initializes the device. So, it looks like slow_virt_to_phys() may be
acceptable for configuous memory pages as well.

> Can you please look into making *one* function that works on either kind
> of mapping?

Ok. Looking into this using slow_virt_to_phys() for direct map
addresses as well.