On Tue, Sep 5, 2023 at 12:15 PM Francis, David <David.Francis@xxxxxxx> wrote: > > [AMD Official Use Only - General] > > > [AMD Official Use Only - General] > > Yeah we've had JIRAs (e.g. https://ontrack-internal.amd.com/browse/SWDEV-409711 ) raised that ASAN can't compile the thunk due to using = {0} . Using memset is definitely preferred to save trouble later. > > Kent > > This is kernel code, not thunk. {} and {0} are extensively used throughout > the kernel in general and our driver in particular, so I don't see this causing problems. Speaking for the kernel, we've seen problematic behavior when using {} vs {0} vs memset. memset always seems to do the right thing, the others don't. E.g., there was a series of patch sets to switch everything from {} to {0} or vice versa, we just traded one issue for another. For this patch, you can probably just drop that hunk as I don't see a need to change it. Whether you switch to memset or not is up to you. Alex > > David > > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Alex > Deucher > Sent: Tuesday, September 5, 2023 10:53 AM > To: Francis, David <David.Francis@xxxxxxx> > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm/amdgpu: Handle null atom context in VBIOS info ioctl > > On Tue, Sep 5, 2023 at 10:50 AM David Francis <David.Francis@xxxxxxx> wrote: > > On some APU systems, there is no atom context and so the > atom_context struct is null. > > Add a check to the VBIOS_INFO branch of amdgpu_info_ioctl > to handle this case, returning all zeroes. > > Signed-off-by: David Francis <David.Francis@xxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > index 3a48bec10aea..86748290ead7 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -947,16 +947,21 @@ int amdgpu_info_ioctl(struct drm_device *dev, void > > *data, struct drm_file *filp) > > ? -EFAULT : 0; > } > case AMDGPU_INFO_VBIOS_INFO: { > - struct drm_amdgpu_info_vbios vbios_info = {}; > + struct drm_amdgpu_info_vbios vbios_info = {0}; > > IIRC, these should be equivalent. That said, I believe memset is > generally preferred as not all compilers seem to handle these cases > correctly. > > Alex > > struct atom_context *atom_context; > > atom_context = adev->mode_info.atom_context; > - memcpy(vbios_info.name, atom_context->name, > > sizeof(atom_context->name)); > > - memcpy(vbios_info.vbios_pn, atom_context->vbios_pn, > > sizeof(atom_context->vbios_pn)); > > - vbios_info.version = atom_context->version; > - memcpy(vbios_info.vbios_ver_str, atom_context->vbios_ver_str, > - sizeof(atom_context->vbios_ver_str)); > - memcpy(vbios_info.date, atom_context->date, > > sizeof(atom_context->date)); > > + if (atom_context) { > + memcpy(vbios_info.name, atom_context->name, > + sizeof(atom_context->name)); > + memcpy(vbios_info.vbios_pn, atom_context->vbios_pn, > + sizeof(atom_context->vbios_pn)); > + vbios_info.version = atom_context->version; > + memcpy(vbios_info.vbios_ver_str, atom_context- > vbios_ver_str, > + sizeof(atom_context->vbios_ver_str)); > + memcpy(vbios_info.date, atom_context->date, > + sizeof(atom_context->date)); > + } > > return copy_to_user(out, &vbios_info, > min((size_t)size, sizeof(vbios_info))) ? -EFAULT : 0; > -- > 2.34.1 >
