Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint

[Madhavan Srinivasan <maddy@linux.vnet.ibm.com>]

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?
				
> 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.

> 
> Alex
>