linux-kernel - Re: [PATCH v2 3/3] crypto: hisilicon/qm - defining the device isolation strategy

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:   Thu, 16 Jun 2022 14:45:47 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     "yekai(A)" <yekai13@...wei.com>
CC:     Kai Ye via Linux-accelerators 
        <linux-accelerators@...ts.ozlabs.org>,
        <gregkh@...uxfoundation.org>, <herbert@...dor.apana.org.au>,
        <linuxarm@...wei.com>, <linux-kernel@...r.kernel.org>,
        <wangzhou1@...ilicon.com>, <linux-crypto@...r.kernel.org>,
        <zhangfei.gao@...aro.org>
Subject: Re: [PATCH v2 3/3] crypto: hisilicon/qm - defining the device
 isolation strategy

...

> >  
> >> +	hw_err->tick_stamp = jiffies;
> >> +	list_for_each_entry_safe(err, tmp, &qm->uacce_hw_errs, list) {  
> >
> > These are ordered (I think). Could take advantage of that by
> > maintaining count of elements in parallel to the list then walking
> > list in right direction + stop when you reach last one to need
> > deleting.
> >  
> 
> thanks, The current list + jiffies solution seems more simple.

If list always remains relatively short then that's probably fine.

> >  
> >> +		if ((hw_err->tick_stamp - err->tick_stamp) / HZ >
> >> +		    SECONDS_PER_HOUR) {
> >> +			list_del(&err->list);
> >> +			kfree(err);
> >> +		} else {
> >> +			count++;
> >> +		}
> >> +	}
> >> +	list_add(&hw_err->list, &qm->uacce_hw_errs);
> >> +
> >> +	if (count >= isolate->hw_err_isolate_hz)
> >> +		atomic_set(&isolate->is_isolate, 1);
> >> +
> >> +	return 0;
> >> +}
> >> +  
> >
> > ...
> >  
> >> +static int hisi_qm_isolate_strategy_write(struct uacce_device *uacce,
> >> +						const char *buf)
> >> +{
> >> +	struct hisi_qm *qm = uacce->priv;
> >> +	unsigned long val = 0;
> >> +
> >> +#define MAX_ISOLATE_STRATEGY	65535
> >> +
> >> +	if (atomic_read(&qm->uacce_ref))
> >> +		return -EBUSY;
> >> +
> >> +	/* must be set by PF */
> >> +	if (atomic_read(&qm->isolate_data.is_isolate) || uacce->is_vf)  
> >
> > Why is the file visible on the vf?  Hide it or don't register it for vfs.  
> Because VF devices can be registered with UACCE. So this file node can 
> be visited on the vf. We're not sure if someone else's device is the 
> same as qm. So i configure it this way by driver. the 'isolate_strategy' 
> must be set by pf.
> 

If possible have the uacce registration from the driver provide information
on whether this applies to the VF.  Much better to have no file presented
by the VF than one that always returns an error code.

Jonathan