Re: [PATCH 2/5] x86/virt/tdx: Move TDMR metadata fields map table to local variable

[Huang, Kai]

On 3/05/2024 12:29 pm, Edgecombe, Rick P wrote:
> On Fri, 2024-05-03 at 12:18 +1200, Huang, Kai wrote:
> > On 3/05/2024 12:09 pm, Edgecombe, Rick P wrote:
> > > On Sat, 2024-03-02 at 00:20 +1300, 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.
> > > >
> > > > Change the table to function local variable.  This also saves the
> > > > storage of the table from the kernel image.
> > >
> > > It seems like a reasonable change, but I don't see how it helps the purpose
> > > of
> > > this series. It seems more like generic cleanup. Can you explain?
> >
> > It doesn't help KVM from exporting API's perspective.
> >
> > I just uses this series for some small improvement (that I believe) of
> > the current code too.
> >
> > I can certainly drop this if you don't want it, but it's just a small
> > change and I don't see the benefit of sending it out separately.
>
> The change makes sense to me by itself, but it needs to be called out as
> unrelated cleanup. Otherwise it will be confusing to anyone reviewing this from
> the perspective of something KVM TDX needs.
>
> Don't have a super strong opinion. But given the choice, I would prefer it gets
> separated, because to me it's a lower priority then the rest (which is high).

OK I'll drop this one.