linux-kernel - [PATCH + an old question] firewire: ohci: use memory barriers to order descriptor updates

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <tkrat.21d3972644f46dc9@s5r6.in-berlin.de>
Date:	Tue, 27 Jul 2010 13:20:33 +0200 (CEST)
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	linux-kernel@...r.kernel.org
cc:	linux1394-devel@...ts.sourceforge.net
Subject: [PATCH + an old question] firewire: ohci: use memory barriers to
 order descriptor updates

When we append to a DMA program, we need to ensure that the PCI device
sees the updates in the intended order.  We need:

1. a write memory barrier between initialization of new descriptors and
   the update of the last old descriptor to point to the new
   descriptors (i.e. branch_address update),

2. a write memory barrier between branch_address update and wake-up of
   the DMA unit by MMIO register write.

This patch adds only barrier 1.

Barrier 2 is implicit in writel() on most machines --- or at least I
think it is.  See this from arch/alpha/include/asm/io.h:

    #define build_mmio_write(name, size, type, reg, barrier) \
    static inline void name(type val, volatile void __iomem *addr) \
    { asm volatile("mov" size " %0,%1": :reg (val), \
    "m" (*(volatile type __force *)addr) barrier); }

    build_mmio_write(writel, "l", unsigned int, "r", :"memory")

Does this order the mmio write relative to previous memory writes?

I am not so sure about barrier semantics of writel() on some less
popular architectures.  From arch/alpha/include/asm/io.h:

    extern inline void writel(u32 b, volatile void __iomem *addr)
    {
	    __raw_writel(b, addr);
	    mb();
    }

This mb() is nice but we need a barrier in front of the __raw_writel.
Somebody who cares might want to add it in the architecture code or in
hundreds of drivers.

Signed-off-by: Stefan Richter <stefanr@...6.in-berlin.de>
---
 drivers/firewire/ohci.c |    3 +++
 1 file changed, 3 insertions(+)

Index: b/drivers/firewire/ohci.c
===================================================================
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -595,6 +595,7 @@ static int ar_context_add_page(struct ar
 	ab->descriptor.res_count      = cpu_to_le16(PAGE_SIZE - offset);
 	ab->descriptor.branch_address = 0;

+	wmb(); /* finish init of new descriptors before branch_address update */
 	ctx->last_buffer->descriptor.branch_address = cpu_to_le32(ab_bus | 1);
 	ctx->last_buffer->next = ab;
 	ctx->last_buffer = ab;
@@ -982,6 +983,8 @@ static void context_append(struct contex
 	d_bus = desc->buffer_bus + (d - desc->buffer) * sizeof(*d);

 	desc->used += (z + extra) * sizeof(*d);
+
+	wmb(); /* finish init of new descriptors before branch_address update */
 	ctx->prev->branch_address = cpu_to_le32(d_bus | z);
 	ctx->prev = find_branch_descriptor(d, z);

-- 
Stefan Richter
-=====-==-=- -=== ==-==
http://arcgraph.de/sr/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/