From: Waiman Long <longman@redhat.com> Date: 2017-02-13 20:12:49
On 02/13/2017 02:42 PM, Waiman Long wrote:
On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
quoted
On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
quoted
That way we'd end up with something like:
asm("
push %rdi;
movslq %edi, %rdi;
movq __per_cpu_offset(,%rdi,8), %rax;
cmpb $0, %[offset](%rax);
setne %al;
pop %rdi;
" : : [offset] "i" (((unsigned long)&steal_time) + offsetof(struct steal_time, preempted)));
And if we could get rid of the sign extend on edi we could avoid all the
push-pop nonsense, but I'm not sure I see how to do that (then again,
this asm foo isn't my strongest point).
Sorry, I am going to take it back. The displacement or offset can only
be up to 32-bit. So we will still need to use at least one more
register, I think.
Cheers,
Longman
From: Peter Zijlstra <peterz@infradead.org> Date: 2017-02-13 21:54:53
On Mon, Feb 13, 2017 at 03:12:45PM -0500, Waiman Long wrote:
On 02/13/2017 02:42 PM, Waiman Long wrote:
quoted
On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
quoted
On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
quoted
That way we'd end up with something like:
asm("
push %rdi;
movslq %edi, %rdi;
movq __per_cpu_offset(,%rdi,8), %rax;
cmpb $0, %[offset](%rax);
setne %al;
pop %rdi;
" : : [offset] "i" (((unsigned long)&steal_time) + offsetof(struct steal_time, preempted)));
And if we could get rid of the sign extend on edi we could avoid all the
push-pop nonsense, but I'm not sure I see how to do that (then again,
this asm foo isn't my strongest point).
Sorry, I am going to take it back. The displacement or offset can only
be up to 32-bit. So we will still need to use at least one more
register, I think.
I don't think that would be a problem, I very much doubt we declare more
than 4G worth of per-cpu variables in the kernel.
In any case, use "e" or "Z" as constraint (I never quite know when to
use which). That are s32 and u32 displacement immediates resp. and
should fail compile with a semi-sensible failure if the displacement is
too big.
Sorry, I am going to take it back. The displacement or offset can
only
quoted
be up to 32-bit. So we will still need to use at least one more
register, I think.
I don't think that would be a problem, I very much doubt we declare
more
than 4G worth of per-cpu variables in the kernel.
In any case, use "e" or "Z" as constraint (I never quite know when to
use which). That are s32 and u32 displacement immediates resp. and
should fail compile with a semi-sensible failure if the displacement is
too big.
e for signed, Z for unsigned. Obviously you have to use a matching instruction: an immediate or displacement in a 64-bit instruction is sign-extended, in a 32-bit instruction zero-extended. E.g.:
movl %0,%%eax # use Z, all of %rax will be set
movq %0,%%rax # use e
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Sorry, I am going to take it back. The displacement or offset can
only
quoted
be up to 32-bit. So we will still need to use at least one more
register, I think.
I don't think that would be a problem, I very much doubt we declare
more
than 4G worth of per-cpu variables in the kernel.
In any case, use "e" or "Z" as constraint (I never quite know when to
use which). That are s32 and u32 displacement immediates resp. and
should fail compile with a semi-sensible failure if the displacement is
too big.
Oh, and unless you are explicitly forcing 32-bit addressing mode, displacements are always "e" (or "m" if you let gcc pick the addressing mode.)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
From: Waiman Long <longman@redhat.com> Date: 2017-02-13 22:34:14
On 02/13/2017 04:52 PM, Peter Zijlstra wrote:
On Mon, Feb 13, 2017 at 03:12:45PM -0500, Waiman Long wrote:
quoted
On 02/13/2017 02:42 PM, Waiman Long wrote:
quoted
On 02/13/2017 05:53 AM, Peter Zijlstra wrote:
quoted
On Mon, Feb 13, 2017 at 11:47:16AM +0100, Peter Zijlstra wrote:
quoted
That way we'd end up with something like:
asm("
push %rdi;
movslq %edi, %rdi;
movq __per_cpu_offset(,%rdi,8), %rax;
cmpb $0, %[offset](%rax);
setne %al;
pop %rdi;
" : : [offset] "i" (((unsigned long)&steal_time) + offsetof(struct steal_time, preempted)));
And if we could get rid of the sign extend on edi we could avoid all the
push-pop nonsense, but I'm not sure I see how to do that (then again,
this asm foo isn't my strongest point).
Sorry, I am going to take it back. The displacement or offset can only
be up to 32-bit. So we will still need to use at least one more
register, I think.
I don't think that would be a problem, I very much doubt we declare more
than 4G worth of per-cpu variables in the kernel.
In any case, use "e" or "Z" as constraint (I never quite know when to
use which). That are s32 and u32 displacement immediates resp. and
should fail compile with a semi-sensible failure if the displacement is
too big.
It is the address of &steal_time that will exceed the 32-bit limit.
Cheers,
Longman
Sorry, I am going to take it back. The displacement or offset can
only
quoted
quoted
be up to 32-bit. So we will still need to use at least one more
register, I think.
I don't think that would be a problem, I very much doubt we declare
more
quoted
than 4G worth of per-cpu variables in the kernel.
In any case, use "e" or "Z" as constraint (I never quite know when to
use which). That are s32 and u32 displacement immediates resp. and
should fail compile with a semi-sensible failure if the displacement
is
quoted
too big.
It is the address of &steal_time that will exceed the 32-bit limit.
Cheers,
Longman
That seems odd in the extreme?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
From: Waiman Long <longman@redhat.com> Date: 2017-02-14 14:46:33
On 02/14/2017 04:39 AM, Peter Zijlstra wrote:
On Mon, Feb 13, 2017 at 05:34:01PM -0500, Waiman Long wrote:
quoted
It is the address of &steal_time that will exceed the 32-bit limit.
That seems extremely unlikely. That would mean we have more than 4G
worth of per-cpu variables declared in the kernel.
I have some doubt about if the compiler is able to properly use
RIP-relative addressing for this. Anyway, it seems like constraints
aren't allowed for asm() when not in the function context, at least for
the the compiler that I am using (4.8.5). So it is a moot point.
Cheers,
Longman
From: Peter Zijlstra <peterz@infradead.org> Date: 2017-02-14 16:03:35
On Tue, Feb 14, 2017 at 09:46:17AM -0500, Waiman Long wrote:
On 02/14/2017 04:39 AM, Peter Zijlstra wrote:
quoted
On Mon, Feb 13, 2017 at 05:34:01PM -0500, Waiman Long wrote:
quoted
It is the address of &steal_time that will exceed the 32-bit limit.
That seems extremely unlikely. That would mean we have more than 4G
worth of per-cpu variables declared in the kernel.
I have some doubt about if the compiler is able to properly use
RIP-relative addressing for this.
Its not RIP relative, &steal_time lives in the .data..percpu section and
is absolute in that.
Anyway, it seems like constraints
aren't allowed for asm() when not in the function context, at least for
the the compiler that I am using (4.8.5). So it is a moot point.
Well kvm_steal_time is (host/guest) ABI anyway, so the offset is fixed
and hard-coding it isn't a problem.
$ readelf -s defconfig-build/vmlinux | grep steal_time
100843: 0000000000017ac0 64 OBJECT WEAK DEFAULT 35 steal_time
$ objdump -dr defconfig-build/vmlinux | awk '/[<][^>]*[>]:/ { o=0 } /[<]__raw_callee_save___kvm_vcpu_is_preempted[>]:/ {o=1} { if (o) print $0 }'
ffffffff810b4480 <__raw_callee_save___kvm_vcpu_is_preempted>:
ffffffff810b4480: 55 push %rbp
ffffffff810b4481: 48 89 e5 mov %rsp,%rbp
ffffffff810b4484: 48 8b 04 fd 00 94 46 mov -0x7db96c00(,%rdi,8),%rax
ffffffff810b448b: 82
ffffffff810b4488: R_X86_64_32S __per_cpu_offset
ffffffff810b448c: 80 b8 d0 7a 01 00 00 cmpb $0x0,0x17ad0(%rax)
ffffffff810b448e: R_X86_64_32S steal_time+0x10
ffffffff810b4493: 0f 95 c0 setne %al
ffffffff810b4496: 5d pop %rbp
ffffffff810b4497: c3 retq
And as you'll note, the displacement is correct and 'small'.
The below relies on the 'extra' cast in PVOP_CALL_ARG1() to extend the
argument to 64bit on the call side of things.
---
arch/x86/kernel/kvm.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
From: Andrew Cooper <hidden> Date: 2017-02-14 16:18:55
On 14/02/17 14:46, Waiman Long wrote:
On 02/14/2017 04:39 AM, Peter Zijlstra wrote:
quoted
On Mon, Feb 13, 2017 at 05:34:01PM -0500, Waiman Long wrote:
quoted
It is the address of &steal_time that will exceed the 32-bit limit.
That seems extremely unlikely. That would mean we have more than 4G
worth of per-cpu variables declared in the kernel.
I have some doubt about if the compiler is able to properly use
RIP-relative addressing for this. Anyway, it seems like constraints
aren't allowed for asm() when not in the function context, at least for
the the compiler that I am using (4.8.5). So it is a moot point.
You can work the issue of not having parameters in a plain asm()
statement by using an asm-offset, stringizing it, and have C put the
string fragments back together.
"cmpb $0, " STR(STEAL_TIME_preempted) "(%rax);"
~Andrew