linux-kernel - RE: [PATCH v3 2/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] [day] [month] [year] [list]

Date:   Fri, 3 Mar 2023 07:16:41 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
CC:     "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>,
        "dan.j.williams@...el.com" <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>,
        "Michael Kelley (LINUX)" <mikelley@...rosoft.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>
Subject: RE: [PATCH v3 2/6] x86/tdx: Support vmalloc() for
 tdx_enc_status_changed()

> From: Kirill A. Shutemov <kirill@...temov.name>
> Sent: Friday, February 17, 2023 5:20 AM
> To: Dexuan Cui <decui@...rosoft.com>
> > ...
Hi Krill, sorry for my late reply!

> > Hi, Dave, I checked the huge vmalloc mapping code, but still don't know
> > how to get the underlying huge page info (if huge page is in use) and
> > try to use PG_LEVEL_2M/1G in try_accept_page() for vmalloc: I checked
> > is_vm_area_hugepages() and  __vfree() -> __vunmap(), and I think the
> > underlying page allocation info is internal to the mm code, and there
> > is no mm API to for me get the info in tdx_enc_status_changed().
> 
> I also don't obvious way to retrieve this info after vmalloc() is
> complete. split_page() makes all pages independent.
> 
> I think you can try to do this manually: allocate a vmalloc region,
> allocate pages manually, and put into the region. This way you always know
> page sizes and can optimize conversion to shared memory.
> 
> But it is tedious and I'm not sure if it worth the gain.
Thanks, I'll do some research on this idea.

> > Hi, Kirill, the load_unaligned_zeropad() issue is not addressed in
> > this patch. The issue looks like a generic issue that also happens to
> > AMD SNP vTOM mode and C-bit mode. Will need to figure out how to
> > address the issue. If we decide to adjust direct mapping to have the
> > shared bit set, it lools like we need to do the below for each
> > 'start_va' vmalloc page:
> >   pa = slow_virt_to_phys(start_va);
> >   set_memory_decrypted(phys_to_virt(pa), 1); -- this line calls
> > tdx_enc_status_changed() the second time for the same age, which is not
> > great. It looks like we need to find a way to reuse the cpa_flush()
> > related code in __set_memory_enc_pgtable() and make sure we call
> > tdx_enc_status_changed() only once for the same page from vmalloc()?
> 
> Actually, current code will change direct mapping for you. I just
> double-checked: the alias processing in __change_page_attr_set_clr() will
> change direct mapping if you call it on vmalloc()ed memory.
> 
> Splitting direct mapping is still unfortunate, but well.
> 
> >
> > Changes in v3:
> >   No change since v2.
> >
> >  arch/x86/coco/tdx/tdx.c | 69 ++++++++++++++++++++++++++---------------
> >  1 file changed, 44 insertions(+), 25 deletions(-)
> 
> I don't hate what you did here. But I think the code below is a bit
> cleaner.
> 
> Any opinions?
Thanks! Your version looks much better. I'll use it in in v4.