On Fri, 2024-05-03 at 09:01 -0700, Dave Hansen wrote: > On 3/1/24 03:20, Kai Huang wrote: > > The kernel reads all TDMR related global metadata fields based on a > > table which maps the metadata fields to the corresponding members of > > 'struct tdx_tdmr_sysinfo'. > > > > Currently this table is a static variable. But this table is only used > > by the function which reads these metadata fields and becomes useless > > after reading is done. > > Is this intended to be a problem statement? _How_ is this a problem? > > > Change the table to function local variable. This also saves the > > storage of the table from the kernel image. > > I'm confused how this would happen. Could you please explain your logic > a bit here? I think I failed to notice one thing, that although this patch can eliminate the (static) @fields[] array in the data section, it generates more code in the function get_tdx_tdmr_sysinfo() in order to build the same array in the stack. I did experiment and compared the generated code with or without the code change in this patch: before: fields: .quad -7998392933915033592 /* metadata field ID */ .long 0 .zero 4 .quad -7998392933915033591 .long 2 .zero 4 .quad -7998392933915033584 .long 4 .zero 4 .quad -7998392933915033583 .long 6 .zero 4 .quad -7998392933915033582 .long 8 .zero 4 get_tdx_tdmr_sysinfo: pushq %rbp movq %rsp, %rbp subq $24, %rsp movq %rdi, -24(%rbp) movl $0, -4(%rbp) jmp .L8 ...... after: get_tdx_tdmr_sysinfo: pushq %rbp movq %rsp, %rbp subq $112, %rsp movq %rdi, -104(%rbp) movabsq $-7998392933915033592, %rax movq %rax, -96(%rbp) movl $0, -88(%rbp) movabsq $-7998392933915033591, %rax movq %rax, -80(%rbp) movl $2, -72(%rbp) movabsq $-7998392933915033584, %rax movq %rax, -64(%rbp) movl $4, -56(%rbp) movabsq $-7998392933915033583, %rax movq %rax, -48(%rbp) movl $6, -40(%rbp) movabsq $-7998392933915033582, %rax movq %rax, -32(%rbp) movl $8, -24(%rbp) movl $0, -4(%rbp) jmp .L8 ...... So looks we cannot assume moving the static array to function local variable can always save the storage. I think the point is the compiler has to keep those constants (metadata field ID and offset) somewhere in the object file, no matter they are in the data section or in the code in text section, and no matter how does the compiler generate the code. The more reasonable benefit of this patch is to make the name scope of the @fields[] array be only visible in the get_tdx_tdmr_sysinfo() but not the entire file. Thanks for the insight. I hope the above is all I missed, or am I still missing anything? Anyway as replied to Rick I'll drop this patch from this series.
