Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations

On Wed, 2012-08-22 at 07:20 -0700, Linus Torvalds wrote:

On Wed, Aug 22, 2012 at 6:26 AM, Ben Hutchings
[off-list ref] wrote:

quoted

It's [ref].

That doesn't help me in the least. I don't *have* that email. It was
never sent to me. I don't know what list it went on, and googling the
ID doesn't get me anything. So sending me the mail ID is kind of
pointless.

So I still don't see the actual patch. But everything I heard about it
indirectly makes me go "That's just crazy".

Sorry, I'll paste it below.

quoted

When updating a TX DMA ring pointer in sfc, we can push the first new
descriptor at the same time, so that for a linear packet only one DMA
read is then required before the packet goes out on the wire.  Currently
this requires 2-4 MMIO writes (each a separate PCIe transactions),
depending on the host word size.  There is a measurable reduction in
latency if we can reduce it to 1 PCIe transaction.  (But as previously
discussed, write-combining isn't suitable for this.)

Quite frankly, this isn't even *remotely* changing my mind about our
FPU model. I'm like "ok, some random driver is trying to be clever,
and it's almost certainly getting things entirely wrong and doing
things that only work on certain machines".

If you really think it's a big deal, do it in *your* driver, and make
it do the whole irq_fpu_usable() check together with
kernel_fpu_begin/end(). And make it very much x86-specific and with a
fallback to non-atomic accesses.

[...]

Which is what I first submitted, but David wanted it to be generic.

Ben.

---
Subject: sfc: Use __raw_writeo() to perform TX descriptor push where possible

Use the new __raw_writeo() function for TX descriptor push where
available.  This means we now use only a single PCIe transaction
on x86_64 (vs 2 before), reducing TX latency slightly.

Signed-off-by: Ben Hutchings <redacted>
---
 drivers/net/ethernet/sfc/bitfield.h |    3 +++
 drivers/net/ethernet/sfc/io.h       |   18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/bitfield.h b/drivers/net/ethernet/sfc/bitfield.h
index b26a954..5feeba2 100644
--- a/drivers/net/ethernet/sfc/bitfield.h
+++ b/drivers/net/ethernet/sfc/bitfield.h

@@ -83,6 +83,9 @@ typedef union efx_qword {
 
 /* An octword (eight-word, i.e. 16 byte) datatype - little-endian in HW */
 typedef union efx_oword {
+#ifdef HAVE_INT128
+       __le128 u128;
+#endif
        __le64 u64[2];
        efx_qword_t qword[2];
        __le32 u32[4];

diff --git a/drivers/net/ethernet/sfc/io.h b/drivers/net/ethernet/sfc/io.h
index 751d1ec..fbcdc6d 100644
--- a/drivers/net/ethernet/sfc/io.h
+++ b/drivers/net/ethernet/sfc/io.h

@@ -57,10 +57,22 @@
  *   current state.
  */
 
-#if BITS_PER_LONG == 64
+#if defined(writeo)
+#define EFX_USE_OWORD_IO 1
+#endif
+
+#if defined(readq) && defined(writeq)
 #define EFX_USE_QWORD_IO 1
 #endif
 
+#ifdef EFX_USE_OWORD_IO
+static inline void _efx_writeo(struct efx_nic *efx, __le128 value,
+                              unsigned int reg)
+{
+       __raw_writeo((__force u128)value, efx->membase + reg);
+}
+#endif
+
 #ifdef EFX_USE_QWORD_IO
 static inline void _efx_writeq(struct efx_nic *efx, __le64 value,
                                  unsigned int reg)

@@ -235,7 +247,9 @@ static inline void _efx_writeo_page(struct efx_nic *efx, efx_oword_t *value,
                   "writing register %x with " EFX_OWORD_FMT "\n", reg,
                   EFX_OWORD_VAL(*value));
 
-#ifdef EFX_USE_QWORD_IO
+#if defined(EFX_USE_OWORD_IO)
+       _efx_writeo(efx, value->u128, reg);
+#elif defined(EFX_USE_QWORD_IO)
        _efx_writeq(efx, value->u64[0], reg + 0);
        _efx_writeq(efx, value->u64[1], reg + 8);
 #else

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.