lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 11 Jun 2021 11:10:16 +0100
From:   Steven Price <steven.price@....com>
To:     Chunyou Tang <tangchunyou@....com>
Cc:     tangchunyou <tangchunyou@....icubecorp.cn>,
        tomeu.vizoso@...labora.com, airlied@...ux.ie,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        alyssa.rosenzweig@...labora.com, tangchunyou@...becorp.cn
Subject: Re: [PATCH] modified: gpu/drm/panfrost/panfrost_gpu.c

On 10/06/2021 14:06, Chunyou Tang wrote:
> Hi Steven,

Hi Chunyou,

For some reason I'm not directly receiving your emails (only via the
list) - can you double check your email configuration?

>>> The GPU exception fault status register(0x3C),the low 8 bit is the
>>> EXCEPTION_TYPE.We can see the description at P3-78 in spec.
> 
> 	You can see the spec
> 	<arm_heimdall_technical_reference_manual_100612_0001_00_en.pdf>.

Thanks - please include that in the commit message - there are many TRMs
(one for each GPU) so without the information about exactly which
specification the page number is pretty useless. Sadly this
documentation isn't public which would be even better but I don't think
there are any public specs for this information.

>> However this change is correct - panfrost_exception_name() should be
>> taking only the lower 8 bits. Even better though would be to to report
>> the full raw fault information as well as the high bits can contain
>> useful information:
>>
>> 	dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at 0x%016llx\n",
>> 		 fault_status,
>> 		 panfrost_exception_name(pfdev, fault_status & 0xFF),
>> 		 address);
> 
> So I change it according to what you said?

Yes, please send a v2.

Thanks,

Steve

> 于 Thu, 10 Jun 2021 11:41:52 +0100
> Steven Price <steven.price@....com> 写道:
> 
>> The subject should have the prefix "drm/panfrost" and should mention
>> what the patch is changing (not just the filename).
>>
>> On 09/06/2021 07:38, ChunyouTang wrote:
>>> From: tangchunyou <tangchunyou@....icubecorp.cn>
>>>
>>> The GPU exception fault status register(0x3C),the low 8 bit is the
>>> EXCEPTION_TYPE.We can see the description at P3-78 in spec.
>>
>> Nit: When referring to a spec it's always good to mention the name -
>> I'm not sure which specification you found this in.
>>
>>>
>>> Signed-off-by: tangchunyou <tangchunyou@....icubecorp.cn>
>>> ---
>>>  drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
>>> b/drivers/gpu/drm/panfrost/panfrost_gpu.c index
>>> 2aae636f1cf5..1fffb6a0b24f 100644 ---
>>> a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++
>>> b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -33,7 +33,7 @@ static
>>> irqreturn_t panfrost_gpu_irq_handler(int irq, void *data) address
>>> |= gpu_read(pfdev, GPU_FAULT_ADDRESS_LO); 
>>>  		dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at
>>> 0x%016llx\n",
>>> -			 fault_status & 0xFF,
>>> panfrost_exception_name(pfdev, fault_status),
>>> +			 fault_status & 0xFF,
>>> panfrost_exception_name(pfdev, fault_status & 0xFF),
>>
>> However this change is correct - panfrost_exception_name() should be
>> taking only the lower 8 bits. Even better though would be to to report
>> the full raw fault information as well as the high bits can contain
>> useful information:
>>
>> 	dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at 0x%016llx\n",
>> 		 fault_status,
>> 		 panfrost_exception_name(pfdev, fault_status & 0xFF),
>> 		 address);
>>
>> Thanks,
>>
>> Steve
>>
>>>  			 address);
>>>  
>>>  		if (state & GPU_IRQ_MULTIPLE_FAULT)
>>>
> 
> 

Powered by blists - more mailing lists