Re: [PATCH v7 11/20] x86/virt/tdx: Add placeholder to construct TDMRs to cover all TDX memory regions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux