linux-kernel - Re: [PATCH v7 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8

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, 12 Feb 2021 08:56:04 +0900
From:   William Breathitt Gray <vilhelm.gray@...il.com>
To:     David Lechner <david@...hnology.com>
Cc:     jic23@...nel.org, kernel@...gutronix.de,
        linux-stm32@...md-mailman.stormreply.com, a.fatoum@...gutronix.de,
        kamel.bouhara@...tlin.com, gwendal@...omium.org,
        alexandre.belloni@...tlin.com, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        syednwaris@...il.com, patrick.havelange@...ensium.com,
        fabrice.gasnier@...com, mcoquelin.stm32@...il.com,
        alexandre.torgue@...com
Subject: Re: [PATCH v7 5/5] counter: 104-quad-8: Add IRQ support for the
 ACCES 104-QUAD-8

On Wed, Dec 30, 2020 at 11:36:45AM -0600, David Lechner wrote:
> On 12/25/20 6:15 PM, William Breathitt Gray wrote:
> 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 b/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8
> > index eac32180c40d..0ecba24d43aa 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8
> > +++ b/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8
> > @@ -1,3 +1,28 @@
> > +What:		/sys/bus/counter/devices/counterX/countY/irq_trigger
> 
> Do we really need this sysfs attribute? Shouldn't interrupts be configured
> _only_ by the chrdev interface?

I think this attribute can go away because we can implicitly figure out
the correct IRQ configuration from the struct counter_watch data when a
user executes a COUNTER_ADD_WATCH_IOCTL ioctl command.

However, I need some help deciding on an appropriate behavior for
conflicting counter_watch configurations. Let me give some context
first.

The 104-QUAD-8 features 8 channels (essentially 8 independent physical
counters on the device). Each channel can independently issue an event,
but any particular channel can only be set to a single kind of event
(COUNTER_EVENT_INDEX, COUNTER_EVENT_OVERFLOW, etc.).

The purpose of the irq_trigger sysfs attribute I introduced in this
patch is to allow the user to select the event configuration they want
for a particular channel. We can theoretically figure this out
implicitly from the struct counter_watch request, so this sysfs
attribute may not be necessary.

However, how do we handle the case where a user executes two
COUNTER_ADD_WATCH_IOCTL ioctl commands for the same channel but with
different event selections? I'm considering three possible behaviors:

* Fail the second ioctl call; event selection of the first struct
  counter_watch takes precedence and thus second is incompatible.
* Issue a dev_warn() indicating that the second struct counter_watch
  event selection will now be the event configuration for that channel.
* Don't notify the user, just silently reconfigure for the second struct
  counter_watch event selection.

I'm suspecting the first behavior I listed here (ioctl returning failed)
is the most appropriate as a user is explicitly made known of this
particular device's inability to support more than one type of event per
channel.

What do you think?

William Breathitt Gray

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)