From: Alexander Graf <agraf@suse.de>
To: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org, paulus@samba.org,
kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint
Date: Tue, 12 Aug 2014 14:15:46 +0200 [thread overview]
Message-ID: <53EA0572.1070806@suse.de> (raw)
In-Reply-To: <53E9FBF6.5010008@linux.vnet.ibm.com>
On 12.08.14 13:35, Madhavan Srinivasan wrote:
> On Tuesday 12 August 2014 04:49 PM, Alexander Graf wrote:
>> On 12.08.14 07:17, Madhavan Srinivasan wrote:
>>> On Monday 11 August 2014 02:45 PM, Alexander Graf wrote:
>>>> On 11.08.14 10:51, Benjamin Herrenschmidt wrote:
>>>>> On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote:
>>>>>>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
>>>>>>> index da86d9b..d95014e 100644
>>>>>>> --- a/arch/powerpc/kvm/emulate.c
>>>>>>> +++ b/arch/powerpc/kvm/emulate.c
>>>>>> This should be book3s_emulate.c.
>>>>> Any reason we can't make that 00dddd00 opcode as breakpoint common to
>>>>> all powerpc variants ?
>>>> I can't think of a good reason. We use a hypercall on booke (which traps
>>>> into an illegal instruction for pr) today, but I don't think it has to
>>>> be that way.
>>>>
>>>> Given that the user space API allows us to change it dynamically, there
>>>> should be nothing blocking us from going with 00dddd00 always.
>>>>
>>> Kindly correct me if i am wrong. So we can still have a common code in
>>> emulate.c to set the env for both HV and pr incase of illegal
>>> instruction (i will rebase latest src). But suggestion here to use
>>> 00dddd00, in that case current path in embed is kvmppc_handle_exit
>>> (booke.c) -> BOOKE_INTERRUPT_HV_PRIV -> emulation_exit ->
>>> kvmppc_emulate_instruction, will change to kvmppc_handle_exit (booke.c)
>>> -> BOOKE_INTERRUPT_PROGRAM -> if debug instr call emulation_exit else
>>> send to guest?
>> I can't follow your description above.
>>
> My bad.
>
>> With the latest git version HV KVM does not include emulate.c anymore.
>>
>> Also, it would make a lot of sense of have the same soft breakpoint
>> instruction across all ppc targets, so it would make sense to change it
>> to 0x00dddd00 for booke as well.
>>
> Got it. Was describing the current control flow with respect to booke
> and where changes needed (for same software breakpoint inst). This is
> for my understanding and wanted verify.
>
> kvmppc_handle_exit(booke.c)
> -> BOOKE_INTERRUPT_HV_PRIV
> -> emulation_exit
> ->kvmppc_emulate_instruction
>
> Incase of using the same software breakpoint instruction (0x00dddd00),
> then we need to add code in booke something like this
>
> kvmppc_handle_exit (booke.c)
> -> BOOKE_INTERRUPT_PROGRAM
> -> if debug instr
> ->emulation_exit
> else
> ->send to guest?
Bleks. I see your point. I guess you need something like this for booke:
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 074b7fc..1fdeee0 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -876,6 +876,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu,
case BOOKE_INTERRUPT_HV_PRIV:
emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
break;
+ case BOOKE_INTERRUPT_PROGRAM:
+ /* SW breakpoints arrive as illegal instructions on HV */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
+ emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
+ break;
default:
break;
}
@@ -953,7 +958,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu,
break;
case BOOKE_INTERRUPT_PROGRAM:
- if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) {
+ if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) &&
+ (last_inst != KVMPPC_INST_SOFT_BREAKPOINT)) {
/*
* Program traps generated by user-level software must
* be handled by the guest kernel.
>
>> Basically you would have handling code in emulate.c and book3s_hv.c at
>> the end of the day.
>>
> Yes. Will resend the patch with updated code.
Thanks,
Alex
next prev parent reply other threads:[~2014-08-12 12:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-01 4:50 [PATCH v3] powerpc/kvm: support to handle sw breakpoint Madhavan Srinivasan
2014-08-03 15:51 ` Segher Boessenkool
2014-08-11 3:59 ` Madhavan Srinivasan
2014-08-11 7:26 ` Alexander Graf
2014-08-11 8:51 ` Benjamin Herrenschmidt
2014-08-11 9:15 ` Alexander Graf
2014-08-12 5:17 ` Madhavan Srinivasan
2014-08-12 11:19 ` Alexander Graf
2014-08-12 11:35 ` Madhavan Srinivasan
2014-08-12 12:15 ` Alexander Graf [this message]
2014-08-12 12:21 ` Madhavan Srinivasan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53EA0572.1070806@suse.de \
--to=agraf@suse.de \
--cc=benh@kernel.crashing.org \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.vnet.ibm.com \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).