firewire: cdev: fix ABI for FCP and address range mapping, add fw_cdev_event_request2

The problem:

A target-like userspace driver, e.g. AV/C target or SBP-2/3 target,
needs to be able to act as responder and requester.  In the latter role,
it needs to send requests to nods from which it received requests.  This
is currently impossible because fw_cdev_event_request lacks information
about sender node ID.
Reported-by: Jay Fenlason <fenlason@redhat.com>

Libffado + libraw1394 + firewire-core is currently unable to drive two
or more audio devices on the same bus.
Reported-by: Arnold Krille <arnold@arnoldarts.de>

This is because libffado requires destination node ID of FCP requests
and sender node ID of FCP responses to match.  It even prohibits
libffado from working with a bus on which libraw1394 opens a /dev/fw* as
default ioctl device that does not correspond with the audio device.
This is because libraw1394 does not receive the sender node ID from the
kernel.

Moreover, fw_cdev_event_request makes it impossible to tell unicast and
broadcast write requests apart.

The fix:

Add a replacement of struct fw_cdev_event_request request, boringly
called struct fw_cdev_event_request2.  The new event will be sent to a
userspace client instead of the old one if the client claims
compatibility with <linux/firewire-cdev.h> ABI version 4 or later.

libraw1394 needs to be extended to make use of the new event, in order
to properly support libffado and other FCP or address range mapping
users who require correct sender node IDs.

Further notes:

While we are at it, change back the range of possible values of
fw_cdev_event_request.tcode to 0x0...0xb like in ABI version <= 3.
The preceding change "firewire: expose extended tcode of incoming lock
requests to (userspace) drivers" expanded it to 0x0...0x17 which could
catch sloppily coded clients by surprise.  The extended range of codes
is only used in the new fw_cdev_event_request2.tcode.

Jay and I also suggested an alternative approach to fix the ABI for
incoming requests:  Add an FW_CDEV_IOC_GET_REQUEST_INFO ioctl which can
be called after reception of an fw_cdev_event_request, before issuing of
the closing FW_CDEV_IOC_SEND_RESPONSE ioctl.  The new ioctl would reveal
the vital information about a request that fw_cdev_event_request lacks.
Jay showed an implementation of this approach.

The former event approach adds 27 LOC of rather trivial code to
core-cdev.c, the ioctl approach 34 LOC, some of which is nontrivial.
The ioctl approach would certainly also add more LOC to userspace
programs which require the expanded information on inbound requests.
This approach is probably only on the lighter-weight side in case of
clients that want to be compatible with kernels that lack the new
capability, like libraw1394.  However, the code to be added to such
libraw1394-like clients in case of the event approach is a straight-
forward additional switch () case in its event handler.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

1 files changed, 36 insertions, 9 deletions

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 0cf86bcbeea2..9b8df2039155 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -49,7 +49,8 @@
 /*
  * ABI version history is documented in linux/firewire-cdev.h.
  */
-#define FW_CDEV_KERNEL_VERSION 3
+#define FW_CDEV_KERNEL_VERSION          4
+#define FW_CDEV_VERSION_EVENT_REQUEST2  4
 
 struct client {
         u32 version;
@@ -176,7 +177,10 @@ struct outbound_transaction_event {
 
 struct inbound_transaction_event {
         struct event event;
-        struct fw_cdev_event_request request;
+        union {
+                struct fw_cdev_event_request request;
+                struct fw_cdev_event_request2 request2;
+        } req;
 };
 
 struct iso_interrupt_event {
@@ -645,6 +649,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
         struct address_handler_resource *handler = callback_data;
         struct inbound_transaction_resource *r;
         struct inbound_transaction_event *e;
+        size_t event_size0;
         void *fcp_frame = NULL;
         int ret;
 
@@ -678,15 +683,37 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
         if (ret < 0)
                 goto failed;
 
-        e->request.type    = FW_CDEV_EVENT_REQUEST;
-        e->request.tcode   = tcode;
-        e->request.offset  = offset;
-        e->request.length  = length;
-        e->request.handle  = r->resource.handle;
-        e->request.closure = handler->closure;
+        if (handler->client->version < FW_CDEV_VERSION_EVENT_REQUEST2) {
+                struct fw_cdev_event_request *req = &e->req.request;
+
+                if (tcode & 0x10)
+                        tcode = TCODE_LOCK_REQUEST;
+
+                req->type       = FW_CDEV_EVENT_REQUEST;
+                req->tcode      = tcode;
+                req->offset     = offset;
+                req->length     = length;
+                req->handle     = r->resource.handle;
+                req->closure    = handler->closure;
+                event_size0     = sizeof(*req);
+        } else {
+                struct fw_cdev_event_request2 *req = &e->req.request2;
+
+                req->type       = FW_CDEV_EVENT_REQUEST2;
+                req->tcode      = tcode;
+                req->offset     = offset;
+                req->source_node_id = source;
+                req->destination_node_id = destination;
+                req->card       = card->index;
+                req->generation = generation;
+                req->length     = length;
+                req->handle     = r->resource.handle;
+                req->closure    = handler->closure;
+                event_size0     = sizeof(*req);
+        }
 
         queue_event(handler->client, &e->event,
-                    &e->request, sizeof(e->request), r->data, length);
+                    &e->req, event_size0, r->data, length);
         return;
 
  failed: