Re: [PATCH 02/10] crypto: ccp: Add support for displaying PSP firmware versions

[Tom Lendacky]

On 4/20/23 11:31, Mario Limonciello wrote:
> As it's not always obvious what PSP bootloader or TEE version are
> present in OEM systems, add the ability to get this information from
> sysfs for supported platforms.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
>   Documentation/ABI/testing/sysfs-driver-ccp | 18 +++++++
>   drivers/crypto/ccp/sp-dev.h                |  2 +
>   drivers/crypto/ccp/sp-pci.c                | 58 ++++++++++++++++++++++
>   3 files changed, 78 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-ccp b/Documentation/ABI/testing/sysfs-driver-ccp
> index 7aded9b75553..ee6b787eee7a 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ccp
> +++ b/Documentation/ABI/testing/sysfs-driver-ccp
> @@ -85,3 +85,21 @@ Description:
>   		Possible values:
>   		0: Not enforced
>   		1: Enforced
> +
> +What:		/sys/bus/pci/devices/<BDF>/bootloader_version
> +Date:		June 2023
> +KernelVersion:	6.4
> +Contact:	mario.limonciello@xxxxxxx
> +Description:
> +		The /sys/bus/pci/devices/<BDF>/bootloader_version
> +		file reports the firmware version of the AMD AGESA
> +		bootloader.
> +
> +What:		/sys/bus/pci/devices/<BDF>/tee_version
> +Date:		June 2023
> +KernelVersion:	6.4
> +Contact:	mario.limonciello@xxxxxxx
> +Description:
> +		The /sys/bus/pci/devices/<BDF>/tee_version
> +		file reports the firmware version of the AMD Trusted
> +		Execution Environment (TEE).
> diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
> index 1253a0217985..76c32ee6bd65 100644
> --- a/drivers/crypto/ccp/sp-dev.h
> +++ b/drivers/crypto/ccp/sp-dev.h
> @@ -51,6 +51,7 @@ struct tee_vdata {
>   	const unsigned int cmdbuff_addr_hi_reg;
>   	const unsigned int ring_wptr_reg;
>   	const unsigned int ring_rptr_reg;
> +	const unsigned int info_reg;
>   };
>   
>   struct platform_access_vdata {
> @@ -69,6 +70,7 @@ struct psp_vdata {
>   	const unsigned int feature_reg;
>   	const unsigned int inten_reg;
>   	const unsigned int intsts_reg;
> +	const unsigned int bootloader_info_reg;
>   };
>   
>   /* Structure to hold SP device data */
> diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
> index 85e3dc8853e2..4d729aa1b260 100644
> --- a/drivers/crypto/ccp/sp-pci.c
> +++ b/drivers/crypto/ccp/sp-pci.c
> @@ -8,6 +8,7 @@
>    * Author: Gary R Hook <gary.hook@xxxxxxx>
>    */
>   
> +#include <linux/bitfield.h>
>   #include <linux/module.h>
>   #include <linux/kernel.h>
>   #include <linux/device.h>
> @@ -88,8 +89,65 @@ static struct attribute_group psp_security_attr_group = {
>   	.is_visible = psp_security_is_visible,
>   };
>   
> +#define version_attribute_show(name, _offset)					\
> +static ssize_t name##_show(struct device *d, struct device_attribute *attr,	\
> +			   char *buf)						\
> +{										\
> +	struct sp_device *sp = dev_get_drvdata(d);				\
> +	struct psp_device *psp = sp->psp_data;					\
> +	unsigned int val = ioread32(psp->io_regs + _offset);			\
> +	return sysfs_emit(buf, "%02lx.%02lx.%02lx.%02lx\n",			\
> +			  FIELD_GET(GENMASK(31, 24), val),			\
> +			  FIELD_GET(GENMASK(23, 16), val),			\
> +			  FIELD_GET(GENMASK(15, 8), val),			\
> +			  FIELD_GET(GENMASK(7, 0), val));			\
> +}
> +
> +version_attribute_show(bootloader_version, psp->vdata->bootloader_info_reg)
> +static DEVICE_ATTR_RO(bootloader_version);
> +version_attribute_show(tee_version, psp->vdata->tee->info_reg)
> +static DEVICE_ATTR_RO(tee_version);
> +
> +static struct attribute *psp_firmware_attrs[] = {
> +	&dev_attr_bootloader_version.attr,
> +	&dev_attr_tee_version.attr,
> +	NULL,
> +};
> +
> +static umode_t psp_firmware_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct sp_device *sp = dev_get_drvdata(dev);
> +	struct psp_device *psp = sp->psp_data;
> +	unsigned int val = 0xffffffff;
> +
> +	if (!psp)
> +		return 0;
> +
> +	if (attr == &dev_attr_bootloader_version.attr &&
> +	    psp->vdata->bootloader_info_reg)
> +		val = ioread32(psp->io_regs + psp->vdata->bootloader_info_reg);
> +
> +	if (attr == &dev_attr_tee_version.attr &&
> +	    psp->capability & PSP_CAPABILITY_TEE &&
> +	    psp->vdata->tee->info_reg)
> +		val = ioread32(psp->io_regs + psp->vdata->tee->info_reg);
> +
> +

Extra blank line.

Also, add a comment about how the PSP security policy might block access 
to these areas resulting in reading all f's, so this hides the sysfs 
attribute in that case.

Thanks,
Tom

> +	if (val != 0xffffffff)
> +		return 0444;
> +
> +	return 0;
> +}
> +
> +static struct attribute_group psp_firmware_attr_group = {
> +	.attrs = psp_firmware_attrs,
> +	.is_visible = psp_firmware_is_visible,
> +};
> +
>   static const struct attribute_group *psp_groups[] = {
>   	&psp_security_attr_group,
> +	&psp_firmware_attr_group,
>   	NULL,
>   };
>