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 Dec 2020 11:34:26 +0000
From:   "Kelly, Seamus" <seamus.kelly@...el.com>
To:     Joe Perches <joe@...ches.com>,
        "mgross@...ux.intel.com" <mgross@...ux.intel.com>,
        "markgross@...nel.org" <markgross@...nel.org>,
        "arnd@...db.de" <arnd@...db.de>, "bp@...e.de" <bp@...e.de>,
        "damien.lemoal@....com" <damien.lemoal@....com>,
        "dragan.cvetic@...inx.com" <dragan.cvetic@...inx.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "corbet@....net" <corbet@....net>,
        "leonard.crestez@....com" <leonard.crestez@....com>,
        "palmerdabbelt@...gle.com" <palmerdabbelt@...gle.com>,
        "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
        "peng.fan@....com" <peng.fan@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 21/22] xlink-core: add async channel and events



-----Original Message-----
From: Joe Perches <joe@...ches.com> 
Sent: Monday, December 7, 2020 2:55 AM
To: mgross@...ux.intel.com; markgross@...nel.org; arnd@...db.de; bp@...e.de; damien.lemoal@....com; dragan.cvetic@...inx.com; gregkh@...uxfoundation.org; corbet@....net; leonard.crestez@....com; palmerdabbelt@...gle.com; paul.walmsley@...ive.com; peng.fan@....com; robh+dt@...nel.org; shawnguo@...nel.org
Cc: linux-kernel@...r.kernel.org; Kelly, Seamus <seamus.kelly@...el.com>
Subject: Re: [PATCH 21/22] xlink-core: add async channel and events

On Tue, 2020-12-01 at 14:35 -0800, mgross@...ux.intel.com wrote:
> Enable asynchronous channel and event communication.
[]
> diff --git a/drivers/misc/xlink-core/xlink-core.c 
> b/drivers/misc/xlink-core/xlink-core.c
[]
> +
> +// sysfs attribute functions
> +
> +static ssize_t event0_show(struct device *dev, struct 
> +device_attribute *attr, char *buf) {
> +	struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev);
> +	struct xlink_attr *a = &xlink_dev->eventx[0];
> +
> +	return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, 
> +a->value); }
> +
> +static ssize_t event1_show(struct device *dev, struct 
> +device_attribute *attr, char *buf) {
> +	struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev);
> +	struct xlink_attr *a = &xlink_dev->eventx[1];
> +
> +	return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, 
> +a->value); }
> +
> +static ssize_t event2_show(struct device *dev, struct 
> +device_attribute *attr, char *buf) {
> +	struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev);
> +	struct xlink_attr *a = &xlink_dev->eventx[2];
> +
> +	return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, 
> +a->value); }
> +
> +static ssize_t event3_show(struct device *dev, struct 
> +device_attribute *attr, char *buf) {
> +	struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev);
> +	struct xlink_attr *a = &xlink_dev->eventx[3];
> +
> +	return scnprintf(buf, PAGE_SIZE, "0x%x 0x%lx\n", a->sw_dev_id, 
> +a->value); }

These could use the sysfs_emit function and not scnprintf and as well could use a single shared function with an index.

Something like:

static ssize_t eventx_show(struct device *dev, struct device_attr *attr, int index, char *buf) {
	struct keembay_xlink_dev *xlink_dev = dev_get_drvdata(dev);
	struct xlink_attr *a = &xlink_dev->eventx[index];

	return sysfs_emit(buf, "0x%x 0x%lx\n", a->sw_dev_id, a->value); }

static ssize_t event0_show(struct device *dev, struct device_attribute *attr, char *buf) {
	return eventx_show(dev, attr, 0, buf);
}

etc...
[Kelly, Seamus]  Thank you!  will do.

> @@ -270,19 +353,23 @@ static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	struct xlinkconnect		con	= {0};
>  	struct xlinkrelease		rel	= {0};
>  	struct xlinkstartvpu		startvpu = {0};
> +	struct xlinkcallback		cb	= {0};
>  	struct xlinkgetdevicename	devn	= {0};
>  	struct xlinkgetdevicelist	devl	= {0};
>  	struct xlinkgetdevicestatus	devs	= {0};
>  	struct xlinkbootdevice		boot	= {0};
>  	struct xlinkresetdevice		res	= {0};
>  	struct xlinkdevmode		devm	= {0};
> +	struct xlinkregdevevent		regdevevent = {0};

Perhaps significantly less stack would be used if these declarations were moved into the case blocks where used.  Less initialization would be done per ioctl too.

[Kelly, Seamus]  This has changed significantly In next patch and I take your comments ( on next patch also ) Thanks

> @@ -318,6 +405,30 @@ static long xlink_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		if (copy_to_user((void __user *)op.return_code, (void *)&rc, sizeof(rc)))
>  			return -EFAULT;
>  		break;
> +	case XL_DATA_READY_CALLBACK:
> +		if (copy_from_user(&cb, (void __user *)arg,
> +				   sizeof(struct xlinkcallback)))
> +			return -EFAULT;
> +		if (copy_from_user(&devh, (void __user *)cb.handle,
> +				   sizeof(struct xlink_handle)))
> +			return -EFAULT;
> +		CHANNEL_SET_USER_BIT(cb.chan); // set MSbit for user space call
> +		rc = xlink_data_available_event(&devh, cb.chan, cb.callback);
> +		if (copy_to_user((void __user *)cb.return_code, (void *)&rc, sizeof(rc)))
> +			return -EFAULT;
> +		break;

	case XL_DATA_READY_CALLBACK: {
		struct xlinkcallback cb	= {};
		etc...
		break;
	}



--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ