[AMD Official Use Only - Internal Distribution Only]
Hi David,
The snippet of code we posted here is truncated.
Here's the complete current struct:
struct drm_amdgpu_info_vbios {
__u8 name[64];
__u32 dbdf;
__u8 vbios_pn[64];
__u32 version;
__u8 date[32];
__u8 serial[16];
__u32 dev_id;
__u32 rev_id;
__u32 sub_dev_id;
__u32 sub_ved_id;
};
We included vbios name already.
Best regards,
Jiawei
From: Nieto, David M <david.ni...@amd.com>
Sent: Tuesday, May 11, 2021 10:52 AM
To: Gu, JiaWei (Will) <jiawei...@amd.com>; Christian König
<ckoenig.leichtzumer...@gmail.com>
Cc: Alex Deucher <alexdeuc...@gmail.com>; Deng, Emily <emily.d...@amd.com>;
Kees Cook <keesc...@chromium.org>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
[AMD Official Use Only - Internal Distribution Only]
I agree that the serial number should be on number form, but I think we are
still missing one field, which is the vbios name, which is located after the
P/N, ASIC, PCI and memory type strings (skiping 0xD 0xA
David
________________________________
From: Gu, JiaWei (Will) <jiawei...@amd.com<mailto:jiawei...@amd.com>>
Sent: Monday, May 10, 2021 7:23 PM
To: Nieto, David M <david.ni...@amd.com<mailto:david.ni...@amd.com>>; Christian
König
<ckoenig.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Deng,
Emily <emily.d...@amd.com<mailto:emily.d...@amd.com>>; Kees Cook
<keesc...@chromium.org<mailto:keesc...@chromium.org>>; amd-gfx list
<amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
[AMD Official Use Only - Internal Distribution Only]
Got it. Let's keep them both.
Another idea about drm_amdgpu_info_vbios is
Does it make more sense to fill the "serial info" with uint64_t
adev->unique_id, instead of the current char[] in adev->serial?
Sorry about that I was not aware of adev->unique_id exists when I defined
drm_amdgpu_info_vbios.
I think it's clearer to use original numeric variable than a string to expose
serial.
How about that?
>> struct drm_amdgpu_info_vbios {
>> __u8 vbios_pn[64];
>> __u32 version;
>> __u8 date[32];
>> - __u8 serial[16];
>> + __u64 serial;
>> __u32 dev_id;
>> __u32 rev_id;
>> __u32 sub_dev_id;
>> --
Best regards,
Jiawei
From: Nieto, David M <david.ni...@amd.com<mailto:david.ni...@amd.com>>
Sent: Tuesday, May 11, 2021 4:20 AM
To: Christian König
<ckoenig.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>>;
Gu, JiaWei (Will) <jiawei...@amd.com<mailto:jiawei...@amd.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>; Deng,
Emily <emily.d...@amd.com<mailto:emily.d...@amd.com>>; Kees Cook
<keesc...@chromium.org<mailto:keesc...@chromium.org>>; amd-gfx list
<amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
One of the primary usecases is to add this information to the renderer string,
I am not sure if there are other cases of UMD drivers accessing sysfs nodes,
but I think if we think permissions, if a client is authenticated and opens the
render device then it can use the IOCTL, it is unclear to me we can make a such
an assumption for sysfs nodes...
I think there is value in having both tbh.
Regards,
David
From: Christian König
<ckoenig.leichtzumer...@gmail.com<mailto:ckoenig.leichtzumer...@gmail.com>>
Date: Monday, May 10, 2021 at 6:48 AM
To: "Nieto, David M" <david.ni...@amd.com<mailto:david.ni...@amd.com>>, "Gu,
JiaWei (Will)" <jiawei...@amd.com<mailto:jiawei...@amd.com>>
Cc: Alex Deucher <alexdeuc...@gmail.com<mailto:alexdeuc...@gmail.com>>, "Deng,
Emily" <emily.d...@amd.com<mailto:emily.d...@amd.com>>, Kees Cook
<keesc...@chromium.org<mailto:keesc...@chromium.org>>, amd-gfx list
<amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
Well we could add both as sysfs file(s).
Question here is rather what is the primary use case of this and if the
application has the necessary access permissions to the sysfs files?
Regards,
Christian.
Am 10.05.21 um 15:42 schrieb Nieto, David M:
Then the application would need to issue the ioctl and then open a sysfs file
to get all the information it needs. It makes little sense from a programming
perspective to add an incomplete interface in my opinion
________________________________
From: Gu, JiaWei (Will) <jiawei...@amd.com><mailto:jiawei...@amd.com>
Sent: Monday, May 10, 2021 12:13:07 AM
To: Nieto, David M <david.ni...@amd.com><mailto:david.ni...@amd.com>
Cc: Alex Deucher <alexdeuc...@gmail.com><mailto:alexdeuc...@gmail.com>; amd-gfx
list <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>;
Kees Cook <keesc...@chromium.org><mailto:keesc...@chromium.org>; Deng, Emily
<emily.d...@amd.com><mailto:emily.d...@amd.com>
Subject: RE: [PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
[AMD Official Use Only - Internal Distribution Only]
Hi David,
What I meant is to ONLY delete the serial[16] from drm_amdgpu_info_vbios, not
the whole struct.
struct drm_amdgpu_info_vbios {
__u8 name[64];
__u32 dbdf;
__u8 vbios_pn[64];
__u32 version;
__u8 date[32];
__u8 serial[16]; // jiawei: shall we delete this
__u32 dev_id;
__u32 rev_id;
__u32 sub_dev_id;
__u32 sub_ved_id;
};
serial[16] in drm_amdgpu_info_vbios copied from adev->serial, but there's
already a sysfs named serial_number, which exposes it already.
static ssize_t amdgpu_device_get_serial_number(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = ddev->dev_private;
return snprintf(buf, PAGE_SIZE, "%s\n", adev->serial);
}
Thanks,
Jiawei
-----Original Message-----
From: Nieto, David M <david.ni...@amd.com><mailto:david.ni...@amd.com>
Sent: Monday, May 10, 2021 2:53 PM
To: Gu, JiaWei (Will) <jiawei...@amd.com><mailto:jiawei...@amd.com>
Cc: Alex Deucher <alexdeuc...@gmail.com><mailto:alexdeuc...@gmail.com>; amd-gfx
list <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>;
Kees Cook <keesc...@chromium.org><mailto:keesc...@chromium.org>; Deng, Emily
<emily.d...@amd.com><mailto:emily.d...@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
No, this structure contains all the details of the vbios: date, serial number,
name, etc.
The sysfs node only contains the vbios name string
> On May 9, 2021, at 23:33, Gu, JiaWei (Will)
> <jiawei...@amd.com><mailto:jiawei...@amd.com> wrote:
>
> [AMD Official Use Only - Internal Distribution Only]
>
> With a second thought,
> __u8 serial[16] in drm_amdgpu_info_vbios is a bit redundant, sysfs
> serial_number already exposes it.
>
> Is it fine to abandon it from drm_amdgpu_info_vbios struct? @Alex
> Deucher @Nieto, David M
>
> Best regards,
> Jiawei
>
> -----Original Message-----
> From: Alex Deucher <alexdeuc...@gmail.com><mailto:alexdeuc...@gmail.com>
> Sent: Sunday, May 9, 2021 11:59 PM
> To: Gu, JiaWei (Will) <jiawei...@amd.com><mailto:jiawei...@amd.com>
> Cc: amd-gfx list
> <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>; Kees
> Cook
> <keesc...@chromium.org><mailto:keesc...@chromium.org>
> Subject: Re: [PATCH] drm/amdgpu: Align serial size in
> drm_amdgpu_info_vbios
>
>> On Sat, May 8, 2021 at 2:48 AM Jiawei Gu
>> <jiawei...@amd.com><mailto:jiawei...@amd.com> wrote:
>>
>> 20 should be serial char size now instead of 16.
>>
>> Signed-off-by: Jiawei Gu <jiawei...@amd.com><mailto:jiawei...@amd.com>
>
> Please make sure this keeps proper 64 bit alignment in the structure.
>
> Alex
>
>
>> ---
>> include/uapi/drm/amdgpu_drm.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h
>> b/include/uapi/drm/amdgpu_drm.h index 2b487a8d2727..1c20721f90da
>> 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -957,7 +957,7 @@ struct drm_amdgpu_info_vbios {
>> __u8 vbios_pn[64];
>> __u32 version;
>> __u8 date[32];
>> - __u8 serial[16];
>> + __u8 serial[20];
>> __u32 dev_id;
>> __u32 rev_id;
>> __u32 sub_dev_id;
>> --
>> 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%2Flis
>> t
>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CJ
>> i
>> awei.Gu%40amd.com%7Ccea31833184c41e8574508d9130360cc%7C3dd8961fe4884e
>> 6
>> 08e11a82d994e183d%7C0%7C0%7C637561727523880356%7CUnknown%7CTWFpbGZsb3
>> d
>> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
>> C
>> 1000&sdata=kAJiC6WoJUTeExwk6ftrLfMoY2OTAwg9X7mGgJT3kLk%3D&res
>> e
>> rved=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%7C3c0071a8a2a74127027408d913ba53e4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637562513264718308%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=g11aunC78b48N19Q%2FFXJBK%2B2Z0PZBWdqp%2FzhsPiqQ8Q%3D&reserved=0>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
