Add an FW_CDEV_IOC_RECEIVE_PHY_PACKETS ioctl() and
FW_CDEV_EVENT_PHY_PACKET_RECEIVED poll()/read() event for /dev/fw*.
This can be used to get information from remote PHYs by remote access
PHY packets.
This is also the 2nd half of the functionality (the receive part) to
support a userspace implementation of a VersaPHY transaction layer.
Safety considerations:
- PHY packets are generally broadcasts, hence some kind of elevated
privileges should be required of a process to be able to listen in
on PHY packets. This implementation assumes that a process that is
allowed to open the /dev/fw* of a local node does have this
privilege.
There was an inconclusive discussion about introducing POSIX
capabilities as a means to check for user privileges for these
kinds of operations.
Other limitations:
- PHY packet reception may be switched on by ioctl() but cannot be
switched off again. It would be trivial to provide an off switch,
but this is not worth the code. The client should simply close()
the fd then, or just ignore further events.
- For sake of simplicity of API and kernel-side implementation, no
filter per packet content is provided.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Add an FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* which can be
used to implement bus management related functionality in userspace.
This is also half of the functionality (the transmit part) that is
needed to support a userspace implementation of a VersaPHY transaction
layer.
Safety considerations:
- PHY packets are generally broadcasts and may have interesting
effects on PHYs and the bus, e.g. make asynchronous arbitration
impossible due to too low gap count. Hence some kind of elevated
privileges should be required of a process to be able to send
PHY packets. This implementation assumes that a process that is
allowed to open the /dev/fw* of a local node does have this
privilege.
There was an inconclusive discussion about introducing POSIX
capabilities as a means to check for user privileges for these
kinds of operations.
- The kernel does not check integrity of the supplied packet data.
That would be far too much code, considering the many kinds of
PHY packets. A process which got the privilege to send these
packets is trusted to do it correctly.
Just like with the other "send packet" ioctls, a non-blocking API is
chosen; i.e. the ioctl may return even before AT DMA started. After
transmission, an event for poll()/read() is enqueued. Most users are
going to need a blocking API, but a blocking userspace wrapper is easy
to implement, and the second of the two existing libraw1394 calls
raw1394_phy_packet_write() and raw1394_start_phy_packet_write() can be
better supported that way.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Bus resets which are triggered
- by the kernel drivers after updates of the local nodes' config ROM,
- by userspace software via ioctl
shall be deferred until after >=2 seconds after the last bus reset.
If multiple modifications of the local nodes' config ROM happen in a row,
only a single bus reset should happen after them.
When the local node's link goes from inactive to active or vice versa,
and at the two occasions of bus resets mentioned above --- and if the
current gap count differs from 63 --- the bus reset should be preceded
by a PHY configuration packet that reaffirms the gap count. Otherwise a
bus manager would have to reset the bus again right after that.
This is necessary to promote bus stability, e.g. leave grace periods for
allocations and reallocations of isochronous channels and bandwidth,
SBP-2 reconnections etc.; see IEEE 1394 clause 8.2.1.
This change implements all of the above by moving bus reset initiation
into a delayed work (except for bus resets which are triggered by the
bus manager workqueue job and are performed there immediately). It
comes with a necessary addition to the card driver methods that allows
to get the current gap count from PHY registers.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The FW_ISO_ constants of the in-kernel API of firewire-core and
FW_CDEV_ISO_ constants of the userspace API of firewire-core have
nothing to do with each other --- except that the core-cdev.c
implementation relies on them having the same values.
Hence put some compile-time assertions into core-cdev.c. It's lame but
I prefer it over including the userspace API header into the kernelspace
API header and defining kernelspace API constants from userspace API
constants. Nor do I want to expose the kernelspace constants in one of
the two firewire headers that are exported to userland since this only
concerns the core-cdev.c implementation.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Check that the data length of a write quadlet request actually is large
enough for a quadlet. Otherwise, fw_fill_request could access the four
bytes after the end of the outbound_transaction_event structure.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Modification of Clemens' change: Consolidate the check into
init_request() which is used by the affected ioctl_send_request() and
ioctl_send_broadcast_request() and the unaffected
ioctl_send_stream_packet(), to save a few lines of code.
Note, since struct outbound_transaction_event *e is slab-allocated, such
an out-of-bounds access won't hit unallocated memory but may result in a
(virtually impossible to exploit) information disclosure.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fix an obscure ABI feature that is a bit of a hassle to implement.
However, somebody put it into the ABI, so let's fill in a sensible
value there.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
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>
libraw1394 v2.0.0...v2.0.5 takes FW_CDEV_VERSION from an externally
installed header file and uses it to declare its own implementation
level in FW_CDEV_IOC_GET_INFO. This is wrong; it should set the real
version for which it was actually written.
If we add features to the kernel ABI that require the kernel to check
a client's implementation level, we can not trust the client version if
it was set from FW_CDEV_VERSION.
Hence freeze FW_CDEV_VERSION at the current value (no damage has been
done yet), clearly document FW_CDEV_VERSION as a dummy version and what
clients are expected to do with fw_cdev_get_info.version, and use a new
defined constant (which is not placed into the exported header file) as
kernel implementation level.
Note, in order to check in client program source code which features are
present in an externally installed linux/firewire-cdev.h, use
preprocessor directives like
#ifdef FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE
or
#ifdef FW_CDEV_EVENT_ISO_RESOURCE_ALLOCATED
instead of a check of FW_CDEV_VERSION.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If a request comes in to an address range managed by a userspace driver
i.e. <linux/firewire-cdev.h> client, the card instance of request and
response may differ from the card instance of the client device.
Therefore we need to take a reference of the card until the response was
sent.
I thought about putting the reference counting into core-transaction.c,
but the various high-level drivers besides cdev clients (firewire-net,
firewire-sbp2, firedtv) use the card pointer in their fw_address_handler
address_callback method only to look up devices of which they already
hold the necessary references. So this seems to be a specific
firewire-cdev issue which is better addressed locally.
We do not need the reference
- in case of FCP_REQUEST or FCP_RESPONSE requests because then the
firewire-core will send the split transaction response for us
already in the context of the request handler,
- if it is the same card as the client device's because we hold a
card reference indirectly via teh client->device reference.
To keep things simple, we take the reference nevertheless.
Jay Fenlason wrote:
> there's no way for the core to tell cdev "this card is gone,
> kill any inbound transactions on it", while cdev holds the transaction
> open until userspace issues a SEND_RESPONSE ioctl, which may be a very,
> very long time. But when it does, it calls fw_send_response(), which
> will dereference the card...
>
> So how unhappy are we about userspace potentially holding a fw_card
> open forever?
While termination of inbound transcations at card removal could be
implemented, it is IMO not worth the effort. Currently, the effect of
holding a reference of a card that has been removed is to block the
process that called the pci_remove of the card. This is
- either a user process ran by root. Root can find and kill processes
that have /dev/fw* open, if desired.
- a kernel thread (which one?) in case of hot removal of a PCCard or
ExpressCard.
The latter case could be a problem indeed. firewire-core's card
shutdown and card release should probably be improved not to block in
shutdown, just to defer freeing of memory until release.
This is not a new problem though; the same already always happens with
the client->device->card without the need of inbound transactions or
other special conditions involved, other than the client not closing the
file.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
My box has two firewire cards in it: card0 and card1.
My application opens /dev/fw0 (card 0) and allocates an address space.
The core makes the address space available on both cards.
Along comes the remote device, which sends a READ_QUADLET_REQUEST to
card1. The request gets passed up to my application, which calls
ioctl_send_response().
ioctl_send_response() then calls fw_send_response() with card0,
because that's the card it's bound to.
Card0's driver drops the response, because it isn't part of
a transaction that it has outstanding.
So in core-cdev: handle_request(), we need to stash the
card of the inbound request in the struct inbound_transaction_resource and
use that card to send the response to.
The hard part will be refcounting the card correctly
so it can't get deallocated while we hold a pointer to it.
Here's a trivial patch, which does not do the card refcounting, but at
least demonstrates what the problem is.
Note that we can't depend on the fact that the core-cdev:client
structure holds a card open, because in this case the card it holds
open is not the card the request came in on.
..and there's no way for the core to tell cdev "this card is gone,
kill any inbound transactions on it", while cdev holds the transaction
open until userspace issues a SEND_RESPONSE ioctl, which may be a very,
very long time. But when it does, it calls fw_send_response(), which
will dereference the card...
So how unhappy are we about userspace potentially holding a fw_card
open forever?
Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Reference counting to be addressed in a separate change.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (whitespace)
Protect the client's iso context pointer against a race that can happen
when more than one creation call is executed at the same time.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
void (*fw_address_callback_t)(..., int speed, ...) is the speed that a
remote node chose to transmit a request to us. In case of split
transactions, firewire-core will transmit the response at that speed.
Upper layer drivers on the other hand (firewire-net, -sbp2, firedtv, and
userspace drivers) cannot do anything useful with that speed datum,
except log it for debug purposes. But data that is merely potentially
(not even actually) used for debug purposes does not belong into the API.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
All of the fields of the iso_interrupt_event instance are overwritten
right after it was allocated.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Rather than "read a Control and Status Registers (CSR) Architecture
register" I prefer to say "read a Control and Status Register".
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
To prepare for the following additions of more OHCI-implemented CSR
registers, replace the get_cycle_time driver callback with a generic
CSR register callback.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Add a check that the data length in the SEND_RESPONSE ioctl is correct.
Incidentally, this also fixes the previously wrong response length of
software-handled lock requests.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The <linux/firewire-cdev.h> character device file ABI (i.e. /dev/fw*
character device file interface) does not make any use of lseek(),
pread(), pwrite() (or any kind of write() at all).
Use nonseekable_open() and, redundantly, set file_operations.llseek to
no_llseek to remove any doubt whether the BKL-grabbing default_llseek
handler is used. (Also shuffle file_operations initialization according
to the order of handler definitions.)
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
A userspace client got to see uninitialized stack-allocated memory if it
specified an _IOC_READ type of ioctl and an argument size larger than
expected by firewire-core's ioctl handlers (but not larger than the
core's union ioctl_arg).
Fix this by clearing the requested buffer size to zero, but only at _IOR
ioctls. This way, there is almost no runtime penalty to legitimate
ioctls. The only legitimate _IOR is FW_CDEV_IOC_GET_CYCLE_TIMER with 12
or 16 bytes to memset.
[Another way to fix this would be strict checking of argument size (and
possibly direction) vs. command number. However, we then need a lookup
table, and we need to allow for slight size deviations in case of 32bit
userland on 64bit kernel.]
Reported-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The definition of struct fw_cdev_iso_packet seems to imply that the
header_length must be quadlet-aligned, and in fact, specifying an
unaligned header has never really worked when using multiple packet
structures, because the position of the next control word is computed by
rounding the header_length _down_, so the last one to three bytes of the
header would overlap the next control word.
To avoid this problem, check that the header length is properly aligned.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
In receive contexts, reject packets with header_length==0. This would
be an instruction to queue zero packets which would not make sense.
This prevents a division by zero in the OHCI driver.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
from array of char to union of structs. I already used a union to size
the buffer which holds ioctl arguments; more consequent is to define it
as an instance of this union in the first place.
Also rename several local variables from "request" to "a"(rgument) since
the term request can be mistaken to mean a transaction subaction, e.g.
an instance of struct fw_request.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The system time from CLOCK_REALTIME is not monotonic, hence problematic
for the main user of the FW_CDEV_IOC_GET_CYCLE_TIMER ioctl. This issue
exists in its successor ABI, i.e. raw1394, too.
http://subversion.ffado.org/ticket/242
We now offer an alternative ioctl which lets the caller choose between
CLOCK_REALTIME, CLOCK_MONOTONIC, and CLOCK_MONOTONIC_RAW as source of
the local time, very similar to the clock_gettime libc function. The
format of the local time return value matches that of clock_gettime
(seconds and nanoseconds, instead of a single microseconds value from
the existing ioctl).
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The current implementation of Bus_Time read access was buggy since it
did not ensure that Bus_Time.second_count_hi and second_count_lo came
from the same 128 seconds period.
Reported-by: Håkan Johansson <f96hajo@chalmers.se>
Instead of a fix, remove Bus_Time register support altogether. The spec
requires all cycle master capable nodes to implement this (all Linux
nodes are cycle master capable) while it also says that it "may" be
initialized by the bus manager or by the IRM standing in for a bus
manager. (Neither Linux' firewire-core nor ieee1394 nodemgr implement
this.)
Since we cannot rely on Bus_Time having been initialized by a bus
manager, it is better to return an error instead of a nonsensical value
on a read request to Bus_Time.
Alternatively, we could fix the Bus_Time read integrity bug _and_
implement (a) cycle master's write support of the register as well as
(b) bus manager's Bus_Time initialization service, i.e. preservation of
the Bus_Time when the cycle master node of a bus changes. However, that
would be quite some code for a feature that is unreliable to begin with
and very likely unused in practice.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
ohci: Break out of the retry loop if too many attempts were necessary.
This may theoretically happen if the chip is fatally defective or if the
get_cycle_timer ioctl was performed after a CardBus controller was
ejected.
Also micro-optimize the loop by re-using the last two register reads in
the next iteration, remove a questionable inline keyword, and shuffle a
comment around.
core: ioctl_get_cycle_timer() is always called with interrupts on,
therefore local_irq_save() can be replaced by local_irq_disable().
Disabled local IRQs imply disabled preemption, hence preempt_disable()
can be removed.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Commit db5d247a "firewire: fix use of multiple AV/C devices, allow
multiple FCP listeners" introduced a regression into 2.6.33-rc3:
The core freed payloads of incoming requests to FCP_Request or
FCP_Response before a userspace driver accessed them.
We need to copy such payloads for each registered userspace client
and free the copies according to the lifetime rules of non-FCP client
request resources.
(This could possibly be optimized by reference counts instead of
copies.)
The presently only kernelspace driver which listens for FCP requests,
firedtv, was not affected because it already copies FCP frames into an
own buffer before returning to firewire-core's FCP handler dispatcher.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If copy_from_user in an FW_CDEV_IOC_SEND_RESPONSE ioctl failed, the
fw_request pointed to by the inbound_transaction_resource is no
longer referenced and needs to be freed.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Control of more than one AV/C device at once --- e.g. camcorders, tape
decks, audio devices, TV tuners --- failed or worked only unreliably,
depending on driver implementation. This affected kernelspace and
userspace drivers alike and was caused by firewire-core's inability to
accept multiple registrations of FCP listeners.
The fix allows multiple address handlers to be registered for the FCP
command and response registers. When a request for these registers is
received, all handlers are invoked, and the Firewire response is
generated by the core and not by any handler.
The cdev API does not change, i.e., userspace is still expected to send
a response for FCP requests; this response is silently ignored.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (changelog, rebased, whitespace)
Replace a hardcoded buffer size by a sizeof union {}. This shrinks the
stack-allocated ioctl argument buffer from 256 to 40 bytes. (This is
not much, but subsequent stack usage particularly by the queue_iso ioctl
handler adds up.)
The new form is also easier to keep up to date than a hardcoded size if
more ioctls are added.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Unify some names:
- "e" for pointers to subtypes of struct event,
- "event" for struct members and pointers to struct event,
- "r" for pointers to subtypes of struct client_resource,
- "resource" for struct members and pointers to struct client_resource,
- other names for struct members and pointers to other types.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
A few stylistic changes to unify some code patterns in the subsystem:
- The similar queue_delayed_work helpers fw_schedule_bm_work,
schedule_iso_resource, and sbp2_queue_work now have the same call
convention.
- Two conditional calls of schedule_iso_resource are factored into
another small helper.
- An sbp2_target_get helper is added as counterpart to
sbp2_target_put.
Object size of firewire-core is decreased a little bit, object size of
firewire-sbp2 remains unchanged.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
If copy_from_user in an FW_CDEV_IOC_SEND_RESPONSE ioctl failed, an
inbound_transaction_resource instance is no longer referenced and needs
to be freed.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The DMA mapping API cannot map on-stack addresses, as explained in
Documentation/DMA-mapping.txt. Convert the two cases of on-stack packet
payload buffers in firewire-core (payload of lock requests in the bus
manager work and in iso resource management) to slab-allocated memory.
There are a number on-stack buffers for quadlet write or quadlet read
requests in firewire-core and firewire-sbp2. These are harmless; they
are copied to/ from card driver internal DMA buffers since quadlet
payloads are inlined with packet headers.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
The source files of firewire-core, firewire-ohci, firewire-sbp2, i.e.
"drivers/firewire/fw-*.c"
are renamed to
"drivers/firewire/core-*.c",
"drivers/firewire/ohci.c",
"drivers/firewire/sbp2.c".
The old fw- prefix was redundant to the directory name. The new core-
prefix distinguishes the files according to which driver they belong to.
This change comes a little late, but still before further firewire
drivers are added as anticipated RSN.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Stefan Richter