From: Ben Hutchings <hidden> Date: 2012-08-22 01:18:00
Current Solarflare network controllers have 128-bit memory-mapped
registers which are normally accessed through a series of I/O
operations. However, it is also possible to access them with a single
MOVAPS instruction on x86_64, and this is measurably faster as it
requires only one PCIe transaction.
This series introduces reado(), writeo() and the corresponding raw
operations on x86_64. These could also be implemented on i386 with an
appropriate CPU selection, but it would require u128 etc. to be defined
as structure types.
The sfc driver can potentially take advantage of both 128-bit reads and
writes, but as a first step this changes only one particular write that
may be done on the data path.
Ben.
Ben Hutchings (3):
x86_64: Define 128-bit types for kernel code only
x86_64: Define 128-bit memory-mapped I/O operations
sfc: Use __raw_writeo() to perform TX descriptor push where possible
arch/x86/Kconfig.cpu | 4 ++
arch/x86/include/asm/io.h | 14 +++++++
arch/x86/include/asm/types.h | 8 ++++
arch/x86/lib/Makefile | 1 +
arch/x86/lib/oword_io.c | 65 +++++++++++++++++++++++++++++++++++
drivers/net/ethernet/sfc/bitfield.h | 3 ++
drivers/net/ethernet/sfc/io.h | 18 ++++++++-
include/linux/types.h | 12 ++++++
8 files changed, 123 insertions(+), 2 deletions(-)
create mode 100644 arch/x86/lib/oword_io.c
--
1.7.7.6
--
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.
From: Ben Hutchings <hidden> Date: 2012-08-22 01:20:25
These types will initially be used for 128-bit I/O operations.
Signed-off-by: Ben Hutchings <redacted>
---
I'm not sure whether there is any point in all the _t type aliases, but
they're defined for all the other explicit-width types.
Ben.
arch/x86/include/asm/types.h | 8 ++++++++
include/linux/types.h | 12 ++++++++++++
2 files changed, 20 insertions(+), 0 deletions(-)
--
1.7.7.6
--
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.
From: Ben Hutchings <hidden> Date: 2012-08-22 01:23:25
Define reado(), writeo() and their raw counterparts using SSE.
Based on work by Stuart Hodgson [off-list ref].
Signed-off-by: Ben Hutchings <redacted>
---
arch/x86/Kconfig.cpu | 4 +++
arch/x86/include/asm/io.h | 14 +++++++++
arch/x86/lib/Makefile | 1 +
arch/x86/lib/oword_io.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 84 insertions(+), 0 deletions(-)
create mode 100644 arch/x86/lib/oword_io.c
@@ -95,6 +95,20 @@ build_mmio_write(writeq, "q", unsigned long, "r", :"memory")#endif+#ifdef CONFIG_X86_USE_SSE++u128reado(constvolatilevoid__iomem*addr);+voidwriteo(u128val,volatilevoid__iomem*addr);++#define __raw_reado(addr) reado(addr)+#define __raw_writeo(val, addr) writeo(val, addr)++/* Let people know that we have them */+#define reado reado+#define writeo writeo++#endif+/***virt_to_phys-mapvirtualaddressestophysical*@address:addresstoremap
--
1.7.7.6
--
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.
From: Ben Hutchings <hidden> Date: 2012-08-22 01:26:47
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>
---
David,
Assuming the previous 2 patches are accepted for x86, I expect you'll
still want to take this into the net-next tree. So perhaps they should
go onto a separate branch that you can pull?
Ben.
drivers/net/ethernet/sfc/bitfield.h | 3 +++
drivers/net/ethernet/sfc/io.h | 18 ++++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
--
1.7.7.6
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 01:37:44
On 08/21/2012 06:23 PM, Ben Hutchings wrote:
Define reado(), writeo() and their raw counterparts using SSE.
Based on work by Stuart Hodgson [off-list ref].
It would be vastly better if we explicitly controlled this with
kernel_fpu_begin()/kernel_fpu_end() rather than hiding it in primitives
than might tempt the user to do very much the wrong thing.
Also, it needs to be extremely clear to the user that these operations
use the FPU, and all the requirements there need to be met, including
not using them at interrupt time.
-hpa
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 01:38:41
On 08/21/2012 06:17 PM, Ben Hutchings wrote:
Current Solarflare network controllers have 128-bit memory-mapped
registers which are normally accessed through a series of I/O
operations. However, it is also possible to access them with a single
MOVAPS instruction on x86_64, and this is measurably faster as it
requires only one PCIe transaction.
Also, have you considered doing this with write combining instead?
-hpa
From: Ben Hutchings <hidden> Date: 2012-08-22 01:43:07
On Tue, 2012-08-21 at 18:38 -0700, H. Peter Anvin wrote:
On 08/21/2012 06:17 PM, Ben Hutchings wrote:
quoted
Current Solarflare network controllers have 128-bit memory-mapped
registers which are normally accessed through a series of I/O
operations. However, it is also possible to access them with a single
MOVAPS instruction on x86_64, and this is measurably faster as it
requires only one PCIe transaction.
Also, have you considered doing this with write combining instead?
We tried it, and it goes horribly wrong. On some systems, the writes
are not combined, but they are reordered in a way the hardware doesn't
support. See the comment at the top of drivers/net/ethernet/sfc/io.h.
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 02:00:09
On 08/21/2012 06:43 PM, Ben Hutchings wrote:
On Tue, 2012-08-21 at 18:38 -0700, H. Peter Anvin wrote:
quoted
On 08/21/2012 06:17 PM, Ben Hutchings wrote:
quoted
Current Solarflare network controllers have 128-bit memory-mapped
registers which are normally accessed through a series of I/O
operations. However, it is also possible to access them with a single
MOVAPS instruction on x86_64, and this is measurably faster as it
requires only one PCIe transaction.
Also, have you considered doing this with write combining instead?
We tried it, and it goes horribly wrong. On some systems, the writes
are not combined, but they are reordered in a way the hardware doesn't
support. See the comment at the top of drivers/net/ethernet/sfc/io.h.
Yes, you have to make sure you properly enforce the necessary ordering
requirements manually (I think you can do that with sfence).
-hpa
From: Ben Hutchings <hidden> Date: 2012-08-22 02:04:15
On Tue, 2012-08-21 at 18:37 -0700, H. Peter Anvin wrote:
On 08/21/2012 06:23 PM, Ben Hutchings wrote:
quoted
Define reado(), writeo() and their raw counterparts using SSE.
Based on work by Stuart Hodgson [off-list ref].
It would be vastly better if we explicitly controlled this with
kernel_fpu_begin()/kernel_fpu_end() rather than hiding it in primitives
than might tempt the user to do very much the wrong thing.
Also, it needs to be extremely clear to the user that these operations
use the FPU, and all the requirements there need to be met, including
not using them at interrupt time.
Well we can sometimes use the FPU state at IRQ time, can't we
(irq_fpu_usable())? So we might need, say, try_reado() and
try_writeo() with callers expected to fall back to alternatives. (Which
they must have anyway for any architecture that doesn't support this.)
Ben.
--
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.
From: Ben Hutchings <hidden> Date: 2012-08-22 02:10:27
On Tue, 2012-08-21 at 18:59 -0700, H. Peter Anvin wrote:
On 08/21/2012 06:43 PM, Ben Hutchings wrote:
quoted
On Tue, 2012-08-21 at 18:38 -0700, H. Peter Anvin wrote:
quoted
On 08/21/2012 06:17 PM, Ben Hutchings wrote:
quoted
Current Solarflare network controllers have 128-bit memory-mapped
registers which are normally accessed through a series of I/O
operations. However, it is also possible to access them with a single
MOVAPS instruction on x86_64, and this is measurably faster as it
requires only one PCIe transaction.
Also, have you considered doing this with write combining instead?
We tried it, and it goes horribly wrong. On some systems, the writes
are not combined, but they are reordered in a way the hardware doesn't
support. See the comment at the top of drivers/net/ethernet/sfc/io.h.
Yes, you have to make sure you properly enforce the necessary ordering
requirements manually (I think you can do that with sfence).
We did put an sfence after the writes to each register. But some
systems only want to combine writes that cover an entire cache line, and
the writes covering a 128-bit register get broken back up into multiple
writes at the PCIe level. And on some systems these are sent in
decreasing address order, which breaks the rules for writing to
TX_DESC_UPD.
To avoid this we'd have to put an sfence in between the writes to a
register, leaving us back where we started.
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 02:31:36
On 08/21/2012 07:10 PM, Ben Hutchings wrote:
quoted
Yes, you have to make sure you properly enforce the necessary ordering
requirements manually (I think you can do that with sfence).
We did put an sfence after the writes to each register. But some
systems only want to combine writes that cover an entire cache line, and
the writes covering a 128-bit register get broken back up into multiple
writes at the PCIe level. And on some systems these are sent in
decreasing address order, which breaks the rules for writing to
TX_DESC_UPD.
To avoid this we'd have to put an sfence in between the writes to a
register, leaving us back where we started.
You realize the same applies to 128-bit writes, right? Some cores
and/or systems will break them up.
-hpa
From: David Miller <davem@davemloft.net> Date: 2012-08-22 02:34:48
From: Ben Hutchings <redacted>
Date: Wed, 22 Aug 2012 03:04:11 +0100
On Tue, 2012-08-21 at 18:37 -0700, H. Peter Anvin wrote:
quoted
On 08/21/2012 06:23 PM, Ben Hutchings wrote:
quoted
Define reado(), writeo() and their raw counterparts using SSE.
Based on work by Stuart Hodgson [off-list ref].
It would be vastly better if we explicitly controlled this with
kernel_fpu_begin()/kernel_fpu_end() rather than hiding it in primitives
than might tempt the user to do very much the wrong thing.
Also, it needs to be extremely clear to the user that these operations
use the FPU, and all the requirements there need to be met, including
not using them at interrupt time.
Well we can sometimes use the FPU state at IRQ time, can't we
(irq_fpu_usable())? So we might need, say, try_reado() and
try_writeo() with callers expected to fall back to alternatives. (Which
they must have anyway for any architecture that doesn't support this.)
I really hope we eventually get rid of this rediculous restriction the
x86 code has.
It really needs a proper stack of FPU state saves like sparc64 has.
Half of the code and complexity in arch/x86/crypto/ would just
disappear, because most of it has to do with handling this obtuse
FPU usage restriction which shouldn't even be an issue in the first
place.
I continually see more and more code that has to check this
irq_fpu_usable() thing, and have ugly fallback code, and therefore is
a sign that this really needs to be fixed properly.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 03:24:40
On 08/21/2012 07:34 PM, David Miller wrote:
I really hope we eventually get rid of this rediculous restriction the
x86 code has.
It really needs a proper stack of FPU state saves like sparc64 has.
Half of the code and complexity in arch/x86/crypto/ would just
disappear, because most of it has to do with handling this obtuse
FPU usage restriction which shouldn't even be an issue in the first
place.
I continually see more and more code that has to check this
irq_fpu_usable() thing, and have ugly fallback code, and therefore is
a sign that this really needs to be fixed properly.
[Cc: Linus, since he has had very strong opinions on this in the past.]
I'm all ears... tell me how sparc64 deals with this, maybe we can
implement something similar. At the same time, do keep in mind that on
x86 this is not just a matter of the FPU state, but the entire "extended
state" which can be very large.
Given the cost of state save/enable, however, nothing is going to change
the need for kernel_fpu_begin/end, nor the fact that those things will
want to bracket large regions.
-hpa
From: David Miller <davem@davemloft.net> Date: 2012-08-22 03:29:47
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 21 Aug 2012 20:24:18 -0700
I'm all ears... tell me how sparc64 deals with this, maybe we can
implement something similar. At the same time, do keep in mind that on
x86 this is not just a matter of the FPU state, but the entire "extended
state" which can be very large.
Sparc's state is pretty huge too. 256 bytes worth of FPU registers,
plus a set of 64-bit control registers.
What we do is we have a FPU stack that grows up from the end of the
thread_info struct, towards the bottom of the kernel stack.
Slot 0 is always the user FPU state.
Slot 1 and further are kernel FPU state save areas.
We hold a counter which keep track of how far deeply saved we are
in the stack.
Not for the purpose of space saving, but for overhead reduction we
sometimes can get away with only saving away half of the FPU
registers. The chip provides a pair of dirty bits, one for the lower
half of the FPU register file and one for the upper half. We only
save the bits that are actually dirty.
Furthermore, when we have FPU using code in the kernel that only uses
the lower half of the registers, we only save away that part of the
state around the routine.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 03:49:39
On 08/21/2012 08:29 PM, David Miller wrote:
What we do is we have a FPU stack that grows up from the end of the
thread_info struct, towards the bottom of the kernel stack.
We have 8K of kernel stack, and an xstate which is pushing a kilobyte
already. This seems like a nightmare. Even if we allocate a larger
stack for this sole purpose, we'd have to put a pretty hard cap on how
far it could grow.
Slot 0 is always the user FPU state.
Slot 1 and further are kernel FPU state save areas.
We hold a counter which keep track of how far deeply saved we are
in the stack.
Not for the purpose of space saving, but for overhead reduction we
sometimes can get away with only saving away half of the FPU
registers. The chip provides a pair of dirty bits, one for the lower
half of the FPU register file and one for the upper half. We only
save the bits that are actually dirty.
Furthermore, when we have FPU using code in the kernel that only uses
the lower half of the registers, we only save away that part of the
state around the routine.
This is messy on x86; it is somewhat doable but it gets really hairy
because of the monolithic [f]xsave/[f]xrstor instruction.
-hpa
On Tue, Aug 21, 2012 at 8:24 PM, H. Peter Anvin [off-list ref] wrote:
quoted
I continually see more and more code that has to check this
irq_fpu_usable() thing, and have ugly fallback code, and therefore is
a sign that this really needs to be fixed properly.
[Cc: Linus, since he has had very strong opinions on this in the past.]
I have strong opinions, because I don't think you *can* do it any
other way on x86.
Saving FPU state on kernel entry IS NOT GOING TO HAPPEN. End of story.
It's too f*cking slow.
And if we don't save it, we cannot use the FP state in kernel mode
without explicitly checking, because the FPU state may not be there,
and touching FP registers can cause exceptions etc. In fact, under
many loads the exception is going to be the common case.
Keeping the FP state live all the time (and then perhaps just saving a
single MMX register for kernel use) and switching it synchronously on
every task switch sounds like a really really bad idea too. It
apparently works better on modern CPU's, but it sucked horribly on
older ones. The FPU state save is horribly expensive, to the point
that we saw it very clearly on benchmarks for signal handling and task
switching.
So the normal operation is going to be "touching the FPU causes a fault".
That said, we might *try* to make the kernel FPU use be cheaper. We
could do something like:
- if we need to restore FPU state for user mode, don't do it
synchronously in kernel_fpu_end(), just set a work-flag, and do it
just once before returning to user mode
- that makes kernel_fpu_end() a no-op, and while we can't get rid of
kernel_fpu_begin(), we could at least make it cheap to do many times
consecutively (ie we'd have to check "who owns FPU state" and perhaps
save it, but the save would have to be done only the first time).
I haven't seen the patch being discussed, or the rationale for it. But
I doubt it makes sense to do 128-bit MMIO and expect any kind of
atomicity things anyway, and I very much doubt that using SSE would
make all that much sense. What's the background, and why would you
want to do this crap?
Linus
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 03:59:43
On 08/21/2012 08:52 PM, Linus Torvalds wrote:
That said, we might *try* to make the kernel FPU use be cheaper. We
could do something like:
- if we need to restore FPU state for user mode, don't do it
synchronously in kernel_fpu_end(), just set a work-flag, and do it
just once before returning to user mode
- that makes kernel_fpu_end() a no-op, and while we can't get rid of
kernel_fpu_begin(), we could at least make it cheap to do many times
consecutively (ie we'd have to check "who owns FPU state" and perhaps
save it, but the save would have to be done only the first time).
The zeroeth order thing we should do is to make it nest; there are cases
where we do multiple kernel_fpu_begin/_end in sequence for no good reason.
kernel_fpu_end() would still have to re-enable preemption (and
preemption would have to check the work flag), but that should be cheap.
We could allow the FPU in the kernel to have preemption, if we allocated
space for two xstates per thread instead of one. That is, however, a
fair hunk of memory.
-hpa
From: David Miller <davem@davemloft.net> Date: 2012-08-22 04:14:31
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 21 Aug 2012 20:59:26 -0700
kernel_fpu_end() would still have to re-enable preemption (and
preemption would have to check the work flag), but that should be cheap.
We could allow the FPU in the kernel to have preemption, if we allocated
space for two xstates per thread instead of one. That is, however, a
fair hunk of memory.
Once you have done the first FPU save for the sake of the kernel, you
can minimize what you save for any deeper nesting because the kernel
only cares about a very limited part of that FPU state not the whole
1K thing.
Those bits you can save by hand with a bunch of explicit stores of the
XMM registers, or something like that.
On Tue, Aug 21, 2012 at 8:59 PM, H. Peter Anvin [off-list ref] wrote:
kernel_fpu_end() would still have to re-enable preemption (and
preemption would have to check the work flag), but that should be cheap.
No, done right, you don't even have to disable preemption. Sure, you
might disable preemption inside of kernel_fpu_begin() itself (around
the test for "do we need to save FPU state and set the flag to restore
it at return-to-user-mode time"), but I think that you should be able
to run the code afterwards with preemption enabled.
Why? Because you've saved the FPU state, and you've set the per-thread
flag saying "I will restore at return to user mode". And that, coupled
with teaching the scheduler about this case ("don't set TS, only save
xmm0 in a special kernel-xmm0-save-area") means that the thing can
preempt fine.
The nesting case would be about just saving (again) that xmm0 register
in any nested kernel_fpu_begin/end pair. But that doesn't need
preemption, that just needs local storage for the kernel_fpu_begin/end
(and it can do the xmm0 save/restore with regular "mov" instructions,
because the kernel owns the FPU at that point, since it's nested).
So it's doable. One question is how many registers to save for kernel
use. It might be that we'd need to make that a "actual FPU user needs
to save/restore the registers it uses", and not do the saving in
kernel_fpu_begin()/end() at all.
My biggest reason to question this all is that I don't think it's
worth it. Why would we ever care to do all this in the first place?
There's no really sane use for it.
Judging from the subject line, some crazy person wants to do 128-bit
MMIO. That's just crap. Nobody sane cares. Do it non-atomically with
regular accesses. If it's high-performance IO, it uses DMA. If it
doesn't use DMA and depends on CPU MMIO accesses, WHO THE F*CK CARES?
Nobody.
And if some crazy device depends on 128-bit atomic writes, the device
is shit. Forget about it. Tell people to stop digging themselves
deeper down a hole it's simply not worth going.
Linus
On Tue, Aug 21, 2012 at 8:52 PM, Linus Torvalds
[off-list ref] wrote:
I haven't seen the patch being discussed, or the rationale for it. But
I doubt it makes sense to do 128-bit MMIO and expect any kind of
atomicity things anyway, and I very much doubt that using SSE would
make all that much sense. What's the background, and why would you
want to do this crap?
Btw, for the 64-bit case, we did have ordering issues, and maybe some
128-bit model has similar ordering issues. Fine. You can't rely on
128-bit atomic accesses anyway on 99% of all hardware - either the CPU
itself cannot do it, or it's too damn inconvenient with XMM only
registers, or the bus itself is limited to 64 bits at a time anyway,
so the CPU or the IO interface would split such an access *anyway*.
So the whole concept of "we rely on atomic 128-bit MMIO accesses"
seems terminally broken. Any driver that thinks it needs that is just
crazy.
And those issues have nothing to do with x86 kernel_fpu_begin/end()
what-so-ever.
So judging by that, I would say that some driver writer needs to take
a few pills, clear up their head, and take another look at their life.
Tell them to look at
include/asm-generic/io-64-nonatomic-hi-lo.h
(and *-lo-hi.h) instead, and ponder the wisdom of just doing it that
way. Tell them to go f*ck themselves if they think they need XMM
registers. They are wrong, for one reason or another.
Linus
My biggest reason to question this all is that I don't think it's
worth it. Why would we ever care to do all this in the first place?
There's no really sane use for it.
All the x86 crypto code hits this case all the time, easiest example
is doing a dm-crypt on a block device when an IPSEC packet arrives.
The crypto code has all of this special code and layering that is
there purely so it can fall back to the slow non-optimized version
of the crypto operation when it hits this can't-nest-fpu-saving
situation.
From: Ben Hutchings <hidden> Date: 2012-08-22 13:27:05
On Tue, 2012-08-21 at 20:52 -0700, Linus Torvalds wrote:
[...]
I haven't seen the patch being discussed, or the rationale for it. But
I doubt it makes sense to do 128-bit MMIO and expect any kind of
atomicity things anyway, and I very much doubt that using SSE would
make all that much sense. What's the background, and why would you
want to do this crap?
It's [off-list ref].
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.)
Ben.
--
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.
On Tue, Aug 21, 2012 at 10:00 PM, David Miller [off-list ref] wrote:
All the x86 crypto code hits this case all the time, easiest example
is doing a dm-crypt on a block device when an IPSEC packet arrives.
The crypto code has all of this special code and layering that is
there purely so it can fall back to the slow non-optimized version
of the crypto operation when it hits this can't-nest-fpu-saving
situation.
Right. And it needs to be there. The current interface is fine and correct.
We can maybe optimize the current model (as outlined earlier), but no,
there's no way in hell we're doing lazy saving of FPU state from
interrupts etc. So all the "check if I can use FPU state at all", and
the explicit kernel_fpu_begin/end() interfaces are absolutely the
right model.
I realize that the people who write that code think that *their* code
is the most important thing in the world, and everything else should
revolve around them, and we should make everything else slower just to
make them happy. But they are wrong.
Deal with it, or do not. You can do the crypto entirely in software. I
think the current model is absolutely the right one, exactly because
it *allows* you to use the FPU for crypto, but it doesn't force the
rest of the kernel to make sure the FPU is always available to you.
I think your complaining about the interface is entirely bogus, and
comes from not appreciating that other areas and uses have other
concerns.
What I would suggest is trying to do profiling, and seeing if the
"let's try to save only once, and restore only when returning to user
space" approach helps. But that's an *implementation* change, not an
interface change. The interface is doing the right thing, the
implementation is just not perhaps optimal.
Linus
On Wed, Aug 22, 2012 at 6:26 AM, Ben Hutchings
[off-list ref] wrote:
It's [off-list 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".
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.
Any patch that exports some "atomic 128-bit MMIO writes" for general
use sounds totally and utterly broken. You can't do it. It's
*fundamentally* an operation that many CPU's cannot even do. 64-bit
buses (or even 32-bit ones) will make the 128-bit write be split up
*anyway*, regardless of any 128-bit register issues.
And nobody else sane cares about this, so it shouldn't even be a
x86-64 specific thing. It should be a driver hack, since that's what
it is. We don't want to support crazy stuff like this in general, that
is not just architecture-, but microarchitecture-specific.
I think you'd be crazy to even want to do this in the first place, but
if you do, keep it internal to your driver, and don't expose the crazy
to anybody else.
Linus
From: Ben Hutchings <hidden> Date: 2012-08-22 14:25:05
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 [off-list 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(-)
--
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.
From: David Laight <hidden> Date: 2012-08-22 14:44:12
Any patch that exports some "atomic 128-bit MMIO writes" for general
use sounds totally and utterly broken. You can't do it. It's
*fundamentally* an operation that many CPU's cannot even do. 64-bit
buses (or even 32-bit ones) will make the 128-bit write be split up
*anyway*, regardless of any 128-bit register issues.
And nobody else sane cares about this, so it shouldn't even be a
x86-64 specific thing...
There are several other processors that can generate long
PCIe transfers, sometimes by using a DMA engine associated
with the PCIe interface (eg freescale 83xx ppc).
Given the slow speed of PCIe transactions (think ISA bus
speeds - at least with some targets), it is important to
be able to request multi-word transfers in a generic way
on systems that can support it.
This support is a property of the PCIe interface block,
not that of the driver that wishes to use the function.
I don't know if XMM register transfers generate 16byte
TLP on any x86 cpus - they might on some.
Perhaps claiming the function is atomic is the real
problem - otherwise a single TLP could be used on
systems (and in contexts) where it is possible, but
a slower mulit-TLP transfer done (possibly without
guaranteeing the transfer order) done where it is not.
David
On Wed, Aug 22, 2012 at 7:24 AM, Ben Hutchings
[off-list ref] wrote:
Sorry, I'll paste it below.
The thing you pasted isn't actually the thing in the subject line.
It's just you *using* it.
I wanted to see what that "writeo()" looks like for x86-64.
But I got google to find it for me by looking for "__raw_writeo", so I
can see the patch now. It looks like it might work. Does it really
help performance despite always doing those TS games in CR0 for each
access?
Linus
This looks like a perfect fit for write combining. I have traces showing
that enabling write combining on MMIO does indeed generate a single PCIe
transaction on at least a couple of different current systems. Why is
that not an option?
-ben
--
"Thought is the essence of where you are now."
On Wed, Aug 22, 2012 at 7:50 AM, Linus Torvalds
[off-list ref] wrote:
But I got google to find it for me by looking for "__raw_writeo", so I
can see the patch now. It looks like it might work. Does it really
help performance despite always doing those TS games in CR0 for each
access?
Btw, are we even certain that a 128-bit PCIe write is going to remain
atomic across a bus (ie over various PCIe bridges etc)? Do you you
care? Is it just a "one transaction is cheaper than two", and it
doesn't really have any ordering constraints? If the thing gets split
into two 64-bit transactions (in whatever order) by a bridge on the
way, would that be ok?
We've seen buses split accesses before (ie 64-bit writes on 32-bit PCI).
Linus
This looks like a perfect fit for write combining. I have traces showing
that enabling write combining on MMIO does indeed generate a single PCIe
transaction on at least a couple of different current systems. Why is
that not an option?
Because reordering, and see the comment at the top of this file.
Ben.
--
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.
From: David Laight <hidden> Date: 2012-08-22 15:06:34
Btw, are we even certain that a 128-bit PCIe write is going to remain
atomic across a bus (ie over various PCIe bridges etc)? Do you you
care? Is it just a "one transaction is cheaper than two", and it
doesn't really have any ordering constraints? If the thing gets split
into two 64-bit transactions (in whatever order) by a bridge on the
way, would that be ok?
PCIe transfers are basically hdlc packets containing the address,
command and any associated data. Unless they get bridged
though some strange PCIe<->PCI<->PCIe system they are very
unlikely to get broken up.
Maybe if they are longer than the maximum TLP size for the
target somewhere - but that is probably at least 128 bytes.
The time taken is largely independent of the transfer size.
The systems I've used (ppc accessing an Altera FPGA) have
PCIe cycles types of the order of microseconds.
Even for slow comms it is important to generate long TLP.
David
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 15:13:58
On 08/22/2012 07:58 AM, Ben Hutchings wrote:
Because reordering, and see the comment at the top of this file.
Your architecture sounds similar to one I once worked on (Orion
Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
device memory, and a single trigger bit would move the head pointer.
We used write combining to write out a set of descriptors, and then used
a non-write-combining write to do the final write which bumps the head
pointer. The UC write flushes the write combiners ahead of it, so it
ends up with two transactions (one for the WC data and one for the UC
trigger) but it could frequently push quite a few descriptors in that
operation.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Wed, Aug 22, 2012 at 8:05 AM, David Laight [off-list ref] wrote:
PCIe transfers are basically hdlc packets containing the address,
command and any associated data. Unless they get bridged
though some strange PCIe<->PCI<->PCIe system they are very
unlikely to get broken up.
It's exactly the odd kind of "mix in a non-native PCIe bridge" setups
I'd worry about. But I guess that is pretty unlikely in any modern
machine (except for thunderbolt, and I think that's going to pass any
PCIe stuff through unchanged).
Linus
From: David Laight <hidden> Date: 2012-08-22 15:30:14
Your architecture sounds similar to one I once worked on (Orion
Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
device memory, and a single trigger bit would move the head pointer.
We used write combining to write out a set of descriptors, and then
used
a non-write-combining write to do the final write which bumps the head
pointer. The UC write flushes the write combiners ahead of it, so it
ends up with two transactions (one for the WC data and one for the UC
trigger) but it could frequently push quite a few descriptors in that
operation.
The code actually looks more like a normal ethernet ring interface
with an 'owner' bit in each entry.
So it is important to write the owner bit last.
It might be possibly to set multiple ring entries in two TLPs
by first writing all of them (maybe with write combining)
but without changing the ownership of the first entry.
Then doing a second transfer to update the owner bit it
the first entry.
The order of the writes in the first transfer would then not
matter.
FWIW can you even guarantee to do an atomic 64bit PCIe transfer
on many systems (without resorting to a dma unit).
David
From: Ben Hutchings <hidden> Date: 2012-08-22 15:41:19
On Wed, 2012-08-22 at 07:50 -0700, Linus Torvalds wrote:
On Wed, Aug 22, 2012 at 7:24 AM, Ben Hutchings
[off-list ref] wrote:
quoted
Sorry, I'll paste it below.
The thing you pasted isn't actually the thing in the subject line.
It's just you *using* it.
I wanted to see what that "writeo()" looks like for x86-64.
But I got google to find it for me by looking for "__raw_writeo", so I
can see the patch now. It looks like it might work. Does it really
help performance despite always doing those TS games in CR0 for each
access?
I haven't run the experiment myself, but my colleagues observed a net
reduction of 100s of nanoseconds of latency. That may not sound like
much, but for a small packet traversing an uncongested twinax link it's
around 5-10% of the total latency from the descriptor pointer write to
DMA completion on the peer.
Later, you wrote:
Btw, are we even certain that a 128-bit PCIe write is going to remain
atomic across a bus (ie over various PCIe bridges etc)?
I don't think PCIe bridges are allowed to split up TLPs (this is why the
PCI core has to be so careful about programming Max Payload Size). What
happens between the processor core and the host bridge is another
matter, though.
Do you you
care? Is it just a "one transaction is cheaper than two", and it
doesn't really have any ordering constraints? If the thing gets split
into two 64-bit transactions (in whatever order) by a bridge on the
way, would that be ok?
We care if the two transactions are not in ascending address order;
that's why we had to abandon write combining.
We've seen buses split accesses before (ie 64-bit writes on 32-bit
PCI).
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 15:50:00
On 08/22/2012 08:27 AM, David Laight wrote:
quoted
Your architecture sounds similar to one I once worked on (Orion
Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
device memory, and a single trigger bit would move the head pointer.
We used write combining to write out a set of descriptors, and then
used
a non-write-combining write to do the final write which bumps the head
pointer. The UC write flushes the write combiners ahead of it, so it
ends up with two transactions (one for the WC data and one for the UC
trigger) but it could frequently push quite a few descriptors in that
operation.
The code actually looks more like a normal ethernet ring interface
with an 'owner' bit in each entry.
So it is important to write the owner bit last.
It might be possibly to set multiple ring entries in two TLPs
by first writing all of them (maybe with write combining)
but without changing the ownership of the first entry.
Then doing a second transfer to update the owner bit it
the first entry.
The order of the writes in the first transfer would then not
matter.
FWIW can you even guarantee to do an atomic 64bit PCIe transfer
on many systems (without resorting to a dma unit).
On many systems, perhaps, but I suspect that 32 bits is the maximum you
can truly guarantee.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
From: Ben Hutchings <hidden> Date: 2012-08-22 15:51:33
On Wed, 2012-08-22 at 16:27 +0100, David Laight wrote:
quoted
Your architecture sounds similar to one I once worked on (Orion
Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
device memory, and a single trigger bit would move the head pointer.
We used write combining to write out a set of descriptors, and then
used
a non-write-combining write to do the final write which bumps the head
pointer. The UC write flushes the write combiners ahead of it, so it
ends up with two transactions (one for the WC data and one for the UC
trigger) but it could frequently push quite a few descriptors in that
operation.
The code actually looks more like a normal ethernet ring interface
with an 'owner' bit in each entry.
So it is important to write the owner bit last.
You're confused. The 'owner' field in the descriptor pointer is part of
the memory protection mechanism for user-level networking. And we don't
have up to 1024 TX descriptors in a single ring, we have up to 1024
separate rings - in host memory, of course. Which is why we have the
'TX push' feature to reduce latency for a currently empty TX queue.
It might be possibly to set multiple ring entries in two TLPs
by first writing all of them (maybe with write combining)
but without changing the ownership of the first entry.
Then doing a second transfer to update the owner bit it
the first entry.
The order of the writes in the first transfer would then not
matter.
FWIW can you even guarantee to do an atomic 64bit PCIe transfer
on many systems (without resorting to a dma unit).
On any architecture that implements readq and writeq these had better be
atomic.
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 15:51:59
On 08/22/2012 08:27 AM, David Laight wrote:
quoted
Your architecture sounds similar to one I once worked on (Orion
Microsystems CNIC/OPA-2). That architecture had a descriptor ring in
device memory, and a single trigger bit would move the head pointer.
We used write combining to write out a set of descriptors, and then
used
a non-write-combining write to do the final write which bumps the head
pointer. The UC write flushes the write combiners ahead of it, so it
ends up with two transactions (one for the WC data and one for the UC
trigger) but it could frequently push quite a few descriptors in that
operation.
The code actually looks more like a normal ethernet ring interface
with an 'owner' bit in each entry.
So it is important to write the owner bit last.
It might be possibly to set multiple ring entries in two TLPs
by first writing all of them (maybe with write combining)
but without changing the ownership of the first entry.
Then doing a second transfer to update the owner bit it
the first entry.
The order of the writes in the first transfer would then not
matter.
The design flaw in that kind of design would be the need to set the
owner bit on every entry. Now, in the case of CNIC/OPA-2 support for
write combining was an explicit design goal, so writes are inert until
the trigger bit is written, at which point the head pointer is moved to
the entry containing the trigger bit. Very effective.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 15:54:18
On 08/22/2012 08:51 AM, Ben Hutchings wrote:
quoted
FWIW can you even guarantee to do an atomic 64bit PCIe transfer
on many systems (without resorting to a dma unit).
On any architecture that implements readq and writeq these had better be
atomic.
Sorry, you fail. There are definitely systems in the field where
readq() and writeq() are implemented, because the CPU supports them,
where the fabric does not guarantee they are intact.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
From: Ben Hutchings <hidden> Date: 2012-08-22 16:44:08
On Wed, 2012-08-22 at 08:54 -0700, H. Peter Anvin wrote:
On 08/22/2012 08:51 AM, Ben Hutchings wrote:
quoted
quoted
FWIW can you even guarantee to do an atomic 64bit PCIe transfer
on many systems (without resorting to a dma unit).
On any architecture that implements readq and writeq these had better be
atomic.
Sorry, you fail. There are definitely systems in the field where
readq() and writeq() are implemented, because the CPU supports them,
where the fabric does not guarantee they are intact.
Well, when the issue of 64-bit MMIO was discussed earlier this year, you
said nothing about this. I thought the conclusion was that any
definitions provided by <asm/io.h> *must* be atomic and drivers can use
<asm-generic/io-64-nonatomic-hi-lo.h> or
<asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 16:49:23
On 08/22/2012 09:44 AM, Ben Hutchings wrote:
quoted
Sorry, you fail. There are definitely systems in the field where
readq() and writeq() are implemented, because the CPU supports them,
where the fabric does not guarantee they are intact.
Well, when the issue of 64-bit MMIO was discussed earlier this year, you
said nothing about this. I thought the conclusion was that any
definitions provided by <asm/io.h> *must* be atomic and drivers can use
<asm-generic/io-64-nonatomic-hi-lo.h> or
<asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.
That is true at the exit interface from the CPU core. Beyond that
drivers have to keep in mind the possible limitations of the
communications fabric between the CPU and the device.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Wed, Aug 22, 2012 at 8:54 AM, H. Peter Anvin [off-list ref] wrote:
Sorry, you fail. There are definitely systems in the field where readq()
and writeq() are implemented, because the CPU supports them, where the
fabric does not guarantee they are intact.
Indeed.
It's unlikely to be an issue with a PCIe driver, though. I'm pretty
sure you can rely on 64-bit transfers there, especially with a CPU
that is modern enough to run 64-bit mode.
That said, even with PCIe, I wonder if older CPU's (think Intel with a
front-side bus, rather than PCIe on die) necessarily always do 128-bit
writes. The FSB is just 64 bits wide, and I could *imagine* that a
PCIe chipset behind the FSB might end up just always generating at
most 64-bit PCIe transactions for host accesses just because that
would be "natural".
Sounds unlikely, but hey, hardware sometimes does odd things.
Linus
On Wed, Aug 22, 2012 at 9:44 AM, Ben Hutchings
[off-list ref] wrote:
Well, when the issue of 64-bit MMIO was discussed earlier this year, you
said nothing about this. I thought the conclusion was that any
definitions provided by <asm/io.h> *must* be atomic and drivers can use
<asm-generic/io-64-nonatomic-hi-lo.h> or
<asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.
Think 32-bit PCI with a 64-bit CPU.
The CPU itself does the 64-bit access no problem. The bus? Not so
much. Even if it's a burst transaction with a single packet, the
actual device on the other side will see the 64-bit value as two
separate parts. Sometimes that matters, sometimes it doesn't (ask
yourself: "What's the atomicity guarantee at the device end? Burst
transaction or individual word of a transaction?").
Again, being limited to PCIe, you are unlikely to hit these issues,
but system bridges can do odd things sometimes, and in the *general*
case it's definitely true that "writeq()" can generate multiple
accesses at the device end even if the *CPU* only generated a single
one.
Linus
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 16:59:18
On 08/22/2012 09:51 AM, Linus Torvalds wrote:
On Wed, Aug 22, 2012 at 8:54 AM, H. Peter Anvin [off-list ref] wrote:
quoted
Sorry, you fail. There are definitely systems in the field where readq()
and writeq() are implemented, because the CPU supports them, where the
fabric does not guarantee they are intact.
Indeed.
It's unlikely to be an issue with a PCIe driver, though. I'm pretty
sure you can rely on 64-bit transfers there, especially with a CPU
that is modern enough to run 64-bit mode.
That said, even with PCIe, I wonder if older CPU's (think Intel with a
front-side bus, rather than PCIe on die) necessarily always do 128-bit
writes. The FSB is just 64 bits wide, and I could *imagine* that a
PCIe chipset behind the FSB might end up just always generating at
most 64-bit PCIe transactions for host accesses just because that
would be "natural".
Sounds unlikely, but hey, hardware sometimes does odd things.
I'm wondering how e.g. a K8 would work (CPU -> HT -> PCIe) on UC memory
there. I know for a fact that some CPU cores break up SSE transactions
into 64-bit transactions.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
From: Ben Hutchings <hidden> Date: 2012-08-22 17:09:08
On Wed, 2012-08-22 at 09:55 -0700, Linus Torvalds wrote:
On Wed, Aug 22, 2012 at 9:44 AM, Ben Hutchings
[off-list ref] wrote:
quoted
Well, when the issue of 64-bit MMIO was discussed earlier this year, you
said nothing about this. I thought the conclusion was that any
definitions provided by <asm/io.h> *must* be atomic and drivers can use
<asm-generic/io-64-nonatomic-hi-lo.h> or
<asm-generic/io-64-nonatomic-lo-hi.h> as a fallback.
Think 32-bit PCI with a 64-bit CPU.
[...]
Well, sure, I'm assuming that the driver is responsible for checking
that the device and its bus interface support an MMIO of the requested
width.
But the architecture code must be responsible for reporting whether the
host supports it, right?
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 17:12:55
On 08/22/2012 10:09 AM, Ben Hutchings wrote:
Well, sure, I'm assuming that the driver is responsible for checking
that the device and its bus interface support an MMIO of the requested
width.
But the architecture code must be responsible for reporting whether the
host supports it, right?
No, the architecture code *can't*.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Wed, Aug 22, 2012 at 10:09 AM, Ben Hutchings
[off-list ref] wrote:
But the architecture code must be responsible for reporting whether the
host supports it, right?
How? It's impossible. As far as the CPU is concerned, the writes
happen atomically. Look how you use it: you don't even query
dynamically about whether the stuff outside the CPU can handle atomic
128-bit writes. You just assume it at compile-time with an #ifdef. How
the hell do you expect that to be able to then say "oh, on this
machine the device you are doing the access to is behind an odd
PCI->PCIe bridge that will split the access"?
Not that we even tend to *know* those kinds of things. It's really
esoteric chipset knowledge. I wouldn't even expect it to be
necessarily documented in the chipset docs, it *might* be in some
NDA'd BIOS writer's guide thing.
You don't even seem to realize that things like the Intel FSB was
patented and wasn't fully documented by Intel at all? And that's for a
bus interface that was used for over a decade from the dominant CPU
manufacturer. What do you think happens with odd random chipsets? Who
do you expects to know?
I *suspect* that 128-bit writes would generally make it intact over
PCIe in real life, but I absolutely wouldn't guarantee it on all
machines. Exactly because of issues like "what happens with a nVidia
host bridge and the old FSB model on older Intel chips?" or "What does
the AMD memory pipeline do?".
Many CPU cores have 64-bit buses even *internally*, much less
externally. Yes, they have atomicity guarantees in their architecture
manual, but go look at it: those talk about memory accesses, and they
are based on cache coherency (these days - they *used* to be based on
certain bus guarantees). The MMIO side is a completely different
animal, and is still based on the bus - and nobody documents that,
afaik.
Linus
From: Ben Hutchings <hidden> Date: 2012-08-22 17:27:30
On Wed, 2012-08-22 at 10:12 -0700, H. Peter Anvin wrote:
On 08/22/2012 10:09 AM, Ben Hutchings wrote:
quoted
Well, sure, I'm assuming that the driver is responsible for checking
that the device and its bus interface support an MMIO of the requested
width.
But the architecture code must be responsible for reporting whether the
host supports it, right?
No, the architecture code *can't*.
So, let me check that I understand this right:
- To support 32-bit architectures, a driver should include one of two
different definitions of readq/writeq depending on which order the
device needs to receive 32-bit operations.
- On 64-bit architectures (or at least x86_64), the system might split
up readq/writeq into 32-bit operations in unspecified order, and the
driver can't control this.
If this is right, how can it be safe to use readq/writeq at all?
Ben.
--
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.
On Wed, Aug 22, 2012 at 10:27 AM, Ben Hutchings
[off-list ref] wrote:
If this is right, how can it be safe to use readq/writeq at all?
Pray.
Or don't care about ordering: use hardware that is well-designed and
doesn't have crap interfaces that are fragile.
If you care about ordering, you need to do them as two separate
accesses, and have a fence in between. Which, quite frankly, sounds
like the right model for you *anyway*, since then you could use
write-combining memory and you might even go faster, despite an
explicit fence and thus a minimum of 2 transactions.
Seriously. If you care that deeply about the ordering of the bytes you
write out, MAKE THAT ORDERING VERY EXPLICIT IN THE SOURCE CODE. Don't
say "oh, with this hack, I win 100ns". You need to ask yourself: what
do you care about more? Going really fast on some machine that you can
test, or being safe?
With PCIe, it's *probably* fine to just say "we expect 64-bit accesses
to make it through unmolested".
The 128-bit case I really don't know about. It probably works too. But
while I'd call the 64-bit case almost certain (in the absence of truly
crap hardware), the 128-bit case I have a hard time judging how
certain it is going to be.
Linus
From: Ben Hutchings <hidden> Date: 2012-08-22 18:11:18
On Wed, 2012-08-22 at 10:54 -0700, Linus Torvalds wrote:
On Wed, Aug 22, 2012 at 10:27 AM, Ben Hutchings
[off-list ref] wrote:
quoted
If this is right, how can it be safe to use readq/writeq at all?
Pray.
Or don't care about ordering: use hardware that is well-designed and
doesn't have crap interfaces that are fragile.
Well the whole point of having the two 32-bit generic implementations is
that hardware may care about the order! How can it be right that a
64-bit implementation assumes it doesn't?
If you care about ordering, you need to do them as two separate
accesses, and have a fence in between. Which, quite frankly, sounds
like the right model for you *anyway*, since then you could use
write-combining memory and you might even go faster, despite an
explicit fence and thus a minimum of 2 transactions.
Yes, which unfortunately is no better than we have at the moment.
Seriously. If you care that deeply about the ordering of the bytes you
write out, MAKE THAT ORDERING VERY EXPLICIT IN THE SOURCE CODE. Don't
say "oh, with this hack, I win 100ns". You need to ask yourself: what
do you care about more? Going really fast on some machine that you can
test, or being safe?
I have to care quite a lot about both. :-) But yes, safety first.
With PCIe, it's *probably* fine to just say "we expect 64-bit accesses
to make it through unmolested".
I have to hope so.
The 128-bit case I really don't know about. It probably works too. But
while I'd call the 64-bit case almost certain (in the absence of truly
crap hardware), the 128-bit case I have a hard time judging how
certain it is going to be.
Right, I think it's been made pretty clear that it's going to be
dependent on more than just architecture.
Ben.
--
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.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2012-08-22 18:18:39
On 08/22/2012 11:11 AM, Ben Hutchings wrote:
Well the whole point of having the two 32-bit generic implementations is
that hardware may care about the order! How can it be right that a
64-bit implementation assumes it doesn't?
On x86 platforms it is pretty much universal (as in: I have never seen
an exception) that transactions that are broken up are broken up in
littleendian order. That was the original readq/writeq implementation
on x86-32, but it screwed up some other architectures.
The other reason to not have readq/writeq by default where we *know* it
can't be supported is that some drivers (e.g. the CNIC/OPA-2 driver I
mentioned) can do better, performance-wise, if it knows that.
-hpa
On Wed, Aug 22, 2012 at 11:11 AM, Ben Hutchings
[off-list ref] wrote:
Right, I think it's been made pretty clear that it's going to be
dependent on more than just architecture.
Well, it's entirely possible that the 128-bit case will work correctly
on all x86-64 hardware out there on PCIe.
I just can't guarantee it, because we certainly have had issues with
hw doing odd things before. But maybe PCIe really is well-specified
enough, and maybe nobody has done a odd PCIe bridges, and maybe every
time some 128-bit access is split, the bus in question still always
remembers the original 128-bit size in the transaction. It's not at
all impossible. I just wouldn't *guarantee* it.
And to some degree, for high-end server-only hardware in particular,
it really *is* acceptable to say "If you have odd hardware, odd things
will happen". So for this particular driver, maybe the right approach
is simply to say "we require that your fabric works right". And see if
anybody ever complains.
The 100ns may be worth those kinds of "you'd better not have old/crap
hardware" decisions. It's not acceptable for some drivers (a driver
for some consumer ATA chip might not want to make that kind of choice,
and say "whatever, we'll be really conservative), but "Quod licet
Jovi, non licet bovi".
The fact that something might not be *guaranteed* to always work
doesn't necessarily mean that it is always the wrong thing to do..
Linus
From: Ben Hutchings <hidden> Date: 2012-08-22 19:01:55
On Wed, 2012-08-22 at 11:28 -0700, Linus Torvalds wrote:
On Wed, Aug 22, 2012 at 11:11 AM, Ben Hutchings
[off-list ref] wrote:
quoted
Right, I think it's been made pretty clear that it's going to be
dependent on more than just architecture.
Well, it's entirely possible that the 128-bit case will work correctly
on all x86-64 hardware out there on PCIe.
I just can't guarantee it, because we certainly have had issues with
hw doing odd things before. But maybe PCIe really is well-specified
enough, and maybe nobody has done a odd PCIe bridges, and maybe every
time some 128-bit access is split, the bus in question still always
remembers the original 128-bit size in the transaction. It's not at
all impossible. I just wouldn't *guarantee* it.
Even then, everything works out OK if the particular MMIO write I'm
concerned about *is* split - just as long as the resulting operations
are in ascending address order. This is not true on all systems if we
enable write-combining (without a fence in the middle, which defeats the
purpose), and I think hpa was saying that it may not be the case with
SSE writes either.
And to some degree, for high-end server-only hardware in particular,
it really *is* acceptable to say "If you have odd hardware, odd things
will happen". So for this particular driver, maybe the right approach
is simply to say "we require that your fabric works right". And see if
anybody ever complains.
Maybe. At the moment reordering tends to cause the hardware to complain
that we sent an invalid sequence of DMA descriptors, but that's only
because we're not being as smart as we could about using TX push. I
don't want to run the risk of sending out corrupted packets (with
offloaded checksums, so they're not that obviously invalid) on the wire.
Ben.
The 100ns may be worth those kinds of "you'd better not have old/crap
hardware" decisions. It's not acceptable for some drivers (a driver
for some consumer ATA chip might not want to make that kind of choice,
and say "whatever, we'll be really conservative), but "Quod licet
Jovi, non licet bovi".
The fact that something might not be *guaranteed* to always work
doesn't necessarily mean that it is always the wrong thing to do..
--
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.
From: David Miller <davem@davemloft.net> Date: 2012-08-22 21:14:36
From: David Miller <davem@davemloft.net>
Date: Tue, 21 Aug 2012 21:14:27 -0700 (PDT)
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 21 Aug 2012 20:59:26 -0700
quoted
kernel_fpu_end() would still have to re-enable preemption (and
preemption would have to check the work flag), but that should be cheap.
We could allow the FPU in the kernel to have preemption, if we allocated
space for two xstates per thread instead of one. That is, however, a
fair hunk of memory.
Once you have done the first FPU save for the sake of the kernel, you
can minimize what you save for any deeper nesting because the kernel
only cares about a very limited part of that FPU state not the whole
1K thing.
Those bits you can save by hand with a bunch of explicit stores of the
XMM registers, or something like that.
BTW, just to clarify, I'm not saying that we should save the FPU on
every trap where we find the FPU enabled or anything stupid like that.
Definitely keep the kern_fpu_begin()/kern_fpu_end() type markers
around FPU usage, but allow some kind of nesting facility.
Here's one idea. Anyone using the existing kern_fpu_*() markers get
the existing behavior. Only one level of kernel FPU usage is allowed.
But a new interface allows specification of a state-save mask. And it
is only users of this interface for which we allow nesting past the
first FPU user.
If this is the first kernel FPU user, we always do the full fxsave or
whatever to push out the full state. For any level of kernel FPU
nesting we save only what is in the save-mask, by hand.
On Wed, Aug 22, 2012 at 2:14 PM, David Miller [off-list ref] wrote:
BTW, just to clarify, I'm not saying that we should save the FPU on
every trap where we find the FPU enabled or anything stupid like that.
Definitely keep the kern_fpu_begin()/kern_fpu_end() type markers
around FPU usage, but allow some kind of nesting facility.
So nesting shouldn't be horrible, but the thing that really screws
with people like the crypto use is not nesting, but the fact that
sometimes you can't save at all, and the whole "kernel_fpu_possible()"
or whatever we call the checking function.
IOW, in [soft]irq context, to avoid races with the irq happening as
the process is going to do something with the FPU state, we don't
allow saving and changing state, because that would mean that the
normal FP state paths would have to be irq-safe, and they aren't.
And once you have to have that fpu possible check, if it happens to
also disallow nested use, I doubt that's going to really affect
anybody. The code has to take the case of "I'm not allowed to change
FPU state" case into account regardless.
Linus
On Wed, Aug 22, 2012 at 2:14 PM, David Miller [off-list ref] wrote:
quoted
BTW, just to clarify, I'm not saying that we should save the FPU on
every trap where we find the FPU enabled or anything stupid like that.
Definitely keep the kern_fpu_begin()/kern_fpu_end() type markers
around FPU usage, but allow some kind of nesting facility.
So nesting shouldn't be horrible, but the thing that really screws
with people like the crypto use is not nesting, but the fact that
sometimes you can't save at all, and the whole "kernel_fpu_possible()"
or whatever we call the checking function.
IOW, in [soft]irq context, to avoid races with the irq happening as
the process is going to do something with the FPU state, we don't
allow saving and changing state, because that would mean that the
normal FP state paths would have to be irq-safe, and they aren't.
And once you have to have that fpu possible check, if it happens to
also disallow nested use, I doubt that's going to really affect
anybody. The code has to take the case of "I'm not allowed to change
FPU state" case into account regardless.
I don't think you really have to do anything special to handle
interrupts properly.
Let's assume that we use some variable length save area at the end of
thread_info to do this nested saving.
When you are asked for FPU usage, you first figure out how much you're
going to save.
Then you advance the allocation pointer in the thread_info, and save
into the space you allocated.
If an interrupt wants to use the FPU, that should be fine as well.
Whether the interrupt FPU save does it's save after you did, or
before, it should work out fine.
I suppose you might have some issues in determining whether we need to
do the full fxsave stuff or not. There could be a state bit for that,
or similar.
Another idea, instead of doing this in thread_info, is to do it on the
local stack. That way if we're in an interrupt, we'll use that
interrupt type's kernel stack.
You might be able to get away with always doing the full FPU
save/restore in that situation.