Re: [PATCH] KVM: PPC: BookE: Include trap in pt_regs

[Scott Wood <scottwood@freescale.com>]

On 05/17/2012 04:03 PM, Alexander Graf wrote:
> 
> On 17.05.2012, at 22:58, Scott Wood wrote:
> 
>> On 05/17/2012 03:56 PM, Alexander Graf wrote:
>>>
>>> On 17.05.2012, at 18:37, Scott Wood wrote:
>>>
>>>> On 05/16/2012 08:25 AM, Alexander Graf wrote:
>>>>> When reinjecting host interrupt requests in the exit handler code,
>>>>> let's also tell the interrupt handler which interrupt number we're
>>>>> coming from.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>> arch/powerpc/kvm/booke.c |   12 +++++++-----
>>>>> 1 files changed, 7 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>>>> index 72f13f4..0512bd7 100644
>>>>> --- a/arch/powerpc/kvm/booke.c
>>>>> +++ b/arch/powerpc/kvm/booke.c
>>>>> @@ -596,7 +596,8 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
>>>>> 	}
>>>>> }
>>>>>
>>>>> -static void kvmppc_fill_pt_regs(struct pt_regs *regs)
>>>>> +static void kvmppc_fill_pt_regs(struct pt_regs *regs,
>>>>> +				int exit_nr)
>>>>> {
>>>>> 	ulong r1, ip, msr, lr;
>>>>>
>>>>> @@ -610,6 +611,7 @@ static void kvmppc_fill_pt_regs(struct pt_regs *regs)
>>>>> 	regs->nip = ip;
>>>>> 	regs->msr = msr;
>>>>> 	regs->link = lr;
>>>>> +	regs->trap = exit_nr;
>>>>> }
>>>>>
>>>>> static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
>>>>> @@ -619,16 +621,16 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
>>>>>
>>>>> 	switch (exit_nr) {
>>>>> 	case BOOKE_INTERRUPT_EXTERNAL:
>>>>> -		kvmppc_fill_pt_regs(&regs);
>>>>> +		kvmppc_fill_pt_regs(&regs, exit_nr);
>>>>> 		do_IRQ(&regs);
>>>>> 		break;
>>>>
>>>> exit_nr is not the same as the trap number that the rest of the kernel
>>>> wants to see.
>>>
>>> It's not? What does the kernel want to see then?
>>
>> Remember the commit message in the e500mc patchset about an
>> undocumented, unnamed mess of magic numbers?
> 
> Yeah. So in KVM we're using the IVOR numbers, right? Any reason we can't do the same for the generic handler? That should simplify the code significantly, no?

Yeah, I had that as a FIXME on the original patch.  You just need to
make sure all users are updated, which is more difficult due to the fact
that they're just numbers and not symbolic names.

-Scott