summaryrefslogtreecommitdiff
path: root/drivers/firewire/core-cdev.c
AgeCommit message (Collapse)Author
2017-02-13idr: Return the deleted entry from idr_removeMatthew Wilcox
It is a relatively common idiom (8 instances) to first look up an IDR entry, and then remove it from the tree if it is found, possibly doing further operations upon the entry afterwards. If we change idr_remove() to return the removed object, all of these users can save themselves a walk of the IDR tree. Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
2016-03-22firewire: use in_compat_syscall to check ioctl compatnessAndy Lutomirski
Firewire was using is_compat_task to check whether it was in a compat ioctl or a non-compat ioctl. Use is_compat_syscall instead so it works properly on all architectures. Signed-off-by: Andy Lutomirski <luto@kernel.org> Cc: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2015-11-06mm, page_alloc: distinguish between being unable to sleep, unwilling to ↵Mel Gorman
sleep and avoiding waking kswapd __GFP_WAIT has been used to identify atomic context in callers that hold spinlocks or are in interrupts. They are expected to be high priority and have access one of two watermarks lower than "min" which can be referred to as the "atomic reserve". __GFP_HIGH users get access to the first lower watermark and can be called the "high priority reserve". Over time, callers had a requirement to not block when fallback options were available. Some have abused __GFP_WAIT leading to a situation where an optimisitic allocation with a fallback option can access atomic reserves. This patch uses __GFP_ATOMIC to identify callers that are truely atomic, cannot sleep and have no alternative. High priority users continue to use __GFP_HIGH. __GFP_DIRECT_RECLAIM identifies callers that can sleep and are willing to enter direct reclaim. __GFP_KSWAPD_RECLAIM to identify callers that want to wake kswapd for background reclaim. __GFP_WAIT is redefined as a caller that is willing to enter direct reclaim and wake kswapd for background reclaim. This patch then converts a number of sites o __GFP_ATOMIC is used by callers that are high priority and have memory pools for those requests. GFP_ATOMIC uses this flag. o Callers that have a limited mempool to guarantee forward progress clear __GFP_DIRECT_RECLAIM but keep __GFP_KSWAPD_RECLAIM. bio allocations fall into this category where kswapd will still be woken but atomic reserves are not used as there is a one-entry mempool to guarantee progress. o Callers that are checking if they are non-blocking should use the helper gfpflags_allow_blocking() where possible. This is because checking for __GFP_WAIT as was done historically now can trigger false positives. Some exceptions like dm-crypt.c exist where the code intent is clearer if __GFP_DIRECT_RECLAIM is used instead of the helper due to flag manipulations. o Callers that built their own GFP flags instead of starting with GFP_KERNEL and friends now also need to specify __GFP_KSWAPD_RECLAIM. The first key hazard to watch out for is callers that removed __GFP_WAIT and was depending on access to atomic reserves for inconspicuous reasons. In some cases it may be appropriate for them to use __GFP_HIGH. The second key hazard is callers that assembled their own combination of GFP flags instead of starting with something like GFP_KERNEL. They may now wish to specify __GFP_KSWAPD_RECLAIM. It's almost certainly harmless if it's missed in most cases as other activity will wake kswapd. Signed-off-by: Mel Gorman <mgorman@techsingularity.net> Acked-by: Vlastimil Babka <vbabka@suse.cz> Acked-by: Michal Hocko <mhocko@suse.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> Cc: Christoph Lameter <cl@linux.com> Cc: David Rientjes <rientjes@google.com> Cc: Vitaly Wool <vitalywool@gmail.com> Cc: Rik van Riel <riel@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2014-11-14firewire: cdev: prevent kernel stack leaking into ioctl argumentsStefan Richter
Found by the UC-KLEE tool: A user could supply less input to firewire-cdev ioctls than write- or write/read-type ioctl handlers expect. The handlers used data from uninitialized kernel stack then. This could partially leak back to the user if the kernel subsequently generated fw_cdev_event_'s (to be read from the firewire-cdev fd) which notably would contain the _u64 closure field which many of the ioctl argument structures contain. The fact that the handlers would act on random garbage input is a lesser issue since all handlers must check their input anyway. The fix simply always null-initializes the entire ioctl argument buffer regardless of the actual length of expected user input. That is, a runtime overhead of memset(..., 40) is added to each firewirew-cdev ioctl() call. [Comment from Clemens Ladisch: This part of the stack is most likely to be already in the cache.] Remarks: - There was never any leak from kernel stack to the ioctl output buffer itself. IOW, it was not possible to read kernel stack by a read-type or write/read-type ioctl alone; the leak could at most happen in combination with read()ing subsequent event data. - The actual expected minimum user input of each ioctl from include/uapi/linux/firewire-cdev.h is, in bytes: [0x00] = 32, [0x05] = 4, [0x0a] = 16, [0x0f] = 20, [0x14] = 16, [0x01] = 36, [0x06] = 20, [0x0b] = 4, [0x10] = 20, [0x15] = 20, [0x02] = 20, [0x07] = 4, [0x0c] = 0, [0x11] = 0, [0x16] = 8, [0x03] = 4, [0x08] = 24, [0x0d] = 20, [0x12] = 36, [0x17] = 12, [0x04] = 20, [0x09] = 24, [0x0e] = 4, [0x13] = 40, [0x18] = 4. Reported-by: David Ramos <daramos@stanford.edu> Cc: <stable@vger.kernel.org> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2014-06-12firewire: Use ktime_get_ts()Thomas Gleixner
do_posix_clock_monotonic_gettime() is a leftover from the initial posix timer implementation which maps to ktime_get_ts() Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: John Stultz <john.stultz@linaro.org> Cc: Peter Zijlstra <peterz@infradead.org> Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Link: http://lkml.kernel.org/r/20140611234607.351283464@linutronix.de Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
2013-07-30firewire: core: typecast from gfp_t to bool more safelyStefan Richter
An idr related patch introduced the following sparse warning: drivers/firewire/core-cdev.c:488:33: warning: incorrect type in initializer (different base types) drivers/firewire/core-cdev.c:488:33: expected bool [unsigned] [usertype] preload drivers/firewire/core-cdev.c:488:33: got restricted gfp_t So let's convert from gfp_t bitfield to Boolean explicitly and safely. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2013-07-27firewire: fix libdc1394/FlyCap2 iso event regressionClemens Ladisch
Commit 18d627113b83 (firewire: prevent dropping of completed iso packet header data) was intended to be an obvious bug fix, but libdc1394 and FlyCap2 depend on the old behaviour by ignoring all returned information and thus not noticing that not all packets have been received yet. The result was that the video frame buffers would be saved before they contained the correct data. Reintroduce the old behaviour for old clients. Tested-by: Stepan Salenikovich <stepan.salenikovich@gmail.com> Tested-by: Josep Bosch <jep250@gmail.com> Cc: <stable@vger.kernel.org> # 3.4+ Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2013-04-28firewire: remove unnecessary alloc/OOM messagesStefan Richter
These are redundant to log messages from the mm core. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2013-04-28firewire: core: remove an always false testStefan Richter
struct fw_cdev_allocate_iso_resource.bandwidth is unsigned. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2013-02-27firewire: convert to idr_alloc()Tejun Heo
Convert to the much saner new idr interface. v2: Stefan pointed out that add_client_resource() may be called from non-process context. Preload iff @gfp_mask contains __GFP_WAIT. Also updated to include minor upper limit check. [tim.gardner@canonical.com: fix accidentally orphaned 'minor'[ Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2013-02-27firewire: don't use idr_remove_all()Tejun Heo
idr_destroy() can destroy idr by itself and idr_remove_all() is being deprecated. Drop its usage. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Stefan Richter <stefanr@s5r6.in-berlin.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2012-10-09firewire: cdev: fix user memory corruption (i386 userland on amd64 kernel)Stefan Richter
Fix two bugs of the /dev/fw* character device concerning the FW_CDEV_IOC_GET_INFO ioctl with nonzero fw_cdev_get_info.bus_reset. (Practically all /dev/fw* clients issue this ioctl right after opening the device.) Both bugs are caused by sizeof(struct fw_cdev_event_bus_reset) being 36 without natural alignment and 40 with natural alignment. 1) Memory corruption, affecting i386 userland on amd64 kernel: Userland reserves a 36 bytes large buffer, kernel writes 40 bytes. This has been first found and reported against libraw1394 if compiled with gcc 4.7 which happens to order libraw1394's stack such that the bug became visible as data corruption. 2) Information leak, affecting all kernel architectures except i386: 4 bytes of random kernel stack data were leaked to userspace. Hence limit the respective copy_to_user() to the 32-bit aligned size of struct fw_cdev_event_bus_reset. Reported-by: Simon Kirby <sim@hostway.ca> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Cc: stable@kernel.org
2012-04-17firewire: core: fix DMA mapping directionStefan Richter
Seen with recent libdc1394: If a client mmap()s the buffer of an isochronous reception buffer with PROT_READ|PROT_WRITE instead of just PROT_READ, firewire-core sets the wrong DMA mapping direction during buffer initialization. The fix is to split fw_iso_buffer_init() into allocation and DMA mapping and to perform the latter after both buffer and DMA context were allocated. Buffer allocation and context allocation may happen in any order, but we need the context type (reception or transmission) in order to set the DMA direction of the buffer. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2012-03-28Remove all #inclusions of asm/system.hDavid Howells
Remove all #inclusions of asm/system.h preparatory to splitting and killing it. Performed with the following command: perl -p -i -e 's!^#\s*include\s*<asm/system[.]h>.*\n!!' `grep -Irl '^#\s*include\s*<asm/system[.]h>' *` Signed-off-by: David Howells <dhowells@redhat.com>
2012-03-18firewire: allow explicit flushing of iso packet completionsClemens Ladisch
Extend the kernel and userspace APIs to allow reporting all currently completed isochronous packets, even if the next interrupt packet has not yet been reached. This is required to determine the status of the packets at the end of a paused or stopped stream, and useful for more precise synchronization of audio streams. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2012-03-18firewire: prevent dropping of completed iso packet header dataClemens Ladisch
The buffer for the header data of completed iso packets has a fixed size, so it is possible to configure a stream with a big interval between interrupt packets or with big headers so that this buffer would overflow. Previously, ohci.c would drop any data that would not fit, but this could make unsuspecting applications believe that fewer than the actual number of packets have completed. Instead of dropping data, add calls to flush_iso_completion() so that there are as many events as needed to report all of the data. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2012-02-22firewire: core: prefix log messages with card nameStefan Richter
Associate all log messages from firewire-core with the respective card because some people have more than one card. E.g. firewire_ohci 0000:04:00.0: added OHCI v1.10 device as card 0, 8 IR + 8 IT contexts, quirks 0x0 firewire_ohci 0000:05:00.0: added OHCI v1.10 device as card 1, 8 IR + 8 IT contexts, quirks 0x0 firewire_core: created device fw0: GUID 0814438400000389, S800 firewire_core: phy config: new root=ffc1, gap_count=5 firewire_core: created device fw1: GUID 0814438400000388, S800 firewire_core: created device fw2: GUID 0001d202e06800d1, S800 turns into firewire_ohci 0000:04:00.0: added OHCI v1.10 device as card 0, 8 IR + 8 IT contexts, quirks 0x0 firewire_ohci 0000:05:00.0: added OHCI v1.10 device as card 1, 8 IR + 8 IT contexts, quirks 0x0 firewire_core 0000:04:00.0: created device fw0: GUID 0814438400000389, S800 firewire_core 0000:04:00.0: phy config: new root=ffc1, gap_count=5 firewire_core 0000:05:00.0: created device fw1: GUID 0814438400000388, S800 firewire_core 0000:04:00.0: created device fw2: GUID 0001d202e06800d1, S800 This increases the module size slightly; to keep this in check, turn the former printk wrapper macros into functions. Their implementation is largely copied from driver core's dev_printk counterparts. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2011-08-12firewire: cdev: fix 32 bit userland on 64 bit kernel compat corner casesStefan Richter
Clemens points out that we need to use compat_ptr() in order to safely cast from u64 to addresses of a 32-bit usermode client. Before, our conversion went wrong - in practice if the client cast from pointer to integer such that sign-extension happened, (libraw1394 and libdc1394 at least were not doing that, IOW were not affected) or - in theory on s390 (which doesn't have FireWire though) and on the tile architecture, regardless of what the client does. The bug would usually be observed as the initial get_info ioctl failing with "Bad address" (EFAULT). Reported-by: Carl Karsten <carl@personnelware.com> Reported-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2011-07-16firewire: cdev: prevent race between first get_info ioctl and bus reset ↵Stefan Richter
event queuing Between open(2) of a /dev/fw* and the first FW_CDEV_IOC_GET_INFO ioctl(2) on it, the kernel already queues FW_CDEV_EVENT_BUS_RESET events to be read(2) by the client. The get_info ioctl is practically always issued right away after open, hence this condition only occurs if the client opens during a bus reset, especially during a rapid series of bus resets. The problem with this condition is twofold: - These bus reset events carry the (as yet undocumented) @closure value of 0. But it is not the kernel's place to choose closures; they are privat to the client. E.g., this 0 value forced from the kernel makes it unsafe for clients to dereference it as a pointer to a closure object without NULL pointer check. - It is impossible for clients to determine the relative order of bus reset events from get_info ioctl(2) versus those from read(2), except in one way: By comparison of closure values. Again, such a procedure imposes complexity on clients and reduces freedom in use of the bus reset closure. So, change the ABI to suppress queuing of bus reset events before the first FW_CDEV_IOC_GET_INFO ioctl was issued by the client. Note, this ABI change cannot be version-controlled. The kernel cannot distinguish old from new clients before the first FW_CDEV_IOC_GET_INFO ioctl. We will try to back-merge this change into currently maintained stable/ longterm series, and we only document the new behaviour. The old behavior is now considered a kernel bug, which it basically is. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Cc: <stable@kernel.org>
2011-07-16firewire: cdev: return -ENOTTY for unimplemented ioctls, not -EINVALStefan Richter
On Jun 27 Linus Torvalds wrote: > The correct error code for "I don't understand this ioctl" is ENOTTY. > The naming may be odd, but you should think of that error value as a > "unrecognized ioctl number, you're feeding me random numbers that I > don't understand and I assume for historical reasons that you tried to > do some tty operation on me". [...] > The EINVAL thing goes way back, and is a disaster. It predates Linux > itself, as far as I can tell. You'll find lots of man-pages that have > this line in it: > > EINVAL Request or argp is not valid. > > and it shows up in POSIX etc. And sadly, it generally shows up > _before_ the line that says > > ENOTTY The specified request does not apply to the kind of object > that the descriptor d references. > > so a lot of people get to the EINVAL, and never even notice the ENOTTY. [...] > At least glibc (and hopefully other C libraries) use a _string_ that > makes much more sense: strerror(ENOTTY) is "Inappropriate ioctl for > device" So let's correct this in the <linux/firewire-cdev.h> ABI while it is still young, relative to distributor adoption. Side note: We return -ENOTTY not only on _IOC_TYPE or _IOC_NR mismatch, but also on _IOC_SIZE mismatch. An ioctl with an unsupported size of argument structure can be seen as an unsupported version of that ioctl. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Cc: <stable@kernel.org>
2011-05-10firewire: sbp2: parallelize login, reconnect, logoutStefan Richter
The struct sbp2_logical_unit.work items can all be executed in parallel but are not reentrant. Furthermore, reconnect or re-login work must be executed in a WQ_MEM_RECLAIM workqueue. Hence replace the old single-threaded firewire-sbp2 workqueue by a concurrency-managed but non-reentrant workqueue with rescuer. firewire-core already maintains one, hence use this one. In earlier versions of this change, I observed occasional failures of parallel INQUIRY to an Initio INIC-2430 FireWire 800 to dual IDE bridge. More testing indicates that parallel INQUIRY is not actually a problem, but too quick successions of logout and login + INQUIRY, e.g. a quick sequence of cable plugout and plugin, can result in failed INQUIRY. This does not seem to be something that should or could be addressed by serialization. Another dual-LU device to which I currently have access to, an OXUF924DSB FireWire 800 to dual SATA bridge with firmware from MacPower, has been successfully tested with this too. This change is beneficial to environments with two or more FireWire storage devices, especially if they are located on the same bus. Management tasks that should be performed as soon and as quickly as possible, especially reconnect, are no longer held up by tasks on other devices that may take a long time, especially login with INQUIRY and sd or sr driver probe. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2011-05-10firewire: core: use non-reentrant workqueue with rescuerStefan Richter
firewire-core manages the following types of work items: fw_card.br_work: - resets the bus on a card and possibly sends a PHY packet before that - does not sleep for long or not at all - is scheduled via fw_schedule_bus_reset() by - firewire-ohci's pci_probe method - firewire-ohci's set_config_rom method, called by kernelspace protocol drivers and userspace drivers which add/remove Configuration ROM descriptors - userspace drivers which use the bus reset ioctl - itself if the last reset happened less than 2 seconds ago fw_card.bm_work: - performs bus management duties - usually does not (but may in corner cases) sleep for long - is scheduled via fw_schedule_bm_work() by - firewire-ohci's self-ID-complete IRQ handler tasklet - firewire-core's fw_device.work instances whenever the root node device was (successfully or unsuccessfully) discovered, refreshed, or rediscovered - itself in case of resource allocation failures or in order to obey the 125ms bus manager arbitration interval fw_device.work: - performs node probe, update, shutdown, revival, removal; including kernel driver probe, update, shutdown and bus reset notification to userspace drivers - usually sleeps moderately long, in corner cases very long - is scheduled by - firewire-ohci's self-ID-complete IRQ handler tasklet via the core's fw_node_event - firewire-ohci's pci_remove method via core's fw_destroy_nodes/ fw_node_event - itself during retries, e.g. while a node is powering up iso_resource.work: - accesses registers at the Isochronous Resource Manager node - usually does not (but may in corner cases) sleep for long - is scheduled via schedule_iso_resource() by - the owning userspace driver at addition and removal of the resource - firewire-core's fw_device.work instances after bus reset - itself in case of resource allocation if necessary to obey the 1000ms reallocation period after bus reset fw_card.br_work instances should not, and instances of the others must not, be executed in parallel by multiple CPUs -- but were not protected against that. Hence allocate a non-reentrant workqueue for them. fw_device.work may be used in the memory reclaim path in case of SBP-2 device updates. Hence we need a workqueue with rescuer and cannot use system_nrt_wq. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Reviewed-by: Tejun Heo <tj@kernel.org>
2011-05-10firewire: optimize iso queueing by setting wake only after the last packetClemens Ladisch
When queueing iso packets, the run time is dominated by the two MMIO accesses that set the DMA context's wake bit. Because most drivers submit packets in batches, we can save much time by removing all but the last wakeup. The internal kernel API is changed to require a call to fw_iso_context_queue_flush() after a batch of queued packets. The user space API does not change, so one call to FW_CDEV_IOC_QUEUE_ISO must specify multiple packets to take advantage of this optimization. In my measurements, this patch reduces the time needed to queue fifty skip packets from userspace to one sixth on a 2.5 GHz CPU, or to one third at 800 MHz. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2011-05-10firewire: octlet AT payloads can be stack-allocatedStefan Richter
We do not need slab allocations anymore in order to satisfy streaming DMA mapping constraints, thanks to commit da28947e7e36 "firewire: ohci: avoid separate DMA mapping for small AT payloads". (Besides, the slab-allocated buffers that firewire-core, firewire-sbp2, and firedtv used to provide for 8-byte write and lock requests were still not fully portable since they crossed cacheline boundaries or shared a cacheline with unrelated CPU-accessed data. snd-firewire-lib got this aspect right by using an extra kmalloc/ kfree just for the 8-byte transaction buffer.) This change replaces kmalloc'ed lock transaction scratch buffers in firewire-core, firedtv, and snd-firewire-lib by local stack allocations. Perhaps the most notable result of the change is simpler locking because there is no need to serialize usages of preallocated per-device buffers anymore. Also, allocations and deallocations are simpler. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> Acked-by: Clemens Ladisch <clemens@ladisch.de>
2011-01-23firewire: core: fix card->reset_jiffies overflowClemens Ladisch
On a 32-bit machine with, e.g., HZ=1000, jiffies will overflow after about 50 days, so if there are between 25 and 50 days between bus resets, the card->reset_jiffies comparisons can get wrong results. To fix this, ensure that this timestamp always uses 64 bits. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
2011-01-23firewire: cdev: remove unneeded referenceClemens Ladisch
For outbound transactions, the IDR's and the callback's references now have exactly the same lifetime, so we do not need both of them. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
2011-01-23firewire: cdev: always wait for outbound transactions to completeClemens Ladisch
We must not use fw_cancel_transaction() because it cannot correctly abort still-active transactions. The only place in core-cdev where this matters is when the file is released. Instead of trying to abort the transactions, we wait for them to complete normally, i.e., until all outbound transaction resources have been removed from the IDR tree. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
2011-01-23firewire: cdev: remove unneeded idr_find() from complete_transaction()Clemens Ladisch
Outbound transactions are never aborted with release_client_resource(), so it is not necessary for complete_transaction() to check whether the resource is still registered. Only shutdown_resource() can abort such an transaction, and this is already handled with the in_shutdown check. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: "Stefan Richter" <stefanr@s5r6.in-berlin.de>
2010-12-13firewire: make PHY packet header format consistentClemens Ladisch
Change the header of PHY packets to be sent to include a pseudo transaction code. This makes the header consistent with that of received PHY packets, and allows at_context_queue_packet() and log_ar_at_event() to see the packet type directly instead of having to deduce it from the header length or even from the header contents. Signed-off-by: Clemens Ladisch <clemens@ladisch.de> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-08-02Merge firewire branches to be released post v2.6.35Stefan Richter
Conflicts: drivers/firewire/core-card.c drivers/firewire/core-cdev.c and forgotten #include <linux/time.h> in drivers/firewire/ohci.c Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-29firewire: add isochronous multichannel receptionStefan Richter
This adds the DMA context programming and userspace ABI for multichannel reception, i.e. for listening on multiple channel numbers by means of a single DMA context. The use case is reception of more streams than there are IR DMA units offered by the link layer. This is already implemented by the older ohci1394 + ieee1394 + raw1394 stack. And as discussed recently on linux1394-devel, this feature is occasionally used in practice. The big drawbacks of this mode are that buffer layout and interrupt generation necessarily differ from single-channel reception: Headers and trailers are not stripped from packets, packets are not aligned with buffer chunks, interrupts are per buffer chunk, not per packet. These drawbacks also cause a rather hefty code footprint to support this rarely used OHCI-1394 feature. (367 lines added, among them 94 lines of added userspace ABI documentation.) This implementation enforces that a multichannel reception context may only listen to channels to which no single-channel context on the same link layer is presently listening to. OHCI-1394 would allow to overlay single-channel contexts by the multi-channel context, but this would be a departure from the present first-come-first-served policy of IR context creation. The implementation is heavily based on an earlier one by Jay Fenlason. Thanks Jay. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-29firewire: core: small clarifications in core-cdevStefan Richter
Make a note on the seemingly unused linux/sched.h. Rename an irritatingly named variable. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-29firewire: core: remove unused codeStefan Richter
ioctl_create_iso_context enforces ctx->header_size >= 4. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-23firewire: cdev: improve FW_CDEV_IOC_ALLOCATEStefan Richter
In both the ieee1394 stack and the firewire stack, the core treats kernelspace drivers better than userspace drivers when it comes to CSR address range allocation: The former may request a register to be placed automatically at a free spot anywhere inside a specified address range. The latter may only request a register at a fixed offset. Hence, userspace drivers which do not require a fixed offset potentially need to implement a retry loop with incremented offset in each retry until the kernel does not fail allocation with EBUSY. This awkward procedure is not fundamentally necessary as the core already provides a superior allocation API to kernelspace drivers. Therefore change the ioctl() ABI by addition of a region_end member in the existing struct fw_cdev_allocate. Userspace and kernelspace APIs work the same way now. There is a small cost to pay by clients though: If client source code is required to compile with older kernel headers too, then any use of the new member fw_cdev_allocate.region_end needs to be enclosed by #ifdef/#endif directives. However, any client program that seriously wants to use address range allocations will require a kernel of cdev ABI version >= 4 at runtime and a linux/firewire-cdev.h header of >= 4 anyway. This is because v4 brings FW_CDEV_EVENT_REQUEST2. The only client program in which build-time compatibility with struct fw_cdev_allocate as found in older kernel headers makes sense is libraw1394. (libraw1394 uses the older broken FW_CDEV_EVENT_REQUEST to implement a makeshift, incorrect transaction responder that does at least work somewhat in many simple scenarios, relying on guesswork by libraw1394 and by libraw1394 based applications. Plus, address range allocation and transaction responder is only one of many features that libraw1394 needs to provide, and these other features need to work with kernel and kernel-headers as old as possible. Any new linux/firewire-cdev.h based client that implements a transaction responder should never attempt to do it like libraw1394; instead it should make a header and kernel of v4 or later a hard requirement.) While we are at it, update the struct fw_cdev_allocate documentation to better reflect the recent fw_cdev_event_request2 ABI addition. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-23firewire: cdev: add PHY pingingStefan Richter
This extends the FW_CDEV_IOC_SEND_PHY_PACKET ioctl() for /dev/fw* to be useful for ping time measurements. One application for it would be gap count optimization in userspace that is based on ping times rather than hop count. (The latter is implemented in firewire-core itself but is not applicable to beta PHYs that act as repeater.) Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-23firewire: cdev: add PHY packet receptionStefan Richter
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>
2010-07-23firewire: cdev: add PHY packet transmissionStefan Richter
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>
2010-07-23firewire: core: use C99 initializer in array of ioctl handlersStefan Richter
to make the correspondence of ioctl numbers and handlers more obvious. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-07-13firewire: core: integrate software-forced bus resets with bus managementStefan Richter
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>
2010-07-13firewire: core: ensure some userspace API constants match corresponding ↵Stefan Richter
kernel API constants 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>
2010-07-13firewire: cdev: check write quadlet request length to avoid buffer overflowClemens Ladisch
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>
2010-07-08firewire: cdev: fix fw_cdev_event_bus_reset.bm_node_idStefan Richter
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>
2010-06-20firewire: cdev: fix ABI for FCP and address range mapping, add ↵Stefan Richter
fw_cdev_event_request2 The problem: A target-like userspace driver, e.g. AV/C target or SBP-2/3 target, needs to be able to act as responder and requester. In the latter role, it needs to send requests to nods from which it received requests. This is currently impossible because fw_cdev_event_request lacks information about sender node ID. Reported-by: Jay Fenlason <fenlason@redhat.com> Libffado + libraw1394 + firewire-core is currently unable to drive two or more audio devices on the same bus. Reported-by: Arnold Krille <arnold@arnoldarts.de> This is because libffado requires destination node ID of FCP requests and sender node ID of FCP responses to match. It even prohibits libffado from working with a bus on which libraw1394 opens a /dev/fw* as default ioctl device that does not correspond with the audio device. This is because libraw1394 does not receive the sender node ID from the kernel. Moreover, fw_cdev_event_request makes it impossible to tell unicast and broadcast write requests apart. The fix: Add a replacement of struct fw_cdev_event_request request, boringly called struct fw_cdev_event_request2. The new event will be sent to a userspace client instead of the old one if the client claims compatibility with <linux/firewire-cdev.h> ABI version 4 or later. libraw1394 needs to be extended to make use of the new event, in order to properly support libffado and other FCP or address range mapping users who require correct sender node IDs. Further notes: While we are at it, change back the range of possible values of fw_cdev_event_request.tcode to 0x0...0xb like in ABI version <= 3. The preceding change "firewire: expose extended tcode of incoming lock requests to (userspace) drivers" expanded it to 0x0...0x17 which could catch sloppily coded clients by surprise. The extended range of codes is only used in the new fw_cdev_event_request2.tcode. Jay and I also suggested an alternative approach to fix the ABI for incoming requests: Add an FW_CDEV_IOC_GET_REQUEST_INFO ioctl which can be called after reception of an fw_cdev_event_request, before issuing of the closing FW_CDEV_IOC_SEND_RESPONSE ioctl. The new ioctl would reveal the vital information about a request that fw_cdev_event_request lacks. Jay showed an implementation of this approach. The former event approach adds 27 LOC of rather trivial code to core-cdev.c, the ioctl approach 34 LOC, some of which is nontrivial. The ioctl approach would certainly also add more LOC to userspace programs which require the expanded information on inbound requests. This approach is probably only on the lighter-weight side in case of clients that want to be compatible with kernels that lack the new capability, like libraw1394. However, the code to be added to such libraw1394-like clients in case of the event approach is a straight- forward additional switch () case in its event handler. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2010-06-20firewire: cdev: freeze FW_CDEV_VERSION due to libraw1394 bugStefan Richter
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>
2010-06-20firewire: cdev: count references of cards during inbound transactionsStefan Richter
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>
2010-06-20firewire: cdev: fix responses to nodes at different cardJay Fenlason
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)
2010-06-20firewire: cdev: fix race in iso context creationClemens Ladisch
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>
2010-06-20firewire: remove an unused function argumentStefan Richter
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>
2010-06-20firewire: core: remove an unnecessary zero initializationStefan Richter
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>
2010-06-19firewire: rename CSR access driver methodsStefan Richter
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>