Re: [PATCH RESEND v4 0/1] add sysfs exports for TPM 2 PCR registers

On Tue, Sep 08, 2020 at 11:14:11AM -0700, James Bottomley wrote:

On Tue, 2020-09-08 at 21:05 +0300, Jarkko Sakkinen wrote:

quoted

On Tue, Sep 08, 2020 at 07:45:52AM +0200, Greg KH wrote:

quoted

On Mon, Sep 07, 2020 at 02:52:08PM -0700, James Bottomley wrote:

[...]

quoted

quoted

quoted

I've got to say I think binary attributes are actively evil.  I
can see
they're a necessity when there's no good way to represent the
data they
contain, like the bios measurement log or firmware code or a raw
interface like we do for the SMP frame code in libsas.  But when
there's a well understood and easy to produce user friendly non-
binary
representation, I think dumping binary is inimical to being a
good API.

Agreed.

thanks,

greg k-h

Looking at the patch, something like <device>/pcrs/<hash>/<index>
would be a bit cleaner representation than the current <device>/pcrs-
<hash>/<index>.

That's actually a technical limitation of using the current attribute
groups API: It's designed to support single level directories in sysfs
(or no directory at all).  That's not to say we can't do multi-level
ones, but if we do we have to roll our own machinery for managing the
files rather than relying on the groups API.

Given that the current groups API does all the nasty lifetime
management that I'd otherwise have to do in the patch, I have a strong
incentive for keeping it, which is why the single <device>/pcrs-
<hash>/<index> format.

OK, I do get that. I've tried to do something similar in past and it
turnd out to be a tremendous job.  I guess I'll have to re-review the
whole patch again. I'll prioritize that next week.

James

/Jarkko