Re: [PATCH] tpm: Add major_version sysfs file

[Jerry Snitselaar]

On Fri Oct 25 19, Mimi Zohar wrote:
> On Fri, 2019-10-25 at 07:28 -0700, Jerry Snitselaar wrote:
> > Easily determining what TCG version a tpm device implements
> > has been a pain point for userspace for a long time, so
> > add a sysfs file to report the tcg version of a tpm device.
>
> Use "TCG" uppercase consistently.
>  
> > Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> > Cc: Peter Huewe <peterhuewe@xxxxxx>
> > Cc: Jason Gunthorpe <jgg@xxxxxxxx>
> > Cc: linux-integrity@xxxxxxxxxxxxxxx
> > Signed-off-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>
>
> thanks!
>
> Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
>
> FYI, on my system(s) the new file is accessible as
> /sys/class/tpm/tpm0/version_major.  Does this need to be documented
> anywhere?

Yes, there should be an entry added to
Documentation/ABI/stable/sysfs-class-tpm.
I will fix that up and the TCG not being uppercase in a v2.

Should Documentation/ABI/stable/sysfs-class-tpm updated in
some way to reflect that those are all links under device
now and not actually there.

> > ---
> >  drivers/char/tpm/tpm-sysfs.c | 34 +++++++++++++++++++++++++++-------
> >  1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> > index edfa89160010..9372c2d6f0b3 100644
> > --- a/drivers/char/tpm/tpm-sysfs.c
> > +++ b/drivers/char/tpm/tpm-sysfs.c
> > @@ -309,7 +309,17 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
> >  }
> >  static DEVICE_ATTR_RO(timeouts);
> >
> > -static struct attribute *tpm_dev_attrs[] = {
> > +static ssize_t major_version_show(struct device *dev,
> > +				  struct device_attribute *attr, char *buf)
> > +{
> > +	struct tpm_chip *chip = to_tpm_chip(dev);
> > +
> > +	return sprintf(buf, "%s\n", chip->flags & TPM_CHIP_FLAG_TPM2
> > +		       ? "2.0" : "1.2");
> > +}
> > +static DEVICE_ATTR_RO(major_version);
> > +
> > +static struct attribute *tpm12_dev_attrs[] = {
> >  	&dev_attr_pubek.attr,
> >  	&dev_attr_pcrs.attr,
> >  	&dev_attr_enabled.attr,
> > @@ -320,18 +330,28 @@ static struct attribute *tpm_dev_attrs[] = {
> >  	&dev_attr_cancel.attr,
> >  	&dev_attr_durations.attr,
> >  	&dev_attr_timeouts.attr,
> > +	&dev_attr_major_version.attr,
> >  	NULL,
> >  };
> >
> > -static const struct attribute_group tpm_dev_group = {
> > -	.attrs = tpm_dev_attrs,
> > +static struct attribute *tpm20_dev_attrs[] = {
> > +	&dev_attr_major_version.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group tpm12_dev_group = {
> > +	.attrs = tpm12_dev_attrs,
> > +};
> > +
> > +static const struct attribute_group tpm20_dev_group = {
> > +	.attrs = tpm20_dev_attrs,
> >  };
> >
> >  void tpm_sysfs_add_device(struct tpm_chip *chip)
> >  {
> > -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > -		return;
> > -
> >  	WARN_ON(chip->groups_cnt != 0);
> > -	chip->groups[chip->groups_cnt++] = &tpm_dev_group;
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		chip->groups[chip->groups_cnt++] = &tpm20_dev_group;
> > +	else
> > +		chip->groups[chip->groups_cnt++] = &tpm12_dev_group;
> >  }