lkml.org 
[lkml]   [2022]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH v7 11/20] x86/virt/tdx: Add placeholder to construct TDMRs to cover all TDX memory regions
Date
On Mon, 2022-11-28 at 22:50 +0000, Huang, Kai wrote:
> On Mon, 2022-11-28 at 14:19 -0800, Dave Hansen wrote:
> > On 11/28/22 14:13, Huang, Kai wrote:
> > > Apologize I am not entirely sure whether I fully got your point. Do you mean
> > > something like below?
> > ...
> >
> > No, something like this:
> >
> > static int init_tdx_module(void)
> > {
> > static struct tdsysinfo_struct tdx_sysinfo; /* too rotund for the stack */
> > ...
> > tdx_get_sysinfo(&tdx_sysinfo, ...);
> > ...
> >
> > But, also, seriously, 3k on the stack is *fine* if you can shut up the
> > warnings. This isn't going to be a deep call stack to begin with.
> >
>
> Let me try to find out whether it is possible to silent the warning. If I
> cannot, then I'll use your above way. Thanks!

Hi Dave,

Sorry to double asking.

Adding below build flag to Makefile can silent the warning:

index 38d534f2c113..f8a40d15fdfc 100644
--- a/arch/x86/virt/vmx/tdx/Makefile
+++ b/arch/x86/virt/vmx/tdx/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
+CFLAGS_tdx.o += -Wframe-larger-than=4096

So to confirm you want to add this flag to Makefile and just make tdx_sysinfo
and tdx_cmr_array as local variables?

Another reason I am double asking is, 'tdx_global_keyid' in this series can also
be a local variable in init_tdx_module() but currently it is a static (as KVM
will need it too). If I change to use local variable in the patch
"x86/virt/tdx: Reserve TDX module global KeyID" like below:

--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -50,9 +50,6 @@ static DEFINE_MUTEX(tdx_module_lock);
/* All TDX-usable memory regions */
static LIST_HEAD(tdx_memlist);

-/* TDX module global KeyID. Used in TDH.SYS.CONFIG ABI. */
-static u32 tdx_global_keyid;
-
/*
* tdx_keyid_start and nr_tdx_keyids indicate that TDX is uninitialized.
* This is used in TDX initialization error paths to take it from
@@ -928,6 +925,7 @@ static int init_tdx_module(void)
__aligned(CMR_INFO_ARRAY_ALIGNMENT);
struct tdsysinfo_struct *sysinfo = &PADDED_STRUCT(tdsysinfo);
struct tdmr_info_list tdmr_list;
+ u32 global_keyid;
int ret;

ret = tdx_get_sysinfo(sysinfo, cmr_array);
@@ -964,7 +962,7 @@ static int init_tdx_module(void)
* Pick the first TDX KeyID as global KeyID to protect
* TDX module metadata.
*/
- tdx_global_keyid = tdx_keyid_start;
+ global_keyid = tdx_keyid_start;

I got a warning for this particular patch:

arch/x86/virt/vmx/tdx/tdx.c: In function ‘init_tdx_module’:
arch/x86/virt/vmx/tdx/tdx.c:928:13: warning: variable ‘global_keyid’ set but not
used [-Wunused-but-set-variable]
928 | u32 global_keyid;
| ^~~~~~~~~~~~

To get rid of this warning, we need to merge this patch to the later patch
(which configures the TDMRs and global keyid to the TDX module).

Should I make the tdx_global_keyid as local variable too and merge patch
"x86/virt/tdx: Reserve TDX module global KeyID" to the later patch?
\
 
 \ /
  Last update: 2022-12-07 12:49    [W:0.186 / U:0.932 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site