| 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
| ||
|
Message-ID: <CAMpxmJX+1sSceOe5ZN9iViWYfede50KwzdDduVgRb=8VtxbC5Q@mail.gmail.com>
Date: Wed, 22 Sep 2021 11:53:54 +0200
From: Bartosz Golaszewski <bgolaszewski@...libre.com>
To: lakshmi.sowjanya.d@...el.com
Cc: Linus Walleij <linus.walleij@...aro.org>,
linux-gpio <linux-gpio@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, mgross@...ux.intel.com,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
tamal.saha@...el.com, bala.senthil@...el.com
Subject: Re: [RFC PATCH v1 11/20] gpio: Add event count interface to gpiolib
On Tue, Aug 24, 2021 at 6:48 PM <lakshmi.sowjanya.d@...el.com> wrote:
>
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
>
> Add a flag for event count and an extended structure to capture the event
> count when the flag is enabled.
>
> Intel(R) PMC Timed I/O device has an event count register counting
> the number of missed input edges. The register interface captures the
> event count and timestamp of the last event. For an event rate
> exceeding the rate that software can read events, the software can use
> the missed event count to calculate average event rates.
>
> The application requests one or both rising and falling edges when
> initializing the interface. The count of the selected edge type is
> optionally selected with an added event type flag. The event count is
> returned in an extended buffer using the read() interface.
>
> Co-developed-by: Christopher Hall <christopher.s.hall@...el.com>
> Signed-off-by: Christopher Hall <christopher.s.hall@...el.com>
> Signed-off-by: Tamal Saha <tamal.saha@...el.com>
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@...el.com>
> Reviewed-by: Mark Gross <mgross@...ux.intel.com>
> ---
> drivers/gpio/gpiolib-cdev.c | 28 +++++++++++++++++++---------
> include/linux/gpio/driver.h | 1 +
> include/uapi/linux/gpio.h | 12 ++++++++++++
> 3 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 1df28a71f88b..3b5719d5e2dc 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -518,7 +518,8 @@ struct linereq {
> GPIO_V2_LINE_DRIVE_FLAGS | \
> GPIO_V2_LINE_EDGE_FLAGS | \
> GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
> - GPIO_V2_LINE_BIAS_FLAGS)
> + GPIO_V2_LINE_BIAS_FLAGS | \
> + GPIO_V2_LINE_FLAG_EVENT_COUNT)
>
> static void linereq_put_event(struct linereq *lr,
> struct gpio_v2_line_event *le)
> @@ -1252,10 +1253,14 @@ static ssize_t linereq_read(struct file *file,
> struct linereq *lr = file->private_data;
> struct gpioevent_poll_data poll_data;
> struct gpio_v2_line_event le;
> + size_t min_userbuf_size;
> ssize_t bytes_read = 0;
> int ret, offset;
>
> - if (count < sizeof(le))
> + min_userbuf_size = sizeof(le);
> + min_userbuf_size += lr->lines[0].eflags & GPIO_V2_LINE_FLAG_EVENT_COUNT ?
> + sizeof(struct gpio_v2_line_event_ext) : 0;
> + if (count < min_userbuf_size)
> return -EINVAL;
>
> /* Without an IRQ, we can only poll */
> @@ -1270,12 +1275,17 @@ static ssize_t linereq_read(struct file *file,
> lr->lines[offset].eflags, &poll_data);
> if (ret)
> return ret;
> - event = kzalloc(sizeof(*event), GFP_KERNEL);
> + event = kzalloc(min_userbuf_size, GFP_KERNEL);
> event->timestamp_ns = poll_data.timestamp;
> event->id = poll_data.id;
> - if (copy_to_user(buf, (void *)&event, sizeof(event)))
> - return -EFAULT;
> - return sizeof(event);
> + if (lr->lines[offset].eflags & GPIO_V2_LINE_FLAG_EVENT_COUNT)
> + event->ext[0].event_count = poll_data.event_count;
> +
> + ret = copy_to_user(buf, (void *)event, min_userbuf_size);
> + if (ret)
> + ret = -EFAULT;
> + kfree(event);
> + return ret ? ret : min_userbuf_size;
> }
>
> do {
> @@ -1396,7 +1406,7 @@ static int setup_input(struct linereq *lr, struct gpio_v2_line_config *lc,
> ret = edge_detector_setup(&lr->lines[line_no], lc, line_no,
> lflags & GPIO_V2_LINE_EDGE_FLAGS);
> if (ret < 0) {
> - if (ret != -ENXIO) {
> + if (ret == -ENXIO) {
> if (lr->gdev->chip->setup_poll &&
> lr->gdev->chip->setup_poll(lr->gdev->chip, offset,
> &lflags) == 0 &&
> @@ -1513,7 +1523,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
> goto out_free_linereq;
> }
>
> - file_flags = O_RDONLY | O_CLOEXEC;
> + file_flags = O_CLOEXEC;
> file_flags |= output ? O_WRONLY : O_RDONLY;
> file_flags |= (!output && !lr->lines[i].irq) ? O_NONBLOCK : 0;
>
> @@ -1524,7 +1534,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
> offset);
> }
>
> - fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
> + fd = get_unused_fd_flags(file_flags);
> if (fd < 0) {
> ret = fd;
> goto out_free_linereq;
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 561e289434aa..09637fcbfd52 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -493,6 +493,7 @@ struct gpio_chip {
> struct gpioevent_poll_data {
> __u64 timestamp;
> __u32 id;
> + __u32 event_count;
> };
>
> struct gpio_output_event_data {
> diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
> index c39efc459b9f..e7fff2a205ec 100644
> --- a/include/uapi/linux/gpio.h
> +++ b/include/uapi/linux/gpio.h
> @@ -80,6 +80,7 @@ enum gpio_v2_line_flag {
> GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN = _BITULL(9),
> GPIO_V2_LINE_FLAG_BIAS_DISABLED = _BITULL(10),
> GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME = _BITULL(11),
> + GPIO_V2_LINE_FLAG_EVENT_COUNT = _BITULL(12),
> };
>
> /**
> @@ -270,6 +271,15 @@ enum gpio_v2_line_event_id {
> GPIO_V2_LINE_EVENT_UNKNOWN_EDGE = 3,
> };
>
> +/**
> + * struct gpio_v2_line_event_ext - Extended gpio line event
> + * @event_count: count of events
> + */
> +struct gpio_v2_line_event_ext {
> + __u32 event_count;
> + __u32 reserved[3];
> +};
> +
> /**
> * struct gpio_v2_line_event - The actual event being pushed to userspace
> * @timestamp_ns: best estimate of time of event occurrence, in nanoseconds.
> @@ -280,6 +290,7 @@ enum gpio_v2_line_event_id {
> * @line_seqno: the sequence number for this event in the sequence of
> * events on this particular line
> * @padding: reserved for future use
> + * @gpio_v2_line_event_ext: Extended gpio line event
> *
> * By default the @timestamp_ns is read from %CLOCK_MONOTONIC and is
> * intended to allow the accurate measurement of the time between events.
> @@ -296,6 +307,7 @@ struct gpio_v2_line_event {
> __u32 line_seqno;
> /* Space reserved for future use. */
> __u32 padding[6];
> + struct gpio_v2_line_event_ext ext[];
This bit is an instant NAK as it breaks the ABI.
While I understand this is a bit of a different problem than the one
handled by the seqno fields, I think you should just think about
adding a field called something like "real_seqno" for those hardware
counted sequence numbers.
Bart
> };
>
> /**
> --
> 2.17.1
>
Powered by blists - more mailing lists