* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 11:31 UTC (permalink / raw)
To: Madhavan Srinivasan, Benjamin Herrenschmidt
Cc: linuxppc-dev, paulus, kvm-ppc, kvm
In-Reply-To: <53A0248D.7070909@linux.vnet.ibm.com>
On 17.06.14 13:20, Madhavan Srinivasan wrote:
> On Tuesday 17 June 2014 03:13 PM, Alexander Graf wrote:
>> On 17.06.14 11:32, Benjamin Herrenschmidt wrote:
>>> On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
>>>> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
>>>>> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>>>>>> Also, why don't we use twi always or something else that actually is
>>>>>> defined as illegal instruction? I would like to see this shared with
>>>>>> book3s_32 PR.
>>>>> twi will be directed to the guest on HV no ? We want a real illegal
>>>>> because those go to the host (for potential emulation by the HV).
>>>> Ah, good point. I guess we need different one for PR and HV then to
>>>> ensure compatibility with older ISAs on PR.
>>> Well, we also need to be careful with what happens if a PR guest puts
>>> that instruction in, do that stop its HV guest/host ?
>>>
>>> What if it's done in userspace ? Do that stop the kernel ? :-)
>> The way SW breakpointing is handled is that when we see one, it gets
>> deflected into user space. User space then has an array of breakpoints
>> it configured itself. If the breakpoint is part of that list, it
>> consumes it. If not, it injects a debug interrupt (program in this case)
>> into the guest.
>>
>> That way we can overlay that one instruction with as many layers as we
>> like :). We only get a performance hit on execution of that instruction.
>>
>>> Maddy, I haven't checked, does your patch ensure that we only ever stop
>>> if the instruction is at a recorded bkpt address ? It still means that a
>>> userspace process can practically DOS its kernel by issuing a lot of
>>> these causing a crapload of exits.
>> Only user space knows about its breakpoint addresses, so we have to
>> deflect. However since time still ticks on, we only increase jitter of
>> the guest. The process would still get scheduled away after the same
> ^^^ Where is this taken care. I am still trying to understand. Kindly
> can you explain or point to the code. Will help.
We tell the guest via VPA about its steal time which includes QEMU time.
Alex
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-06-17 11:20 UTC (permalink / raw)
To: Alexander Graf, Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus, kvm-ppc, kvm
In-Reply-To: <53A00DB8.4030207@suse.de>
On Tuesday 17 June 2014 03:13 PM, Alexander Graf wrote:
>
> On 17.06.14 11:32, Benjamin Herrenschmidt wrote:
>> On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
>>> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
>>>> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>>>>> Also, why don't we use twi always or something else that actually is
>>>>> defined as illegal instruction? I would like to see this shared with
>>>>> book3s_32 PR.
>>>> twi will be directed to the guest on HV no ? We want a real illegal
>>>> because those go to the host (for potential emulation by the HV).
>>> Ah, good point. I guess we need different one for PR and HV then to
>>> ensure compatibility with older ISAs on PR.
>> Well, we also need to be careful with what happens if a PR guest puts
>> that instruction in, do that stop its HV guest/host ?
>>
>> What if it's done in userspace ? Do that stop the kernel ? :-)
>
> The way SW breakpointing is handled is that when we see one, it gets
> deflected into user space. User space then has an array of breakpoints
> it configured itself. If the breakpoint is part of that list, it
> consumes it. If not, it injects a debug interrupt (program in this case)
> into the guest.
>
> That way we can overlay that one instruction with as many layers as we
> like :). We only get a performance hit on execution of that instruction.
>
>> Maddy, I haven't checked, does your patch ensure that we only ever stop
>> if the instruction is at a recorded bkpt address ? It still means that a
>> userspace process can practically DOS its kernel by issuing a lot of
>> these causing a crapload of exits.
>
> Only user space knows about its breakpoint addresses, so we have to
> deflect. However since time still ticks on, we only increase jitter of
> the guest. The process would still get scheduled away after the same
^^^ Where is this taken care. I am still trying to understand. Kindly
can you explain or point to the code. Will help.
> amount of real time, no?
>
>
> Alex
>
Thanks for review.
Regards
Maddy
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-06-17 11:13 UTC (permalink / raw)
To: Alexander Graf, benh, paulus; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <53A021B4.9040500@suse.de>
On Tuesday 17 June 2014 04:38 PM, Alexander Graf wrote:
>
> On 17.06.14 13:07, Madhavan Srinivasan wrote:
>> On Tuesday 17 June 2014 02:24 PM, Alexander Graf wrote:
>>> On 14.06.14 23:08, Madhavan Srinivasan wrote:
>>>> This patch adds kernel side support for software breakpoint.
>>>> Design is that, by using an illegal instruction, we trap to hypervisor
>>>> via Emulation Assistance interrupt, where we check for the illegal
>>>> instruction
>>>> and accordingly we return to Host or Guest. Patch mandates use of
>>>> "abs" instruction
>>>> (primary opcode 31 and extended opcode 360) as sw breakpoint
>>>> instruction.
>>>> Based on PowerISA v2.01, ABS instruction has been dropped from the
>>>> architecture
>>>> and treated an illegal instruction.
>>>>
>>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>>>> ---
>>>> arch/powerpc/kvm/book3s.c | 3 ++-
>>>> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
>>>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>>> index c254c27..b40fe5d 100644
>>>> --- a/arch/powerpc/kvm/book3s.c
>>>> +++ b/arch/powerpc/kvm/book3s.c
>>>> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
>>>> *vcpu,
>>>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>>> struct kvm_guest_debug *dbg)
>>>> {
>>>> - return -EINVAL;
>>>> + vcpu->guest_debug = dbg->control;
>>>> + return 0;
>>>> }
>>>> void kvmppc_decrementer_func(unsigned long data)
>>>> diff --git a/arch/powerpc/kvm/book3s_hv.c
>>>> b/arch/powerpc/kvm/book3s_hv.c
>>>> index 7a12edb..688421d 100644
>>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>>> @@ -67,6 +67,14 @@
>>>> /* Used as a "null" value for timebase values */
>>>> #define TB_NIL (~(u64)0)
>>>> +/*
>>>> + * SW_BRK_DBG_INT is debug Instruction for supporting Software
>>>> Breakpoint.
>>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>>> opcode is 360.
>>>> + * Based on PowerISA v2.01, ABS instruction has been dropped from the
>>>> architecture
>>>> + * and treated an illegal instruction.
>>>> + */
>>>> +#define SW_BRK_DBG_INT 0x7c0002d0
>>> The instruction we use to trap needs to get exposed to user space via a
>>> ONE_REG property.
>>>
>> Yes. I got to know about that from Bharat (patchset "ppc debug: Add
>> debug stub support"). I will change it.
>>
>>> Also, why don't we use twi always or something else that actually is
>>> defined as illegal instruction? I would like to see this shared with
>>> book3s_32 PR.
>>>
>>>> +
>>>> static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
>>>> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>>>> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct
>>>> kvm_run *run, struct kvm_vcpu *vcpu,
>>>> break;
>>>> /*
>>>> * This occurs if the guest executes an illegal instruction.
>>>> - * We just generate a program interrupt to the guest, since
>>>> - * we don't emulate any guest instructions at this stage.
>>>> + * To support software breakpoint, we check for the sw breakpoint
>>>> + * instruction to return back to host, if not we just generate a
>>>> + * program interrupt to the guest.
>>>> */
>>>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>>>> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>>>> - r = RESUME_GUEST;
>>>> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
>>> Don't access last_inst directly. Instead use the provided helpers.
>>>
>> Ok. Will look and replace it.
>>
>>>> + run->exit_reason = KVM_EXIT_DEBUG;
>>>> + run->debug.arch.address = vcpu->arch.pc;
>>>> + r = RESUME_HOST;
>>>> + } else {
>>>> + kvmppc_core_queue_program(vcpu, 0x80000);
>>> magic numbers
>> ^^^^^
>> I did not understand this?
>
> You're replacing the readable SRR1_PROGILL with the unreadable 0x80000.
> That's bad.
>
Oops. My bad. Will undo that. I guess I messed up when was re basing it.
>
> Alex
>
Thanks for review
Regards
Maddy
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 11:08 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <53A0216F.9060504@linux.vnet.ibm.com>
On 17.06.14 13:07, Madhavan Srinivasan wrote:
> On Tuesday 17 June 2014 02:24 PM, Alexander Graf wrote:
>> On 14.06.14 23:08, Madhavan Srinivasan wrote:
>>> This patch adds kernel side support for software breakpoint.
>>> Design is that, by using an illegal instruction, we trap to hypervisor
>>> via Emulation Assistance interrupt, where we check for the illegal
>>> instruction
>>> and accordingly we return to Host or Guest. Patch mandates use of
>>> "abs" instruction
>>> (primary opcode 31 and extended opcode 360) as sw breakpoint instruction.
>>> Based on PowerISA v2.01, ABS instruction has been dropped from the
>>> architecture
>>> and treated an illegal instruction.
>>>
>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/kvm/book3s.c | 3 ++-
>>> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
>>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>> index c254c27..b40fe5d 100644
>>> --- a/arch/powerpc/kvm/book3s.c
>>> +++ b/arch/powerpc/kvm/book3s.c
>>> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
>>> *vcpu,
>>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>>> struct kvm_guest_debug *dbg)
>>> {
>>> - return -EINVAL;
>>> + vcpu->guest_debug = dbg->control;
>>> + return 0;
>>> }
>>> void kvmppc_decrementer_func(unsigned long data)
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index 7a12edb..688421d 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -67,6 +67,14 @@
>>> /* Used as a "null" value for timebase values */
>>> #define TB_NIL (~(u64)0)
>>> +/*
>>> + * SW_BRK_DBG_INT is debug Instruction for supporting Software
>>> Breakpoint.
>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>> opcode is 360.
>>> + * Based on PowerISA v2.01, ABS instruction has been dropped from the
>>> architecture
>>> + * and treated an illegal instruction.
>>> + */
>>> +#define SW_BRK_DBG_INT 0x7c0002d0
>> The instruction we use to trap needs to get exposed to user space via a
>> ONE_REG property.
>>
> Yes. I got to know about that from Bharat (patchset "ppc debug: Add
> debug stub support"). I will change it.
>
>> Also, why don't we use twi always or something else that actually is
>> defined as illegal instruction? I would like to see this shared with
>> book3s_32 PR.
>>
>>> +
>>> static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
>>> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>>> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct
>>> kvm_run *run, struct kvm_vcpu *vcpu,
>>> break;
>>> /*
>>> * This occurs if the guest executes an illegal instruction.
>>> - * We just generate a program interrupt to the guest, since
>>> - * we don't emulate any guest instructions at this stage.
>>> + * To support software breakpoint, we check for the sw breakpoint
>>> + * instruction to return back to host, if not we just generate a
>>> + * program interrupt to the guest.
>>> */
>>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>>> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>>> - r = RESUME_GUEST;
>>> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
>> Don't access last_inst directly. Instead use the provided helpers.
>>
> Ok. Will look and replace it.
>
>>> + run->exit_reason = KVM_EXIT_DEBUG;
>>> + run->debug.arch.address = vcpu->arch.pc;
>>> + r = RESUME_HOST;
>>> + } else {
>>> + kvmppc_core_queue_program(vcpu, 0x80000);
>> magic numbers
> ^^^^^
> I did not understand this?
You're replacing the readable SRR1_PROGILL with the unreadable 0x80000.
That's bad.
Alex
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-06-17 11:07 UTC (permalink / raw)
To: Alexander Graf, benh, paulus; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <53A0022D.5020108@suse.de>
On Tuesday 17 June 2014 02:24 PM, Alexander Graf wrote:
>
> On 14.06.14 23:08, Madhavan Srinivasan wrote:
>> This patch adds kernel side support for software breakpoint.
>> Design is that, by using an illegal instruction, we trap to hypervisor
>> via Emulation Assistance interrupt, where we check for the illegal
>> instruction
>> and accordingly we return to Host or Guest. Patch mandates use of
>> "abs" instruction
>> (primary opcode 31 and extended opcode 360) as sw breakpoint instruction.
>> Based on PowerISA v2.01, ABS instruction has been dropped from the
>> architecture
>> and treated an illegal instruction.
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/kvm/book3s.c | 3 ++-
>> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index c254c27..b40fe5d 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
>> *vcpu,
>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>> struct kvm_guest_debug *dbg)
>> {
>> - return -EINVAL;
>> + vcpu->guest_debug = dbg->control;
>> + return 0;
>> }
>> void kvmppc_decrementer_func(unsigned long data)
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 7a12edb..688421d 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -67,6 +67,14 @@
>> /* Used as a "null" value for timebase values */
>> #define TB_NIL (~(u64)0)
>> +/*
>> + * SW_BRK_DBG_INT is debug Instruction for supporting Software
>> Breakpoint.
>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>> opcode is 360.
>> + * Based on PowerISA v2.01, ABS instruction has been dropped from the
>> architecture
>> + * and treated an illegal instruction.
>> + */
>> +#define SW_BRK_DBG_INT 0x7c0002d0
>
> The instruction we use to trap needs to get exposed to user space via a
> ONE_REG property.
>
Yes. I got to know about that from Bharat (patchset "ppc debug: Add
debug stub support"). I will change it.
> Also, why don't we use twi always or something else that actually is
> defined as illegal instruction? I would like to see this shared with
> book3s_32 PR.
>
>> +
>> static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
>> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct
>> kvm_run *run, struct kvm_vcpu *vcpu,
>> break;
>> /*
>> * This occurs if the guest executes an illegal instruction.
>> - * We just generate a program interrupt to the guest, since
>> - * we don't emulate any guest instructions at this stage.
>> + * To support software breakpoint, we check for the sw breakpoint
>> + * instruction to return back to host, if not we just generate a
>> + * program interrupt to the guest.
>> */
>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> - r = RESUME_GUEST;
>> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
>
> Don't access last_inst directly. Instead use the provided helpers.
>
Ok. Will look and replace it.
>> + run->exit_reason = KVM_EXIT_DEBUG;
>> + run->debug.arch.address = vcpu->arch.pc;
>> + r = RESUME_HOST;
>> + } else {
>> + kvmppc_core_queue_program(vcpu, 0x80000);
>
> magic numbers
^^^^^
I did not understand this?
>> + r = RESUME_GUEST;
>> + }
>> break;
>> /*
>> * This occurs if the guest (kernel or userspace), does
>> something that
>
> Please enable PR KVM as well while you're at it.
>
My bad, I did not try the PR KVM. I will try it out.
>
> Alex
>
Thanks for review
Regards
Maddy
^ permalink raw reply
* RE: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: David Laight @ 2014-06-17 11:03 UTC (permalink / raw)
To: 'Peter Hurley', Arnd Bergmann,
linuxppc-dev@lists.ozlabs.org
Cc: Greg Kroah-Hartman, Karsten Keil, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org, One Thousand Gnomes
In-Reply-To: <53A01F02.7000202@hurleysoftware.com>
RnJvbTogUGV0ZXIgSHVybGV5DQouLi4NCj4gPiBJIGRvbid0IHVuZGVyc3RhbmQgdGhlIHNlY29u
ZCBoYWxmIG9mIHRoZSBjaGFuZ2Vsb2csIGl0IGRvZXNuJ3Qgc2VlbQ0KPiA+IHRvIGZpdCBoZXJl
OiB0aGVyZSBkZWFkbG9jayB0aGF0IHdlIGFyZSB0cnlpbmcgdG8gYXZvaWQgaGVyZSBoYXBwZW5z
DQo+ID4gd2hlbiB0aGUgKnNhbWUqIHR0eSBuZWVkcyB0aGUgbG9jayB0byBjb21wbGV0ZSB0aGUg
ZnVuY3Rpb24gdGhhdA0KPiA+IHNlbmRzIHRoZSBwZW5kaW5nIGRhdGEuIEkgZG9uJ3QgdGhpbmsg
d2UgZG8gc3RpbGwgZG8gdGhhdCBhbnkgbW9yZSwNCj4gPiBidXQgaXQgZG9lc24ndCBzZWVtIHJl
bGF0ZWQgdG8gdGhlIHR0eSBsb2NrIGJlaW5nIHN5c3RlbS13aWRlIG9yIG5vdC4NCj4gDQo+IFRo
ZSB0dHkgbG9jayBpcyBub3QgdXNlZCBpbiB0aGUgaS9vIHBhdGg7IGl0J3MgcHVycG9zZSBpcyB0
bw0KPiBtdXR1YWxseSBleGNsdWRlIHN0YXRlIGNoYW5nZXMgaW4gb3BlbigpLCBjbG9zZSgpIGFu
ZCBoYW5ndXAoKS4NCj4gDQo+IFRoZSBjb21taXQgdGhhdCBhZGRlZCB0aGlzIFsxXSBjb21tZW50
cyB0aGF0IF9vdGhlcl8gdHR5cyBtYXkgd2FpdA0KPiBmb3IgdGhpcyB0dHkgdG8gY29tcGxldGUs
IGFuZCBjb21tZW50cyBpbiB0aGUgY29kZSBub3RlIHRoYXQgdGhpcw0KPiBmdW5jdGlvbiBzaG91
bGQgYmUgcmVtb3ZlZCB3aGVuIHRoZSBzeXN0ZW0td2lkZSB0dHkgbXV0ZXggd2FzIHJlbW92ZWQN
Cj4gKHdoaWNoIGhhcHBlbmVkIHdpdGggdGhlIGNvbW1pdCBub3RlZCBpbiB0aGUgY2hhbmdlbG9n
KS4NCg0KV2hhdCBoYXBwZW5zIGlmIGFub3RoZXIgcHJvY2VzcyB0cmllcyB0byBkbyBhIG5vbi1i
bG9ja2luZyBvcGVuDQp3aGlsZSB5b3UgYXJlIHNsZWVwaW5nIGluIGNsb3NlIHdhaXRpbmcgZm9y
IG91dHB1dCB0byBkcmFpbj8NCg0KSG9wZWZ1bGx5IHRoaXMgcmV0dXJucyBiZWZvcmUgdGhhdCBk
YXRhIGhhcyBkcmFpbmVkLg0KDQoJRGF2aWQNCg0K
^ permalink raw reply
* Re: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: Peter Hurley @ 2014-06-17 10:57 UTC (permalink / raw)
To: Arnd Bergmann, linuxppc-dev
Cc: Greg Kroah-Hartman, Karsten Keil, linux-kernel, linux-serial,
One Thousand Gnomes
In-Reply-To: <4575870.N9RCpZ4UMg@wuerfel>
On 06/17/2014 04:00 AM, Arnd Bergmann wrote:
> On Monday 16 June 2014 09:17:11 Peter Hurley wrote:
>> tty_wait_until_sent_from_close() drops the tty lock while waiting
>> for the tty driver to finish sending previously accepted data (ie.,
>> data remaining in its write buffer and transmit fifo).
>>
>> However, dropping the tty lock is a hold-over from when the tty
>> lock was system-wide; ie., one lock for all ttys.
>>
>> Since commit 89c8d91e31f267703e365593f6bfebb9f6d2ad01,
>> 'tty: localise the lock', dropping the tty lock has not been necessary.
>>
>> CC: Karsten Keil <isdn@linux-pingi.de>
>> CC: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>
> I don't understand the second half of the changelog, it doesn't seem
> to fit here: there deadlock that we are trying to avoid here happens
> when the *same* tty needs the lock to complete the function that
> sends the pending data. I don't think we do still do that any more,
> but it doesn't seem related to the tty lock being system-wide or not.
The tty lock is not used in the i/o path; it's purpose is to
mutually exclude state changes in open(), close() and hangup().
The commit that added this [1] comments that _other_ ttys may wait
for this tty to complete, and comments in the code note that this
function should be removed when the system-wide tty mutex was removed
(which happened with the commit noted in the changelog).
Regards,
Peter Hurley
[1]
commit a57a7bf3fc7eff00f07eb9c805774d911a3f2472
Author: Jiri Slaby <jslaby@suse.cz>
Date: Thu Aug 25 15:12:06 2011 +0200
TTY: define tty_wait_until_sent_from_close
We need this helper to fix system stalls. The issue is that the rest
of the system TTYs wait for us to finish waiting. This wasn't an issue
with BKL. BKL used to unlock implicitly.
This is based on the Arnd suggestion.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-06-17 10:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Alexander Graf; +Cc: linuxppc-dev, paulus, kvm-ppc, kvm
In-Reply-To: <1402997541.7661.128.camel@pasglop>
On Tuesday 17 June 2014 03:02 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
>> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
>>> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>>>> Also, why don't we use twi always or something else that actually is
>>>> defined as illegal instruction? I would like to see this shared with
>>>> book3s_32 PR.
>>> twi will be directed to the guest on HV no ? We want a real illegal
>>> because those go to the host (for potential emulation by the HV).
>>
>> Ah, good point. I guess we need different one for PR and HV then to
>> ensure compatibility with older ISAs on PR.
>
> Well, we also need to be careful with what happens if a PR guest puts
> that instruction in, do that stop its HV guest/host ?
>
Damn, my mail client is messed up. did not see the mail till now.
I havent tried this incase of PR guest kernel. I will need to try this
before commenting.
> What if it's done in userspace ? Do that stop the kernel ? :-)
>
Basically flow is that, when we see this instruction, we return to host,
and host checks for address in the SW array and if not it returns to kernel.
> Maddy, I haven't checked, does your patch ensure that we only ever stop
> if the instruction is at a recorded bkpt address ? It still means that a
> userspace process can practically DOS its kernel by issuing a lot of
> these causing a crapload of exits.
>
This is valid, userspace can create a mess, need to handle this, meaning
incase if we dont find a valid SW breakpoint for this address in the
HOST, we need to route it to guest and kill it at app.
Regards
Maddy
> Cheers,
> Ben.
>
>> Alex
>>
>>> I'm
>>> trying to see if I can get the architect to set one in stone in a future
>>> proof way.
>>>
>>> Cheers,
>>> Ben.
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* Boot failure in Power7 pSeries
From: Mike Qiu @ 2014-06-17 9:45 UTC (permalink / raw)
To: linuxppc
Hi all,
I use newest linux-next( top commit:
5f295cdf5c5dbbb0c40f10f2ddae02ff46bbf773) to boot up my Power7 machine,
PowerVM mode(HypMode 01), use defualt config file in /boot/, it show
error log below:
OF stdout device is: /vdevice/vty@30000000
Preparing to boot Linux version 3.16.0-rc1-next-20140617+ (root@shui)
(gcc version 4.8.2 20131212 (Red Hat 4.8.2-7) (GCC) ) #5 SMP Tue Jun 17
05:16:21 EDT 2014
Detected machine type: 0000000000000101
Max number of cores passed to firmware: 256 (NR_CPUS = 1024)
Calling ibm,client-architecture-support... done
command line: BOOT_IMAGE=/vmlinux-3.16.0-rc1-next-20140617+
root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/swap rd.md=0 rd.dm=0
vconsole.keymap=us rd.luks=0 vconsole.font=latarcyrheb-sun16 rd.lvt
memory layout at init:
memory_limit : 0000000000000000 (16 MB aligned)
alloc_bottom : 0000000005910000
alloc_top : 0000000010000000
alloc_top_hi : 0000000010000000
rmo_top : 0000000010000000
ram_top : 0000000010000000
instantiating rtas at 0x000000000ee80000... done
Querying for OPAL presence... DEFAULT CATCH!, exception-handler=fff00700
at %SRR0: 00000000041a1c14 %SRR1: 0000000000081002
Open Firmware exception handler entered from non-OF code
Client's Fix Pt Regs:
00 00000000042c017c 00000000042c2ce8 0000000004ae8d58 00000000042c2f38
04 000000000369aafc 00000000042c2f38 0000000001adc100 00000000042c2f38
08 0000000000000000 0000000004328d58 0000000028002024 0000000000001002
0c a000000000000001 0000000000000000 0000000001a9fd20 00000000041a7df8
10 00000000041a2130 00000000041a1e70 f821ff913d220005 0000000001a9fd20
14 0000000000007962 000000000ee80000 0000000001180000 000000000ee80000
18 00000000041a2610 0000000003690000 00000000042c3070 00000000041a1ce8
1c 00000000041a1ce0 00000000041b89f0 0000000000000003 0000000000000001
Special Regs:
%IV: 00000700 %CR: 48002024 %XER: 00000000 %DSISR: 40000000
%SRR0: 00000000041a1c14 %SRR1: 0000000000081002
%LR: 000000000369aafc %CTR: 0000000000000000
%DAR: f821ff913d220035
Virtual PID = 0
ok
0 >
Thanks
Mike
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 9:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <1402997541.7661.128.camel@pasglop>
On 17.06.14 11:32, Benjamin Herrenschmidt wrote:
> On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
>> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
>>> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>>>> Also, why don't we use twi always or something else that actually is
>>>> defined as illegal instruction? I would like to see this shared with
>>>> book3s_32 PR.
>>> twi will be directed to the guest on HV no ? We want a real illegal
>>> because those go to the host (for potential emulation by the HV).
>> Ah, good point. I guess we need different one for PR and HV then to
>> ensure compatibility with older ISAs on PR.
> Well, we also need to be careful with what happens if a PR guest puts
> that instruction in, do that stop its HV guest/host ?
>
> What if it's done in userspace ? Do that stop the kernel ? :-)
The way SW breakpointing is handled is that when we see one, it gets
deflected into user space. User space then has an array of breakpoints
it configured itself. If the breakpoint is part of that list, it
consumes it. If not, it injects a debug interrupt (program in this case)
into the guest.
That way we can overlay that one instruction with as many layers as we
like :). We only get a performance hit on execution of that instruction.
> Maddy, I haven't checked, does your patch ensure that we only ever stop
> if the instruction is at a recorded bkpt address ? It still means that a
> userspace process can practically DOS its kernel by issuing a lot of
> these causing a crapload of exits.
Only user space knows about its breakpoint addresses, so we have to
deflect. However since time still ticks on, we only increase jitter of
the guest. The process would still get scheduled away after the same
amount of real time, no?
Alex
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Benjamin Herrenschmidt @ 2014-06-17 9:32 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <53A0096D.3020108@suse.de>
On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
> > On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
> >> Also, why don't we use twi always or something else that actually is
> >> defined as illegal instruction? I would like to see this shared with
> >> book3s_32 PR.
> > twi will be directed to the guest on HV no ? We want a real illegal
> > because those go to the host (for potential emulation by the HV).
>
> Ah, good point. I guess we need different one for PR and HV then to
> ensure compatibility with older ISAs on PR.
Well, we also need to be careful with what happens if a PR guest puts
that instruction in, do that stop its HV guest/host ?
What if it's done in userspace ? Do that stop the kernel ? :-)
Maddy, I haven't checked, does your patch ensure that we only ever stop
if the instruction is at a recorded bkpt address ? It still means that a
userspace process can practically DOS its kernel by issuing a lot of
these causing a crapload of exits.
Cheers,
Ben.
> Alex
>
> > I'm
> > trying to see if I can get the architect to set one in stone in a future
> > proof way.
> >
> > Cheers,
> > Ben.
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 9:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <1402996939.7661.126.camel@pasglop>
On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>> Also, why don't we use twi always or something else that actually is
>> defined as illegal instruction? I would like to see this shared with
>> book3s_32 PR.
> twi will be directed to the guest on HV no ? We want a real illegal
> because those go to the host (for potential emulation by the HV).
Ah, good point. I guess we need different one for PR and HV then to
ensure compatibility with older ISAs on PR.
Alex
> I'm
> trying to see if I can get the architect to set one in stone in a future
> proof way.
>
> Cheers,
> Ben.
>
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Benjamin Herrenschmidt @ 2014-06-17 9:22 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <53A0022D.5020108@suse.de>
On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>
> Also, why don't we use twi always or something else that actually is
> defined as illegal instruction? I would like to see this shared with
> book3s_32 PR.
twi will be directed to the guest on HV no ? We want a real illegal
because those go to the host (for potential emulation by the HV). I'm
trying to see if I can get the architect to set one in stone in a future
proof way.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
From: Alexander Graf @ 2014-06-17 9:08 UTC (permalink / raw)
To: Scott Wood
Cc: mihai.caraman@freescale.com, linuxppc-dev@lists.ozlabs.org,
kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <1402688561.6603.583.camel@snotra.buserror.net>
On 13.06.14 21:42, Scott Wood wrote:
> On Fri, 2014-06-13 at 16:55 +0200, Alexander Graf wrote:
>> On 13.06.14 16:43, mihai.caraman@freescale.com wrote:
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Thursday, June 12, 2014 8:05 PM
>>>> To: Caraman Mihai Claudiu-B02008
>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
>>>> dev@lists.ozlabs.org; Wood Scott-B07421
>>>> Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition
>>>> on vcpu schedule
>>>>
>>>> On 06/12/2014 04:00 PM, Mihai Caraman wrote:
>>>>> On vcpu schedule, the condition checked for tlb pollution is too tight.
>>>>> The tlb entries of one vcpu are polluted when a different vcpu from the
>>>>> same partition runs in-between. Relax the current tlb invalidation
>>>>> condition taking into account the lpid.
> Can you quantify the performance improvement from this? We've had bugs
> in this area before, so let's make sure it's worth it before making this
> more complicated.
>
>>>>> Signed-off-by: Mihai Caraman <mihai.caraman <at> freescale.com>
>>>> Your mailer is broken? :)
>>>> This really should be an @.
>>>>
>>>> I think this should work. Scott, please ack.
>>> Alex, you were right. I screwed up the patch description by inverting relax
>>> and tight terms :) It should have been more like this:
>>>
>>> KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
>>>
>>> On vcpu schedule, the condition checked for tlb pollution is too loose.
>>> The tlb entries of a vcpu are polluted (vs stale) only when a different vcpu
>>> within the same logical partition runs in-between. Optimize the tlb invalidation
>>> condition taking into account the lpid.
>> Can't we give every vcpu its own lpid? Or don't we trap on global
>> invalidates?
> That would significantly increase the odds of exhausting LPIDs,
> especially on large chips like t4240 with similarly large VMs. If we
> were to do that, the LPIDs would need to be dynamically assigned (like
> PIDs), and should probably be a separate numberspace per physical core.
True, I didn't realize we only have so few of them. It would however
save us from most flushing as long as we have spare LPIDs available :).
Alex
^ permalink raw reply
* [PATCH v2] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
From: Mihai Caraman @ 2014-06-17 9:02 UTC (permalink / raw)
To: kvm-ppc; +Cc: Scott Wood, Mihai Caraman, linuxppc-dev, kvm
On vcpu schedule, the condition checked for tlb pollution is too loose.
The tlb entries of a vcpu become polluted (vs stale) only when a different
vcpu within the same logical partition runs in-between. Optimize the tlb
invalidation condition taking into account the logical partition id.
With the new invalidation condition, a guest shows 4% performance improvement
on P5020DS while running a memory stress application with the cpu oversubscribed,
the other guest running a cpu intensive workload.
Guest - old invalidation condition
real 3.89
user 3.87
sys 0.01
Guest - enhanced invalidation condition
real 3.75
user 3.73
sys 0.01
Host
real 3.70
user 1.85
sys 0.00
The memory stress application accesses 4KB pages backed by 75% of available
TLB0 entries:
char foo[ENTRIES][4096] __attribute__ ((aligned (4096)));
int main()
{
char bar;
int i, j;
for (i = 0; i < ITERATIONS; i++)
for (j = 0; j < ENTRIES; j++)
bar = foo[j][0];
return 0;
}
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
Cc: Scott Wood <scottwood@freescale.com>
---
v2:
- improve patch name and description
- add performance results
arch/powerpc/kvm/e500mc.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 17e4562..d3b814b0 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -111,10 +111,12 @@ void kvmppc_mmu_msr_notify(struct kvm_vcpu *vcpu, u32 old_msr)
}
static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu);
+static DEFINE_PER_CPU(int, last_lpid_on_cpu);
static void kvmppc_core_vcpu_load_e500mc(struct kvm_vcpu *vcpu, int cpu)
{
struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
+ bool update_last = false, inval_tlb = false;
kvmppc_booke_vcpu_load(vcpu, cpu);
@@ -140,12 +142,24 @@ static void kvmppc_core_vcpu_load_e500mc(struct kvm_vcpu *vcpu, int cpu)
mtspr(SPRN_GDEAR, vcpu->arch.shared->dar);
mtspr(SPRN_GESR, vcpu->arch.shared->esr);
- if (vcpu->arch.oldpir != mfspr(SPRN_PIR) ||
- __get_cpu_var(last_vcpu_on_cpu) != vcpu) {
- kvmppc_e500_tlbil_all(vcpu_e500);
+ if (vcpu->arch.oldpir != mfspr(SPRN_PIR)) {
+ /* stale tlb entries */
+ inval_tlb = update_last = true;
+ } else if (__get_cpu_var(last_vcpu_on_cpu) != vcpu) {
+ update_last = true;
+ /* polluted tlb entries */
+ inval_tlb = __get_cpu_var(last_lpid_on_cpu) ==
+ vcpu->kvm->arch.lpid;
+ }
+
+ if (update_last) {
__get_cpu_var(last_vcpu_on_cpu) = vcpu;
+ __get_cpu_var(last_lpid_on_cpu) = vcpu->kvm->arch.lpid;
}
+ if (inval_tlb)
+ kvmppc_e500_tlbil_all(vcpu_e500);
+
kvmppc_load_guest_fp(vcpu);
}
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 8:54 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1402780097-28827-1-git-send-email-maddy@linux.vnet.ibm.com>
On 14.06.14 23:08, Madhavan Srinivasan wrote:
> This patch adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to hypervisor
> via Emulation Assistance interrupt, where we check for the illegal instruction
> and accordingly we return to Host or Guest. Patch mandates use of "abs" instruction
> (primary opcode 31 and extended opcode 360) as sw breakpoint instruction.
> Based on PowerISA v2.01, ABS instruction has been dropped from the architecture
> and treated an illegal instruction.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> arch/powerpc/kvm/book3s.c | 3 ++-
> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index c254c27..b40fe5d 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> - return -EINVAL;
> + vcpu->guest_debug = dbg->control;
> + return 0;
> }
>
> void kvmppc_decrementer_func(unsigned long data)
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 7a12edb..688421d 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -67,6 +67,14 @@
> /* Used as a "null" value for timebase values */
> #define TB_NIL (~(u64)0)
>
> +/*
> + * SW_BRK_DBG_INT is debug Instruction for supporting Software Breakpoint.
> + * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is 360.
> + * Based on PowerISA v2.01, ABS instruction has been dropped from the architecture
> + * and treated an illegal instruction.
> + */
> +#define SW_BRK_DBG_INT 0x7c0002d0
The instruction we use to trap needs to get exposed to user space via a
ONE_REG property.
Also, why don't we use twi always or something else that actually is
defined as illegal instruction? I would like to see this shared with
book3s_32 PR.
> +
> static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>
> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
> /*
> * This occurs if the guest executes an illegal instruction.
> - * We just generate a program interrupt to the guest, since
> - * we don't emulate any guest instructions at this stage.
> + * To support software breakpoint, we check for the sw breakpoint
> + * instruction to return back to host, if not we just generate a
> + * program interrupt to the guest.
> */
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> - r = RESUME_GUEST;
> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
Don't access last_inst directly. Instead use the provided helpers.
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.address = vcpu->arch.pc;
> + r = RESUME_HOST;
> + } else {
> + kvmppc_core_queue_program(vcpu, 0x80000);
magic numbers
> + r = RESUME_GUEST;
> + }
> break;
> /*
> * This occurs if the guest (kernel or userspace), does something that
Please enable PR KVM as well while you're at it.
Alex
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/book3s: Fix guest MC delivery mechanism to avoid soft lockups in guest.
From: Mahesh J Salgaonkar @ 2014-06-17 8:44 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Michael Neuling
In-Reply-To: <20140617062358.GB12120@drongo>
On 2014-06-17 16:23:58 Tue, Paul Mackerras wrote:
> On Wed, Jun 11, 2014 at 02:18:21PM +0530, Mahesh J Salgaonkar wrote:
> > From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >
> > Currently we forward MCEs to guest which have been recovered by guest.
> > And for unhandled errors we do not deliver the MCE to guest. It looks like
> > with no support of FWNMI in qemu, guest just panics whenever we deliver the
> > recovered MCEs to guest. Also, the existig code used to return to host for
> > unhandled errors which was casuing guest to hang with soft lockups inside
> > guest and makes it difficult to recover guest instance.
> >
> > This patch now forwards all fatal MCEs to guest causing guest to crash/panic.
> > And, for recovered errors we just go back to normal functioning of guest
> > instead of returning to host.
>
> ... having corrupted possibly live values that the guest had in SRR0/1.
>
> Ideally the guest should have cleared MSR[RI] before putting values in
> SRR0/1, so perhaps you could check that and return to the guest
> without giving it a machine check if MSR[RI] is set. But if MSR[RI]
> is clear, the guest is unfixably corrupted because the machine check
> overwrote SRR0/1, and the only thing we can do, in the absence of
> FWNMI support, is give the guest a machine check interrupt and let it
> crash.
Yes agree. I have patch (below) ready for the same, will test/verify and send it
out soon.
Thanks,
-Mahesh.
-------------
Deliver machine check with MSR(RI=0) to guest as MCE
From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 868347e..c9c56ee 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2257,7 +2257,6 @@ machine_check_realmode:
mr r3, r9 /* get vcpu pointer */
bl kvmppc_realmode_machine_check
nop
- cmpdi r3, 0 /* Did we handle MCE ? */
ld r9, HSTATE_KVM_VCPU(r13)
li r12, BOOK3S_INTERRUPT_MACHINE_CHECK
/*
@@ -2270,13 +2269,18 @@ machine_check_realmode:
* The old code used to return to host for unhandled errors which
* was causing guest to hang with soft lockups inside guest and
* makes it difficult to recover guest instance.
+ *
+ * if we receive machine check with MSR(RI=0) then deliver it to
+ * guest as machine check causing guest to crash.
*/
- ld r10, VCPU_PC(r9)
ld r11, VCPU_MSR(r9)
+ andi. r10, r11, MSR_RI /* check for unrecoverable exception */
+ beq 1f /* Deliver a machine check to guest */
+ ld r10, VCPU_PC(r9)
+ cmpdi r3, 0 /* Did we handle MCE ? */
bne 2f /* Continue guest execution. */
/* If not, deliver a machine check. SRR0/1 are already set */
- li r10, BOOK3S_INTERRUPT_MACHINE_CHECK
- ld r11, VCPU_MSR(r9)
+1: li r10, BOOK3S_INTERRUPT_MACHINE_CHECK
bl kvmppc_msr_interrupt
2: b fast_interrupt_c_return
^ permalink raw reply related
* RE: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: David Laight @ 2014-06-17 8:18 UTC (permalink / raw)
To: 'Arnd Bergmann', linuxppc-dev@lists.ozlabs.org
Cc: One Thousand Gnomes, Karsten Keil, Peter Hurley,
Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <4575870.N9RCpZ4UMg@wuerfel>
RnJvbTogQXJuZCBCZXJnbWFubg0KPiBPbiBNb25kYXkgMTYgSnVuZSAyMDE0IDA5OjE3OjExIFBl
dGVyIEh1cmxleSB3cm90ZToNCj4gPiB0dHlfd2FpdF91bnRpbF9zZW50X2Zyb21fY2xvc2UoKSBk
cm9wcyB0aGUgdHR5IGxvY2sgd2hpbGUgd2FpdGluZw0KPiA+IGZvciB0aGUgdHR5IGRyaXZlciB0
byBmaW5pc2ggc2VuZGluZyBwcmV2aW91c2x5IGFjY2VwdGVkIGRhdGEgKGllLiwNCj4gPiBkYXRh
IHJlbWFpbmluZyBpbiBpdHMgd3JpdGUgYnVmZmVyIGFuZCB0cmFuc21pdCBmaWZvKS4NCj4gPg0K
PiA+IEhvd2V2ZXIsIGRyb3BwaW5nIHRoZSB0dHkgbG9jayBpcyBhIGhvbGQtb3ZlciBmcm9tIHdo
ZW4gdGhlIHR0eQ0KPiA+IGxvY2sgd2FzIHN5c3RlbS13aWRlOyBpZS4sIG9uZSBsb2NrIGZvciBh
bGwgdHR5cy4NCj4gPg0KPiA+IFNpbmNlIGNvbW1pdCA4OWM4ZDkxZTMxZjI2NzcwM2UzNjU1OTNm
NmJmZWJiOWY2ZDJhZDAxLA0KPiA+ICd0dHk6IGxvY2FsaXNlIHRoZSBsb2NrJywgZHJvcHBpbmcg
dGhlIHR0eSBsb2NrIGhhcyBub3QgYmVlbiBuZWNlc3NhcnkuDQo+ID4NCj4gPiBDQzogS2Fyc3Rl
biBLZWlsIDxpc2RuQGxpbnV4LXBpbmdpLmRlPg0KPiA+IENDOiBsaW51eHBwYy1kZXZAbGlzdHMu
b3psYWJzLm9yZw0KPiA+IFNpZ25lZC1vZmYtYnk6IFBldGVyIEh1cmxleSA8cGV0ZXJAaHVybGV5
c29mdHdhcmUuY29tPg0KPiANCj4gSSBkb24ndCB1bmRlcnN0YW5kIHRoZSBzZWNvbmQgaGFsZiBv
ZiB0aGUgY2hhbmdlbG9nLCBpdCBkb2Vzbid0IHNlZW0NCj4gdG8gZml0IGhlcmU6IHRoZXJlIGRl
YWRsb2NrIHRoYXQgd2UgYXJlIHRyeWluZyB0byBhdm9pZCBoZXJlIGhhcHBlbnMNCj4gd2hlbiB0
aGUgKnNhbWUqIHR0eSBuZWVkcyB0aGUgbG9jayB0byBjb21wbGV0ZSB0aGUgZnVuY3Rpb24gdGhh
dA0KPiBzZW5kcyB0aGUgcGVuZGluZyBkYXRhLiBJIGRvbid0IHRoaW5rIHdlIGRvIHN0aWxsIGRv
IHRoYXQgYW55IG1vcmUsDQo+IGJ1dCBpdCBkb2Vzbid0IHNlZW0gcmVsYXRlZCB0byB0aGUgdHR5
IGxvY2sgYmVpbmcgc3lzdGVtLXdpZGUgb3Igbm90Lg0KDQpXaGlsZSBJJ3ZlIG5vdCBsb29rZWQg
YXQgdGhlIGNvZGUgaW4gcXVlc3Rpb247IG15IHRob3VnaHRzIHdlcmUgdGhhdA0KaG9sZGluZyBh
bnkgbG9jayB3aGlsZSB3YWl0aW5nIGZvciBvdXRwdXQgdG8gZHJhaW4gKG9yIGFueXRoaW5nIGVs
c2UNCnJlYWxseSkgcHJvYmFibHkgaXNuJ3QgYSBnb29kIGlkZWEuDQpZb3UgbWlnaHQgZmluZCB0
aGF0IHNvbWV0aGluZyBlbHNlIG5lZWRzIHRoZSBsb2NrIC0gZXZlbiBpZiBvbmx5DQpzb21lIGtp
bmQgb2Ygc3RhdHVzIHJlcXVlc3QuDQoNCglEYXZpZA0KDQo=
^ permalink raw reply
* Re: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: Arnd Bergmann @ 2014-06-17 8:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: One Thousand Gnomes, Karsten Keil, Peter Hurley,
Greg Kroah-Hartman, linux-kernel, linux-serial
In-Reply-To: <1402924639-5164-15-git-send-email-peter@hurleysoftware.com>
On Monday 16 June 2014 09:17:11 Peter Hurley wrote:
> tty_wait_until_sent_from_close() drops the tty lock while waiting
> for the tty driver to finish sending previously accepted data (ie.,
> data remaining in its write buffer and transmit fifo).
>
> However, dropping the tty lock is a hold-over from when the tty
> lock was system-wide; ie., one lock for all ttys.
>
> Since commit 89c8d91e31f267703e365593f6bfebb9f6d2ad01,
> 'tty: localise the lock', dropping the tty lock has not been necessary.
>
> CC: Karsten Keil <isdn@linux-pingi.de>
> CC: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
I don't understand the second half of the changelog, it doesn't seem
to fit here: there deadlock that we are trying to avoid here happens
when the *same* tty needs the lock to complete the function that
sends the pending data. I don't think we do still do that any more,
but it doesn't seem related to the tty lock being system-wide or not.
Arnd
^ permalink raw reply
* Re: [PATCH v1] fs2dt: Refine kdump device_tree sort
From: Yang,Wei @ 2014-06-17 6:03 UTC (permalink / raw)
To: Yang,Wei, Wei.Yang, horms; +Cc: linuxppc-dev, linux-kernel, linux-arm-kernel
In-Reply-To: <539FD9A9.1070307@windriver.com>
Simon, I missed "kexec" string in subject, so please ignore this
version. I would resend it with adding "kexec" in subject.
Thanks
Wei
On 06/17/2014 02:01 PM, Yang,Wei wrote:
> Hi Simon,
>
> How about this patch?
>
> Thanks
> Wei
> On 06/12/2014 01:16 PM, Wei.Yang@windriver.com wrote:
>> From: Yang Wei <Wei.Yang@windriver.com>
>>
>> The commit b02d735bf was to rearrange the device-tree entries, and
>> assumed that these entries are sorted in the ascending order. but
>> acctually when I was validating kexec and kdump, the order of
>> serial node still is changed. We should not only compare the length
>> of directory name, but also compare the directory name, it would
>> ensure that the order of device node is really in ascending order.
>>
>> Signed-off-by: Yang Wei <Wei.Yang@windriver.com>
>> ---
>> kexec/fs2dt.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> It is validated on Freescale t4240qds.
>>
>> diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
>> index 1e5f074..0bffaf5 100644
>> --- a/kexec/fs2dt.c
>> +++ b/kexec/fs2dt.c
>> @@ -479,6 +479,9 @@ static int comparefunc(const struct dirent
>> **dentry1,
>> {
>> char *str1 = (*(struct dirent **)dentry1)->d_name;
>> char *str2 = (*(struct dirent **)dentry2)->d_name;
>> + char* ptr1 = strchr(str1, '@');
>> + char* ptr2 = strchr(str2, '@');
>> + int len1, len2;
>> /*
>> * strcmp scans from left to right and fails to idetify for some
>> @@ -486,9 +489,13 @@ static int comparefunc(const struct dirent
>> **dentry1,
>> * Therefore, we get the wrong sorted order like
>> memory@10000000 and
>> * memory@f000000.
>> */
>> - if (strchr(str1, '@') && strchr(str2, '@') &&
>> - (strlen(str1) > strlen(str2)))
>> - return 1;
>> + if (ptr1 && ptr2) {
>> + len1 = ptr1 - str1;
>> + len2 = ptr2 - str2;
>> + if (!strncmp(str1, str2, len1 >len2 ? len1: len2) &&
>> + (strlen(str1) > strlen(str2)))
>> + return 1;
>> + }
>> return strcmp(str1, str2);
>> }
>
>
>
^ permalink raw reply
* [PATCH v1] kexec:fs2dt: Refine kdump device_tree sort
From: Wei.Yang @ 2014-06-17 6:06 UTC (permalink / raw)
To: horms; +Cc: linuxppc-dev, wei.yang, linux-arm-kernel, linux-kernel
From: Yang Wei <Wei.Yang@windriver.com>
The commit b02d735bf was to rearrange the device-tree entries, and
assumed that these entries are sorted in the ascending order. but
acctually when I was validating kexec and kdump, the order of
serial node still is changed. We should not only compare the length
of directory name, but also compare the directory name, it would
ensure that the order of device node is really in ascending order.
Signed-off-by: Yang Wei <Wei.Yang@windriver.com>
---
Hi Simon,
Please help me take a look at this patch. I validated it on freescale t4240qds.
Thanks
Wei
kexec/fs2dt.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
index 1e5f074..0bffaf5 100644
--- a/kexec/fs2dt.c
+++ b/kexec/fs2dt.c
@@ -479,6 +479,9 @@ static int comparefunc(const struct dirent **dentry1,
{
char *str1 = (*(struct dirent **)dentry1)->d_name;
char *str2 = (*(struct dirent **)dentry2)->d_name;
+ char* ptr1 = strchr(str1, '@');
+ char* ptr2 = strchr(str2, '@');
+ int len1, len2;
/*
* strcmp scans from left to right and fails to idetify for some
@@ -486,9 +489,13 @@ static int comparefunc(const struct dirent **dentry1,
* Therefore, we get the wrong sorted order like memory@10000000 and
* memory@f000000.
*/
- if (strchr(str1, '@') && strchr(str2, '@') &&
- (strlen(str1) > strlen(str2)))
- return 1;
+ if (ptr1 && ptr2) {
+ len1 = ptr1 - str1;
+ len2 = ptr2 - str2;
+ if (!strncmp(str1, str2, len1 >len2 ? len1: len2) &&
+ (strlen(str1) > strlen(str2)))
+ return 1;
+ }
return strcmp(str1, str2);
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 4/4] powerpc/book3s: Fix guest MC delivery mechanism to avoid soft lockups in guest.
From: Paul Mackerras @ 2014-06-17 6:23 UTC (permalink / raw)
To: Mahesh J Salgaonkar; +Cc: linuxppc-dev, Michael Neuling
In-Reply-To: <20140611084821.9634.7119.stgit@mars.in.ibm.com>
On Wed, Jun 11, 2014 at 02:18:21PM +0530, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> Currently we forward MCEs to guest which have been recovered by guest.
> And for unhandled errors we do not deliver the MCE to guest. It looks like
> with no support of FWNMI in qemu, guest just panics whenever we deliver the
> recovered MCEs to guest. Also, the existig code used to return to host for
> unhandled errors which was casuing guest to hang with soft lockups inside
> guest and makes it difficult to recover guest instance.
>
> This patch now forwards all fatal MCEs to guest causing guest to crash/panic.
> And, for recovered errors we just go back to normal functioning of guest
> instead of returning to host.
... having corrupted possibly live values that the guest had in SRR0/1.
Ideally the guest should have cleared MSR[RI] before putting values in
SRR0/1, so perhaps you could check that and return to the guest
without giving it a machine check if MSR[RI] is set. But if MSR[RI]
is clear, the guest is unfixably corrupted because the machine check
overwrote SRR0/1, and the only thing we can do, in the absence of
FWNMI support, is give the guest a machine check interrupt and let it
crash.
Paul.
^ permalink raw reply
* [PATCH 5/5] powerpc/ftrace: Use pr_fmt() to namespace error messages
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
The printks() in our ftrace code have no prefix, so they appear on the
console with very little context, eg:
Branch out of range
Use pr_fmt() & pr_err() to add a prefix. While we're at it, collapse a
few split lines that don't need to be, and add a missing newline to one
message.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 96efc66..d178834 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -10,6 +10,8 @@
*
*/
+#define pr_fmt(fmt) "ftrace-powerpc: " fmt
+
#include <linux/spinlock.h>
#include <linux/hardirq.h>
#include <linux/uaccess.h>
@@ -115,7 +117,7 @@ __ftrace_make_nop(struct module *mod,
/* Make sure that that this is still a 24bit jump */
if (!is_bl_op(op)) {
- printk(KERN_ERR "Not expected bl: opcode is %x\n", op);
+ pr_err("Not expected bl: opcode is %x\n", op);
return -EINVAL;
}
@@ -125,12 +127,12 @@ __ftrace_make_nop(struct module *mod,
pr_devel("ip:%lx jumps to %p", ip, tramp);
if (!is_module_trampoline(tramp)) {
- printk(KERN_ERR "Not a trampoline\n");
+ pr_err("Not a trampoline\n");
return -EINVAL;
}
if (module_trampoline_target(mod, tramp, &ptr)) {
- printk(KERN_ERR "Failed to get trampoline target\n");
+ pr_err("Failed to get trampoline target\n");
return -EFAULT;
}
@@ -139,8 +141,7 @@ __ftrace_make_nop(struct module *mod,
entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
if (ptr != entry) {
- printk(KERN_ERR "addr %lx does not match expected %lx\n",
- ptr, entry);
+ pr_err("addr %lx does not match expected %lx\n", ptr, entry);
return -EINVAL;
}
@@ -180,7 +181,7 @@ __ftrace_make_nop(struct module *mod,
/* Make sure that that this is still a 24bit jump */
if (!is_bl_op(op)) {
- printk(KERN_ERR "Not expected bl: opcode is %x\n", op);
+ pr_err("Not expected bl: opcode is %x\n", op);
return -EINVAL;
}
@@ -199,7 +200,7 @@ __ftrace_make_nop(struct module *mod,
/* Find where the trampoline jumps to */
if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
- printk(KERN_ERR "Failed to read %lx\n", tramp);
+ pr_err("Failed to read %lx\n", tramp);
return -EFAULT;
}
@@ -210,7 +211,7 @@ __ftrace_make_nop(struct module *mod,
((jmp[1] & 0xffff0000) != 0x398c0000) ||
(jmp[2] != 0x7d8903a6) ||
(jmp[3] != 0x4e800420)) {
- printk(KERN_ERR "Not a trampoline\n");
+ pr_err("Not a trampoline\n");
return -EINVAL;
}
@@ -222,8 +223,7 @@ __ftrace_make_nop(struct module *mod,
pr_devel(" %lx ", tramp);
if (tramp != addr) {
- printk(KERN_ERR
- "Trampoline location %08lx does not match addr\n",
+ pr_err("Trampoline location %08lx does not match addr\n",
tramp);
return -EINVAL;
}
@@ -264,15 +264,13 @@ int ftrace_make_nop(struct module *mod,
*/
if (!rec->arch.mod) {
if (!mod) {
- printk(KERN_ERR "No module loaded addr=%lx\n",
- addr);
+ pr_err("No module loaded addr=%lx\n", addr);
return -EFAULT;
}
rec->arch.mod = mod;
} else if (mod) {
if (mod != rec->arch.mod) {
- printk(KERN_ERR
- "Record mod %p not equal to passed in mod %p\n",
+ pr_err("Record mod %p not equal to passed in mod %p\n",
rec->arch.mod, mod);
return -EINVAL;
}
@@ -309,25 +307,24 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
* just mask it out when doing the compare.
*/
if ((op[0] != 0x48000008) || ((op[1] & 0xffff0000) != 0xe8410000)) {
- printk(KERN_ERR "Unexpected call sequence: %x %x\n",
- op[0], op[1]);
+ pr_err("Unexpected call sequence: %x %x\n", op[0], op[1]);
return -EINVAL;
}
/* If we never set up a trampoline to ftrace_caller, then bail */
if (!rec->arch.mod->arch.tramp) {
- printk(KERN_ERR "No ftrace trampoline\n");
+ pr_err("No ftrace trampoline\n");
return -EINVAL;
}
/* Ensure branch is within 24 bits */
if (!create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
- printk(KERN_ERR "Branch out of range");
+ pr_err("Branch out of range\n");
return -EINVAL;
}
if (patch_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
- printk(KERN_ERR "REL24 out of range!\n");
+ pr_err("REL24 out of range!\n");
return -EINVAL;
}
@@ -346,13 +343,13 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
/* It should be pointing to a nop */
if (op != PPC_INST_NOP) {
- printk(KERN_ERR "Expected NOP but have %x\n", op);
+ pr_err("Expected NOP but have %x\n", op);
return -EINVAL;
}
/* If we never set up a trampoline to ftrace_caller, then bail */
if (!rec->arch.mod->arch.tramp) {
- printk(KERN_ERR "No ftrace trampoline\n");
+ pr_err("No ftrace trampoline\n");
return -EINVAL;
}
@@ -360,7 +357,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
op = create_branch((unsigned int *)ip,
rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
if (!op) {
- printk(KERN_ERR "REL24 out of range!\n");
+ pr_err("REL24 out of range!\n");
return -EINVAL;
}
@@ -398,7 +395,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
* already have a module defined.
*/
if (!rec->arch.mod) {
- printk(KERN_ERR "No module loaded\n");
+ pr_err("No module loaded\n");
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 4/5] powerpc/ftrace: Fix nop of modules on 64bit LE (ABIv2)
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
There is a bug in the handling of the function entry when we are nopping
out a branch from a module in ftrace.
We compare the result of module_trampoline_target() with the value of
ppc_function_entry(), and expect them to be true. But they never will
be.
module_trampoline_target() will always return the global entry point of
the function, whereas ppc_function_entry() will always return the local.
Fix it by using the newly added ppc_global_function_entry().
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 8fc0c17..96efc66 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -105,7 +105,7 @@ __ftrace_make_nop(struct module *mod,
struct dyn_ftrace *rec, unsigned long addr)
{
unsigned int op;
- unsigned long ptr;
+ unsigned long entry, ptr;
unsigned long ip = rec->ip;
void *tramp;
@@ -136,10 +136,11 @@ __ftrace_make_nop(struct module *mod,
pr_devel("trampoline target %lx", ptr);
+ entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
- if (ptr != ppc_function_entry((void *)addr)) {
+ if (ptr != entry) {
printk(KERN_ERR "addr %lx does not match expected %lx\n",
- ptr, ppc_function_entry((void *)addr));
+ ptr, entry);
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 3/5] powerpc/ftrace: Fix inverted check of create_branch()
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
In commit 24a1bdc35, "Fix ABIv2 issues with __ftrace_make_call", Anton
changed the logic that creates and patches the branch, and added a
thinko in the check of create_branch(). create_branch() returns the
instruction that was generated, so if we get zero then it succeeded.
The result is we can't ftrace modules:
Branch out of range
WARNING: at ../kernel/trace/ftrace.c:1638
ftrace failed to modify [<d000000004ba001c>] fuse_req_init_context+0x1c/0x90 [fuse]
We should probably fix patch_instruction() to do that check and make the
API saner, but that's a separate patch. For now just invert the test.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index f5d1a34..8fc0c17 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -320,7 +320,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
}
/* Ensure branch is within 24 bits */
- if (create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
+ if (!create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
printk(KERN_ERR "Branch out of range");
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox