[PATCH] drm/amdgpu: Align serial size in drm_amdgpu_info_vbios
Christian König
ckoenig.leichtzumerken at gmail.com
Tue May 11 14:07:42 UTC 2021
Yeah, but umr is making strong use of sysfs as well.
The only justification of this interface would be if we want to use it
in Mesa.
And I agree with Marek that looks redundant with the device structure to
me as well.
Christian.
Am 11.05.21 um 16:04 schrieb Deucher, Alexander:
>
> [AMD Public Use]
>
>
> It's being used by umr and some other smi tools to provide vbios
> information for debugging.
>
> Alex
>
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of
> Marek Olšák <maraeo at gmail.com>
> *Sent:* Tuesday, May 11, 2021 4:18 AM
> *To:* Christian König <ckoenig.leichtzumerken at gmail.com>
> *Cc:* Kees Cook <keescook at chromium.org>; Gu, JiaWei (Will)
> <JiaWei.Gu at amd.com>; amd-gfx list <amd-gfx at lists.freedesktop.org>;
> Deng, Emily <Emily.Deng at amd.com>; Alex Deucher
> <alexdeucher at gmail.com>; Nieto, David M <David.Nieto at amd.com>
> *Subject:* Re: [PATCH] drm/amdgpu: Align serial size in
> drm_amdgpu_info_vbios
> Mesa doesn't use sysfs.
>
> Note that this is a uapi, meaning that once it's in the kernel, it
> can't be changed like that.
>
> What's the use case for this new interface? Isn't it partially
> redundant with the current device info structure, which seems to have
> the equivalent of dev_id and rev_id?
>
> Marek
>
> On Tue, May 11, 2021 at 3:51 AM Christian König
> <ckoenig.leichtzumerken at gmail.com
> <mailto:ckoenig.leichtzumerken at gmail.com>> wrote:
>
> Marek and other userspace folks need to decide that.
>
> Basic question here is if Mesa is already accessing sysfs nodes
> for OpenGL or RADV. If that is the case then we should probably
> expose the information there as well.
>
> If that isn't the case (which I think it is) then we should
> implement it as IOCTL.
>
> Regards,
> Christian.
>
> Am 10.05.21 um 22:19 schrieb Nieto, David M:
>>
>> 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.leichtzumerken at gmail.com>
>> <mailto:ckoenig.leichtzumerken at gmail.com>
>> *Date: *Monday, May 10, 2021 at 6:48 AM
>> *To: *"Nieto, David M" <David.Nieto at amd.com>
>> <mailto:David.Nieto at amd.com>, "Gu, JiaWei (Will)"
>> <JiaWei.Gu at amd.com> <mailto:JiaWei.Gu at amd.com>
>> *Cc: *Alex Deucher <alexdeucher at gmail.com>
>> <mailto:alexdeucher at gmail.com>, "Deng, Emily"
>> <Emily.Deng at amd.com> <mailto:Emily.Deng at amd.com>, Kees Cook
>> <keescook at chromium.org> <mailto:keescook at chromium.org>, amd-gfx
>> list <amd-gfx at lists.freedesktop.org>
>> <mailto:amd-gfx at 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.Gu at amd.com>
>> <mailto:JiaWei.Gu at amd.com>
>> *Sent:* Monday, May 10, 2021 12:13:07 AM
>> *To:* Nieto, David M <David.Nieto at amd.com>
>> <mailto:David.Nieto at amd.com>
>> *Cc:* Alex Deucher <alexdeucher at gmail.com>
>> <mailto:alexdeucher at gmail.com>; amd-gfx list
>> <amd-gfx at lists.freedesktop.org>
>> <mailto:amd-gfx at lists.freedesktop.org>; Kees Cook
>> <keescook at chromium.org> <mailto:keescook at chromium.org>; Deng,
>> Emily <Emily.Deng at amd.com> <mailto:Emily.Deng at 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.Nieto at amd.com>
>> <mailto:David.Nieto at amd.com>
>> Sent: Monday, May 10, 2021 2:53 PM
>> To: Gu, JiaWei (Will) <JiaWei.Gu at amd.com>
>> <mailto:JiaWei.Gu at amd.com>
>> Cc: Alex Deucher <alexdeucher at gmail.com>
>> <mailto:alexdeucher at gmail.com>; amd-gfx list
>> <amd-gfx at lists.freedesktop.org>
>> <mailto:amd-gfx at lists.freedesktop.org>; Kees Cook
>> <keescook at chromium.org> <mailto:keescook at chromium.org>; Deng,
>> Emily <Emily.Deng at amd.com> <mailto:Emily.Deng at 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.Gu at amd.com> <mailto:JiaWei.Gu at 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 <alexdeucher at gmail.com>
>> <mailto:alexdeucher at gmail.com>
>> > Sent: Sunday, May 9, 2021 11:59 PM
>> > To: Gu, JiaWei (Will) <JiaWei.Gu at amd.com>
>> <mailto:JiaWei.Gu at amd.com>
>> > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
>> <mailto:amd-gfx at lists.freedesktop.org>; Kees Cook
>> > <keescook at chromium.org> <mailto:keescook at 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.Gu at amd.com> <mailto:Jiawei.Gu at amd.com> wrote:
>> >>
>> >> 20 should be serial char size now instead of 16.
>> >>
>> >> Signed-off-by: Jiawei Gu <Jiawei.Gu at amd.com>
>> <mailto:Jiawei.Gu at 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 at lists.freedesktop.org
>> <mailto:amd-gfx at lists.freedesktop.org>
>> >>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis>
>> >> t
>> >> s.freedesktop.org
>> <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fs.freedesktop.org%2F&data=04%7C01%7Calexander.deucher%40amd.com%7C3503a51f2fa04376040b08d914558033%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563179729003008%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=5c730zpnFFOm3EgabDFoiYlsl2tsaxImaTlVfap%2BfHQ%3D&reserved=0>%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CJ
>> >> i
>> >> awei.Gu%40amd.com
>> <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2F40amd.com%2F&data=04%7C01%7Calexander.deucher%40amd.com%7C3503a51f2fa04376040b08d914558033%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563179729003008%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=FzFHfe095%2FDzOYR%2BZ3tAzlF2wDRYsD5kOt%2Bf37nNVP4%3D&reserved=0>%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 at lists.freedesktop.org <mailto:amd-gfx at 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%7Calexander.deucher%40amd.com%7C3503a51f2fa04376040b08d914558033%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563179729012969%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=GA4tfIXNrbU98WUMQl%2Bkd28DyNgqjlZcIAryQtVIn%2Bw%3D&reserved=0>
>>
>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at 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%7Calexander.deucher%40amd.com%7C3503a51f2fa04376040b08d914558033%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563179729012969%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=GA4tfIXNrbU98WUMQl%2Bkd28DyNgqjlZcIAryQtVIn%2Bw%3D&reserved=0>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210511/53f28dd6/attachment-0001.htm>
More information about the amd-gfx
mailing list