Date:   Fri,  4 Jun 2021 01:45:19 +0200
From:   Maximilian Luz <luzmaximilian@...il.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     Maximilian Luz <luzmaximilian@...il.com>,
        Mark Gross <mgross@...ux.intel.com>,
        Jonathan Corbet <corbet@....net>,
        platform-driver-x86@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH 0/7] platform/surface: aggregator: Extend user-space interface for events

Extend the user-space debug interface so that it can be used to receive
SSAM events in user-space.

Currently, inspecting SSAM events requires writing a custom client
device and corresponding driver. This is not particularly user-friendly
for quick testing and comes with higher iteration times. Since we
already have a user-space interface, we can extend this to forward
events from SSAM via the controller device file to user-space. With this
we can then essentially write user-space SSAM clients for testing and
reverse-engineering, providing us with all the essential functionality
that previously only a kernel driver would have access to. Note that
this is still only intended to be an interface for debugging and
reverse-engineering purposes.

To achieve this, we need to extend the core to decouple events from
notifiers. Right now, enabling an event group requires registering a
notifier for that group. This notifier provides a callback that is
called when the event occurs. For user-space forwarding, we need to run
all events through the same file. In the current implementation, this
presents a problem as, when we don't know the exact events or can't
filter for them, multiple notifiers for the same target category will
lead to duplicate events to be sent through the file, one per notifier.

Decoupling notifier registration from event enable-/disablement (and the
corresponding reference counting) allows us to avoid this issue. We can
then register one notifier for a whole target category and enable or
disable events independently of this notifier. Since events are strictly
separated by their target category, this will not lead to duplicate
events.

With this, we can then provide user-space with two new IOCTLs for
registering notifiers for a specific target category of events they are
interested in. This allows us to forward all events received by those
notifiers to the internal buffer of the device file, from which they can
be read by user-space. In other words, user-space can, via those two
IOCTLs, select which event target categories they are interested in.

Furthermore, we add another two IOCTLs for enabling and disabling events
via the controller. While events can already be enabled and disabled via
generic requests, this does not respect the controller-internal
reference counting mechanism. Due to that, this can lead to an event
group being disabled even though a kernel-driver has requested it to be
enabled. Or in other words: Without this, a user-space client cannot
safely reset the state as it has only two options, keeping the event
group enabled and not attempt cleanup at all, or disable the event group
for all clients and potentially stop them from working properly.

Also update the copyright lines since we're already doing some work on
the core.

Maximilian Luz (7):
  platform/surface: aggregator: Allow registering notifiers without
    enabling events
  platform/surface: aggregator: Allow enabling of events without
    notifiers
  platform/surface: aggregator: Update copyright
  platform/surface: aggregator_cdev: Add support for forwarding events
    to user-space
  platform/surface: aggregator_cdev: Allow enabling of events from
    user-space
  platform/surface: aggregator_cdev: Add lockdep support
  docs: driver-api: Update Surface Aggregator user-space interface
    documentation

 .../surface_aggregator/clients/cdev.rst       | 127 ++++-
 .../userspace-api/ioctl/ioctl-number.rst      |   2 +-
 drivers/platform/surface/aggregator/Kconfig   |   2 +-
 drivers/platform/surface/aggregator/Makefile  |   2 +-
 drivers/platform/surface/aggregator/bus.c     |   2 +-
 drivers/platform/surface/aggregator/bus.h     |   2 +-
 .../platform/surface/aggregator/controller.c  | 206 ++++++-
 .../platform/surface/aggregator/controller.h  |   2 +-
 drivers/platform/surface/aggregator/core.c    |   2 +-
 .../platform/surface/aggregator/ssh_msgb.h    |   2 +-
 .../surface/aggregator/ssh_packet_layer.c     |   2 +-
 .../surface/aggregator/ssh_packet_layer.h     |   2 +-
 .../platform/surface/aggregator/ssh_parser.c  |   2 +-
 .../platform/surface/aggregator/ssh_parser.h  |   2 +-
 .../surface/aggregator/ssh_request_layer.c    |   2 +-
 .../surface/aggregator/ssh_request_layer.h    |   2 +-
 drivers/platform/surface/aggregator/trace.h   |   2 +-
 .../surface/surface_aggregator_cdev.c         | 531 +++++++++++++++++-
 include/linux/surface_aggregator/controller.h |  27 +-
 include/linux/surface_aggregator/device.h     |   2 +-
 include/linux/surface_aggregator/serial_hub.h |   2 +-
 include/uapi/linux/surface_aggregator/cdev.h  |  73 ++-
 22 files changed, 921 insertions(+), 77 deletions(-)

-- 
2.31.1

Powered by blists - more mailing lists