* [PATCH 01/11] firewire: cdev: reference-count client instances
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
@ 2009-01-04 15:24 ` Stefan Richter
2009-01-11 20:31 ` David Moore
2009-01-04 15:25 ` [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances Stefan Richter
` (10 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:24 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
The lifetime of struct client instances must be longer than the lifetime
of any client resource.
This fixes a possible race between fw_device_op_release and transaction
completions. It also prepares for new ioctls for isochronous resource
management which will involve delayed processing of client resources.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 55 ++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 9 deletions(-)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -20,6 +20,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/kref.h>
#include <linux/wait.h>
#include <linux/errno.h>
#include <linux/device.h>
@@ -94,8 +95,27 @@ struct client {
unsigned long vm_start;
struct list_head link;
+ struct kref kref;
};
+static inline void client_get(struct client *client)
+{
+ kref_get(&client->kref);
+}
+
+static void client_release(struct kref *kref)
+{
+ struct client *client = container_of(kref, struct client, kref);
+
+ fw_device_put(client->device);
+ kfree(client);
+}
+
+static void client_put(struct client *client)
+{
+ kref_put(&client->kref, client_release);
+}
+
static inline void __user *u64_to_uptr(__u64 value)
{
return (void __user *)(unsigned long)value;
@@ -131,6 +151,7 @@ static int fw_device_op_open(struct inod
idr_init(&client->resource_idr);
INIT_LIST_HEAD(&client->event_list);
init_waitqueue_head(&client->wait);
+ kref_init(&client->kref);
file->private_data = client;
@@ -326,6 +347,8 @@ static int add_client_resource(struct cl
else
ret = idr_get_new(&client->resource_idr, resource,
&resource->handle);
+ if (ret >= 0)
+ client_get(client);
spin_unlock_irqrestore(&client->lock, flags);
if (ret == -EAGAIN)
@@ -358,6 +381,8 @@ static int release_client_resource(struc
else
r->release(client, r);
+ client_put(client);
+
return 0;
}
@@ -385,11 +410,21 @@ static void complete_transaction(struct
spin_lock_irqsave(&client->lock, flags);
/*
- * If called while in shutdown, the idr tree must be left untouched.
- * The idr handle will be removed later.
+ * 1. If called while in shutdown, the idr tree must be left untouched.
+ * The idr handle will be removed and the client reference will be
+ * dropped later.
+ * 2. If the call chain was release_client_resource ->
+ * release_transaction -> complete_transaction (instead of a normal
+ * conclusion of the transaction), i.e. if this resource was already
+ * unregistered from the idr, the client reference will be dropped
+ * by release_client_resource and we must not drop it here.
*/
- if (!client->in_shutdown)
+ if (!client->in_shutdown &&
+ idr_find(&client->resource_idr, response->resource.handle)) {
idr_remove(&client->resource_idr, response->resource.handle);
+ /* Drop the idr's reference */
+ client_put(client);
+ }
spin_unlock_irqrestore(&client->lock, flags);
r->type = FW_CDEV_EVENT_RESPONSE;
@@ -408,6 +443,9 @@ static void complete_transaction(struct
else
queue_event(client, &response->event, r, sizeof(*r) + r->length,
NULL, 0);
+
+ /* Drop the transaction callback's reference */
+ client_put(client);
}
static int ioctl_send_request(struct client *client, void *buffer)
@@ -459,6 +497,9 @@ static int ioctl_send_request(struct cli
if (ret < 0)
goto failed;
+ /* Get a reference for the transaction callback */
+ client_get(client);
+
fw_send_request(device->card, &response->transaction,
request->tcode & 0x1f,
device->node->node_id,
@@ -1044,6 +1085,7 @@ static int shutdown_resource(int id, voi
struct client *client = data;
r->release(client, r);
+ client_put(client);
return 0;
}
@@ -1076,12 +1118,7 @@ static int fw_device_op_release(struct i
list_for_each_entry_safe(e, next_e, &client->event_list, link)
kfree(e);
- /*
- * FIXME: client should be reference-counted. It's extremely unlikely
- * but there may still be transactions being completed at this point.
- */
- fw_device_put(client->device);
- kfree(client);
+ client_put(client);
return 0;
}
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
@ 2009-01-04 15:25 ` Stefan Richter
2009-01-04 15:25 ` [PATCH 03/11] firewire: cdev: sort includes Stefan Richter
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:25 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
to indicate that they are specializations of struct event or of struct
client_resource, respectively.
struct response was both an event and a client_resource; it is now split
into struct outbound_transaction_resource and ~_event in order to
document more explicitly which types of client resources exist.
struct request and struct_request_event are renamed to struct
inbound_transaction_resource and ~_event because requests and responses
occur in outbound and in inbound transactions.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 337 ++++++++++++++++++-------------------
1 file changed, 169 insertions(+), 168 deletions(-)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -41,43 +41,6 @@
#include "fw-topology.h"
#include "fw-device.h"
-struct client;
-struct client_resource;
-typedef void (*client_resource_release_fn_t)(struct client *,
- struct client_resource *);
-struct client_resource {
- client_resource_release_fn_t release;
- int handle;
-};
-
-/*
- * dequeue_event() just kfree()'s the event, so the event has to be
- * the first field in the struct.
- */
-
-struct event {
- struct { void *data; size_t size; } v[2];
- struct list_head link;
-};
-
-struct bus_reset {
- struct event event;
- struct fw_cdev_event_bus_reset reset;
-};
-
-struct response {
- struct event event;
- struct fw_transaction transaction;
- struct client *client;
- struct client_resource resource;
- struct fw_cdev_event_response response;
-};
-
-struct iso_interrupt {
- struct event event;
- struct fw_cdev_event_iso_interrupt interrupt;
-};
-
struct client {
u32 version;
struct fw_device *device;
@@ -116,6 +79,70 @@ static void client_put(struct client *cl
kref_put(&client->kref, client_release);
}
+struct client_resource;
+typedef void (*client_resource_release_fn_t)(struct client *,
+ struct client_resource *);
+struct client_resource {
+ client_resource_release_fn_t release;
+ int handle;
+};
+
+struct address_handler_resource {
+ struct client_resource resource;
+ struct fw_address_handler handler;
+ __u64 closure;
+ struct client *client;
+};
+
+struct outbound_transaction_resource {
+ struct client_resource resource;
+ struct fw_transaction transaction;
+};
+
+struct inbound_transaction_resource {
+ struct client_resource resource;
+ struct fw_request *request;
+ void *data;
+ size_t length;
+};
+
+struct descriptor_resource {
+ struct client_resource resource;
+ struct fw_descriptor descriptor;
+ u32 data[0];
+};
+
+/*
+ * dequeue_event() just kfree()'s the event, so the event has to be
+ * the first field in a struct XYZ_event.
+ */
+struct event {
+ struct { void *data; size_t size; } v[2];
+ struct list_head link;
+};
+
+struct bus_reset_event {
+ struct event event;
+ struct fw_cdev_event_bus_reset reset;
+};
+
+struct outbound_transaction_event {
+ struct event event;
+ struct client *client;
+ struct outbound_transaction_resource r;
+ struct fw_cdev_event_response response;
+};
+
+struct inbound_transaction_event {
+ struct event event;
+ struct fw_cdev_event_request request;
+};
+
+struct iso_interrupt_event {
+ struct event event;
+ struct fw_cdev_event_iso_interrupt interrupt;
+};
+
static inline void __user *u64_to_uptr(__u64 value)
{
return (void __user *)(unsigned long)value;
@@ -263,18 +290,18 @@ static void for_each_client(struct fw_de
static void queue_bus_reset_event(struct client *client)
{
- struct bus_reset *bus_reset;
+ struct bus_reset_event *e;
- bus_reset = kzalloc(sizeof(*bus_reset), GFP_KERNEL);
- if (bus_reset == NULL) {
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
+ if (e == NULL) {
fw_notify("Out of memory when allocating bus reset event\n");
return;
}
- fill_bus_reset_event(&bus_reset->reset, client);
+ fill_bus_reset_event(&e->reset, client);
- queue_event(client, &bus_reset->event,
- &bus_reset->reset, sizeof(bus_reset->reset), NULL, 0);
+ queue_event(client, &e->event,
+ &e->reset, sizeof(e->reset), NULL, 0);
}
void fw_device_cdev_update(struct fw_device *device)
@@ -389,24 +416,24 @@ static int release_client_resource(struc
static void release_transaction(struct client *client,
struct client_resource *resource)
{
- struct response *response =
- container_of(resource, struct response, resource);
+ struct outbound_transaction_resource *r = container_of(resource,
+ struct outbound_transaction_resource, resource);
- fw_cancel_transaction(client->device->card, &response->transaction);
+ fw_cancel_transaction(client->device->card, &r->transaction);
}
static void complete_transaction(struct fw_card *card, int rcode,
void *payload, size_t length, void *data)
{
- struct response *response = data;
- struct client *client = response->client;
+ struct outbound_transaction_event *e = data;
+ struct fw_cdev_event_response *rsp = &e->response;
+ struct client *client = e->client;
unsigned long flags;
- struct fw_cdev_event_response *r = &response->response;
- if (length < r->length)
- r->length = length;
+ if (length < rsp->length)
+ rsp->length = length;
if (rcode == RCODE_COMPLETE)
- memcpy(r->data, payload, r->length);
+ memcpy(rsp->data, payload, rsp->length);
spin_lock_irqsave(&client->lock, flags);
/*
@@ -420,28 +447,28 @@ static void complete_transaction(struct
* by release_client_resource and we must not drop it here.
*/
if (!client->in_shutdown &&
- idr_find(&client->resource_idr, response->resource.handle)) {
- idr_remove(&client->resource_idr, response->resource.handle);
+ idr_find(&client->resource_idr, e->r.resource.handle)) {
+ idr_remove(&client->resource_idr, e->r.resource.handle);
/* Drop the idr's reference */
client_put(client);
}
spin_unlock_irqrestore(&client->lock, flags);
- r->type = FW_CDEV_EVENT_RESPONSE;
- r->rcode = rcode;
+ rsp->type = FW_CDEV_EVENT_RESPONSE;
+ rsp->rcode = rcode;
/*
- * In the case that sizeof(*r) doesn't align with the position of the
+ * In the case that sizeof(*rsp) doesn't align with the position of the
* data, and the read is short, preserve an extra copy of the data
* to stay compatible with a pre-2.6.27 bug. Since the bug is harmless
* for short reads and some apps depended on it, this is both safe
* and prudent for compatibility.
*/
- if (r->length <= sizeof(*r) - offsetof(typeof(*r), data))
- queue_event(client, &response->event, r, sizeof(*r),
- r->data, r->length);
+ if (rsp->length <= sizeof(*rsp) - offsetof(typeof(*rsp), data))
+ queue_event(client, &e->event, rsp, sizeof(*rsp),
+ rsp->data, rsp->length);
else
- queue_event(client, &response->event, r, sizeof(*r) + r->length,
+ queue_event(client, &e->event, rsp, sizeof(*rsp) + rsp->length,
NULL, 0);
/* Drop the transaction callback's reference */
@@ -452,23 +479,23 @@ static int ioctl_send_request(struct cli
{
struct fw_device *device = client->device;
struct fw_cdev_send_request *request = buffer;
- struct response *response;
+ struct outbound_transaction_event *e;
int ret;
/* What is the biggest size we'll accept, really? */
if (request->length > 4096)
return -EINVAL;
- response = kmalloc(sizeof(*response) + request->length, GFP_KERNEL);
- if (response == NULL)
+ e = kmalloc(sizeof(*e) + request->length, GFP_KERNEL);
+ if (e == NULL)
return -ENOMEM;
- response->client = client;
- response->response.length = request->length;
- response->response.closure = request->closure;
+ e->client = client;
+ e->response.length = request->length;
+ e->response.closure = request->closure;
if (request->data &&
- copy_from_user(response->response.data,
+ copy_from_user(e->response.data,
u64_to_uptr(request->data), request->length)) {
ret = -EFAULT;
goto failed;
@@ -492,86 +519,66 @@ static int ioctl_send_request(struct cli
goto failed;
}
- response->resource.release = release_transaction;
- ret = add_client_resource(client, &response->resource, GFP_KERNEL);
+ e->r.resource.release = release_transaction;
+ ret = add_client_resource(client, &e->r.resource, GFP_KERNEL);
if (ret < 0)
goto failed;
/* Get a reference for the transaction callback */
client_get(client);
- fw_send_request(device->card, &response->transaction,
+ fw_send_request(device->card, &e->r.transaction,
request->tcode & 0x1f,
device->node->node_id,
request->generation,
device->max_speed,
request->offset,
- response->response.data, request->length,
- complete_transaction, response);
+ e->response.data, request->length,
+ complete_transaction, e);
if (request->data)
return sizeof(request) + request->length;
else
return sizeof(request);
failed:
- kfree(response);
+ kfree(e);
return ret;
}
-struct address_handler {
- struct fw_address_handler handler;
- __u64 closure;
- struct client *client;
- struct client_resource resource;
-};
-
-struct request {
- struct fw_request *request;
- void *data;
- size_t length;
- struct client_resource resource;
-};
-
-struct request_event {
- struct event event;
- struct fw_cdev_event_request request;
-};
-
static void release_request(struct client *client,
struct client_resource *resource)
{
- struct request *request =
- container_of(resource, struct request, resource);
+ struct inbound_transaction_resource *r = container_of(resource,
+ struct inbound_transaction_resource, resource);
- fw_send_response(client->device->card, request->request,
+ fw_send_response(client->device->card, r->request,
RCODE_CONFLICT_ERROR);
- kfree(request);
+ kfree(r);
}
-static void handle_request(struct fw_card *card, struct fw_request *r,
+static void handle_request(struct fw_card *card, struct fw_request *request,
int tcode, int destination, int source,
int generation, int speed,
unsigned long long offset,
void *payload, size_t length, void *callback_data)
{
- struct address_handler *handler = callback_data;
- struct request *request;
- struct request_event *e;
- struct client *client = handler->client;
+ struct address_handler_resource *handler = callback_data;
+ struct inbound_transaction_resource *r;
+ struct inbound_transaction_event *e;
int ret;
- request = kmalloc(sizeof(*request), GFP_ATOMIC);
+ r = kmalloc(sizeof(*r), GFP_ATOMIC);
e = kmalloc(sizeof(*e), GFP_ATOMIC);
- if (request == NULL || e == NULL)
+ if (r == NULL || e == NULL)
goto failed;
- request->request = r;
- request->data = payload;
- request->length = length;
+ r->request = request;
+ r->data = payload;
+ r->length = length;
- request->resource.release = release_request;
- ret = add_client_resource(client, &request->resource, GFP_ATOMIC);
+ r->resource.release = release_request;
+ ret = add_client_resource(handler->client, &r->resource, GFP_ATOMIC);
if (ret < 0)
goto failed;
@@ -579,61 +586,61 @@ static void handle_request(struct fw_car
e->request.tcode = tcode;
e->request.offset = offset;
e->request.length = length;
- e->request.handle = request->resource.handle;
+ e->request.handle = r->resource.handle;
e->request.closure = handler->closure;
- queue_event(client, &e->event,
+ queue_event(handler->client, &e->event,
&e->request, sizeof(e->request), payload, length);
return;
failed:
- kfree(request);
+ kfree(r);
kfree(e);
- fw_send_response(card, r, RCODE_CONFLICT_ERROR);
+ fw_send_response(card, request, RCODE_CONFLICT_ERROR);
}
static void release_address_handler(struct client *client,
struct client_resource *resource)
{
- struct address_handler *handler =
- container_of(resource, struct address_handler, resource);
+ struct address_handler_resource *r =
+ container_of(resource, struct address_handler_resource, resource);
- fw_core_remove_address_handler(&handler->handler);
- kfree(handler);
+ fw_core_remove_address_handler(&r->handler);
+ kfree(r);
}
static int ioctl_allocate(struct client *client, void *buffer)
{
struct fw_cdev_allocate *request = buffer;
- struct address_handler *handler;
+ struct address_handler_resource *r;
struct fw_address_region region;
int ret;
- handler = kmalloc(sizeof(*handler), GFP_KERNEL);
- if (handler == NULL)
+ r = kmalloc(sizeof(*r), GFP_KERNEL);
+ if (r == NULL)
return -ENOMEM;
region.start = request->offset;
region.end = request->offset + request->length;
- handler->handler.length = request->length;
- handler->handler.address_callback = handle_request;
- handler->handler.callback_data = handler;
- handler->closure = request->closure;
- handler->client = client;
+ r->handler.length = request->length;
+ r->handler.address_callback = handle_request;
+ r->handler.callback_data = r;
+ r->closure = request->closure;
+ r->client = client;
- ret = fw_core_add_address_handler(&handler->handler, ®ion);
+ ret = fw_core_add_address_handler(&r->handler, ®ion);
if (ret < 0) {
- kfree(handler);
+ kfree(r);
return ret;
}
- handler->resource.release = release_address_handler;
- ret = add_client_resource(client, &handler->resource, GFP_KERNEL);
+ r->resource.release = release_address_handler;
+ ret = add_client_resource(client, &r->resource, GFP_KERNEL);
if (ret < 0) {
- release_address_handler(client, &handler->resource);
+ release_address_handler(client, &r->resource);
return ret;
}
- request->handle = handler->resource.handle;
+ request->handle = r->resource.handle;
return 0;
}
@@ -650,13 +657,14 @@ static int ioctl_send_response(struct cl
{
struct fw_cdev_send_response *request = buffer;
struct client_resource *resource;
- struct request *r;
+ struct inbound_transaction_resource *r;
if (release_client_resource(client, request->handle,
release_request, &resource) < 0)
return -EINVAL;
- r = container_of(resource, struct request, resource);
+ r = container_of(resource, struct inbound_transaction_resource,
+ resource);
if (request->length < r->length)
r->length = request->length;
if (copy_from_user(r->data, u64_to_uptr(request->data), r->length))
@@ -678,62 +686,55 @@ static int ioctl_initiate_bus_reset(stru
return fw_core_initiate_bus_reset(client->device->card, short_reset);
}
-struct descriptor {
- struct fw_descriptor d;
- struct client_resource resource;
- u32 data[0];
-};
-
static void release_descriptor(struct client *client,
struct client_resource *resource)
{
- struct descriptor *descriptor =
- container_of(resource, struct descriptor, resource);
+ struct descriptor_resource *r =
+ container_of(resource, struct descriptor_resource, resource);
- fw_core_remove_descriptor(&descriptor->d);
- kfree(descriptor);
+ fw_core_remove_descriptor(&r->descriptor);
+ kfree(r);
}
static int ioctl_add_descriptor(struct client *client, void *buffer)
{
struct fw_cdev_add_descriptor *request = buffer;
- struct descriptor *descriptor;
+ struct descriptor_resource *r;
int ret;
if (request->length > 256)
return -EINVAL;
- descriptor =
- kmalloc(sizeof(*descriptor) + request->length * 4, GFP_KERNEL);
- if (descriptor == NULL)
+ r = kmalloc(sizeof(*r) + request->length * 4, GFP_KERNEL);
+ if (r == NULL)
return -ENOMEM;
- if (copy_from_user(descriptor->data,
+ if (copy_from_user(r->data,
u64_to_uptr(request->data), request->length * 4)) {
ret = -EFAULT;
goto failed;
}
- descriptor->d.length = request->length;
- descriptor->d.immediate = request->immediate;
- descriptor->d.key = request->key;
- descriptor->d.data = descriptor->data;
+ r->descriptor.length = request->length;
+ r->descriptor.immediate = request->immediate;
+ r->descriptor.key = request->key;
+ r->descriptor.data = r->data;
- ret = fw_core_add_descriptor(&descriptor->d);
+ ret = fw_core_add_descriptor(&r->descriptor);
if (ret < 0)
goto failed;
- descriptor->resource.release = release_descriptor;
- ret = add_client_resource(client, &descriptor->resource, GFP_KERNEL);
+ r->resource.release = release_descriptor;
+ ret = add_client_resource(client, &r->resource, GFP_KERNEL);
if (ret < 0) {
- fw_core_remove_descriptor(&descriptor->d);
+ fw_core_remove_descriptor(&r->descriptor);
goto failed;
}
- request->handle = descriptor->resource.handle;
+ request->handle = r->resource.handle;
return 0;
failed:
- kfree(descriptor);
+ kfree(r);
return ret;
}
@@ -750,19 +751,19 @@ static void iso_callback(struct fw_iso_c
size_t header_length, void *header, void *data)
{
struct client *client = data;
- struct iso_interrupt *irq;
+ struct iso_interrupt_event *e;
- irq = kzalloc(sizeof(*irq) + header_length, GFP_ATOMIC);
- if (irq == NULL)
+ e = kzalloc(sizeof(*e) + header_length, GFP_ATOMIC);
+ if (e == NULL)
return;
- irq->interrupt.type = FW_CDEV_EVENT_ISO_INTERRUPT;
- irq->interrupt.closure = client->iso_closure;
- irq->interrupt.cycle = cycle;
- irq->interrupt.header_length = header_length;
- memcpy(irq->interrupt.header, header, header_length);
- queue_event(client, &irq->event, &irq->interrupt,
- sizeof(irq->interrupt) + header_length, NULL, 0);
+ e->interrupt.type = FW_CDEV_EVENT_ISO_INTERRUPT;
+ e->interrupt.closure = client->iso_closure;
+ e->interrupt.cycle = cycle;
+ e->interrupt.header_length = header_length;
+ memcpy(e->interrupt.header, header, header_length);
+ queue_event(client, &e->event, &e->interrupt,
+ sizeof(e->interrupt) + header_length, NULL, 0);
}
static int ioctl_create_iso_context(struct client *client, void *buffer)
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 03/11] firewire: cdev: sort includes
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
2009-01-04 15:24 ` [PATCH 01/11] firewire: cdev: reference-count client instances Stefan Richter
2009-01-04 15:25 ` [PATCH 02/11] firewire: cdev: unify names of struct types and of their instances Stefan Richter
@ 2009-01-04 15:25 ` Stefan Richter
2009-01-04 15:26 ` [PATCH 04/11] firewire: core: topology header fix Stefan Richter
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:25 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -18,28 +18,30 @@
* Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
*/
-#include <linux/module.h>
+#include <linux/compat.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/firewire-cdev.h>
+#include <linux/idr.h>
#include <linux/kernel.h>
#include <linux/kref.h>
-#include <linux/wait.h>
-#include <linux/errno.h>
-#include <linux/device.h>
-#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/poll.h>
#include <linux/preempt.h>
-#include <linux/time.h>
#include <linux/spinlock.h>
-#include <linux/delay.h>
-#include <linux/mm.h>
-#include <linux/idr.h>
-#include <linux/compat.h>
-#include <linux/firewire-cdev.h>
+#include <linux/time.h>
+#include <linux/vmalloc.h>
+#include <linux/wait.h>
+
#include <asm/system.h>
#include <asm/uaccess.h>
-#include "fw-transaction.h"
-#include "fw-topology.h"
+
#include "fw-device.h"
+#include "fw-topology.h"
+#include "fw-transaction.h"
struct client {
u32 version;
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 04/11] firewire: core: topology header fix
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (2 preceding siblings ...)
2009-01-04 15:25 ` [PATCH 03/11] firewire: cdev: sort includes Stefan Richter
@ 2009-01-04 15:26 ` Stefan Richter
2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:26 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-topology.h | 6 ++++++
1 file changed, 6 insertions(+)
Index: linux/drivers/firewire/fw-topology.h
===================================================================
--- linux.orig/drivers/firewire/fw-topology.h
+++ linux/drivers/firewire/fw-topology.h
@@ -19,6 +19,11 @@
#ifndef __fw_topology_h
#define __fw_topology_h
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include <asm/atomic.h>
+
enum {
FW_NODE_CREATED,
FW_NODE_UPDATED,
@@ -64,6 +69,7 @@ static inline void fw_node_put(struct fw
kfree(node);
}
+struct fw_card;
void fw_destroy_nodes(struct fw_card *card);
int fw_compute_block_crc(u32 *block);
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (3 preceding siblings ...)
2009-01-04 15:26 ` [PATCH 04/11] firewire: core: topology header fix Stefan Richter
@ 2009-01-04 15:26 ` Stefan Richter
[not found] ` <1231404355.18613.68.camel@localhost.localdomain>
2009-01-11 20:32 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management David Moore
2009-01-04 15:27 ` [PATCH 06/11] firewire: cdev: add ioctls for manual iso " Stefan Richter
` (6 subsequent siblings)
11 siblings, 2 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:26 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Based on
Date: Tue, 18 Nov 2008 11:41:27 -0500
From: Jay Fenlason <fenlason@redhat.com>
Subject: [Patch V4] Add ISO resource management support
with several changes to the ABI and implementation. Only the part of
the ABI which enables auto-reallocation and auto-deallocation is
included here.
This implements ioctls for kernel-assisted allocation of isochronous
channels and isochronous bandwidth. The benefits are:
- The client does not have to have write access to the /dev/fw* device
corresponding to the IRM.
- The client does not have to perform reallocation after bus resets.
- Channel and bandwidth are deallocated by the kernel if the file is
closed before the client deallocated the resources. Thus resources
are released even if the client crashes.
It is anticipated that future in-kernel code (firewire-core IRM code;
the firewire port of firedtv), will use the fw-iso.c portions of this
code too.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 215 +++++++++++++++++++++++++++++-
drivers/firewire/fw-iso.c | 176 +++++++++++++++++++++++-
drivers/firewire/fw-transaction.h | 4
include/linux/firewire-cdev.h | 100 ++++++++++++-
4 files changed, 475 insertions(+), 20 deletions(-)
Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -25,10 +25,12 @@
#include <linux/types.h>
#include <linux/firewire-constants.h>
-#define FW_CDEV_EVENT_BUS_RESET 0x00
-#define FW_CDEV_EVENT_RESPONSE 0x01
-#define FW_CDEV_EVENT_REQUEST 0x02
-#define FW_CDEV_EVENT_ISO_INTERRUPT 0x03
+#define FW_CDEV_EVENT_BUS_RESET 0x00
+#define FW_CDEV_EVENT_RESPONSE 0x01
+#define FW_CDEV_EVENT_REQUEST 0x02
+#define FW_CDEV_EVENT_ISO_INTERRUPT 0x03
+#define FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED 0x04
+#define FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED 0x05
/**
* struct fw_cdev_event_common - Common part of all fw_cdev_event_ types
@@ -147,12 +149,46 @@ struct fw_cdev_event_iso_interrupt {
};
/**
+ * struct fw_cdev_event_iso_resource - Iso resources were allocated or freed
+ * @closure: See &fw_cdev_event_common;
+ * set by %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl
+ * @type: %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
+ * %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
+ * @handle: Reference by which an allocated resource can be deallocated
+ * @channel: Isochronous channel which was (de)allocated, if any
+ * @bandwidth: Bandwidth allocation units which were (de)allocated, if any
+ * @channels_available: Last known availability of channels
+ * @bandwidth_available: Last known availability of bandwidth
+ *
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED event is sent after an isochronous
+ * resource was allocated at the IRM. The client has to check @channel and
+ * @bandwidth for whether the allocation actually succeeded.
+ *
+ * @channel is <0 if no channel was allocated.
+ * @bandwidth is 0 if no bandwidth was allocated.
+ *
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event is sent after an isochronous
+ * resource was deallocated at the IRM. It is also sent when automatic
+ * reallocation after a bus reset failed.
+ */
+struct fw_cdev_event_iso_resource {
+ __u64 closure;
+ __u32 type;
+ __u32 handle;
+ __s32 channel;
+ __s32 bandwidth;
+};
+
+/**
* union fw_cdev_event - Convenience union of fw_cdev_event_ types
* @common: Valid for all types
* @bus_reset: Valid if @common.type == %FW_CDEV_EVENT_BUS_RESET
* @response: Valid if @common.type == %FW_CDEV_EVENT_RESPONSE
* @request: Valid if @common.type == %FW_CDEV_EVENT_REQUEST
* @iso_interrupt: Valid if @common.type == %FW_CDEV_EVENT_ISO_INTERRUPT
+ * @iso_resource: Valid if @common.type ==
+ * %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
+ * %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
*
* Convenience union for userspace use. Events could be read(2) into an
* appropriately aligned char buffer and then cast to this union for further
@@ -163,13 +199,15 @@ struct fw_cdev_event_iso_interrupt {
* not fit will be discarded so that the next read(2) will return a new event.
*/
union fw_cdev_event {
- struct fw_cdev_event_common common;
- struct fw_cdev_event_bus_reset bus_reset;
- struct fw_cdev_event_response response;
- struct fw_cdev_event_request request;
- struct fw_cdev_event_iso_interrupt iso_interrupt;
+ struct fw_cdev_event_common common;
+ struct fw_cdev_event_bus_reset bus_reset;
+ struct fw_cdev_event_response response;
+ struct fw_cdev_event_request request;
+ struct fw_cdev_event_iso_interrupt iso_interrupt;
+ struct fw_cdev_event_iso_resource iso_resource;
};
+/* available since kernel version 2.6.22 */
#define FW_CDEV_IOC_GET_INFO _IOWR('#', 0x00, struct fw_cdev_get_info)
#define FW_CDEV_IOC_SEND_REQUEST _IOW('#', 0x01, struct fw_cdev_send_request)
#define FW_CDEV_IOC_ALLOCATE _IOWR('#', 0x02, struct fw_cdev_allocate)
@@ -178,13 +216,18 @@ union fw_cdev_event {
#define FW_CDEV_IOC_INITIATE_BUS_RESET _IOW('#', 0x05, struct fw_cdev_initiate_bus_reset)
#define FW_CDEV_IOC_ADD_DESCRIPTOR _IOWR('#', 0x06, struct fw_cdev_add_descriptor)
#define FW_CDEV_IOC_REMOVE_DESCRIPTOR _IOW('#', 0x07, struct fw_cdev_remove_descriptor)
-
#define FW_CDEV_IOC_CREATE_ISO_CONTEXT _IOWR('#', 0x08, struct fw_cdev_create_iso_context)
#define FW_CDEV_IOC_QUEUE_ISO _IOWR('#', 0x09, struct fw_cdev_queue_iso)
#define FW_CDEV_IOC_START_ISO _IOW('#', 0x0a, struct fw_cdev_start_iso)
#define FW_CDEV_IOC_STOP_ISO _IOW('#', 0x0b, struct fw_cdev_stop_iso)
+
+/* available since kernel version 2.6.24 */
#define FW_CDEV_IOC_GET_CYCLE_TIMER _IOR('#', 0x0c, struct fw_cdev_get_cycle_timer)
+/* available since kernel version 2.6.30 */
+#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE _IOWR('#', 0x0d, struct fw_cdev_allocate_iso_resource)
+#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE _IOW('#', 0x0e, struct fw_cdev_deallocate)
+
/* FW_CDEV_VERSION History
*
* 1 Feb 18, 2007: Initial version.
@@ -284,9 +327,9 @@ struct fw_cdev_allocate {
};
/**
- * struct fw_cdev_deallocate - Free an address range allocation
- * @handle: Handle to the address range, as returned by the kernel when the
- * range was allocated
+ * struct fw_cdev_deallocate - Free a CSR address range or isochronous resource
+ * @handle: Handle to the address range or iso resource, as returned by the
+ * kernel when the range or resource was allocated
*/
struct fw_cdev_deallocate {
__u32 handle;
@@ -479,4 +522,35 @@ struct fw_cdev_get_cycle_timer {
__u32 cycle_timer;
};
+/**
+ * struct fw_cdev_allocate_iso_resource - Allocate a channel or bandwidth
+ * @closure: Passed back to userspace in correponding iso resource events
+ * @channels: Isochronous channels of which one is to be allocated
+ * @bandwidth: Isochronous bandwidth units to be allocated
+ * @handle: Handle to the allocation, written by the kernel
+ *
+ * The %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl initiates allocation of an
+ * isochronous channel and/or of isochronous bandwidth at the isochronous
+ * resource manager (IRM). Only one of the channels specified in @channels is
+ * allocated. An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED is sent after
+ * communication with the IRM, indicating success or failure in the event data.
+ * The kernel will automatically reallocate the resources after bus resets.
+ * Should a reallocation fail, an %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event
+ * will be sent. The kernel will also automatically deallocate the resources
+ * when the file descriptor is closed.
+ *
+ * @channels is a host-endian bitfield with the most significant bit
+ * representing channel 0 and the least significant bit representing channel 63:
+ * 1ULL << (63 - c)
+ *
+ * @bandwidth is expressed in bandwidth allocation units, i.e. the time to send
+ * one quadlet of data (payload or header data) at speed S1600.
+ */
+struct fw_cdev_allocate_iso_resource {
+ __u64 closure;
+ __u64 channels;
+ __u32 bandwidth;
+ __u32 handle;
+};
+
#endif /* _LINUX_FIREWIRE_CDEV_H */
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -24,6 +24,7 @@
#include <linux/errno.h>
#include <linux/firewire-cdev.h>
#include <linux/idr.h>
+#include <linux/jiffies.h>
#include <linux/kernel.h>
#include <linux/kref.h>
#include <linux/mm.h>
@@ -35,6 +36,7 @@
#include <linux/time.h>
#include <linux/vmalloc.h>
#include <linux/wait.h>
+#include <linux/workqueue.h>
#include <asm/system.h>
#include <asm/uaccess.h>
@@ -114,6 +116,21 @@ struct descriptor_resource {
u32 data[0];
};
+struct iso_resource {
+ struct client_resource resource;
+ struct client *client;
+ /* Schedule work and access todo only with client->lock held. */
+ struct delayed_work work;
+ enum {ISO_RES_ALLOC, ISO_RES_REALLOC, ISO_RES_DEALLOC,} todo;
+ int generation;
+ u64 channels;
+ s32 bandwidth;
+ struct iso_resource_event *e_alloc, *e_dealloc;
+};
+
+static void schedule_iso_resource(struct iso_resource *);
+static void release_iso_resource(struct client *, struct client_resource *);
+
/*
* dequeue_event() just kfree()'s the event, so the event has to be
* the first field in a struct XYZ_event.
@@ -145,6 +162,11 @@ struct iso_interrupt_event {
struct fw_cdev_event_iso_interrupt interrupt;
};
+struct iso_resource_event {
+ struct event event;
+ struct fw_cdev_event_iso_resource resource;
+};
+
static inline void __user *u64_to_uptr(__u64 value)
{
return (void __user *)(unsigned long)value;
@@ -290,6 +312,16 @@ static void for_each_client(struct fw_de
mutex_unlock(&device->client_list_mutex);
}
+static int schedule_reallocations(int id, void *p, void *data)
+{
+ struct client_resource *r = p;
+
+ if (r->release == release_iso_resource)
+ schedule_iso_resource(container_of(r,
+ struct iso_resource, resource));
+ return 0;
+}
+
static void queue_bus_reset_event(struct client *client)
{
struct bus_reset_event *e;
@@ -304,6 +336,10 @@ static void queue_bus_reset_event(struct
queue_event(client, &e->event,
&e->reset, sizeof(e->reset), NULL, 0);
+
+ spin_lock_irq(&client->lock);
+ idr_for_each(&client->resource_idr, schedule_reallocations, client);
+ spin_unlock_irq(&client->lock);
}
void fw_device_cdev_update(struct fw_device *device)
@@ -376,8 +412,12 @@ static int add_client_resource(struct cl
else
ret = idr_get_new(&client->resource_idr, resource,
&resource->handle);
- if (ret >= 0)
+ if (ret >= 0) {
client_get(client);
+ if (resource->release == release_iso_resource)
+ schedule_iso_resource(container_of(resource,
+ struct iso_resource, resource));
+ }
spin_unlock_irqrestore(&client->lock, flags);
if (ret == -EAGAIN)
@@ -970,6 +1010,177 @@ static int ioctl_get_cycle_timer(struct
return 0;
}
+static void iso_resource_work(struct work_struct *work)
+{
+ struct iso_resource_event *e;
+ struct iso_resource *r =
+ container_of(work, struct iso_resource, work.work);
+ struct client *client = r->client;
+ int generation, channel, bandwidth, todo;
+ bool skip, free, success;
+
+ spin_lock_irq(&client->lock);
+ generation = client->device->generation;
+ todo = r->todo;
+ /* Allow 1000ms grace period for other reallocations. */
+ if (todo == ISO_RES_ALLOC &&
+ time_is_after_jiffies(client->device->card->reset_jiffies + HZ)) {
+ if (schedule_delayed_work(&r->work, DIV_ROUND_UP(HZ, 3)))
+ client_get(client);
+ skip = true;
+ } else {
+ /* We could be called twice within the same generation. */
+ skip = todo == ISO_RES_REALLOC &&
+ r->generation == generation;
+ }
+ free = todo == ISO_RES_DEALLOC;
+ r->generation = generation;
+ spin_unlock_irq(&client->lock);
+
+ if (skip)
+ goto out;
+
+ bandwidth = r->bandwidth;
+
+ fw_iso_resource_manage(client->device->card, generation,
+ r->channels, &channel, &bandwidth,
+ todo == ISO_RES_ALLOC || todo == ISO_RES_REALLOC);
+ /*
+ * Is this generation outdated already? As long as this resource sticks
+ * in the idr, it will be scheduled again for a newer generation or at
+ * shutdown.
+ */
+ if (channel == -EAGAIN &&
+ (todo == ISO_RES_ALLOC || todo == ISO_RES_REALLOC))
+ goto out;
+
+ success = channel >= 0 || bandwidth > 0;
+
+ spin_lock_irq(&client->lock);
+ /*
+ * Transit from allocation to reallocation, except if the client
+ * requested deallocation in the meantime.
+ */
+ if (r->todo == ISO_RES_ALLOC)
+ r->todo = ISO_RES_REALLOC;
+ /*
+ * Allocation or reallocation failure? Pull this resource out of the
+ * idr and prepare for deletion, unless the client is shutting down.
+ */
+ if (r->todo == ISO_RES_REALLOC && !success &&
+ !client->in_shutdown &&
+ idr_find(&client->resource_idr, r->resource.handle)) {
+ idr_remove(&client->resource_idr, r->resource.handle);
+ client_put(client);
+ free = true;
+ }
+ spin_unlock_irq(&client->lock);
+
+ if (todo == ISO_RES_ALLOC && channel >= 0)
+ r->channels = 1ULL << (63 - channel);
+
+ if (todo == ISO_RES_REALLOC && success)
+ goto out;
+
+ if (todo == ISO_RES_ALLOC) {
+ e = r->e_alloc;
+ r->e_alloc = NULL;
+ } else {
+ e = r->e_dealloc;
+ r->e_dealloc = NULL;
+ }
+ e->resource.handle = r->resource.handle;
+ e->resource.channel = channel;
+ e->resource.bandwidth = bandwidth;
+
+ queue_event(client, &e->event,
+ &e->resource, sizeof(e->resource), NULL, 0);
+
+ if (free) {
+ cancel_delayed_work(&r->work);
+ kfree(r->e_alloc);
+ kfree(r->e_dealloc);
+ kfree(r);
+ }
+ out:
+ client_put(client);
+}
+
+static void schedule_iso_resource(struct iso_resource *r)
+{
+ if (schedule_delayed_work(&r->work, 0))
+ client_get(r->client);
+}
+
+static void release_iso_resource(struct client *client,
+ struct client_resource *resource)
+{
+ struct iso_resource *r =
+ container_of(resource, struct iso_resource, resource);
+
+ spin_lock_irq(&client->lock);
+ r->todo = ISO_RES_DEALLOC;
+ schedule_iso_resource(r);
+ spin_unlock_irq(&client->lock);
+}
+
+static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+{
+ struct fw_cdev_allocate_iso_resource *request = buffer;
+ struct iso_resource_event *e1, *e2;
+ struct iso_resource *r;
+ int ret;
+
+ if ((request->channels == 0 && request->bandwidth == 0) ||
+ request->bandwidth > BANDWIDTH_AVAILABLE_INITIAL ||
+ request->bandwidth < 0)
+ return -EINVAL;
+
+ r = kmalloc(sizeof(*r), GFP_KERNEL);
+ e1 = kmalloc(sizeof(*e1), GFP_KERNEL);
+ e2 = kmalloc(sizeof(*e2), GFP_KERNEL);
+ if (r == NULL || e1 == NULL || e2 == NULL) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ INIT_DELAYED_WORK(&r->work, iso_resource_work);
+ r->client = client;
+ r->todo = ISO_RES_ALLOC;
+ r->generation = -1;
+ r->channels = request->channels;
+ r->bandwidth = request->bandwidth;
+ r->e_alloc = e1;
+ r->e_dealloc = e2;
+
+ e1->resource.closure = request->closure;
+ e1->resource.type = FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED;
+ e2->resource.closure = request->closure;
+ e2->resource.type = FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED;
+
+ r->resource.release = release_iso_resource;
+ ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+ if (ret < 0)
+ goto fail;
+ request->handle = r->resource.handle;
+
+ return 0;
+ fail:
+ kfree(r);
+ kfree(e1);
+ kfree(e2);
+
+ return ret;
+}
+
+static int ioctl_deallocate_iso_resource(struct client *client, void *buffer)
+{
+ struct fw_cdev_deallocate *request = buffer;
+
+ return release_client_resource(client, request->handle,
+ release_iso_resource, NULL);
+}
+
static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
ioctl_get_info,
ioctl_send_request,
@@ -984,6 +1195,8 @@ static int (* const ioctl_handlers[])(st
ioctl_start_iso,
ioctl_stop_iso,
ioctl_get_cycle_timer,
+ ioctl_allocate_iso_resource,
+ ioctl_deallocate_iso_resource,
};
static int dispatch_ioctl(struct client *client,
Index: linux/drivers/firewire/fw-iso.c
===================================================================
--- linux.orig/drivers/firewire/fw-iso.c
+++ linux/drivers/firewire/fw-iso.c
@@ -1,5 +1,7 @@
/*
- * Isochronous IO functionality
+ * Isochronous I/O functionality:
+ * - Isochronous DMA context management
+ * - Isochronous bus resource management (channels, bandwidth), client side
*
* Copyright (C) 2006 Kristian Hoegsberg <krh@bitplanet.net>
*
@@ -18,15 +20,20 @@
* Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
*/
-#include <linux/kernel.h>
-#include <linux/module.h>
#include <linux/dma-mapping.h>
-#include <linux/vmalloc.h>
+#include <linux/errno.h>
+#include <linux/firewire-constants.h>
+#include <linux/kernel.h>
#include <linux/mm.h>
+#include <linux/spinlock.h>
+#include <linux/vmalloc.h>
-#include "fw-transaction.h"
#include "fw-topology.h"
-#include "fw-device.h"
+#include "fw-transaction.h"
+
+/*
+ * Isochronous DMA context management
+ */
int fw_iso_buffer_init(struct fw_iso_buffer *buffer, struct fw_card *card,
int page_count, enum dma_data_direction direction)
@@ -153,3 +160,160 @@ int fw_iso_context_stop(struct fw_iso_co
{
return ctx->card->driver->stop_iso(ctx);
}
+
+/*
+ * Isochronous bus resource management (channels, bandwidth), client side
+ */
+
+static int manage_bandwidth(struct fw_card *card, int irm_id, int generation,
+ int bandwidth, bool allocate)
+{
+ __be32 data[2];
+ int try, new, old = allocate ? BANDWIDTH_AVAILABLE_INITIAL : 0;
+
+ /*
+ * On a 1394a IRM with low contention, try < 1 is enough.
+ * On a 1394-1995 IRM, we need at least try < 2.
+ * Let's just do try < 5.
+ */
+ for (try = 0; try < 5; try++) {
+ new = allocate ? old - bandwidth : old + bandwidth;
+ if (new < 0 || new > BANDWIDTH_AVAILABLE_INITIAL)
+ break;
+
+ data[0] = cpu_to_be32(old);
+ data[1] = cpu_to_be32(new);
+ switch (fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP,
+ irm_id, generation, SCODE_100,
+ CSR_REGISTER_BASE + CSR_BANDWIDTH_AVAILABLE,
+ data, sizeof(data))) {
+ case RCODE_GENERATION:
+ /* A generation change frees all bandwidth. */
+ return allocate ? -EAGAIN : bandwidth;
+
+ case RCODE_COMPLETE:
+ if (be32_to_cpup(data) == old)
+ return bandwidth;
+
+ old = be32_to_cpup(data);
+ /* Fall through. */
+ }
+ }
+
+ return -EIO;
+}
+
+static int manage_channel(struct fw_card *card, int irm_id, int generation,
+ __be32 channels_mask, u64 offset, bool allocate)
+{
+ __be32 data[2], c, old = allocate ? cpu_to_be32(~0) : 0;
+ int i, retry = 5;
+
+ for (i = 0; i < 32; i++) {
+ c = cpu_to_be32(1 << (31 - i));
+ if (!(channels_mask & c))
+ continue;
+
+ if (allocate == !(old & c))
+ continue;
+
+ data[0] = old;
+ data[1] = old ^ c;
+ switch (fw_run_transaction(card, TCODE_LOCK_COMPARE_SWAP,
+ irm_id, generation, SCODE_100,
+ offset, data, sizeof(data))) {
+ case RCODE_GENERATION:
+ /* A generation change frees all channels. */
+ return allocate ? -EAGAIN : i;
+
+ case RCODE_COMPLETE:
+ if (data[0] == old)
+ return i;
+
+ old = data[0];
+
+ /* Is the IRM 1394a-2000 compliant? */
+ if ((data[0] & c) != (data[1] & c))
+ continue;
+
+ /* 1394-1995 IRM, fall through to retry. */
+ default:
+ if (retry--)
+ i--;
+ }
+ }
+
+ return -EIO;
+}
+
+static void deallocate_channel(struct fw_card *card, int irm_id,
+ int generation, int channel)
+{
+ __be32 mask;
+ u64 offset;
+
+ mask = channel < 32 ? cpu_to_be32(1 << (31 - channel)) :
+ cpu_to_be32(1 << (63 - channel));
+ offset = channel < 32 ? CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI :
+ CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO;
+
+ manage_channel(card, irm_id, generation, mask, offset, false);
+}
+
+/**
+ * fw_iso_resource_manage - Allocate or deallocate a channel and/or bandwidth
+ *
+ * In parameters: card, generation, channels_mask, bandwidth, allocate
+ * Out parameters: channel, bandwidth
+ * This function blocks (sleeps) during communication with the IRM.
+ * Allocates or deallocates at most one channel out of channels_mask.
+ *
+ * Returns channel < 0 if no channel was allocated or deallocated.
+ * Returns bandwidth = 0 if no bandwidth was allocated or deallocated.
+ *
+ * If generation is stale, deallocations succeed but allocations fail with
+ * channel = -EAGAIN.
+ *
+ * If channel (de)allocation fails, bandwidth (de)allocation fails too.
+ * If bandwidth allocation fails, no channel will be allocated either.
+ * If bandwidth deallocation fails, channel deallocation may still have been
+ * successful.
+ */
+void fw_iso_resource_manage(struct fw_card *card, int generation,
+ u64 channels_mask, int *channel, int *bandwidth,
+ bool allocate)
+{
+ __be32 channels_hi = cpu_to_be32(channels_mask >> 32);
+ __be32 channels_lo = cpu_to_be32(channels_mask);
+ int irm_id, ret, c = -EINVAL;
+
+ spin_lock_irq(&card->lock);
+ irm_id = card->irm_node->node_id;
+ spin_unlock_irq(&card->lock);
+
+ if (channels_hi)
+ c = manage_channel(card, irm_id, generation, channels_hi,
+ CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_HI, allocate);
+ if (channels_lo && c < 0) {
+ c = manage_channel(card, irm_id, generation, channels_lo,
+ CSR_REGISTER_BASE + CSR_CHANNELS_AVAILABLE_LO, allocate);
+ if (c >= 0)
+ c += 32;
+ }
+ *channel = c;
+
+ if (channels_mask != 0 && c < 0)
+ *bandwidth = 0;
+
+ if (*bandwidth == 0)
+ return;
+
+ ret = manage_bandwidth(card, irm_id, generation, *bandwidth, allocate);
+ if (ret < 0)
+ *bandwidth = 0;
+
+ if (ret < 0 && c >= 0 && allocate) {
+ deallocate_channel(card, irm_id, generation, c);
+ *channel = ret;
+ }
+}
Index: linux/drivers/firewire/fw-transaction.h
===================================================================
--- linux.orig/drivers/firewire/fw-transaction.h
+++ linux/drivers/firewire/fw-transaction.h
@@ -82,6 +82,7 @@
#define CSR_SPEED_MAP 0x2000
#define CSR_SPEED_MAP_END 0x3000
+#define BANDWIDTH_AVAILABLE_INITIAL 4915
#define BROADCAST_CHANNEL_INITIAL (1 << 31 | 31)
#define BROADCAST_CHANNEL_VALID (1 << 30)
@@ -343,6 +344,9 @@ int fw_iso_context_start(struct fw_iso_c
int fw_iso_context_stop(struct fw_iso_context *ctx);
void fw_iso_context_destroy(struct fw_iso_context *ctx);
+void fw_iso_resource_manage(struct fw_card *card, int generation,
+ u64 channels_mask, int *channel, int *bandwidth, bool allocate);
+
struct fw_card_driver {
/*
* Enable the given card with the given initial config rom.
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread[parent not found: <1231404355.18613.68.camel@localhost.localdomain>]
* Re: [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management
2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
[not found] ` <1231404355.18613.68.camel@localhost.localdomain>
@ 2009-01-11 20:32 ` David Moore
1 sibling, 0 replies; 17+ messages in thread
From: David Moore @ 2009-01-11 20:32 UTC (permalink / raw)
To: Stefan Richter; +Cc: linux1394-devel, linux-kernel
On Sun, 2009-01-04 at 16:26 +0100, Stefan Richter wrote:
> Based on
> Date: Tue, 18 Nov 2008 11:41:27 -0500
> From: Jay Fenlason <fenlason@redhat.com>
> Subject: [Patch V4] Add ISO resource management support
> with several changes to the ABI and implementation. Only the part of
> the ABI which enables auto-reallocation and auto-deallocation is
> included here.
>
> This implements ioctls for kernel-assisted allocation of isochronous
> channels and isochronous bandwidth. The benefits are:
> - The client does not have to have write access to the /dev/fw* device
> corresponding to the IRM.
> - The client does not have to perform reallocation after bus resets.
> - Channel and bandwidth are deallocated by the kernel if the file is
> closed before the client deallocated the resources. Thus resources
> are released even if the client crashes.
>
> It is anticipated that future in-kernel code (firewire-core IRM code;
> the firewire port of firedtv), will use the fw-iso.c portions of this
> code too.
>
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Tested-by: David Moore <dcm@acm.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 06/11] firewire: cdev: add ioctls for manual iso resource management
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (4 preceding siblings ...)
2009-01-04 15:26 ` [PATCH 05/11] firewire: cdev: add ioctls for isochronous resource management Stefan Richter
@ 2009-01-04 15:27 ` Stefan Richter
[not found] ` <1231643968.3538.59.camel@localhost.localdomain>
2009-01-04 15:28 ` [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed Stefan Richter
` (5 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:27 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
This adds ioctls for allocation and deallocation of a channel or/and
bandwidth without auto-reallocation and without auto-deallocation.
The benefit of these ioctls is that libraw1394-style isochronous
resource management can be implemented without write access to the IRM's
character device file.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 67 +++++++++++++++++++++++++++-------
include/linux/firewire-cdev.h | 38 +++++++++++++++----
2 files changed, 84 insertions(+), 21 deletions(-)
Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -151,7 +151,7 @@ struct fw_cdev_event_iso_interrupt {
/**
* struct fw_cdev_event_iso_resource - Iso resources were allocated or freed
* @closure: See &fw_cdev_event_common;
- * set by %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl
+ * set by %FW_CDEV_IOC_(DE)ALLOCATE_ISO_RESOURCE(_ONCE) ioctl
* @type: %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED or
* %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED
* @handle: Reference by which an allocated resource can be deallocated
@@ -164,12 +164,12 @@ struct fw_cdev_event_iso_interrupt {
* resource was allocated at the IRM. The client has to check @channel and
* @bandwidth for whether the allocation actually succeeded.
*
- * @channel is <0 if no channel was allocated.
- * @bandwidth is 0 if no bandwidth was allocated.
- *
* An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event is sent after an isochronous
* resource was deallocated at the IRM. It is also sent when automatic
* reallocation after a bus reset failed.
+ *
+ * @channel is <0 if no channel was (de)allocated or if reallocation failed.
+ * @bandwidth is 0 if no bandwidth was (de)allocated or if reallocation failed.
*/
struct fw_cdev_event_iso_resource {
__u64 closure;
@@ -227,6 +227,8 @@ union fw_cdev_event {
/* available since kernel version 2.6.30 */
#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE _IOWR('#', 0x0d, struct fw_cdev_allocate_iso_resource)
#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE _IOW('#', 0x0e, struct fw_cdev_deallocate)
+#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x0f, struct fw_cdev_allocate_iso_resource)
+#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x10, struct fw_cdev_allocate_iso_resource)
/* FW_CDEV_VERSION History
*
@@ -523,11 +525,12 @@ struct fw_cdev_get_cycle_timer {
};
/**
- * struct fw_cdev_allocate_iso_resource - Allocate a channel or bandwidth
+ * struct fw_cdev_allocate_iso_resource - (De)allocate a channel or bandwidth
* @closure: Passed back to userspace in correponding iso resource events
- * @channels: Isochronous channels of which one is to be allocated
- * @bandwidth: Isochronous bandwidth units to be allocated
- * @handle: Handle to the allocation, written by the kernel
+ * @channels: Isochronous channels of which one is to be (de)allocated
+ * @bandwidth: Isochronous bandwidth units to be (de)allocated
+ * @handle: Handle to the allocation, written by the kernel (only valid in
+ * case of %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctls)
*
* The %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE ioctl initiates allocation of an
* isochronous channel and/or of isochronous bandwidth at the isochronous
@@ -539,6 +542,25 @@ struct fw_cdev_get_cycle_timer {
* will be sent. The kernel will also automatically deallocate the resources
* when the file descriptor is closed.
*
+ * The %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE ioctl can be used to initiate
+ * deallocation of resources which were allocated as described above.
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event concludes this operation.
+ *
+ * The %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE ioctl is a variant of allocation
+ * without automatic re- or deallocation.
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED event concludes this operation,
+ * indicating success or failure in its data.
+ *
+ * The %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE ioctl works like
+ * %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE except that resources are freed
+ * instead of allocated. At most one channel may be specified in this ioctl.
+ * An %FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED event concludes this operation.
+ *
+ * To summarize, %FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE allocates iso resources
+ * for the lifetime of the fd or handle.
+ * In contrast, %FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE allocates iso resources
+ * for the duration of a bus generation.
+ *
* @channels is a host-endian bitfield with the most significant bit
* representing channel 0 and the least significant bit representing channel 63:
* 1ULL << (63 - c)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -121,14 +121,15 @@ struct iso_resource {
struct client *client;
/* Schedule work and access todo only with client->lock held. */
struct delayed_work work;
- enum {ISO_RES_ALLOC, ISO_RES_REALLOC, ISO_RES_DEALLOC,} todo;
+ enum {ISO_RES_ALLOC, ISO_RES_REALLOC, ISO_RES_DEALLOC,
+ ISO_RES_ALLOC_ONCE, ISO_RES_DEALLOC_ONCE,} todo;
int generation;
u64 channels;
s32 bandwidth;
struct iso_resource_event *e_alloc, *e_dealloc;
};
-static void schedule_iso_resource(struct iso_resource *);
+static int schedule_iso_resource(struct iso_resource *);
static void release_iso_resource(struct client *, struct client_resource *);
/*
@@ -1033,7 +1034,9 @@ static void iso_resource_work(struct wor
skip = todo == ISO_RES_REALLOC &&
r->generation == generation;
}
- free = todo == ISO_RES_DEALLOC;
+ free = todo == ISO_RES_DEALLOC ||
+ todo == ISO_RES_ALLOC_ONCE ||
+ todo == ISO_RES_DEALLOC_ONCE;
r->generation = generation;
spin_unlock_irq(&client->lock);
@@ -1044,7 +1047,9 @@ static void iso_resource_work(struct wor
fw_iso_resource_manage(client->device->card, generation,
r->channels, &channel, &bandwidth,
- todo == ISO_RES_ALLOC || todo == ISO_RES_REALLOC);
+ todo == ISO_RES_ALLOC ||
+ todo == ISO_RES_REALLOC ||
+ todo == ISO_RES_ALLOC_ONCE);
/*
* Is this generation outdated already? As long as this resource sticks
* in the idr, it will be scheduled again for a newer generation or at
@@ -1082,7 +1087,7 @@ static void iso_resource_work(struct wor
if (todo == ISO_RES_REALLOC && success)
goto out;
- if (todo == ISO_RES_ALLOC) {
+ if (todo == ISO_RES_ALLOC || todo == ISO_RES_ALLOC_ONCE) {
e = r->e_alloc;
r->e_alloc = NULL;
} else {
@@ -1106,10 +1111,17 @@ static void iso_resource_work(struct wor
client_put(client);
}
-static void schedule_iso_resource(struct iso_resource *r)
+static int schedule_iso_resource(struct iso_resource *r)
{
- if (schedule_delayed_work(&r->work, 0))
- client_get(r->client);
+ int scheduled;
+
+ client_get(r->client);
+
+ scheduled = schedule_delayed_work(&r->work, 0);
+ if (!scheduled)
+ client_put(r->client);
+
+ return scheduled;
}
static void release_iso_resource(struct client *client,
@@ -1124,9 +1136,9 @@ static void release_iso_resource(struct
spin_unlock_irq(&client->lock);
}
-static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+static int init_iso_resource(struct client *client,
+ struct fw_cdev_allocate_iso_resource *request, int todo)
{
- struct fw_cdev_allocate_iso_resource *request = buffer;
struct iso_resource_event *e1, *e2;
struct iso_resource *r;
int ret;
@@ -1146,7 +1158,7 @@ static int ioctl_allocate_iso_resource(s
INIT_DELAYED_WORK(&r->work, iso_resource_work);
r->client = client;
- r->todo = ISO_RES_ALLOC;
+ r->todo = todo;
r->generation = -1;
r->channels = request->channels;
r->bandwidth = request->bandwidth;
@@ -1158,8 +1170,14 @@ static int ioctl_allocate_iso_resource(s
e2->resource.closure = request->closure;
e2->resource.type = FW_CDEV_EVENT_ISO_RESOURCE_DEALLOCATED;
- r->resource.release = release_iso_resource;
- ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+ if (todo == ISO_RES_ALLOC) {
+ r->resource.release = release_iso_resource;
+ ret = add_client_resource(client, &r->resource, GFP_KERNEL);
+ } else {
+ r->resource.release = NULL;
+ r->resource.handle = -1;
+ ret = schedule_iso_resource(r) ? 0 : -ENOMEM;
+ }
if (ret < 0)
goto fail;
request->handle = r->resource.handle;
@@ -1173,6 +1191,13 @@ static int ioctl_allocate_iso_resource(s
return ret;
}
+static int ioctl_allocate_iso_resource(struct client *client, void *buffer)
+{
+ struct fw_cdev_allocate_iso_resource *request = buffer;
+
+ return init_iso_resource(client, request, ISO_RES_ALLOC);
+}
+
static int ioctl_deallocate_iso_resource(struct client *client, void *buffer)
{
struct fw_cdev_deallocate *request = buffer;
@@ -1181,6 +1206,20 @@ static int ioctl_deallocate_iso_resource
release_iso_resource, NULL);
}
+static int ioctl_allocate_iso_resource_once(struct client *client, void *buffer)
+{
+ struct fw_cdev_allocate_iso_resource *request = buffer;
+
+ return init_iso_resource(client, request, ISO_RES_ALLOC_ONCE);
+}
+
+static int ioctl_deallocate_iso_resource_once(struct client *client, void *buffer)
+{
+ struct fw_cdev_allocate_iso_resource *request = buffer;
+
+ return init_iso_resource(client, request, ISO_RES_DEALLOC_ONCE);
+}
+
static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
ioctl_get_info,
ioctl_send_request,
@@ -1197,6 +1236,8 @@ static int (* const ioctl_handlers[])(st
ioctl_get_cycle_timer,
ioctl_allocate_iso_resource,
ioctl_deallocate_iso_resource,
+ ioctl_allocate_iso_resource_once,
+ ioctl_deallocate_iso_resource_once,
};
static int dispatch_ioctl(struct client *client,
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (5 preceding siblings ...)
2009-01-04 15:27 ` [PATCH 06/11] firewire: cdev: add ioctls for manual iso " Stefan Richter
@ 2009-01-04 15:28 ` Stefan Richter
2009-01-04 15:29 ` [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests Stefan Richter
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:28 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
While the speed of asynchronous transactions is automatically chosen by
the kernel, the speed of isochronous streams has to be chosen by the
initiating client.
In case of 1394a bus topologies, the maximum possible speed could be
figured out with some effort by evaluation of the remote node's link
speed field in the config ROM, the local node's link speed field, and
the PHY speeds and topologic information in the local node's or IRM's
topology map CSR. However, this does not work in case of 1394b buses.
Hence add an ioctl to export the maximum speed which the kernel already
determined.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 10 ++++++++++
include/linux/firewire-cdev.h | 10 ++++++++++
2 files changed, 20 insertions(+)
Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -229,6 +229,7 @@ union fw_cdev_event {
#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE _IOW('#', 0x0e, struct fw_cdev_deallocate)
#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x0f, struct fw_cdev_allocate_iso_resource)
#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x10, struct fw_cdev_allocate_iso_resource)
+#define FW_CDEV_IOC_GET_SPEED _IOR('#', 0x11, struct fw_cdev_get_speed)
/* FW_CDEV_VERSION History
*
@@ -575,4 +576,13 @@ struct fw_cdev_allocate_iso_resource {
__u32 handle;
};
+/**
+ * struct fw_cdev_get_speed - Query maximum speed to or from this device
+ * @max_speed: Speed code; minimum of the device's link speed, the local node's
+ * link speed, and all PHY port speeds between the two links
+ */
+struct fw_cdev_get_speed {
+ __u32 max_speed;
+};
+
#endif /* _LINUX_FIREWIRE_CDEV_H */
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -1220,6 +1220,15 @@ static int ioctl_deallocate_iso_resource
return init_iso_resource(client, request, ISO_RES_DEALLOC_ONCE);
}
+static int ioctl_get_speed(struct client *client, void *buffer)
+{
+ struct fw_cdev_get_speed *request = buffer;
+
+ request->max_speed = client->device->max_speed;
+
+ return 0;
+}
+
static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
ioctl_get_info,
ioctl_send_request,
@@ -1238,6 +1247,7 @@ static int (* const ioctl_handlers[])(st
ioctl_deallocate_iso_resource,
ioctl_allocate_iso_resource_once,
ioctl_deallocate_iso_resource_once,
+ ioctl_get_speed,
};
static int dispatch_ioctl(struct client *client,
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (6 preceding siblings ...)
2009-01-04 15:28 ` [PATCH 07/11] firewire: cdev: add ioctl to query maximum transmission speed Stefan Richter
@ 2009-01-04 15:29 ` Stefan Richter
2009-01-04 15:30 ` [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space Stefan Richter
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:29 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Write transactions to the broadcast node ID are a convenient way to
trigger functions of multiple nodes at once. IIDC is a protocol which
can make use of this if multiple cameras with same command_regs_base are
connected at the same bus.
Based on
Date: Wed, 10 Sep 2008 11:32:16 -0400
From: Jay Fenlason <fenlason@redhat.com>
Subject: [patch] SEND_BROADCAST_REQUEST
Changes: ioctl_send_request() and ioctl_send_broadcast_request() now
share code. Broadcast speed corrected to S100. Check for proper tcode.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 74 +++++++++++++++++++++-------------
include/linux/firewire-cdev.h | 1
2 files changed, 48 insertions(+), 27 deletions(-)
Index: linux/include/linux/firewire-cdev.h
===================================================================
--- linux.orig/include/linux/firewire-cdev.h
+++ linux/include/linux/firewire-cdev.h
@@ -230,6 +230,7 @@ union fw_cdev_event {
#define FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x0f, struct fw_cdev_allocate_iso_resource)
#define FW_CDEV_IOC_DEALLOCATE_ISO_RESOURCE_ONCE _IOW('#', 0x10, struct fw_cdev_allocate_iso_resource)
#define FW_CDEV_IOC_GET_SPEED _IOR('#', 0x11, struct fw_cdev_get_speed)
+#define FW_CDEV_IOC_SEND_BROADCAST_REQUEST _IOW('#', 0x12, struct fw_cdev_send_request)
/* FW_CDEV_VERSION History
*
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -518,10 +518,10 @@ static void complete_transaction(struct
client_put(client);
}
-static int ioctl_send_request(struct client *client, void *buffer)
+static int init_request(struct client *client,
+ struct fw_cdev_send_request *request,
+ int destination_id, int speed)
{
- struct fw_device *device = client->device;
- struct fw_cdev_send_request *request = buffer;
struct outbound_transaction_event *e;
int ret;
@@ -544,24 +544,6 @@ static int ioctl_send_request(struct cli
goto failed;
}
- switch (request->tcode) {
- case TCODE_WRITE_QUADLET_REQUEST:
- case TCODE_WRITE_BLOCK_REQUEST:
- case TCODE_READ_QUADLET_REQUEST:
- case TCODE_READ_BLOCK_REQUEST:
- case TCODE_LOCK_MASK_SWAP:
- case TCODE_LOCK_COMPARE_SWAP:
- case TCODE_LOCK_FETCH_ADD:
- case TCODE_LOCK_LITTLE_ADD:
- case TCODE_LOCK_BOUNDED_ADD:
- case TCODE_LOCK_WRAP_ADD:
- case TCODE_LOCK_VENDOR_DEPENDENT:
- break;
- default:
- ret = -EINVAL;
- goto failed;
- }
-
e->r.resource.release = release_transaction;
ret = add_client_resource(client, &e->r.resource, GFP_KERNEL);
if (ret < 0)
@@ -570,12 +552,9 @@ static int ioctl_send_request(struct cli
/* Get a reference for the transaction callback */
client_get(client);
- fw_send_request(device->card, &e->r.transaction,
- request->tcode & 0x1f,
- device->node->node_id,
- request->generation,
- device->max_speed,
- request->offset,
+ fw_send_request(client->device->card, &e->r.transaction,
+ request->tcode & 0x1f, destination_id,
+ request->generation, speed, request->offset,
e->response.data, request->length,
complete_transaction, e);
@@ -589,6 +568,31 @@ static int ioctl_send_request(struct cli
return ret;
}
+static int ioctl_send_request(struct client *client, void *buffer)
+{
+ struct fw_cdev_send_request *request = buffer;
+
+ switch (request->tcode) {
+ case TCODE_WRITE_QUADLET_REQUEST:
+ case TCODE_WRITE_BLOCK_REQUEST:
+ case TCODE_READ_QUADLET_REQUEST:
+ case TCODE_READ_BLOCK_REQUEST:
+ case TCODE_LOCK_MASK_SWAP:
+ case TCODE_LOCK_COMPARE_SWAP:
+ case TCODE_LOCK_FETCH_ADD:
+ case TCODE_LOCK_LITTLE_ADD:
+ case TCODE_LOCK_BOUNDED_ADD:
+ case TCODE_LOCK_WRAP_ADD:
+ case TCODE_LOCK_VENDOR_DEPENDENT:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return init_request(client, request, client->device->node->node_id,
+ client->device->max_speed);
+}
+
static void release_request(struct client *client,
struct client_resource *resource)
{
@@ -1229,6 +1233,21 @@ static int ioctl_get_speed(struct client
return 0;
}
+static int ioctl_send_broadcast_request(struct client *client, void *buffer)
+{
+ struct fw_cdev_send_request *request = buffer;
+
+ switch (request->tcode) {
+ case TCODE_WRITE_QUADLET_REQUEST:
+ case TCODE_WRITE_BLOCK_REQUEST:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return init_request(client, request, LOCAL_BUS | 0x3f, SCODE_100);
+}
+
static int (* const ioctl_handlers[])(struct client *client, void *buffer) = {
ioctl_get_info,
ioctl_send_request,
@@ -1248,6 +1267,7 @@ static int (* const ioctl_handlers[])(st
ioctl_allocate_iso_resource_once,
ioctl_deallocate_iso_resource_once,
ioctl_get_speed,
+ ioctl_send_broadcast_request,
};
static int dispatch_ioctl(struct client *client,
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (7 preceding siblings ...)
2009-01-04 15:29 ` [PATCH 08/11] firewire: cdev: add ioctl for broadcast write requests Stefan Richter
@ 2009-01-04 15:30 ` Stefan Richter
2009-01-04 15:30 ` [PATCH 10/11] firewire: cdev: extend transaction payload size check Stefan Richter
` (2 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:30 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
We don't want random users write to Memory Space (e.g. PCs with physical
DMA filters down) or to core CSRs like Reset_Start.
This does not protect SBP-2 target CSRs. But properly behaving SBP-2
targets ignore broadcast write requests to these registers, and the
maximum damage which can happen with laxer targets is DOS. But there
are ways to create DOS situations anyway if there are devices with weak
device file permissions (like audio/video devices) present at the same
bus as an SBP-2 target.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 4 ++++
1 file changed, 4 insertions(+)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -1245,6 +1245,10 @@ static int ioctl_send_broadcast_request(
return -EINVAL;
}
+ /* Security policy: Only allow accesses to Units Space. */
+ if (request->offset < CSR_REGISTER_BASE + CSR_CONFIG_ROM_END)
+ return -EACCES;
+
return init_request(client, request, LOCAL_BUS | 0x3f, SCODE_100);
}
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 10/11] firewire: cdev: extend transaction payload size check
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (8 preceding siblings ...)
2009-01-04 15:30 ` [PATCH 09/11] firewire: cdev: restrict broadcast write requests to Units Space Stefan Richter
@ 2009-01-04 15:30 ` Stefan Richter
2009-01-04 15:31 ` [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq Stefan Richter
2009-01-04 23:28 ` [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:30 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
Make the size check of ioctl_send_request and
ioctl_send_broadcast_request speed dependent. Also change the error
return code from -EINVAL to -EIO to distinguish this from other errors
concerning the ioctl parameters.
Another payload size limit for which we don't check here though is the
remote node's Bus_Info_Block.max_rec.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -525,9 +525,8 @@ static int init_request(struct client *c
struct outbound_transaction_event *e;
int ret;
- /* What is the biggest size we'll accept, really? */
- if (request->length > 4096)
- return -EINVAL;
+ if (request->length > 4096 || request->length > 512 << speed)
+ return -EIO;
e = kmalloc(sizeof(*e) + request->length, GFP_KERNEL);
if (e == NULL)
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (9 preceding siblings ...)
2009-01-04 15:30 ` [PATCH 10/11] firewire: cdev: extend transaction payload size check Stefan Richter
@ 2009-01-04 15:31 ` Stefan Richter
2009-01-04 23:28 ` [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 15:31 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
All of these functions are entered with IRQs enabled.
Hence the unconditional spin_unlock_irq can be used.
Function: Caller context:
dequeue_event() client process, via read(2)
fill_bus_reset_event() fw-device.c update worqueue job
release_client_resource() client process, via ioctl(2)
fw_device_op_release() client process, via close(2)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/firewire/fw-cdev.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -237,7 +237,6 @@ static void queue_event(struct client *c
static int dequeue_event(struct client *client,
char __user *buffer, size_t count)
{
- unsigned long flags;
struct event *event;
size_t size, total;
int i, ret;
@@ -252,10 +251,10 @@ static int dequeue_event(struct client *
fw_device_is_shutdown(client->device))
return -ENODEV;
- spin_lock_irqsave(&client->lock, flags);
+ spin_lock_irq(&client->lock);
event = list_first_entry(&client->event_list, struct event, link);
list_del(&event->link);
- spin_unlock_irqrestore(&client->lock, flags);
+ spin_unlock_irq(&client->lock);
total = 0;
for (i = 0; i < ARRAY_SIZE(event->v) && total < count; i++) {
@@ -286,9 +285,8 @@ static void fill_bus_reset_event(struct
struct client *client)
{
struct fw_card *card = client->device->card;
- unsigned long flags;
- spin_lock_irqsave(&card->lock, flags);
+ spin_lock_irq(&card->lock);
event->closure = client->bus_reset_closure;
event->type = FW_CDEV_EVENT_BUS_RESET;
@@ -299,7 +297,7 @@ static void fill_bus_reset_event(struct
event->irm_node_id = card->irm_node->node_id;
event->root_node_id = card->root_node->node_id;
- spin_unlock_irqrestore(&card->lock, flags);
+ spin_unlock_irq(&card->lock);
}
static void for_each_client(struct fw_device *device,
@@ -432,16 +430,15 @@ static int release_client_resource(struc
struct client_resource **resource)
{
struct client_resource *r;
- unsigned long flags;
- spin_lock_irqsave(&client->lock, flags);
+ spin_lock_irq(&client->lock);
if (client->in_shutdown)
r = NULL;
else
r = idr_find(&client->resource_idr, handle);
if (r && r->release == release)
idr_remove(&client->resource_idr, handle);
- spin_unlock_irqrestore(&client->lock, flags);
+ spin_unlock_irq(&client->lock);
if (!(r && r->release == release))
return -EINVAL;
@@ -1384,7 +1381,6 @@ static int fw_device_op_release(struct i
{
struct client *client = file->private_data;
struct event *e, *next_e;
- unsigned long flags;
mutex_lock(&client->device->client_list_mutex);
list_del(&client->link);
@@ -1397,9 +1393,9 @@ static int fw_device_op_release(struct i
fw_iso_context_destroy(client->iso_context);
/* Freeze client->resource_idr and client->event_list */
- spin_lock_irqsave(&client->lock, flags);
+ spin_lock_irq(&client->lock);
client->in_shutdown = true;
- spin_unlock_irqrestore(&client->lock, flags);
+ spin_unlock_irq(&client->lock);
idr_for_each(&client->resource_idr, shutdown_resource, client);
idr_remove_all(&client->resource_idr);
--
Stefan Richter
-=====-==--= ---= --=--
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 00/11] firewire: cdev: proposed ABI extensions
2009-01-04 15:23 [PATCH 00/11] firewire: cdev: proposed ABI extensions Stefan Richter
` (10 preceding siblings ...)
2009-01-04 15:31 ` [PATCH 11/11] firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq Stefan Richter
@ 2009-01-04 23:28 ` Stefan Richter
11 siblings, 0 replies; 17+ messages in thread
From: Stefan Richter @ 2009-01-04 23:28 UTC (permalink / raw)
To: linux1394-devel; +Cc: linux-kernel
I wrote:
> These patches apply after several as yet uncommitted firewire patches,
I committed the prerequisites to linux1394-2.6.git now, branch: cdev.
I also committed all the new patches to branch: test, for those who'd
rather look through git or gitweb at the patches.
The linux1394-2.6.git branches misc, cdev, topology, for-next, and
master have been rebased onto v2.6.28.
--
Stefan Richter
-=====-==--= ---= --=-=
http://arcgraph.de/sr/
^ permalink raw reply [flat|nested] 17+ messages in thread