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);
>