linux-kernel - Re: [PATCH] uacce: add print information if not enable sva

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]

Message-ID: <3f1590c4-b9f1-18f5-370a-57d9707f38a0@huawei.com>
Date:   Tue, 8 Jun 2021 09:39:52 +0800
From:   "yekai(A)" <yekai13@...wei.com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     <linux-accelerators@...ts.ozlabs.org>,
        <linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
        <zhangfei.gao@...aro.org>, <wangzhou1@...ilicon.com>
Subject: Re: [PATCH] uacce: add print information if not enable sva



On 2021/6/4 15:51, Greg KH wrote:
> On Fri, Jun 04, 2021 at 03:46:09PM +0800, Kai Ye wrote:
>> Add print information necessary if user not enable sva.
>>
>> Signed-off-by: Kai Ye <yekai13@...wei.com>
>> ---
>>  drivers/misc/uacce/uacce.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
>> index bae18ef0..fe38af8 100644
>> --- a/drivers/misc/uacce/uacce.c
>> +++ b/drivers/misc/uacce/uacce.c
>> @@ -387,15 +387,22 @@ static void uacce_release(struct device *dev)
>>
>>  static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags)
>>  {
>> +	int ret;
>> +
>>  	if (!(flags & UACCE_DEV_SVA))
>>  		return flags;
>>
>>  	flags &= ~UACCE_DEV_SVA;
>>
>> -	if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF))
>> +	ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF);
>> +	if (ret) {
>> +		dev_err(parent, "failed to enable IOPF feature! ret = %d\n", ret);
>
> Why is this needed?  Has this ever happened in real life such that the
> log message is now required?
>
>
>
>>  		return flags;
>> +	}
>>
>> -	if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA)) {
>> +	ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA);
>> +	if (ret) {
>> +		dev_err(parent, "failed to enable SVA feature! ret = %d\n", ret);
>
> Same here, does this happen in real systems?
>
> thanks,
>
> greg k-h
> .
>
In a other debug version, the SVA feature failed to be enabled, and no 
related information was printed. we don't know the cause of the problem. 
Finally, Although, it's not the problem here. but we find that the sva 
enable function doesn't have debug information. Therefore, it is 
inconvenient to locate the fault.
so i think the log message is required.

thanks

Kai