Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
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. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
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 > --- > 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
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 > --- > 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 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
On weakly-ordered systems, the reading of an event's content must occur after reading the event's validity. Signed-off-by: Matt Evans --- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev