Messages in this thread | | | From | "Huang, Kai" <> | Subject | Re: [PATCH v14 07/23] x86/virt/tdx: Add skeleton to enable TDX on demand | Date | Wed, 18 Oct 2023 06:51:38 +0000 |
| |
> > +/* > > + * Do the module global initialization once and return its result. > > + * It can be done on any cpu. It's always called with interrupts > > + * disabled. > > + */ > > +static int try_init_module_global(void) > > +{ > > + struct tdx_module_args args = {}; > > + static DEFINE_RAW_SPINLOCK(sysinit_lock); > > + static bool sysinit_done; > > + static int sysinit_ret; > > + > > + lockdep_assert_irqs_disabled(); > > + > > + raw_spin_lock(&sysinit_lock); > > + > > + if (sysinit_done) > > + goto out; > > + > > + /* RCX is module attributes and all bits are reserved */ > > + args.rcx = 0; > > Isn't args.rcx already initialized to 0, why explixitly set it?
The purpose was to indicate SYS.INIT takes RCX as input, but OK I agree it's unnecessary.
I found the existing upstream TDX code uses this pattern (please see tdx_parse_info(), hv_tdx_hypercall() and try_accept_one()):
struct tdx_module_args args = {};
args.xx = xx; args.yy = yy; ...
So I followed in this series.
But I think explicit zeroing the structure isn't needed, so I think I'll just remove the explicit initialization in the @args declaration in all SEAMCALLs which use this pattern in this series:
struct tdx_module_args args;
/* RCX is module attributes and all bits are reserved */ args.rcx = 0;
Or just explicitly init RCX in the declaration:
struct tdx_module_args args = { .rcx = 0; /* Module attributes. All bits reserved. */ }; // other variable declarations
But it seems many people don't like tail comment.
Hi Kirill/Dave,
Do you have any preference? >
| |