* [PATCH 3/5] xhci: Add rmb() between reading event validity & event data access.
@ 2011-03-25 7:44 Matt Evans
2011-03-25 23:11 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Matt Evans @ 2011-03-25 7:44 UTC (permalink / raw)
To: sarah.a.sharp; +Cc: linuxppc-dev, linux-usb
On weakly-ordered systems, the reading of an event's content must occur
after reading the event's validity.
Signed-off-by: Matt Evans <matt@ozlabs.org>
---
drivers/usb/host/xhci-ring.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 45f3b77..b46efd9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2151,7 +2151,7 @@ static void xhci_handle_event(struct xhci_hcd *xhci)
return;
}
xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
-
+ rmb();
/* FIXME: Handle more event types. */
switch ((le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK)) {
case TRB_TYPE(TRB_COMPLETION):
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/5] xhci: Add rmb() between reading event validity & event data access.
2011-03-25 7:44 [PATCH 3/5] xhci: Add rmb() between reading event validity & event data access Matt Evans
@ 2011-03-25 23:11 ` Segher Boessenkool
2011-03-28 4:52 ` [PATCH v2 " Matt Evans
0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2011-03-25 23:11 UTC (permalink / raw)
To: Matt Evans; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb
> On weakly-ordered systems, the reading of an event's content must occur
> after reading the event's validity.
Put a comment on the rmb() saying what it orders?
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
2011-03-25 23:11 ` Segher Boessenkool
@ 2011-03-28 4:52 ` Matt Evans
2011-03-28 22:20 ` Sarah Sharp
2011-03-29 18:56 ` Dmitry Torokhov
0 siblings, 2 replies; 6+ messages in thread
From: Matt Evans @ 2011-03-28 4:52 UTC (permalink / raw)
To: sarah.a.sharp; +Cc: linuxppc-dev, linux-usb
On weakly-ordered systems, the reading of an event's content must occur
after reading the event's validity.
Signed-off-by: Matt Evans <matt@ozlabs.org>
---
Segher, thanks for the comment; explanation added.
drivers/usb/host/xhci-ring.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 45f3b77..d6aa880 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2152,6 +2152,11 @@ static void xhci_handle_event(struct xhci_hcd *xhci)
}
xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
+ /*
+ * Barrier between reading the TRB_CYCLE (valid) flag above and any
+ * speculative reads of the event's flags/data below.
+ */
+ rmb();
/* FIXME: Handle more event types. */
switch ((le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK)) {
case TRB_TYPE(TRB_COMPLETION):
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
2011-03-28 4:52 ` [PATCH v2 " Matt Evans
@ 2011-03-28 22:20 ` Sarah Sharp
2011-03-29 18:56 ` Dmitry Torokhov
1 sibling, 0 replies; 6+ messages in thread
From: Sarah Sharp @ 2011-03-28 22:20 UTC (permalink / raw)
To: Matt Evans; +Cc: linuxppc-dev, linux-usb
This patch looks fine, thanks!
Sarah Sharp
On Mon, Mar 28, 2011 at 03:52:57PM +1100, Matt Evans wrote:
> On weakly-ordered systems, the reading of an event's content must occur
> after reading the event's validity.
>
> Signed-off-by: Matt Evans <matt@ozlabs.org>
> ---
> Segher, thanks for the comment; explanation added.
>
> drivers/usb/host/xhci-ring.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 45f3b77..d6aa880 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2152,6 +2152,11 @@ static void xhci_handle_event(struct xhci_hcd *xhci)
> }
> xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
>
> + /*
> + * Barrier between reading the TRB_CYCLE (valid) flag above and any
> + * speculative reads of the event's flags/data below.
> + */
> + rmb();
> /* FIXME: Handle more event types. */
> switch ((le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK)) {
> case TRB_TYPE(TRB_COMPLETION):
> --
> 1.7.0.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
2011-03-28 4:52 ` [PATCH v2 " Matt Evans
2011-03-28 22:20 ` Sarah Sharp
@ 2011-03-29 18:56 ` Dmitry Torokhov
2011-03-29 21:23 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2011-03-29 18:56 UTC (permalink / raw)
To: Matt Evans; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb
On Sunday, March 27, 2011 09:52:57 pm Matt Evans wrote:
> On weakly-ordered systems, the reading of an event's content must occur
> after reading the event's validity.
>
> Signed-off-by: Matt Evans <matt@ozlabs.org>
> ---
> Segher, thanks for the comment; explanation added.
>
> drivers/usb/host/xhci-ring.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 45f3b77..d6aa880 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2152,6 +2152,11 @@ static void xhci_handle_event(struct xhci_hcd
> *xhci) }
> xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
>
> + /*
> + * Barrier between reading the TRB_CYCLE (valid) flag above and any
> + * speculative reads of the event's flags/data below.
> + */
> + rmb();
> /* FIXME: Handle more event types. */
> switch ((le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK)) {
Isn't it the same memory that is being read the first time around? How
reordering could happen here?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
2011-03-29 18:56 ` Dmitry Torokhov
@ 2011-03-29 21:23 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2011-03-29 21:23 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb, Matt Evans
On Tue, 2011-03-29 at 11:56 -0700, Dmitry Torokhov wrote:
> > + /*
> > + * Barrier between reading the TRB_CYCLE (valid) flag above
> and any
> > + * speculative reads of the event's flags/data below.
> > + */
> > + rmb();
> > /* FIXME: Handle more event types. */
> > switch ((le32_to_cpu(event->event_cmd.flags) &
> TRB_TYPE_BITMASK)) {
>
> Isn't it the same memory that is being read the first time around? How
> reordering could happen here?
We don't try to enforce ordering specifically with the subsequent read
of the flags that is visible in the snipped above (indeed that's the
same address and so hopefully should be in program order), but all
subsequent reads which could have been performed speculatively such as
the rest of the event and including whatever other in memory information
the chip might have updated prior to sending the event.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-29 21:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-25 7:44 [PATCH 3/5] xhci: Add rmb() between reading event validity & event data access Matt Evans
2011-03-25 23:11 ` Segher Boessenkool
2011-03-28 4:52 ` [PATCH v2 " Matt Evans
2011-03-28 22:20 ` Sarah Sharp
2011-03-29 18:56 ` Dmitry Torokhov
2011-03-29 21:23 ` Benjamin Herrenschmidt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.