From: "Huang, Kai" <kai.huang@intel.com>
To: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"Hansen, Dave" <dave.hansen@intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "Luck, Tony" <tony.luck@intel.com>,
"bagasdotme@gmail.com" <bagasdotme@gmail.com>,
"ak@linux.intel.com" <ak@linux.intel.com>,
"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
"kirill.shutemov@linux.intel.com"
<kirill.shutemov@linux.intel.com>, "Christopherson,,
Sean" <seanjc@google.com>,
"Chatre, Reinette" <reinette.chatre@intel.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"Yamahata, Isaku" <isaku.yamahata@intel.com>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"Shahar, Sagi" <sagis@google.com>,
"imammedo@redhat.com" <imammedo@redhat.com>,
"Gao, Chao" <chao.gao@intel.com>,
"Brown, Len" <len.brown@intel.com>,
"peterz@infradead.org" <peterz@infradead.org>,
"sathyanarayanan.kuppuswamy@linux.intel.com"
<sathyanarayanan.kuppuswamy@linux.intel.com>,
"Huang, Ying" <ying.huang@intel.com>,
"Williams, Dan J" <dan.j.williams@intel.com>
Subject: Re: [PATCH v8 08/16] x86/virt/tdx: Add placeholder to construct TDMRs to cover all TDX memory regions
Date: Tue, 10 Jan 2023 02:23:34 +0000 [thread overview]
Message-ID: <cba4c7295b157dc5d0d88b73f335899f0aa5a21c.camel@intel.com> (raw)
In-Reply-To: <92aba287-c839-8841-a9f7-28a2c0b3097a@intel.com>
On Mon, 2023-01-09 at 16:47 -0800, Dave Hansen wrote:
> On 1/9/23 16:40, Huang, Kai wrote:
> > On Fri, 2023-01-06 at 11:24 -0800, Dave Hansen wrote:
> ...
> > > Also, tdmr_sz and max_tdmrs can both be derived from 'sysinfo'. Do they
> > > really need to be stored here?
> >
> > It's not mandatory to keep them here. I did it mainly because I want to avoid
> > passing 'sysinfo' as argument for almost all functions related to constructing
> > TDMRs.
>
> I don't think it hurts readability that much. On the contrary, it makes
> it more clear what data is needed for initialization.
Sorry one thing I forgot to mention is if we keep 'tdmr_sz' in 'struct
tdmr_info_list', it only needs to be calculated at once when allocating the
buffer. Otherwise, we need to calculate it based on sysinfo-
>max_reserved_per_tdmr each time we want to get a TDMR at a given index.
To me putting relevant fields (tdmrs, tdmr_sz, max_tdmrs, nr_consumed_tdmrs)
together makes how the TDMR list is organized more clear. But please let me
know if you prefer removing 'tdmr_sz' and 'max_tdmrs'.
Btw, if we remove 'tdmr_sz' and 'max_tdmrs', even nr_consumed_tdmrs is not
absolutely necessary here. It can be a local variable of init_tdx_module() (as
shown in v7), and the 'struct tdmr_info_list' will only have the 'tdmrs' member
(as you commented in v7):
https://lore.kernel.org/linux-mm/cc195eb6499cf021b4ce2e937200571915bfe66f.camel@intel.com/T/#mb9826e2bcf8bf6399c13cc5f95a948fe4b3a46d9
Please let me know what's your preference?
>
> > > If so, I think I'd probably do something
> > > like this with the structure:
> > >
> > > struct tdmr_info_list {
> > > struct tdmr_info *tdmrs;
> > > int nr_consumed_tdmrs; // How many @tdmrs are in use
> > >
> > > /* Metadata for freeing this structure: */
> > > int tdmr_sz; // Size of one 'tdmr_info' (has a flex array)
> > > int max_tdmrs; // How many @tdmrs are allocated
> > > };
> > >
> > > Modulo whataver folks are doing for comments these days.
> >
> > Looks nice to me. Will use. A slight thing is 'tdmr_sz' is also used to get
> > the TDMR at a given index, but not just freeing the structure.
> >
> > Btw, is C++ style comment "//" OK in kernel code?
>
> It's OK with me, but I don't think there's much consensus on it.
> Probably best to stick with normal arch/x86 style for now.
>
>
Will use normal arch/x86 style for now. Thanks for the info.
next prev parent reply other threads:[~2023-01-10 2:26 UTC|newest]
Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-09 6:52 [PATCH v8 00/16] TDX host kernel support Kai Huang
2022-12-09 6:52 ` [PATCH v8 01/16] x86/tdx: Define TDX supported page sizes as macros Kai Huang
2023-01-06 19:04 ` Dave Hansen
2022-12-09 6:52 ` [PATCH v8 02/16] x86/virt/tdx: Detect TDX during kernel boot Kai Huang
2023-01-06 17:09 ` Dave Hansen
2023-01-08 22:25 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 03/16] x86/virt/tdx: Make INTEL_TDX_HOST depend on X86_X2APIC Kai Huang
2023-01-06 19:04 ` Dave Hansen
2022-12-09 6:52 ` [PATCH v8 04/16] x86/virt/tdx: Add skeleton to initialize TDX on demand Kai Huang
2023-01-06 17:14 ` Dave Hansen
2023-01-08 22:26 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 05/16] x86/virt/tdx: Implement functions to make SEAMCALL Kai Huang
2023-01-06 17:29 ` Dave Hansen
2023-01-09 10:30 ` Huang, Kai
2023-01-09 19:54 ` Dave Hansen
2023-01-09 22:10 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 06/16] x86/virt/tdx: Get information about TDX module and TDX-capable memory Kai Huang
2023-01-06 17:46 ` Dave Hansen
2023-01-09 10:25 ` Huang, Kai
2023-01-09 19:52 ` Dave Hansen
2023-01-09 22:07 ` Huang, Kai
2023-01-09 22:11 ` Dave Hansen
2022-12-09 6:52 ` [PATCH v8 07/16] x86/virt/tdx: Use all system memory when initializing TDX module as TDX memory Kai Huang
2023-01-06 18:18 ` Dave Hansen
2023-01-09 11:48 ` Huang, Kai
2023-01-09 16:51 ` Dave Hansen
2023-01-10 12:09 ` Huang, Kai
2023-01-10 16:18 ` Dave Hansen
2023-01-11 10:00 ` Huang, Kai
2023-01-12 0:56 ` Huang, Ying
2023-01-12 1:18 ` Huang, Kai
2023-01-12 1:59 ` Huang, Ying
2023-01-12 2:22 ` Huang, Kai
2023-01-12 11:33 ` Huang, Kai
2023-01-18 11:08 ` Huang, Kai
2023-01-18 13:57 ` David Hildenbrand
2023-01-18 19:38 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 08/16] x86/virt/tdx: Add placeholder to construct TDMRs to cover all TDX memory regions Kai Huang
2023-01-06 19:24 ` Dave Hansen
2023-01-10 0:40 ` Huang, Kai
2023-01-10 0:47 ` Dave Hansen
2023-01-10 2:23 ` Huang, Kai [this message]
2023-01-10 19:12 ` Dave Hansen
2023-01-11 9:23 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 09/16] x86/virt/tdx: Fill out " Kai Huang
2023-01-06 19:36 ` Dave Hansen
2023-01-10 0:45 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 10/16] x86/virt/tdx: Allocate and set up PAMTs for TDMRs Kai Huang
2023-01-06 21:53 ` Dave Hansen
2023-01-10 0:49 ` Huang, Kai
2023-01-07 0:47 ` Dave Hansen
2023-01-10 0:47 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 11/16] x86/virt/tdx: Designate reserved areas for all TDMRs Kai Huang
2023-01-06 22:07 ` Dave Hansen
2023-01-10 1:19 ` Huang, Kai
2023-01-10 1:22 ` Dave Hansen
2023-01-10 11:01 ` Huang, Kai
2023-01-10 15:19 ` Dave Hansen
2023-01-11 10:57 ` Huang, Kai
2023-01-11 16:16 ` Dave Hansen
2023-01-11 22:10 ` Huang, Kai
2023-01-10 11:01 ` Huang, Kai
2023-01-10 15:17 ` Dave Hansen
2022-12-09 6:52 ` [PATCH v8 12/16] x86/virt/tdx: Designate the global KeyID and configure the TDX module Kai Huang
2023-01-06 22:21 ` Dave Hansen
2023-01-10 10:48 ` Huang, Kai
2023-01-10 16:25 ` Dave Hansen
2023-01-10 23:33 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 13/16] x86/virt/tdx: Configure global KeyID on all packages Kai Huang
2023-01-06 22:49 ` Dave Hansen
2023-01-10 10:15 ` Huang, Kai
2023-01-10 16:53 ` Dave Hansen
2023-01-11 0:06 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 14/16] x86/virt/tdx: Initialize all TDMRs Kai Huang
2023-01-07 0:17 ` Dave Hansen
2023-01-10 10:23 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 15/16] x86/virt/tdx: Flush cache in kexec() when TDX is enabled Kai Huang
2023-01-07 0:35 ` Dave Hansen
2023-01-10 11:29 ` Huang, Kai
2023-01-10 15:27 ` Dave Hansen
2023-01-11 0:13 ` Huang, Kai
2023-01-11 0:30 ` Dave Hansen
2023-01-11 1:58 ` Huang, Kai
2022-12-09 6:52 ` [PATCH v8 16/16] Documentation/x86: Add documentation for TDX host support Kai Huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cba4c7295b157dc5d0d88b73f335899f0aa5a21c.camel@intel.com \
--to=kai.huang@intel.com \
--cc=ak@linux.intel.com \
--cc=bagasdotme@gmail.com \
--cc=chao.gao@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@intel.com \
--cc=imammedo@redhat.com \
--cc=isaku.yamahata@intel.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=rafael.j.wysocki@intel.com \
--cc=reinette.chatre@intel.com \
--cc=sagis@google.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=ying.huang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).