<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [AMD Official Use Only] <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> 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.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> 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.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> So, I think we should do the same as with the unique_id node, if the unique_id is not present, just return.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> David</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">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 <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">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 > <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cdavid.nieto%40amd.com%7Cb6a43b8c156c4a6964e208d91a070e84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637569439877514988%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7lpRnRgRwKASGUmfr3RChO0P6QfRbcpMFggQl6HO%2Bss%3D&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%7Cb6a43b8c156c4a6964e208d91a070e84%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637569439877514988%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7lpRnRgRwKASGUmfr3RChO0P6QfRbcpMFggQl6HO%2Bss%3D&reserved=0</a> </div> </div> </div> </body> </html>