linux-kernel - Re: [PATCH v8 11/16] x86/virt/tdx: Designate reserved areas for all TDMRs

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: <b971cd8b4e6ce9e96c6b4c6192adb74cc6722d54.camel@intel.com>
Date:   Tue, 10 Jan 2023 01:19:11 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "Luck, Tony" <tony.luck@...el.com>,
        "bagasdotme@...il.com" <bagasdotme@...il.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "Chatre, Reinette" <reinette.chatre@...el.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "Shahar, Sagi" <sagis@...gle.com>,
        "imammedo@...hat.com" <imammedo@...hat.com>,
        "Gao, Chao" <chao.gao@...el.com>,
        "Brown, Len" <len.brown@...el.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Huang, Ying" <ying.huang@...el.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>
Subject: Re: [PATCH v8 11/16] x86/virt/tdx: Designate reserved areas for all
 TDMRs

On Fri, 2023-01-06 at 14:07 -0800, Dave Hansen wrote:
> On 12/8/22 22:52, Kai Huang wrote:
> > +static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
> > +			      u64 size, u16 max_reserved_per_tdmr)
> > +{
> > +	struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
> > +	int idx = *p_idx;
> > +
> > +	/* Reserved area must be 4K aligned in offset and size */
> > +	if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
> > +		return -EINVAL;
> > +
> > +	if (idx >= max_reserved_per_tdmr)
> > +		return -E2BIG;
> > +
> > +	rsvd_areas[idx].offset = addr - tdmr->base;
> > +	rsvd_areas[idx].size = size;
> > +
> > +	*p_idx = idx + 1;
> > +
> > +	return 0;
> > +}
> 
> It's probably worth at least a comment here to say:
> 
> 	/*
> 	 * Consume one reserved area per call.  Make no effort to
> 	 * optimize or reduce the number of reserved areas which are
> 	 * consumed by contiguous reserved areas, for instance.
> 	 */

I'll add this comment before the code to set up rsvd_areas[idx].

> 
> I think the -E2BIG is also wrong.  It should be ENOSPC.  I'd also add a
> pr_warn() there.  Especially with how lazy this whole thing is, I can
> start to see how the reserved areas might be exhausted.  Let's be kind
> to our future selves and make the error (and the fix) easier to find.

Yes agreed.  Will change to -ENOSPC and add pr_warn().

> It's probably also worth noting *somewhere* that there's a balance to be
> had between TDMRs and reserved areas.  A system that is running out of
> reserved areas in a TDMR could split a TDMR to get more reserved areas.
> A system that has run out of TDMRs could relatively easily coalesce two
> adjacent TDMRs (before the PAMTs are allocated) and use a reserved area
> if there was a gap between them.

We can add above to the changelog of this patch, or the patch 09 ("x86/virt/tdx:
Fill out TDMRs to cover all TDX memory regions").  The latter perhaps is better
since that patch is the first place where the balance of TDMRs and reserved
areas is related.

What is your suggestion?

> 
> I'm *really* close to acking this patch once those are fixed up.