linux-kernel - Re: [PATCHv2 23/29] x86/tdx: Add helper to convert memory between shared and private

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]

Message-ID: <YgJePUqMEzHNqrsR@zn.tnic>
Date:   Tue, 8 Feb 2022 13:12:45 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:     tglx@...utronix.de, mingo@...hat.com, dave.hansen@...el.com,
        luto@...nel.org, peterz@...radead.org,
        sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 23/29] x86/tdx: Add helper to convert memory between
 shared and private

On Mon, Jan 24, 2022 at 06:02:09PM +0300, Kirill A. Shutemov wrote:
> +static bool tdx_accept_page(phys_addr_t gpa, enum pg_level pg_level)

accept_page() as it is a static function.

> +int tdx_hcall_request_gpa_type(phys_addr_t start, phys_addr_t end, bool enc)
> +{
> +	u64 ret;
> +
> +	if (end <= start)
> +		return -EINVAL;
> +
> +	if (!enc) {
> +		start |= tdx_shared_mask();
> +		end |= tdx_shared_mask();
> +	}
> +
> +	/*
> +	 * Notify the VMM about page mapping conversion. More info about ABI
> +	 * can be found in TDX Guest-Host-Communication Interface (GHCI),
> +	 * sec "TDG.VP.VMCALL<MapGPA>"
> +	 */
> +	ret = _tdx_hypercall(TDVMCALL_MAP_GPA, start, end - start, 0, 0, NULL);
> +


^ Superfluous newline.

> +	if (ret)
> +		ret = -EIO;
> +
> +	if (ret || !enc)

Is the second case here after the "||" the conversion-to-shared where it
only needs to notify with MapGPA and return?

Of all the places, this one needs a comment.

> +		return ret;
> +
> +	/*
> +	 * For shared->private conversion, accept the page using
> +	 * TDX_ACCEPT_PAGE TDX module call.
> +	 */
> +	while (start < end) {
> +		/* Try 2M page accept first if possible */
> +		if (!(start & ~PMD_MASK) && end - start >= PMD_SIZE &&
> +		    !tdx_accept_page(start, PG_LEVEL_2M)) {

What happens here if the module doesn't accept the page? No error
reporting, no error handling, no warning, nada?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette