On 08.01.2010, at 20:35, Hollis Blanchard wrote:
> On Fri, Jan 8, 2010 at 11:32 AM, Alexander Graf <agraf@xxxxxxx> wrote:
>>
>> On 08.01.2010, at 20:29, Hollis Blanchard wrote:
>>
>>> On Thu, Jan 7, 2010 at 5:58 PM, Alexander Graf <agraf@xxxxxxx> wrote:
>>>> Book3S needs some flags in SRR1 to get to know details about an interrupt.
>>>>
>>>> One such example is the trap instruction. It tells the guest kernel that
>>>> a program interrupt is due to a trap using a bit in SRR1.
>>>>
>>>> This patch implements above behavior, making WARN_ON behave like WARN_ON.
>>>
>>> ... "for Book S". It already works properly for Book E, thankyouverymuch. ;)
>>>
>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>>> index 338baf9..e283e44 100644
>>>> --- a/arch/powerpc/kvm/booke.c
>>>> +++ b/arch/powerpc/kvm/booke.c
>>>> @@ -82,8 +82,9 @@ static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
>>>> set_bit(priority, &vcpu->arch.pending_exceptions);
>>>> }
>>>>
>>>> -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
>>>> +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
>>>> {
>>>> + /* BookE does flags in ESR, so ignore those we get here */
>>>> kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
>>>> }
>>>
>>> Actually, I think Book E prematurely sets ESR, since it's done before
>>> the program interrupt is actually delivered. Architecturally, I'm not
>>> sure if it's a problem, but philosophically I've always wanted it to
>>> work the way you've just implemented for Book S.
>>>
>>> Anyways, since we can't test changes at the moment (Yu, can you?), I'd
>>> settle for a comment to the effect that Book E code *should* do this,
>>> but doesn't (rather than the comment above that says it's ok).
>>
>> Hm, can't you just write up a patch that removes the comment I put in, does the ESR setting in the function and do an #ifdef in emulate.c?
>>
>> That way we can incrementally improve things. This series is really about Book3S. Improving BookE should go in a different patch.
>
> Yes, but I'd rather minimize the behavioral changes as long as we can't test it.
Right. That's why the semantics of "flags" are Book3S specific right now and as such the comment is correct.
The behavioral change we need to do later is to take the ESR setting into queue_program, but that should wait until after someone has a board to test things on again ;-). The reminder that we need to do this is hereby archived in the mails :).
Alex--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
