All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
@ 2021-05-17  5:54 Jiawei Gu
  2021-05-17  6:52 ` Christian König
  2021-05-18 14:12 ` Alex Deucher
  0 siblings, 2 replies; 8+ messages in thread
From: Jiawei Gu @ 2021-05-17  5:54 UTC (permalink / raw)
  To: amd-gfx; +Cc: emily.deng, Jiawei Gu, david.nieto

Introduce an RFC 4122 compliant UUID for the GPUs derived
from the unique GPU serial number (from Vega10) on gpus.
Where this serial number is not available, use a compliant
random UUID.

For virtualization, the unique ID is passed by the host driver
in the PF2VF structure.

Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
 drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
 drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
 drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
 6 files changed, 146 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 3147c1c935c8..ad6d4b55be6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -802,6 +802,40 @@ struct amd_powerplay {
 					  (rid == 0x01) || \
 					  (rid == 0x10))))
 
+union amdgpu_uuid_info {
+	struct {
+		union {
+			struct {
+				uint32_t did	: 16;
+				uint32_t fcn	: 8;
+				uint32_t asic_7 : 8;
+			};
+			uint32_t time_low;
+		};
+
+		struct {
+			uint32_t time_mid  : 16;
+			uint32_t time_high : 12;
+			uint32_t version   : 4;
+		};
+
+		struct {
+			struct {
+				uint8_t clk_seq_hi : 6;
+				uint8_t variant    : 2;
+			};
+			union {
+				uint8_t clk_seq_low;
+				uint8_t asic_6;
+			};
+			uint16_t asic_4;
+		};
+
+		uint32_t asic_0;
+	};
+	char as_char[16];
+};
+
 #define AMDGPU_RESET_MAGIC_NUM 64
 #define AMDGPU_MAX_DF_PERFMONS 4
 struct amdgpu_device {
@@ -1074,6 +1108,8 @@ struct amdgpu_device {
 	char				product_name[32];
 	char				serial[20];
 
+	union amdgpu_uuid_info uuid_info;
+
 	struct amdgpu_autodump		autodump;
 
 	atomic_t			throttling_logging_enabled;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7c6c435e5d02..079841e1cb52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -37,6 +37,7 @@
 #include <linux/vgaarb.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/efi.h>
+#include <linux/uuid.h>
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 #include "amdgpu_i2c.h"
@@ -3239,11 +3240,104 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
 	return ret;
 }
 
+static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info *uuid_info)
+{
+	return (uuid_info->time_low    == 0 &&
+			uuid_info->time_mid    == 0 &&
+			uuid_info->time_high   == 0 &&
+			uuid_info->version     == 0 &&
+			uuid_info->clk_seq_hi  == 0 &&
+			uuid_info->variant     == 0 &&
+			uuid_info->clk_seq_low == 0 &&
+			uuid_info->asic_4      == 0 &&
+			uuid_info->asic_0      == 0);
+}
+
+static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
+				uint64_t serial, uint16_t did, uint8_t idx)
+{
+	uint16_t clk_seq = 0;
+
+	/* Step1: insert clk_seq */
+	uuid_info->clk_seq_low = (uint8_t)clk_seq;
+	uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
+
+	/* Step2: insert did */
+	uuid_info->did = did;
+
+	/* Step3: insert vf idx */
+	uuid_info->fcn = idx;
+
+	/* Step4: insert serial */
+	uuid_info->asic_0 = (uint32_t)serial;
+	uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
+	uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
+	uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
+
+	/* Step5: insert version */
+	uuid_info->version = 1;
+	/* Step6: insert variant */
+	uuid_info->variant = 2;
+}
+
+/* byte reverse random uuid */
+static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
+{
+	char b0, b1;
+	int i;
+
+	generate_random_uuid(uuid_info->as_char);
+	for (i = 0; i < 8; i++) {
+		b0 = uuid_info->as_char[i];
+		b1 = uuid_info->as_char[16-i];
+		uuid_info->as_char[16-i] = b0;
+		uuid_info->as_char[i] = b1;
+	}
+}
+
+/**
+ *
+ * The amdgpu driver provides a sysfs API for providing uuid data.
+ * The file uuid_info is used for this, and returns string of amdgpu uuid.
+ */
+static ssize_t amdgpu_get_uuid_info(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(ddev);//ddev->dev_private;
+	union amdgpu_uuid_info *uuid = &adev->uuid_info;
+
+	return sysfs_emit(buf,
+					"%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
+					uuid->time_low,
+					uuid->time_mid,
+					uuid->version,
+					uuid->time_high,
+					uuid->clk_seq_hi |
+					uuid->variant << 6,
+					uuid->clk_seq_low,
+					uuid->asic_4,
+					uuid->asic_0);
+}
+static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
+
+static void amdgpu_uuid_init(struct amdgpu_device *adev)
+{
+	if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
+		if (adev->unique_id)
+			amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, adev->pdev->device, 31);
+		else
+			amdgpu_gen_uuid_random(&adev->uuid_info);
+	}
+}
+
 static const struct attribute *amdgpu_dev_attributes[] = {
 	&dev_attr_product_name.attr,
 	&dev_attr_product_number.attr,
 	&dev_attr_serial_number.attr,
 	&dev_attr_pcie_replay_count.attr,
+	&dev_attr_uuid_info.attr,
 	NULL
 };
 
@@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
 	amdgpu_fbdev_init(adev);
 
+	amdgpu_uuid_init(adev);
+
 	r = amdgpu_pm_sysfs_init(adev);
 	if (r) {
 		adev->pm_sysfs_en = false;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index b71dd1deeb2d..2dfebfe38079 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
 static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
 {
 	struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = adev->virt.fw_reserve.p_pf2vf;
+	union amdgpu_uuid_info *uuid = &adev->uuid_info;
 	uint32_t checksum;
 	uint32_t checkval;
 
@@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
 
 		adev->unique_id =
 			((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid;
+
+		memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid_info_reserved,
+			sizeof(union amdgpu_uuid_info));
 		break;
 	default:
 		DRM_ERROR("invalid pf2vf version\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
index a434c71fde8e..0d1d36e82aeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
 		uint32_t encode_max_frame_pixels;
 	} mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
 	/* UUID info */
-	struct amd_sriov_msg_uuid_info uuid_info;
+	uint32_t uuid_info_reserved[4];
 	/* reserved */
-	uint32_t reserved[256 - 47];
+	uint32_t reserved[256-47];
 };
 
 struct amd_sriov_msg_vf2pf_info_header {
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 32c34470404c..16d4a480f4c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
 		if (amdgpu_sriov_vf(adev))
 			adev->rev_id = 0;
 		adev->external_rev_id = adev->rev_id + 0xa;
+		if (!amdgpu_sriov_vf(adev)) {
+			adev->unique_id = RREG32(mmFUSE_DATA_730);
+			adev->unique_id <<= 32;
+			adev->unique_id |= RREG32(mmFUSE_DATA_729);
+		}
 		break;
 	case CHIP_SIENNA_CICHLID:
 		adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h
index 515d67bf249f..520ac2b98744 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.h
+++ b/drivers/gpu/drm/amd/amdgpu/nv.h
@@ -26,6 +26,9 @@
 
 #include "nbio_v2_3.h"
 
+#define mmFUSE_DATA_729 (0x176D9)
+#define mmFUSE_DATA_730 (0x176DA)
+
 void nv_grbm_select(struct amdgpu_device *adev,
 		    u32 me, u32 pipe, u32 queue, u32 vmid);
 void nv_set_virt_ops(struct amdgpu_device *adev);
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-17  5:54 [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID Jiawei Gu
@ 2021-05-17  6:52 ` Christian König
  2021-05-17 20:08   ` Nieto, David M
  2021-05-18 14:12 ` Alex Deucher
  1 sibling, 1 reply; 8+ messages in thread
From: Christian König @ 2021-05-17  6:52 UTC (permalink / raw)
  To: Jiawei Gu, amd-gfx; +Cc: emily.deng, david.nieto

Am 17.05.21 um 07:54 schrieb Jiawei Gu:
> Introduce an RFC 4122 compliant UUID for the GPUs derived
> from the unique GPU serial number (from Vega10) on gpus.
> Where this serial number is not available, use a compliant
> random UUID.
>
> For virtualization, the unique ID is passed by the host driver
> in the PF2VF structure.

The question is why this is useful.

>
> Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
>   drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
>   drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
>   drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
>   6 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3147c1c935c8..ad6d4b55be6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -802,6 +802,40 @@ struct amd_powerplay {
>   					  (rid == 0x01) || \
>   					  (rid == 0x10))))
>   
> +union amdgpu_uuid_info {
> +	struct {
> +		union {
> +			struct {
> +				uint32_t did	: 16;
> +				uint32_t fcn	: 8;
> +				uint32_t asic_7 : 8;
> +			};

Bitfields are not allowed in structures used for communication with 
hardware or uAPI.

Regards,
Christian.

> +			uint32_t time_low;
> +		};
> +
> +		struct {
> +			uint32_t time_mid  : 16;
> +			uint32_t time_high : 12;
> +			uint32_t version   : 4;
> +		};
> +
> +		struct {
> +			struct {
> +				uint8_t clk_seq_hi : 6;
> +				uint8_t variant    : 2;
> +			};
> +			union {
> +				uint8_t clk_seq_low;
> +				uint8_t asic_6;
> +			};
> +			uint16_t asic_4;
> +		};
> +
> +		uint32_t asic_0;
> +	};
> +	char as_char[16];
> +};
> +
>   #define AMDGPU_RESET_MAGIC_NUM 64
>   #define AMDGPU_MAX_DF_PERFMONS 4
>   struct amdgpu_device {
> @@ -1074,6 +1108,8 @@ struct amdgpu_device {
>   	char				product_name[32];
>   	char				serial[20];
>   
> +	union amdgpu_uuid_info uuid_info;
> +
>   	struct amdgpu_autodump		autodump;
>   
>   	atomic_t			throttling_logging_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7c6c435e5d02..079841e1cb52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -37,6 +37,7 @@
>   #include <linux/vgaarb.h>
>   #include <linux/vga_switcheroo.h>
>   #include <linux/efi.h>
> +#include <linux/uuid.h>
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_i2c.h"
> @@ -3239,11 +3240,104 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>   	return ret;
>   }
>   
> +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info *uuid_info)
> +{
> +	return (uuid_info->time_low    == 0 &&
> +			uuid_info->time_mid    == 0 &&
> +			uuid_info->time_high   == 0 &&
> +			uuid_info->version     == 0 &&
> +			uuid_info->clk_seq_hi  == 0 &&
> +			uuid_info->variant     == 0 &&
> +			uuid_info->clk_seq_low == 0 &&
> +			uuid_info->asic_4      == 0 &&
> +			uuid_info->asic_0      == 0);
> +}
> +
> +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> +				uint64_t serial, uint16_t did, uint8_t idx)
> +{
> +	uint16_t clk_seq = 0;
> +
> +	/* Step1: insert clk_seq */
> +	uuid_info->clk_seq_low = (uint8_t)clk_seq;
> +	uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> +
> +	/* Step2: insert did */
> +	uuid_info->did = did;
> +
> +	/* Step3: insert vf idx */
> +	uuid_info->fcn = idx;
> +
> +	/* Step4: insert serial */
> +	uuid_info->asic_0 = (uint32_t)serial;
> +	uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> +	uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> +	uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> +
> +	/* Step5: insert version */
> +	uuid_info->version = 1;
> +	/* Step6: insert variant */
> +	uuid_info->variant = 2;
> +}
> +
> +/* byte reverse random uuid */
> +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> +{
> +	char b0, b1;
> +	int i;
> +
> +	generate_random_uuid(uuid_info->as_char);
> +	for (i = 0; i < 8; i++) {
> +		b0 = uuid_info->as_char[i];
> +		b1 = uuid_info->as_char[16-i];
> +		uuid_info->as_char[16-i] = b0;
> +		uuid_info->as_char[i] = b1;
> +	}
> +}
> +
> +/**
> + *
> + * The amdgpu driver provides a sysfs API for providing uuid data.
> + * The file uuid_info is used for this, and returns string of amdgpu uuid.
> + */
> +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> +				      struct device_attribute *attr,
> +				      char *buf)
> +{
> +	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct amdgpu_device *adev = drm_to_adev(ddev);//ddev->dev_private;
> +	union amdgpu_uuid_info *uuid = &adev->uuid_info;
> +
> +	return sysfs_emit(buf,
> +					"%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> +					uuid->time_low,
> +					uuid->time_mid,
> +					uuid->version,
> +					uuid->time_high,
> +					uuid->clk_seq_hi |
> +					uuid->variant << 6,
> +					uuid->clk_seq_low,
> +					uuid->asic_4,
> +					uuid->asic_0);
> +}
> +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> +
> +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> +{
> +	if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> +		if (adev->unique_id)
> +			amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, adev->pdev->device, 31);
> +		else
> +			amdgpu_gen_uuid_random(&adev->uuid_info);
> +	}
> +}
> +
>   static const struct attribute *amdgpu_dev_attributes[] = {
>   	&dev_attr_product_name.attr,
>   	&dev_attr_product_number.attr,
>   	&dev_attr_serial_number.attr,
>   	&dev_attr_pcie_replay_count.attr,
> +	&dev_attr_uuid_info.attr,
>   	NULL
>   };
>   
> @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   
>   	amdgpu_fbdev_init(adev);
>   
> +	amdgpu_uuid_init(adev);
> +
>   	r = amdgpu_pm_sysfs_init(adev);
>   	if (r) {
>   		adev->pm_sysfs_en = false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index b71dd1deeb2d..2dfebfe38079 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
>   static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>   {
>   	struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = adev->virt.fw_reserve.p_pf2vf;
> +	union amdgpu_uuid_info *uuid = &adev->uuid_info;
>   	uint32_t checksum;
>   	uint32_t checkval;
>   
> @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>   
>   		adev->unique_id =
>   			((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid;
> +
> +		memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid_info_reserved,
> +			sizeof(union amdgpu_uuid_info));
>   		break;
>   	default:
>   		DRM_ERROR("invalid pf2vf version\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> index a434c71fde8e..0d1d36e82aeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
>   		uint32_t encode_max_frame_pixels;
>   	} mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
>   	/* UUID info */
> -	struct amd_sriov_msg_uuid_info uuid_info;
> +	uint32_t uuid_info_reserved[4];
>   	/* reserved */
> -	uint32_t reserved[256 - 47];
> +	uint32_t reserved[256-47];
>   };
>   
>   struct amd_sriov_msg_vf2pf_info_header {
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 32c34470404c..16d4a480f4c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
>   		if (amdgpu_sriov_vf(adev))
>   			adev->rev_id = 0;
>   		adev->external_rev_id = adev->rev_id + 0xa;
> +		if (!amdgpu_sriov_vf(adev)) {
> +			adev->unique_id = RREG32(mmFUSE_DATA_730);
> +			adev->unique_id <<= 32;
> +			adev->unique_id |= RREG32(mmFUSE_DATA_729);
> +		}
>   		break;
>   	case CHIP_SIENNA_CICHLID:
>   		adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h
> index 515d67bf249f..520ac2b98744 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> @@ -26,6 +26,9 @@
>   
>   #include "nbio_v2_3.h"
>   
> +#define mmFUSE_DATA_729 (0x176D9)
> +#define mmFUSE_DATA_730 (0x176DA)
> +
>   void nv_grbm_select(struct amdgpu_device *adev,
>   		    u32 me, u32 pipe, u32 queue, u32 vmid);
>   void nv_set_virt_ops(struct amdgpu_device *adev);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-17  6:52 ` Christian König
@ 2021-05-17 20:08   ` Nieto, David M
  2021-05-18 13:03     ` Christian König
  0 siblings, 1 reply; 8+ messages in thread
From: Nieto, David M @ 2021-05-17 20:08 UTC (permalink / raw)
  To: Christian König, Gu, JiaWei (Will), amd-gfx; +Cc: Deng, Emily


[-- Attachment #1.1: Type: text/plain, Size: 11391 bytes --]

[AMD Official Use Only]

It is for unique identification of the GPU in system management applications, the 64 bit asic number is only available in Vega10 and later and not compliant with RFC4122.

David
________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Sunday, May 16, 2021 11:52 PM
To: Gu, JiaWei (Will) <JiaWei.Gu@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deng, Emily <Emily.Deng@amd.com>; Nieto, David M <David.Nieto@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID

Am 17.05.21 um 07:54 schrieb Jiawei Gu:
> Introduce an RFC 4122 compliant UUID for the GPUs derived
> from the unique GPU serial number (from Vega10) on gpus.
> Where this serial number is not available, use a compliant
> random UUID.
>
> For virtualization, the unique ID is passed by the host driver
> in the PF2VF structure.

The question is why this is useful.

>
> Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
>   drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
>   drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
>   drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
>   6 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3147c1c935c8..ad6d4b55be6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -802,6 +802,40 @@ struct amd_powerplay {
>                                          (rid == 0x01) || \
>                                          (rid == 0x10))))
>
> +union amdgpu_uuid_info {
> +     struct {
> +             union {
> +                     struct {
> +                             uint32_t did    : 16;
> +                             uint32_t fcn    : 8;
> +                             uint32_t asic_7 : 8;
> +                     };

Bitfields are not allowed in structures used for communication with
hardware or uAPI.

Regards,
Christian.

> +                     uint32_t time_low;
> +             };
> +
> +             struct {
> +                     uint32_t time_mid  : 16;
> +                     uint32_t time_high : 12;
> +                     uint32_t version   : 4;
> +             };
> +
> +             struct {
> +                     struct {
> +                             uint8_t clk_seq_hi : 6;
> +                             uint8_t variant    : 2;
> +                     };
> +                     union {
> +                             uint8_t clk_seq_low;
> +                             uint8_t asic_6;
> +                     };
> +                     uint16_t asic_4;
> +             };
> +
> +             uint32_t asic_0;
> +     };
> +     char as_char[16];
> +};
> +
>   #define AMDGPU_RESET_MAGIC_NUM 64
>   #define AMDGPU_MAX_DF_PERFMONS 4
>   struct amdgpu_device {
> @@ -1074,6 +1108,8 @@ struct amdgpu_device {
>        char                            product_name[32];
>        char                            serial[20];
>
> +     union amdgpu_uuid_info uuid_info;
> +
>        struct amdgpu_autodump          autodump;
>
>        atomic_t                        throttling_logging_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7c6c435e5d02..079841e1cb52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -37,6 +37,7 @@
>   #include <linux/vgaarb.h>
>   #include <linux/vga_switcheroo.h>
>   #include <linux/efi.h>
> +#include <linux/uuid.h>
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_i2c.h"
> @@ -3239,11 +3240,104 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>        return ret;
>   }
>
> +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info *uuid_info)
> +{
> +     return (uuid_info->time_low    == 0 &&
> +                     uuid_info->time_mid    == 0 &&
> +                     uuid_info->time_high   == 0 &&
> +                     uuid_info->version     == 0 &&
> +                     uuid_info->clk_seq_hi  == 0 &&
> +                     uuid_info->variant     == 0 &&
> +                     uuid_info->clk_seq_low == 0 &&
> +                     uuid_info->asic_4      == 0 &&
> +                     uuid_info->asic_0      == 0);
> +}
> +
> +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> +                             uint64_t serial, uint16_t did, uint8_t idx)
> +{
> +     uint16_t clk_seq = 0;
> +
> +     /* Step1: insert clk_seq */
> +     uuid_info->clk_seq_low = (uint8_t)clk_seq;
> +     uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> +
> +     /* Step2: insert did */
> +     uuid_info->did = did;
> +
> +     /* Step3: insert vf idx */
> +     uuid_info->fcn = idx;
> +
> +     /* Step4: insert serial */
> +     uuid_info->asic_0 = (uint32_t)serial;
> +     uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> +     uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> +     uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> +
> +     /* Step5: insert version */
> +     uuid_info->version = 1;
> +     /* Step6: insert variant */
> +     uuid_info->variant = 2;
> +}
> +
> +/* byte reverse random uuid */
> +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> +{
> +     char b0, b1;
> +     int i;
> +
> +     generate_random_uuid(uuid_info->as_char);
> +     for (i = 0; i < 8; i++) {
> +             b0 = uuid_info->as_char[i];
> +             b1 = uuid_info->as_char[16-i];
> +             uuid_info->as_char[16-i] = b0;
> +             uuid_info->as_char[i] = b1;
> +     }
> +}
> +
> +/**
> + *
> + * The amdgpu driver provides a sysfs API for providing uuid data.
> + * The file uuid_info is used for this, and returns string of amdgpu uuid.
> + */
> +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> +                                   struct device_attribute *attr,
> +                                   char *buf)
> +{
> +     struct drm_device *ddev = dev_get_drvdata(dev);
> +     struct amdgpu_device *adev = drm_to_adev(ddev);//ddev->dev_private;
> +     union amdgpu_uuid_info *uuid = &adev->uuid_info;
> +
> +     return sysfs_emit(buf,
> +                                     "%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> +                                     uuid->time_low,
> +                                     uuid->time_mid,
> +                                     uuid->version,
> +                                     uuid->time_high,
> +                                     uuid->clk_seq_hi |
> +                                     uuid->variant << 6,
> +                                     uuid->clk_seq_low,
> +                                     uuid->asic_4,
> +                                     uuid->asic_0);
> +}
> +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> +
> +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> +{
> +     if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> +             if (adev->unique_id)
> +                     amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, adev->pdev->device, 31);
> +             else
> +                     amdgpu_gen_uuid_random(&adev->uuid_info);
> +     }
> +}
> +
>   static const struct attribute *amdgpu_dev_attributes[] = {
>        &dev_attr_product_name.attr,
>        &dev_attr_product_number.attr,
>        &dev_attr_serial_number.attr,
>        &dev_attr_pcie_replay_count.attr,
> +     &dev_attr_uuid_info.attr,
>        NULL
>   };
>
> @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
>        amdgpu_fbdev_init(adev);
>
> +     amdgpu_uuid_init(adev);
> +
>        r = amdgpu_pm_sysfs_init(adev);
>        if (r) {
>                adev->pm_sysfs_en = false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index b71dd1deeb2d..2dfebfe38079 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
>   static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>   {
>        struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = adev->virt.fw_reserve.p_pf2vf;
> +     union amdgpu_uuid_info *uuid = &adev->uuid_info;
>        uint32_t checksum;
>        uint32_t checkval;
>
> @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>
>                adev->unique_id =
>                        ((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid;
> +
> +             memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid_info_reserved,
> +                     sizeof(union amdgpu_uuid_info));
>                break;
>        default:
>                DRM_ERROR("invalid pf2vf version\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> index a434c71fde8e..0d1d36e82aeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
>                uint32_t encode_max_frame_pixels;
>        } mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
>        /* UUID info */
> -     struct amd_sriov_msg_uuid_info uuid_info;
> +     uint32_t uuid_info_reserved[4];
>        /* reserved */
> -     uint32_t reserved[256 - 47];
> +     uint32_t reserved[256-47];
>   };
>
>   struct amd_sriov_msg_vf2pf_info_header {
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 32c34470404c..16d4a480f4c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
>                if (amdgpu_sriov_vf(adev))
>                        adev->rev_id = 0;
>                adev->external_rev_id = adev->rev_id + 0xa;
> +             if (!amdgpu_sriov_vf(adev)) {
> +                     adev->unique_id = RREG32(mmFUSE_DATA_730);
> +                     adev->unique_id <<= 32;
> +                     adev->unique_id |= RREG32(mmFUSE_DATA_729);
> +             }
>                break;
>        case CHIP_SIENNA_CICHLID:
>                adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h
> index 515d67bf249f..520ac2b98744 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> @@ -26,6 +26,9 @@
>
>   #include "nbio_v2_3.h"
>
> +#define mmFUSE_DATA_729 (0x176D9)
> +#define mmFUSE_DATA_730 (0x176DA)
> +
>   void nv_grbm_select(struct amdgpu_device *adev,
>                    u32 me, u32 pipe, u32 queue, u32 vmid);
>   void nv_set_virt_ops(struct amdgpu_device *adev);


[-- Attachment #1.2: Type: text/html, Size: 25825 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-17 20:08   ` Nieto, David M
@ 2021-05-18 13:03     ` Christian König
  0 siblings, 0 replies; 8+ messages in thread
From: Christian König @ 2021-05-18 13:03 UTC (permalink / raw)
  To: Nieto, David M, Gu, JiaWei (Will), amd-gfx; +Cc: Deng, Emily


[-- Attachment #1.1: Type: text/plain, Size: 11670 bytes --]

When this is not compliant with RFC4122 then why do we try to expose a 
UUID which seems to be compliant?

And emulating it for older hardware doesn't sounds like something I 
would want in the kernel driver. That can perfectly be handled in 
userspace as far as I can see.

Christian.

Am 17.05.21 um 22:08 schrieb Nieto, David M:
>
> [AMD Official Use Only]
>
>
> It is for unique identification of the GPU in system management 
> applications, the 64 bit asic number is only available in Vega10 and 
> later and not compliant with RFC4122.
>
> David
> ------------------------------------------------------------------------
> *From:* Christian König <ckoenig.leichtzumerken@gmail.com>
> *Sent:* Sunday, May 16, 2021 11:52 PM
> *To:* Gu, JiaWei (Will) <JiaWei.Gu@amd.com>; 
> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> *Cc:* Deng, Emily <Emily.Deng@amd.com>; Nieto, David M 
> <David.Nieto@amd.com>
> *Subject:* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
> Am 17.05.21 um 07:54 schrieb Jiawei Gu:
> > Introduce an RFC 4122 compliant UUID for the GPUs derived
> > from the unique GPU serial number (from Vega10) on gpus.
> > Where this serial number is not available, use a compliant
> > random UUID.
> >
> > For virtualization, the unique ID is passed by the host driver
> > in the PF2VF structure.
>
> The question is why this is useful.
>
> >
> > Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
> >   drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
> >   drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
> >   drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
> >   6 files changed, 146 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 3147c1c935c8..ad6d4b55be6c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -802,6 +802,40 @@ struct amd_powerplay {
> >                                          (rid == 0x01) || \
> >                                          (rid == 0x10))))
> >
> > +union amdgpu_uuid_info {
> > +     struct {
> > +             union {
> > +                     struct {
> > +                             uint32_t did    : 16;
> > +                             uint32_t fcn    : 8;
> > +                             uint32_t asic_7 : 8;
> > +                     };
>
> Bitfields are not allowed in structures used for communication with
> hardware or uAPI.
>
> Regards,
> Christian.
>
> > +                     uint32_t time_low;
> > +             };
> > +
> > +             struct {
> > +                     uint32_t time_mid  : 16;
> > +                     uint32_t time_high : 12;
> > +                     uint32_t version   : 4;
> > +             };
> > +
> > +             struct {
> > +                     struct {
> > +                             uint8_t clk_seq_hi : 6;
> > +                             uint8_t variant    : 2;
> > +                     };
> > +                     union {
> > +                             uint8_t clk_seq_low;
> > +                             uint8_t asic_6;
> > +                     };
> > +                     uint16_t asic_4;
> > +             };
> > +
> > +             uint32_t asic_0;
> > +     };
> > +     char as_char[16];
> > +};
> > +
> >   #define AMDGPU_RESET_MAGIC_NUM 64
> >   #define AMDGPU_MAX_DF_PERFMONS 4
> >   struct amdgpu_device {
> > @@ -1074,6 +1108,8 @@ struct amdgpu_device {
> >        char product_name[32];
> >        char                            serial[20];
> >
> > +     union amdgpu_uuid_info uuid_info;
> > +
> >        struct amdgpu_autodump          autodump;
> >
> >        atomic_t throttling_logging_enabled;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 7c6c435e5d02..079841e1cb52 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -37,6 +37,7 @@
> >   #include <linux/vgaarb.h>
> >   #include <linux/vga_switcheroo.h>
> >   #include <linux/efi.h>
> > +#include <linux/uuid.h>
> >   #include "amdgpu.h"
> >   #include "amdgpu_trace.h"
> >   #include "amdgpu_i2c.h"
> > @@ -3239,11 +3240,104 @@ static int 
> amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
> >        return ret;
> >   }
> >
> > +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info 
> *uuid_info)
> > +{
> > +     return (uuid_info->time_low    == 0 &&
> > +                     uuid_info->time_mid    == 0 &&
> > +                     uuid_info->time_high   == 0 &&
> > +                     uuid_info->version     == 0 &&
> > +                     uuid_info->clk_seq_hi  == 0 &&
> > +                     uuid_info->variant     == 0 &&
> > +                     uuid_info->clk_seq_low == 0 &&
> > +                     uuid_info->asic_4      == 0 &&
> > +                     uuid_info->asic_0      == 0);
> > +}
> > +
> > +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> > +                             uint64_t serial, uint16_t did, uint8_t 
> idx)
> > +{
> > +     uint16_t clk_seq = 0;
> > +
> > +     /* Step1: insert clk_seq */
> > +     uuid_info->clk_seq_low = (uint8_t)clk_seq;
> > +     uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> > +
> > +     /* Step2: insert did */
> > +     uuid_info->did = did;
> > +
> > +     /* Step3: insert vf idx */
> > +     uuid_info->fcn = idx;
> > +
> > +     /* Step4: insert serial */
> > +     uuid_info->asic_0 = (uint32_t)serial;
> > +     uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> > +     uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> > +     uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> > +
> > +     /* Step5: insert version */
> > +     uuid_info->version = 1;
> > +     /* Step6: insert variant */
> > +     uuid_info->variant = 2;
> > +}
> > +
> > +/* byte reverse random uuid */
> > +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> > +{
> > +     char b0, b1;
> > +     int i;
> > +
> > +     generate_random_uuid(uuid_info->as_char);
> > +     for (i = 0; i < 8; i++) {
> > +             b0 = uuid_info->as_char[i];
> > +             b1 = uuid_info->as_char[16-i];
> > +             uuid_info->as_char[16-i] = b0;
> > +             uuid_info->as_char[i] = b1;
> > +     }
> > +}
> > +
> > +/**
> > + *
> > + * The amdgpu driver provides a sysfs API for providing uuid data.
> > + * The file uuid_info is used for this, and returns string of 
> amdgpu uuid.
> > + */
> > +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> > +                                   struct device_attribute *attr,
> > +                                   char *buf)
> > +{
> > +     struct drm_device *ddev = dev_get_drvdata(dev);
> > +     struct amdgpu_device *adev = 
> drm_to_adev(ddev);//ddev->dev_private;
> > +     union amdgpu_uuid_info *uuid = &adev->uuid_info;
> > +
> > +     return sysfs_emit(buf,
> > + "%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> > + uuid->time_low,
> > + uuid->time_mid,
> > + uuid->version,
> > + uuid->time_high,
> > + uuid->clk_seq_hi |
> > + uuid->variant << 6,
> > + uuid->clk_seq_low,
> > + uuid->asic_4,
> > + uuid->asic_0);
> > +}
> > +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> > +
> > +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> > +{
> > +     if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> > +             if (adev->unique_id)
> > + amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, 
> adev->pdev->device, 31);
> > +             else
> > + amdgpu_gen_uuid_random(&adev->uuid_info);
> > +     }
> > +}
> > +
> >   static const struct attribute *amdgpu_dev_attributes[] = {
> >        &dev_attr_product_name.attr,
> >        &dev_attr_product_number.attr,
> >        &dev_attr_serial_number.attr,
> >        &dev_attr_pcie_replay_count.attr,
> > +     &dev_attr_uuid_info.attr,
> >        NULL
> >   };
> >
> > @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >
> >        amdgpu_fbdev_init(adev);
> >
> > +     amdgpu_uuid_init(adev);
> > +
> >        r = amdgpu_pm_sysfs_init(adev);
> >        if (r) {
> >                adev->pm_sysfs_en = false;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index b71dd1deeb2d..2dfebfe38079 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct 
> amdgpu_device *adev,
> >   static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
> >   {
> >        struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = 
> adev->virt.fw_reserve.p_pf2vf;
> > +     union amdgpu_uuid_info *uuid = &adev->uuid_info;
> >        uint32_t checksum;
> >        uint32_t checkval;
> >
> > @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct 
> amdgpu_device *adev)
> >
> >                adev->unique_id =
> >                        ((struct amd_sriov_msg_pf2vf_info 
> *)pf2vf_info)->uuid;
> > +
> > +             memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info 
> *)pf2vf_info)->uuid_info_reserved,
> > +                     sizeof(union amdgpu_uuid_info));
> >                break;
> >        default:
> >                DRM_ERROR("invalid pf2vf version\n");
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> > index a434c71fde8e..0d1d36e82aeb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> > @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
> >                uint32_t encode_max_frame_pixels;
> >        } mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
> >        /* UUID info */
> > -     struct amd_sriov_msg_uuid_info uuid_info;
> > +     uint32_t uuid_info_reserved[4];
> >        /* reserved */
> > -     uint32_t reserved[256 - 47];
> > +     uint32_t reserved[256-47];
> >   };
> >
> >   struct amd_sriov_msg_vf2pf_info_header {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
> b/drivers/gpu/drm/amd/amdgpu/nv.c
> > index 32c34470404c..16d4a480f4c0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
> >                if (amdgpu_sriov_vf(adev))
> >                        adev->rev_id = 0;
> >                adev->external_rev_id = adev->rev_id + 0xa;
> > +             if (!amdgpu_sriov_vf(adev)) {
> > +                     adev->unique_id = RREG32(mmFUSE_DATA_730);
> > +                     adev->unique_id <<= 32;
> > +                     adev->unique_id |= RREG32(mmFUSE_DATA_729);
> > +             }
> >                break;
> >        case CHIP_SIENNA_CICHLID:
> >                adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h 
> b/drivers/gpu/drm/amd/amdgpu/nv.h
> > index 515d67bf249f..520ac2b98744 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> > @@ -26,6 +26,9 @@
> >
> >   #include "nbio_v2_3.h"
> >
> > +#define mmFUSE_DATA_729 (0x176D9)
> > +#define mmFUSE_DATA_730 (0x176DA)
> > +
> >   void nv_grbm_select(struct amdgpu_device *adev,
> >                    u32 me, u32 pipe, u32 queue, u32 vmid);
> >   void nv_set_virt_ops(struct amdgpu_device *adev);
>


[-- Attachment #1.2: Type: text/html, Size: 22064 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-17  5:54 [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID Jiawei Gu
  2021-05-17  6:52 ` Christian König
@ 2021-05-18 14:12 ` Alex Deucher
  2021-05-18 20:37   ` Nieto, David M
  1 sibling, 1 reply; 8+ messages in thread
From: Alex Deucher @ 2021-05-18 14:12 UTC (permalink / raw)
  To: Jiawei Gu; +Cc: Emily Deng, amd-gfx list, Nieto, David M

On Mon, May 17, 2021 at 1:54 AM Jiawei Gu <Jiawei.Gu@amd.com> wrote:
>
> Introduce an RFC 4122 compliant UUID for the GPUs derived
> from the unique GPU serial number (from Vega10) on gpus.
> Where this serial number is not available, use a compliant
> random UUID.
>
> For virtualization, the unique ID is passed by the host driver
> in the PF2VF structure.
>
> Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
>  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
>  drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
>  drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
>  6 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3147c1c935c8..ad6d4b55be6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -802,6 +802,40 @@ struct amd_powerplay {
>                                           (rid == 0x01) || \
>                                           (rid == 0x10))))
>
> +union amdgpu_uuid_info {
> +       struct {
> +               union {
> +                       struct {
> +                               uint32_t did    : 16;
> +                               uint32_t fcn    : 8;
> +                               uint32_t asic_7 : 8;
> +                       };
> +                       uint32_t time_low;
> +               };
> +
> +               struct {
> +                       uint32_t time_mid  : 16;
> +                       uint32_t time_high : 12;
> +                       uint32_t version   : 4;
> +               };
> +
> +               struct {
> +                       struct {
> +                               uint8_t clk_seq_hi : 6;
> +                               uint8_t variant    : 2;
> +                       };
> +                       union {
> +                               uint8_t clk_seq_low;
> +                               uint8_t asic_6;
> +                       };
> +                       uint16_t asic_4;
> +               };
> +
> +               uint32_t asic_0;
> +       };
> +       char as_char[16];
> +};
> +
>  #define AMDGPU_RESET_MAGIC_NUM 64
>  #define AMDGPU_MAX_DF_PERFMONS 4
>  struct amdgpu_device {
> @@ -1074,6 +1108,8 @@ struct amdgpu_device {
>         char                            product_name[32];
>         char                            serial[20];
>
> +       union amdgpu_uuid_info uuid_info;
> +
>         struct amdgpu_autodump          autodump;
>
>         atomic_t                        throttling_logging_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7c6c435e5d02..079841e1cb52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -37,6 +37,7 @@
>  #include <linux/vgaarb.h>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/efi.h>
> +#include <linux/uuid.h>
>  #include "amdgpu.h"
>  #include "amdgpu_trace.h"
>  #include "amdgpu_i2c.h"
> @@ -3239,11 +3240,104 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>         return ret;
>  }
>
> +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info *uuid_info)
> +{
> +       return (uuid_info->time_low    == 0 &&
> +                       uuid_info->time_mid    == 0 &&
> +                       uuid_info->time_high   == 0 &&
> +                       uuid_info->version     == 0 &&
> +                       uuid_info->clk_seq_hi  == 0 &&
> +                       uuid_info->variant     == 0 &&
> +                       uuid_info->clk_seq_low == 0 &&
> +                       uuid_info->asic_4      == 0 &&
> +                       uuid_info->asic_0      == 0);
> +}
> +
> +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> +                               uint64_t serial, uint16_t did, uint8_t idx)
> +{
> +       uint16_t clk_seq = 0;
> +
> +       /* Step1: insert clk_seq */
> +       uuid_info->clk_seq_low = (uint8_t)clk_seq;
> +       uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> +
> +       /* Step2: insert did */
> +       uuid_info->did = did;
> +
> +       /* Step3: insert vf idx */
> +       uuid_info->fcn = idx;
> +
> +       /* Step4: insert serial */
> +       uuid_info->asic_0 = (uint32_t)serial;
> +       uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> +       uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> +       uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> +
> +       /* Step5: insert version */
> +       uuid_info->version = 1;
> +       /* Step6: insert variant */
> +       uuid_info->variant = 2;
> +}
> +
> +/* byte reverse random uuid */
> +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> +{
> +       char b0, b1;
> +       int i;
> +
> +       generate_random_uuid(uuid_info->as_char);
> +       for (i = 0; i < 8; i++) {
> +               b0 = uuid_info->as_char[i];
> +               b1 = uuid_info->as_char[16-i];
> +               uuid_info->as_char[16-i] = b0;
> +               uuid_info->as_char[i] = b1;
> +       }
> +}
> +
> +/**
> + *
> + * The amdgpu driver provides a sysfs API for providing uuid data.
> + * The file uuid_info is used for this, and returns string of amdgpu uuid.
> + */
> +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     char *buf)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(ddev);//ddev->dev_private;
> +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
> +
> +       return sysfs_emit(buf,
> +                                       "%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> +                                       uuid->time_low,
> +                                       uuid->time_mid,
> +                                       uuid->version,
> +                                       uuid->time_high,
> +                                       uuid->clk_seq_hi |
> +                                       uuid->variant << 6,
> +                                       uuid->clk_seq_low,
> +                                       uuid->asic_4,
> +                                       uuid->asic_0);
> +}
> +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> +
> +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> +{
> +       if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> +               if (adev->unique_id)
> +                       amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, adev->pdev->device, 31);
> +               else
> +                       amdgpu_gen_uuid_random(&adev->uuid_info);
> +       }
> +}
> +
>  static const struct attribute *amdgpu_dev_attributes[] = {
>         &dev_attr_product_name.attr,
>         &dev_attr_product_number.attr,
>         &dev_attr_serial_number.attr,
>         &dev_attr_pcie_replay_count.attr,
> +       &dev_attr_uuid_info.attr,
>         NULL
>  };
>
> @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
>         amdgpu_fbdev_init(adev);
>
> +       amdgpu_uuid_init(adev);
> +
>         r = amdgpu_pm_sysfs_init(adev);
>         if (r) {
>                 adev->pm_sysfs_en = false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index b71dd1deeb2d..2dfebfe38079 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
>  static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>  {
>         struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = adev->virt.fw_reserve.p_pf2vf;
> +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
>         uint32_t checksum;
>         uint32_t checkval;
>
> @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>
>                 adev->unique_id =
>                         ((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid;
> +
> +               memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid_info_reserved,
> +                       sizeof(union amdgpu_uuid_info));
>                 break;
>         default:
>                 DRM_ERROR("invalid pf2vf version\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> index a434c71fde8e..0d1d36e82aeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
>                 uint32_t encode_max_frame_pixels;
>         } mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
>         /* UUID info */
> -       struct amd_sriov_msg_uuid_info uuid_info;
> +       uint32_t uuid_info_reserved[4];
>         /* reserved */
> -       uint32_t reserved[256 - 47];
> +       uint32_t reserved[256-47];
>  };
>
>  struct amd_sriov_msg_vf2pf_info_header {
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 32c34470404c..16d4a480f4c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
>                 if (amdgpu_sriov_vf(adev))
>                         adev->rev_id = 0;
>                 adev->external_rev_id = adev->rev_id + 0xa;
> +               if (!amdgpu_sriov_vf(adev)) {
> +                       adev->unique_id = RREG32(mmFUSE_DATA_730);
> +                       adev->unique_id <<= 32;
> +                       adev->unique_id |= RREG32(mmFUSE_DATA_729);
> +               }

I would suggest putting this in navi10_get_unique_id() in navi10_ppt.c
for consistency since we query this from the SMU on most other asics.

Alex



>                 break;
>         case CHIP_SIENNA_CICHLID:
>                 adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h
> index 515d67bf249f..520ac2b98744 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> @@ -26,6 +26,9 @@
>
>  #include "nbio_v2_3.h"
>
> +#define mmFUSE_DATA_729 (0x176D9)
> +#define mmFUSE_DATA_730 (0x176DA)
> +
>  void nv_grbm_select(struct amdgpu_device *adev,
>                     u32 me, u32 pipe, u32 queue, u32 vmid);
>  void nv_set_virt_ops(struct amdgpu_device *adev);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-18 14:12 ` Alex Deucher
@ 2021-05-18 20:37   ` Nieto, David M
  2021-05-19 10:58     ` Christian König
  0 siblings, 1 reply; 8+ messages in thread
From: Nieto, David M @ 2021-05-18 20:37 UTC (permalink / raw)
  To: Alex Deucher, Gu, JiaWei (Will); +Cc: Deng, Emily, amd-gfx list


[-- Attachment #1.1: Type: text/plain, Size: 12448 bytes --]

[AMD Official Use Only]

I think the sysfs node should be moved into amdgpu_pm instead of the amdgpu_device.c and generation of the unique_id should be moved to navi10_ppt.c, similarly to other chips.

Thinking it better, generating a random UUID makes no sense in the driver level, any application can do the same thing on userspace if the UUID sysfs node is empty.

So, I think we should do the same as with the unique_id node, if the unique_id is not present, just return.

David
________________________________
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Tuesday, May 18, 2021 7:12 AM
To: Gu, JiaWei (Will) <JiaWei.Gu@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Deng, Emily <Emily.Deng@amd.com>; Nieto, David M <David.Nieto@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID

On Mon, May 17, 2021 at 1:54 AM Jiawei Gu <Jiawei.Gu@amd.com> wrote:
>
> Introduce an RFC 4122 compliant UUID for the GPUs derived
> from the unique GPU serial number (from Vega10) on gpus.
> Where this serial number is not available, use a compliant
> random UUID.
>
> For virtualization, the unique ID is passed by the host driver
> in the PF2VF structure.
>
> Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
>  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
>  drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
>  drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
>  6 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3147c1c935c8..ad6d4b55be6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -802,6 +802,40 @@ struct amd_powerplay {
>                                           (rid == 0x01) || \
>                                           (rid == 0x10))))
>
> +union amdgpu_uuid_info {
> +       struct {
> +               union {
> +                       struct {
> +                               uint32_t did    : 16;
> +                               uint32_t fcn    : 8;
> +                               uint32_t asic_7 : 8;
> +                       };
> +                       uint32_t time_low;
> +               };
> +
> +               struct {
> +                       uint32_t time_mid  : 16;
> +                       uint32_t time_high : 12;
> +                       uint32_t version   : 4;
> +               };
> +
> +               struct {
> +                       struct {
> +                               uint8_t clk_seq_hi : 6;
> +                               uint8_t variant    : 2;
> +                       };
> +                       union {
> +                               uint8_t clk_seq_low;
> +                               uint8_t asic_6;
> +                       };
> +                       uint16_t asic_4;
> +               };
> +
> +               uint32_t asic_0;
> +       };
> +       char as_char[16];
> +};
> +
>  #define AMDGPU_RESET_MAGIC_NUM 64
>  #define AMDGPU_MAX_DF_PERFMONS 4
>  struct amdgpu_device {
> @@ -1074,6 +1108,8 @@ struct amdgpu_device {
>         char                            product_name[32];
>         char                            serial[20];
>
> +       union amdgpu_uuid_info uuid_info;
> +
>         struct amdgpu_autodump          autodump;
>
>         atomic_t                        throttling_logging_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7c6c435e5d02..079841e1cb52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -37,6 +37,7 @@
>  #include <linux/vgaarb.h>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/efi.h>
> +#include <linux/uuid.h>
>  #include "amdgpu.h"
>  #include "amdgpu_trace.h"
>  #include "amdgpu_i2c.h"
> @@ -3239,11 +3240,104 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>         return ret;
>  }
>
> +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info *uuid_info)
> +{
> +       return (uuid_info->time_low    == 0 &&
> +                       uuid_info->time_mid    == 0 &&
> +                       uuid_info->time_high   == 0 &&
> +                       uuid_info->version     == 0 &&
> +                       uuid_info->clk_seq_hi  == 0 &&
> +                       uuid_info->variant     == 0 &&
> +                       uuid_info->clk_seq_low == 0 &&
> +                       uuid_info->asic_4      == 0 &&
> +                       uuid_info->asic_0      == 0);
> +}
> +
> +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> +                               uint64_t serial, uint16_t did, uint8_t idx)
> +{
> +       uint16_t clk_seq = 0;
> +
> +       /* Step1: insert clk_seq */
> +       uuid_info->clk_seq_low = (uint8_t)clk_seq;
> +       uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> +
> +       /* Step2: insert did */
> +       uuid_info->did = did;
> +
> +       /* Step3: insert vf idx */
> +       uuid_info->fcn = idx;
> +
> +       /* Step4: insert serial */
> +       uuid_info->asic_0 = (uint32_t)serial;
> +       uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> +       uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> +       uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> +
> +       /* Step5: insert version */
> +       uuid_info->version = 1;
> +       /* Step6: insert variant */
> +       uuid_info->variant = 2;
> +}
> +
> +/* byte reverse random uuid */
> +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> +{
> +       char b0, b1;
> +       int i;
> +
> +       generate_random_uuid(uuid_info->as_char);
> +       for (i = 0; i < 8; i++) {
> +               b0 = uuid_info->as_char[i];
> +               b1 = uuid_info->as_char[16-i];
> +               uuid_info->as_char[16-i] = b0;
> +               uuid_info->as_char[i] = b1;
> +       }
> +}
> +
> +/**
> + *
> + * The amdgpu driver provides a sysfs API for providing uuid data.
> + * The file uuid_info is used for this, and returns string of amdgpu uuid.
> + */
> +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     char *buf)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(ddev);//ddev->dev_private;
> +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
> +
> +       return sysfs_emit(buf,
> +                                       "%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> +                                       uuid->time_low,
> +                                       uuid->time_mid,
> +                                       uuid->version,
> +                                       uuid->time_high,
> +                                       uuid->clk_seq_hi |
> +                                       uuid->variant << 6,
> +                                       uuid->clk_seq_low,
> +                                       uuid->asic_4,
> +                                       uuid->asic_0);
> +}
> +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> +
> +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> +{
> +       if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> +               if (adev->unique_id)
> +                       amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, adev->pdev->device, 31);
> +               else
> +                       amdgpu_gen_uuid_random(&adev->uuid_info);
> +       }
> +}
> +
>  static const struct attribute *amdgpu_dev_attributes[] = {
>         &dev_attr_product_name.attr,
>         &dev_attr_product_number.attr,
>         &dev_attr_serial_number.attr,
>         &dev_attr_pcie_replay_count.attr,
> +       &dev_attr_uuid_info.attr,
>         NULL
>  };
>
> @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
>         amdgpu_fbdev_init(adev);
>
> +       amdgpu_uuid_init(adev);
> +
>         r = amdgpu_pm_sysfs_init(adev);
>         if (r) {
>                 adev->pm_sysfs_en = false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index b71dd1deeb2d..2dfebfe38079 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
>  static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>  {
>         struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = adev->virt.fw_reserve.p_pf2vf;
> +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
>         uint32_t checksum;
>         uint32_t checkval;
>
> @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>
>                 adev->unique_id =
>                         ((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid;
> +
> +               memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid_info_reserved,
> +                       sizeof(union amdgpu_uuid_info));
>                 break;
>         default:
>                 DRM_ERROR("invalid pf2vf version\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> index a434c71fde8e..0d1d36e82aeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
>                 uint32_t encode_max_frame_pixels;
>         } mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
>         /* UUID info */
> -       struct amd_sriov_msg_uuid_info uuid_info;
> +       uint32_t uuid_info_reserved[4];
>         /* reserved */
> -       uint32_t reserved[256 - 47];
> +       uint32_t reserved[256-47];
>  };
>
>  struct amd_sriov_msg_vf2pf_info_header {
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 32c34470404c..16d4a480f4c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
>                 if (amdgpu_sriov_vf(adev))
>                         adev->rev_id = 0;
>                 adev->external_rev_id = adev->rev_id + 0xa;
> +               if (!amdgpu_sriov_vf(adev)) {
> +                       adev->unique_id = RREG32(mmFUSE_DATA_730);
> +                       adev->unique_id <<= 32;
> +                       adev->unique_id |= RREG32(mmFUSE_DATA_729);
> +               }

I would suggest putting this in navi10_get_unique_id() in navi10_ppt.c
for consistency since we query this from the SMU on most other asics.

Alex



>                 break;
>         case CHIP_SIENNA_CICHLID:
>                 adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h
> index 515d67bf249f..520ac2b98744 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> @@ -26,6 +26,9 @@
>
>  #include "nbio_v2_3.h"
>
> +#define mmFUSE_DATA_729 (0x176D9)
> +#define mmFUSE_DATA_730 (0x176DA)
> +
>  void nv_grbm_select(struct amdgpu_device *adev,
>                     u32 me, u32 pipe, u32 queue, u32 vmid);
>  void nv_set_virt_ops(struct amdgpu_device *adev);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cdavid.nieto%40amd.com%7Cb6a43b8c156c4a6964e208d91a070e84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637569439877514988%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7lpRnRgRwKASGUmfr3RChO0P6QfRbcpMFggQl6HO%2Bss%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 28907 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-18 20:37   ` Nieto, David M
@ 2021-05-19 10:58     ` Christian König
  2021-05-19 16:10       ` Nieto, David M
  0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2021-05-19 10:58 UTC (permalink / raw)
  To: Nieto, David M, Alex Deucher, Gu, JiaWei (Will); +Cc: Deng, Emily, amd-gfx list


[-- Attachment #1.1: Type: text/plain, Size: 13203 bytes --]

Well I don't think generating an UUID in the kernel makes sense in general.

What we can do is to expose the serial number of the device, so that 
userspace can create an UUID if necessary.

Christian.

Am 18.05.21 um 22:37 schrieb Nieto, David M:
>
> [AMD Official Use Only]
>
>
> I think the sysfs node should be moved into amdgpu_pm instead of the 
> amdgpu_device.c and generation of the unique_id should be moved to 
> navi10_ppt.c, similarly to other chips.
>
> Thinking it better, generating a random UUID makes no sense in the 
> driver level, any application can do the same thing on userspace if 
> the UUID sysfs node is empty.
>
> So, I think we should do the same as with the unique_id node, if the 
> unique_id is not present, just return.
>
> David
> ------------------------------------------------------------------------
> *From:* Alex Deucher <alexdeucher@gmail.com>
> *Sent:* Tuesday, May 18, 2021 7:12 AM
> *To:* Gu, JiaWei (Will) <JiaWei.Gu@amd.com>
> *Cc:* amd-gfx list <amd-gfx@lists.freedesktop.org>; Deng, Emily 
> <Emily.Deng@amd.com>; Nieto, David M <David.Nieto@amd.com>
> *Subject:* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
> On Mon, May 17, 2021 at 1:54 AM Jiawei Gu <Jiawei.Gu@amd.com> wrote:
> >
> > Introduce an RFC 4122 compliant UUID for the GPUs derived
> > from the unique GPU serial number (from Vega10) on gpus.
> > Where this serial number is not available, use a compliant
> > random UUID.
> >
> > For virtualization, the unique ID is passed by the host driver
> > in the PF2VF structure.
> >
> > Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
> >  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
> >  drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
> >  6 files changed, 146 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 3147c1c935c8..ad6d4b55be6c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -802,6 +802,40 @@ struct amd_powerplay {
> >                                           (rid == 0x01) || \
> >                                           (rid == 0x10))))
> >
> > +union amdgpu_uuid_info {
> > +       struct {
> > +               union {
> > +                       struct {
> > +                               uint32_t did    : 16;
> > +                               uint32_t fcn    : 8;
> > +                               uint32_t asic_7 : 8;
> > +                       };
> > +                       uint32_t time_low;
> > +               };
> > +
> > +               struct {
> > +                       uint32_t time_mid  : 16;
> > +                       uint32_t time_high : 12;
> > +                       uint32_t version   : 4;
> > +               };
> > +
> > +               struct {
> > +                       struct {
> > +                               uint8_t clk_seq_hi : 6;
> > +                               uint8_t variant : 2;
> > +                       };
> > +                       union {
> > +                               uint8_t clk_seq_low;
> > +                               uint8_t asic_6;
> > +                       };
> > +                       uint16_t asic_4;
> > +               };
> > +
> > +               uint32_t asic_0;
> > +       };
> > +       char as_char[16];
> > +};
> > +
> >  #define AMDGPU_RESET_MAGIC_NUM 64
> >  #define AMDGPU_MAX_DF_PERFMONS 4
> >  struct amdgpu_device {
> > @@ -1074,6 +1108,8 @@ struct amdgpu_device {
> >         char product_name[32];
> >         char                            serial[20];
> >
> > +       union amdgpu_uuid_info uuid_info;
> > +
> >         struct amdgpu_autodump          autodump;
> >
> >         atomic_t throttling_logging_enabled;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 7c6c435e5d02..079841e1cb52 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -37,6 +37,7 @@
> >  #include <linux/vgaarb.h>
> >  #include <linux/vga_switcheroo.h>
> >  #include <linux/efi.h>
> > +#include <linux/uuid.h>
> >  #include "amdgpu.h"
> >  #include "amdgpu_trace.h"
> >  #include "amdgpu_i2c.h"
> > @@ -3239,11 +3240,104 @@ static int 
> amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
> >         return ret;
> >  }
> >
> > +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info 
> *uuid_info)
> > +{
> > +       return (uuid_info->time_low    == 0 &&
> > +                       uuid_info->time_mid == 0 &&
> > +                       uuid_info->time_high == 0 &&
> > +                       uuid_info->version == 0 &&
> > +                       uuid_info->clk_seq_hi == 0 &&
> > +                       uuid_info->variant == 0 &&
> > +                       uuid_info->clk_seq_low == 0 &&
> > +                       uuid_info->asic_4 == 0 &&
> > +                       uuid_info->asic_0 == 0);
> > +}
> > +
> > +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> > +                               uint64_t serial, uint16_t did, 
> uint8_t idx)
> > +{
> > +       uint16_t clk_seq = 0;
> > +
> > +       /* Step1: insert clk_seq */
> > +       uuid_info->clk_seq_low = (uint8_t)clk_seq;
> > +       uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> > +
> > +       /* Step2: insert did */
> > +       uuid_info->did = did;
> > +
> > +       /* Step3: insert vf idx */
> > +       uuid_info->fcn = idx;
> > +
> > +       /* Step4: insert serial */
> > +       uuid_info->asic_0 = (uint32_t)serial;
> > +       uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> > +       uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> > +       uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> > +
> > +       /* Step5: insert version */
> > +       uuid_info->version = 1;
> > +       /* Step6: insert variant */
> > +       uuid_info->variant = 2;
> > +}
> > +
> > +/* byte reverse random uuid */
> > +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> > +{
> > +       char b0, b1;
> > +       int i;
> > +
> > + generate_random_uuid(uuid_info->as_char);
> > +       for (i = 0; i < 8; i++) {
> > +               b0 = uuid_info->as_char[i];
> > +               b1 = uuid_info->as_char[16-i];
> > +               uuid_info->as_char[16-i] = b0;
> > +               uuid_info->as_char[i] = b1;
> > +       }
> > +}
> > +
> > +/**
> > + *
> > + * The amdgpu driver provides a sysfs API for providing uuid data.
> > + * The file uuid_info is used for this, and returns string of 
> amdgpu uuid.
> > + */
> > +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> > +                                     struct device_attribute *attr,
> > +                                     char *buf)
> > +{
> > +       struct drm_device *ddev = dev_get_drvdata(dev);
> > +       struct amdgpu_device *adev = 
> drm_to_adev(ddev);//ddev->dev_private;
> > +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
> > +
> > +       return sysfs_emit(buf,
> > + "%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> > + uuid->time_low,
> > + uuid->time_mid,
> > + uuid->version,
> > + uuid->time_high,
> > + uuid->clk_seq_hi |
> > + uuid->variant << 6,
> > + uuid->clk_seq_low,
> > + uuid->asic_4,
> > + uuid->asic_0);
> > +}
> > +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> > +
> > +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> > +{
> > +       if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> > +               if (adev->unique_id)
> > + amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, 
> adev->pdev->device, 31);
> > +               else
> > + amdgpu_gen_uuid_random(&adev->uuid_info);
> > +       }
> > +}
> > +
> >  static const struct attribute *amdgpu_dev_attributes[] = {
> >         &dev_attr_product_name.attr,
> >         &dev_attr_product_number.attr,
> >         &dev_attr_serial_number.attr,
> >         &dev_attr_pcie_replay_count.attr,
> > +       &dev_attr_uuid_info.attr,
> >         NULL
> >  };
> >
> > @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >
> >         amdgpu_fbdev_init(adev);
> >
> > +       amdgpu_uuid_init(adev);
> > +
> >         r = amdgpu_pm_sysfs_init(adev);
> >         if (r) {
> >                 adev->pm_sysfs_en = false;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index b71dd1deeb2d..2dfebfe38079 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct 
> amdgpu_device *adev,
> >  static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
> >  {
> >         struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = 
> adev->virt.fw_reserve.p_pf2vf;
> > +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
> >         uint32_t checksum;
> >         uint32_t checkval;
> >
> > @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct 
> amdgpu_device *adev)
> >
> >                 adev->unique_id =
> >                         ((struct amd_sriov_msg_pf2vf_info 
> *)pf2vf_info)->uuid;
> > +
> > +               memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info 
> *)pf2vf_info)->uuid_info_reserved,
> > +                       sizeof(union amdgpu_uuid_info));
> >                 break;
> >         default:
> >                 DRM_ERROR("invalid pf2vf version\n");
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> > index a434c71fde8e..0d1d36e82aeb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> > @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
> >                 uint32_t encode_max_frame_pixels;
> >         } mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
> >         /* UUID info */
> > -       struct amd_sriov_msg_uuid_info uuid_info;
> > +       uint32_t uuid_info_reserved[4];
> >         /* reserved */
> > -       uint32_t reserved[256 - 47];
> > +       uint32_t reserved[256-47];
> >  };
> >
> >  struct amd_sriov_msg_vf2pf_info_header {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
> b/drivers/gpu/drm/amd/amdgpu/nv.c
> > index 32c34470404c..16d4a480f4c0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
> >                 if (amdgpu_sriov_vf(adev))
> >                         adev->rev_id = 0;
> >                 adev->external_rev_id = adev->rev_id + 0xa;
> > +               if (!amdgpu_sriov_vf(adev)) {
> > +                       adev->unique_id = RREG32(mmFUSE_DATA_730);
> > +                       adev->unique_id <<= 32;
> > +                       adev->unique_id |= RREG32(mmFUSE_DATA_729);
> > +               }
>
> I would suggest putting this in navi10_get_unique_id() in navi10_ppt.c
> for consistency since we query this from the SMU on most other asics.
>
> Alex
>
>
>
> >                 break;
> >         case CHIP_SIENNA_CICHLID:
> >                 adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h 
> b/drivers/gpu/drm/amd/amdgpu/nv.h
> > index 515d67bf249f..520ac2b98744 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> > @@ -26,6 +26,9 @@
> >
> >  #include "nbio_v2_3.h"
> >
> > +#define mmFUSE_DATA_729 (0x176D9)
> > +#define mmFUSE_DATA_730 (0x176DA)
> > +
> >  void nv_grbm_select(struct amdgpu_device *adev,
> >                     u32 me, u32 pipe, u32 queue, u32 vmid);
> >  void nv_set_virt_ops(struct amdgpu_device *adev);
> > --
> > 2.17.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cdavid.nieto%40amd.com%7Cb6a43b8c156c4a6964e208d91a070e84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637569439877514988%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7lpRnRgRwKASGUmfr3RChO0P6QfRbcpMFggQl6HO%2Bss%3D&amp;reserved=0 
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cdavid.nieto%40amd.com%7Cb6a43b8c156c4a6964e208d91a070e84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637569439877514988%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7lpRnRgRwKASGUmfr3RChO0P6QfRbcpMFggQl6HO%2Bss%3D&amp;reserved=0>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 25005 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID
  2021-05-19 10:58     ` Christian König
@ 2021-05-19 16:10       ` Nieto, David M
  0 siblings, 0 replies; 8+ messages in thread
From: Nieto, David M @ 2021-05-19 16:10 UTC (permalink / raw)
  To: Christian König, Alex Deucher, Gu, JiaWei (Will)
  Cc: Deng, Emily, amd-gfx list


[-- Attachment #1.1: Type: text/plain, Size: 14632 bytes --]

[AMD Official Use Only]

For the case of virtualization, for example, the serial number has no relation to the uuid. Which means that at least for virtualization the node needs to be created. This may also be the case on other gpus.

________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Wednesday, May 19, 2021 3:58:35 AM
To: Nieto, David M <David.Nieto@amd.com>; Alex Deucher <alexdeucher@gmail.com>; Gu, JiaWei (Will) <JiaWei.Gu@amd.com>
Cc: Deng, Emily <Emily.Deng@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID

Well I don't think generating an UUID in the kernel makes sense in general.

What we can do is to expose the serial number of the device, so that userspace can create an UUID if necessary.

Christian.

Am 18.05.21 um 22:37 schrieb Nieto, David M:

[AMD Official Use Only]

I think the sysfs node should be moved into amdgpu_pm instead of the amdgpu_device.c and generation of the unique_id should be moved to navi10_ppt.c, similarly to other chips.

Thinking it better, generating a random UUID makes no sense in the driver level, any application can do the same thing on userspace if the UUID sysfs node is empty.

So, I think we should do the same as with the unique_id node, if the unique_id is not present, just return.

David
________________________________
From: Alex Deucher <alexdeucher@gmail.com><mailto:alexdeucher@gmail.com>
Sent: Tuesday, May 18, 2021 7:12 AM
To: Gu, JiaWei (Will) <JiaWei.Gu@amd.com><mailto:JiaWei.Gu@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>; Deng, Emily <Emily.Deng@amd.com><mailto:Emily.Deng@amd.com>; Nieto, David M <David.Nieto@amd.com><mailto:David.Nieto@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID

On Mon, May 17, 2021 at 1:54 AM Jiawei Gu <Jiawei.Gu@amd.com><mailto:Jiawei.Gu@amd.com> wrote:
>
> Introduce an RFC 4122 compliant UUID for the GPUs derived
> from the unique GPU serial number (from Vega10) on gpus.
> Where this serial number is not available, use a compliant
> random UUID.
>
> For virtualization, the unique ID is passed by the host driver
> in the PF2VF structure.
>
> Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com><mailto:Jiawei.Gu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h         | 36 ++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 96 +++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c    |  4 +
>  drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h |  4 +-
>  drivers/gpu/drm/amd/amdgpu/nv.c             |  5 ++
>  drivers/gpu/drm/amd/amdgpu/nv.h             |  3 +
>  6 files changed, 146 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3147c1c935c8..ad6d4b55be6c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -802,6 +802,40 @@ struct amd_powerplay {
>                                           (rid == 0x01) || \
>                                           (rid == 0x10))))
>
> +union amdgpu_uuid_info {
> +       struct {
> +               union {
> +                       struct {
> +                               uint32_t did    : 16;
> +                               uint32_t fcn    : 8;
> +                               uint32_t asic_7 : 8;
> +                       };
> +                       uint32_t time_low;
> +               };
> +
> +               struct {
> +                       uint32_t time_mid  : 16;
> +                       uint32_t time_high : 12;
> +                       uint32_t version   : 4;
> +               };
> +
> +               struct {
> +                       struct {
> +                               uint8_t clk_seq_hi : 6;
> +                               uint8_t variant    : 2;
> +                       };
> +                       union {
> +                               uint8_t clk_seq_low;
> +                               uint8_t asic_6;
> +                       };
> +                       uint16_t asic_4;
> +               };
> +
> +               uint32_t asic_0;
> +       };
> +       char as_char[16];
> +};
> +
>  #define AMDGPU_RESET_MAGIC_NUM 64
>  #define AMDGPU_MAX_DF_PERFMONS 4
>  struct amdgpu_device {
> @@ -1074,6 +1108,8 @@ struct amdgpu_device {
>         char                            product_name[32];
>         char                            serial[20];
>
> +       union amdgpu_uuid_info uuid_info;
> +
>         struct amdgpu_autodump          autodump;
>
>         atomic_t                        throttling_logging_enabled;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7c6c435e5d02..079841e1cb52 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -37,6 +37,7 @@
>  #include <linux/vgaarb.h>
>  #include <linux/vga_switcheroo.h>
>  #include <linux/efi.h>
> +#include <linux/uuid.h>
>  #include "amdgpu.h"
>  #include "amdgpu_trace.h"
>  #include "amdgpu_i2c.h"
> @@ -3239,11 +3240,104 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>         return ret;
>  }
>
> +static bool amdgpu_is_uuid_info_empty(union amdgpu_uuid_info *uuid_info)
> +{
> +       return (uuid_info->time_low    == 0 &&
> +                       uuid_info->time_mid    == 0 &&
> +                       uuid_info->time_high   == 0 &&
> +                       uuid_info->version     == 0 &&
> +                       uuid_info->clk_seq_hi  == 0 &&
> +                       uuid_info->variant     == 0 &&
> +                       uuid_info->clk_seq_low == 0 &&
> +                       uuid_info->asic_4      == 0 &&
> +                       uuid_info->asic_0      == 0);
> +}
> +
> +static void amdgpu_gen_uuid_info(union amdgpu_uuid_info *uuid_info,
> +                               uint64_t serial, uint16_t did, uint8_t idx)
> +{
> +       uint16_t clk_seq = 0;
> +
> +       /* Step1: insert clk_seq */
> +       uuid_info->clk_seq_low = (uint8_t)clk_seq;
> +       uuid_info->clk_seq_hi  = (uint8_t)(clk_seq >> 8) & 0x3F;
> +
> +       /* Step2: insert did */
> +       uuid_info->did = did;
> +
> +       /* Step3: insert vf idx */
> +       uuid_info->fcn = idx;
> +
> +       /* Step4: insert serial */
> +       uuid_info->asic_0 = (uint32_t)serial;
> +       uuid_info->asic_4 = (uint16_t)(serial >> 4 * 8) & 0xFFFF;
> +       uuid_info->asic_6 = (uint8_t)(serial >> 6 * 8) & 0xFF;
> +       uuid_info->asic_7 = (uint8_t)(serial >> 7 * 8) & 0xFF;
> +
> +       /* Step5: insert version */
> +       uuid_info->version = 1;
> +       /* Step6: insert variant */
> +       uuid_info->variant = 2;
> +}
> +
> +/* byte reverse random uuid */
> +static void amdgpu_gen_uuid_random(union amdgpu_uuid_info *uuid_info)
> +{
> +       char b0, b1;
> +       int i;
> +
> +       generate_random_uuid(uuid_info->as_char);
> +       for (i = 0; i < 8; i++) {
> +               b0 = uuid_info->as_char[i];
> +               b1 = uuid_info->as_char[16-i];
> +               uuid_info->as_char[16-i] = b0;
> +               uuid_info->as_char[i] = b1;
> +       }
> +}
> +
> +/**
> + *
> + * The amdgpu driver provides a sysfs API for providing uuid data.
> + * The file uuid_info is used for this, and returns string of amdgpu uuid.
> + */
> +static ssize_t amdgpu_get_uuid_info(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     char *buf)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = drm_to_adev(ddev);//ddev->dev_private;
> +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
> +
> +       return sysfs_emit(buf,
> +                                       "%08x-%04x-%x%03x-%02x%02x-%04x%08x\n",
> +                                       uuid->time_low,
> +                                       uuid->time_mid,
> +                                       uuid->version,
> +                                       uuid->time_high,
> +                                       uuid->clk_seq_hi |
> +                                       uuid->variant << 6,
> +                                       uuid->clk_seq_low,
> +                                       uuid->asic_4,
> +                                       uuid->asic_0);
> +}
> +static DEVICE_ATTR(uuid_info, S_IRUGO, amdgpu_get_uuid_info, NULL);
> +
> +static void amdgpu_uuid_init(struct amdgpu_device *adev)
> +{
> +       if (amdgpu_is_uuid_info_empty(&adev->uuid_info)) {
> +               if (adev->unique_id)
> +                       amdgpu_gen_uuid_info(&adev->uuid_info, adev->unique_id, adev->pdev->device, 31);
> +               else
> +                       amdgpu_gen_uuid_random(&adev->uuid_info);
> +       }
> +}
> +
>  static const struct attribute *amdgpu_dev_attributes[] = {
>         &dev_attr_product_name.attr,
>         &dev_attr_product_number.attr,
>         &dev_attr_serial_number.attr,
>         &dev_attr_pcie_replay_count.attr,
> +       &dev_attr_uuid_info.attr,
>         NULL
>  };
>
> @@ -3551,6 +3645,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>
>         amdgpu_fbdev_init(adev);
>
> +       amdgpu_uuid_init(adev);
> +
>         r = amdgpu_pm_sysfs_init(adev);
>         if (r) {
>                 adev->pm_sysfs_en = false;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index b71dd1deeb2d..2dfebfe38079 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -429,6 +429,7 @@ static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
>  static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>  {
>         struct amd_sriov_msg_pf2vf_info_header *pf2vf_info = adev->virt.fw_reserve.p_pf2vf;
> +       union amdgpu_uuid_info *uuid = &adev->uuid_info;
>         uint32_t checksum;
>         uint32_t checkval;
>
> @@ -498,6 +499,9 @@ static int amdgpu_virt_read_pf2vf_data(struct amdgpu_device *adev)
>
>                 adev->unique_id =
>                         ((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid;
> +
> +               memcpy(uuid, &((struct amd_sriov_msg_pf2vf_info *)pf2vf_info)->uuid_info_reserved,
> +                       sizeof(union amdgpu_uuid_info));
>                 break;
>         default:
>                 DRM_ERROR("invalid pf2vf version\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> index a434c71fde8e..0d1d36e82aeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
> @@ -203,9 +203,9 @@ struct amd_sriov_msg_pf2vf_info {
>                 uint32_t encode_max_frame_pixels;
>         } mm_bw_management[AMD_SRIOV_MSG_RESERVE_VCN_INST];
>         /* UUID info */
> -       struct amd_sriov_msg_uuid_info uuid_info;
> +       uint32_t uuid_info_reserved[4];
>         /* reserved */
> -       uint32_t reserved[256 - 47];
> +       uint32_t reserved[256-47];
>  };
>
>  struct amd_sriov_msg_vf2pf_info_header {
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 32c34470404c..16d4a480f4c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -1167,6 +1167,11 @@ static int nv_common_early_init(void *handle)
>                 if (amdgpu_sriov_vf(adev))
>                         adev->rev_id = 0;
>                 adev->external_rev_id = adev->rev_id + 0xa;
> +               if (!amdgpu_sriov_vf(adev)) {
> +                       adev->unique_id = RREG32(mmFUSE_DATA_730);
> +                       adev->unique_id <<= 32;
> +                       adev->unique_id |= RREG32(mmFUSE_DATA_729);
> +               }

I would suggest putting this in navi10_get_unique_id() in navi10_ppt.c
for consistency since we query this from the SMU on most other asics.

Alex



>                 break;
>         case CHIP_SIENNA_CICHLID:
>                 adev->cg_flags = AMD_CG_SUPPORT_GFX_MGCG |
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h
> index 515d67bf249f..520ac2b98744 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.h
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.h
> @@ -26,6 +26,9 @@
>
>  #include "nbio_v2_3.h"
>
> +#define mmFUSE_DATA_729 (0x176D9)
> +#define mmFUSE_DATA_730 (0x176DA)
> +
>  void nv_grbm_select(struct amdgpu_device *adev,
>                     u32 me, u32 pipe, u32 queue, u32 vmid);
>  void nv_set_virt_ops(struct amdgpu_device *adev);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cdavid.nieto%40amd.com%7Cb6a43b8c156c4a6964e208d91a070e84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637569439877514988%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7lpRnRgRwKASGUmfr3RChO0P6QfRbcpMFggQl6HO%2Bss%3D&amp;reserved=0<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CDavid.Nieto%40amd.com%7C43a11e657f9b44afb21608d91ab50e02%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637570187200925847%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=n1gNKk7klknJ8RNwi8BV%2FiriWegB8RPGa2iBjcnU%2BVM%3D&reserved=0>



_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CDavid.Nieto%40amd.com%7C43a11e657f9b44afb21608d91ab50e02%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637570187200935841%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=YT88Ow13cLBgyzhbzCHgYhnLE5mWaK5GkN60o%2Fpm69o%3D&reserved=0>



[-- Attachment #1.2: Type: text/html, Size: 32301 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-05-19 16:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17  5:54 [PATCH] drm/amdgpu: Expose rfc4122 compliant UUID Jiawei Gu
2021-05-17  6:52 ` Christian König
2021-05-17 20:08   ` Nieto, David M
2021-05-18 13:03     ` Christian König
2021-05-18 14:12 ` Alex Deucher
2021-05-18 20:37   ` Nieto, David M
2021-05-19 10:58     ` Christian König
2021-05-19 16:10       ` Nieto, David M

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.