Messages in this thread | | | From | Kuppuswamy Sathyanarayanan <> | Date | Wed, 1 May 2024 20:40:43 -0700 | Subject | Re: [PATCH v4 13/15] x86/sev: Take advantage of configfs visibility support in TSM |
| |
On Wed, May 1, 2024 at 1:15 PM Dan Williams <dan.j.williams@intel.com> wrote: > > Kuppuswamy Sathyanarayanan wrote: > > On Wed, Apr 24, 2024 at 9:00 AM Tom Lendacky <thomas.lendacky@amd.com> wrote: > > > > > > The TSM attestation report support provides multiple configfs attribute > > > types (both for standard and binary attributes) to allow for additional > > > attributes to be displayed for SNP as compared to TDX. With the ability > > > to hide attributes via configfs, consoldate the multiple attribute groups > > > into a single standard attribute group and a single binary attribute > > > group. Modify the TDX support to hide the attributes that were previously > > > "hidden" as a result of registering the selective attribute groups. > > > > > > Co-developed-by: Dan Williams <dan.j.williams@intel.com> > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> > [..] > > > + */ > > > +enum tsm_attr_index { > > > + TSM_REPORT_GENERATION, > > > > Do we need an index for the generation attribute ? Since it is a core > > function, we can allow it by default. > > That is up to the is_visible() callback to decide the declaration of > which index corresponds to which attribute is just static information. > > > > > > + TSM_REPORT_PROVIDER, > > > > Same as above. > > These numbers need to match the array indices of tsm_report_attrs. > > Your suggestion makes the declaration of tsm_report_attrs more > difficult: > > static struct configfs_attribute *tsm_report_attrs[] = { > [TSM_REPORT_GENERATION] = &tsm_report_attr_generation, > [TSM_REPORT_PROVIDER] = &tsm_report_attr_provider, > [TSM_REPORT_PRIVLEVEL] = &tsm_report_attr_privlevel, > [TSM_REPORT_PRIVLEVEL_FLOOR] = &tsm_report_attr_privlevel_floor, > NULL, > }; > > ...because then the definition of TSM_REPORT_PRIVLEVEL would need to > know how many attributes precede it in the array. So, defining it this > way makes it more robust against future changes that want to > add/delete/reorder attributes in the array.
Got it. Makes sense. It is simpler to do it this way. I am just worried that the vendor driver might mistakenly disable some core attributes like inblob, outblob, provider and generation.
| |