On Wed, Jan 31, 2024 at 02:05:04PM +0100, Pierre-Louis Bossart wrote: > > > On 1/30/24 19:46, Greg Kroah-Hartman wrote: > > From: Dan Williams <dan.j.williams@xxxxxxxxx> > > > > Add a mechanism for named attribute_groups to hide their directory at > > sysfs_update_group() time, or otherwise skip emitting the group > > directory when the group is first registered. It piggybacks on > > is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags > > in the upper bits of the returned mode. To use it, specify a symbol > > prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix > > to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback: > > > > DEFINE_SYSFS_GROUP_VISIBLE($prefix) > > > > struct attribute_group $prefix_group = { > > .name = $name, > > .is_visible = SYSFS_GROUP_VISIBLE($prefix), > > }; > > > > SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible() > > and $prefix_attr_visible(), where $prefix_group_visible() just returns > > true / false and $prefix_attr_visible() behaves as normal. > > > > The motivation for this capability is to centralize PCI device > > authentication in the PCI core with a named sysfs group while keeping > > that group hidden for devices and platforms that do not meet the > > requirements. In a PCI topology, most devices will not support > > authentication, a small subset will support just PCI CMA (Component > > Measurement and Authentication), a smaller subset will support PCI CMA + > > PCIe IDE (Link Integrity and Encryption), and only next generation > > server hosts will start to include a platform TSM (TEE Security > > Manager). > > > > Without this capability the alternatives are: > > > > * Check if all attributes are invisible and if so, hide the directory. > > Beyond trouble getting this to work [1], this is an ABI change for > > scenarios if userspace happens to depend on group visibility absent any > > attributes. I.e. this new capability avoids regression since it does > > not retroactively apply to existing cases. > > > > * Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI > > devices (i.e. for the case when TSM platform support is present, but > > device support is absent). Unfortunate that this will be a vestigial > > empty directory in the vast majority of cases. > > > > * Reintroduce usage of runtime calls to sysfs_{create,remove}_group() > > in the PCI core. Bjorn has already indicated that he does not want to > > see any growth of pci_sysfs_init() [2]. > > > > * Drop the named group and simulate a directory by prefixing all > > TSM-related attributes with "tsm_". Unfortunate to not use the naming > > capability of a sysfs group as intended. > > > > In comparison, there is a small potential for regression if for some > > reason an @is_visible() callback had dependencies on how many times it > > was called. Additionally, it is no longer an error to update a group > > that does not have its directory already present, and it is no longer a > > WARN() to remove a group that was never visible. > > > > Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1] > > Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2] > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > This patch seems to introduce a regression on our Lunar Lake test > devices, where we can't boot to an ssh shell. No issues on older devices > [1]. Bard Liao and I reproduced the same results on different boards. > > We'll need to find someone with direct device access to provide more > information on the problem, remote testing without ssh is a > self-negating proposition. > > Is there a dependency on other patches? Our tests are still based on > 6.7.0-rc3 due to other upstream issues we're currently working through. This should be totally stand-alone and not cause any problems, especially if you don't have any other patches applied. I did make this against 6.8-rc2, perhaps that's the issue? thanks, greg k-h