linux-kernel - Re: [PATCH v7 09/20] x86/virt/tdx: Get information about TDX module and TDX-capable memory

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: <7be59cd82bc3f3c26e835980eb74a8d92c6d67d6.camel@intel.com>
Date:   Fri, 2 Dec 2022 11:19:33 +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>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "imammedo@...hat.com" <imammedo@...hat.com>,
        "Gao, Chao" <chao.gao@...el.com>,
        "Brown, Len" <len.brown@...el.com>,
        "Shahar, Sagi" <sagis@...gle.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 v7 09/20] x86/virt/tdx: Get information about TDX module
 and TDX-capable memory

On Wed, 2022-11-23 at 22:53 +0000, Huang, Kai wrote:
> > > > > > > > > +
> > > > > > > > > +   /*
> > > > > > > > > +    * trim_empty_cmrs() updates the actual number of CMRs by
> > > > > > > > > +    * dropping all tail empty CMRs.
> > > > > > > > > +    */
> > > > > > > > > +   return trim_empty_cmrs(tdx_cmr_array, &tdx_cmr_num);
> > > > > > > > > +}
> > > > > > > 
> > > > > > > Why does this both need to respect the "tdx_cmr_num = out.r9"
> > > > > > > value
> > > > > > > *and* trim the empty ones?  Couldn't it just ignore the
> > > > > > > "tdx_cmr_num =
> > > > > > > out.r9" value and just trim the empty ones either way?  It's not
> > > > > > > like
> > > > > > > there is a billion of them.  It would simplify the code for sure.
> > > > > 
> > > > > OK.  Since spec says MAX_CMRs is 32, so I can use 32 instead of
> > > > > reading out from
> > > > > R9.
> > > 
> > > But then you still have the "trimming" code.  Why not just trust "r9"
> > > and then axe all the trimming code?  Heck, and most of the sanity checks.
> > > 
> > > This code could be a *lot* smaller.
> 
> As I said the only problem is there might be empty CMRs at the tail of the
> cmr_array[] following one or more valid CMRs.  

Hi Dave,

Probably I forgot to mention the "r9" in practice always returns 32, so there
will be empty CMRs at the tail of the cmr_array[].

> 
> But we can also do nothing here, but just skip empty CMRs when comparing the
> memory region to it (in next patch).
> 
> Or, we don't even need to explicitly check memory region against CMRs. If the
> memory regions that we provided in the TDMR doesn't fall into CMR, then
> TDH.SYS.CONFIG will fail. We can just depend on the SEAMCALL to do that.

Sorry to ping, but do you have any comments here?

How about we just don't do any check of TDX memory regions against CMRs, but 
just let the TDH.SYS.CONFIG SEAMCALL to determine?