<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> </head> <body> 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. <div class="moz-cite-prefix">Am 18.05.21 um 22:37 schrieb Nieto, David M: </div> <blockquote type="cite" cite="mid:BYAPR12MB2840D36066EF94573527A8F4F42C9@BYAPR12MB2840.namprd12.prod.outlook.com"> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style> [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> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Alex Deucher <a class="moz-txt-link-rfc2396E" href="mailto:alexdeucher@gmail.com"><alexdeucher@gmail.com></a> Sent: Tuesday, May 18, 2021 7:12 AM To: Gu, JiaWei (Will) <a class="moz-txt-link-rfc2396E" href="mailto:JiaWei.Gu@amd.com"><JiaWei.Gu@amd.com></a> Cc: amd-gfx list <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a>; Deng, Emily <a class="moz-txt-link-rfc2396E" href="mailto:Emily.Deng@amd.com"><Emily.Deng@amd.com></a>; Nieto, David M <a class="moz-txt-link-rfc2396E" href="mailto:David.Nieto@amd.com"><David.Nieto@amd.com></a> 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 <a class="moz-txt-link-rfc2396E" href="mailto:Jiawei.Gu@amd.com"><Jiawei.Gu@amd.com></a> 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 <a class="moz-txt-link-rfc2396E" href="mailto:Jiawei.Gu@amd.com"><Jiawei.Gu@amd.com></a> > --- > 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 > <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> > <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" moz-do-not-send="true"> 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> <fieldset class="mimeAttachmentHeader"></fieldset> <pre class="moz-quote-pre" wrap="">_______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </body> </html>