On 04.01.2013, at 20:40, Scott Wood wrote:
> On 01/04/2013 11:36:37 AM, Alexander Graf wrote:
>> When injecting an interrupt into guest context, we usually don't need
>> to check for requests anymore. At least not until today.
>> With the introduction of EPR, we will have to create a request when the
>> guest has successfully accepted an external interrupt though.
>> So we need to prepare the interrupt delivery to abort guest entry
>> gracefully. Otherwise we'd delay the EPR request.
>> Signed-off-by: Alexander Graf <agraf@xxxxxxx>
>> ---
>> arch/powerpc/kvm/booke.c | 14 ++++++++++----
>> 1 files changed, 10 insertions(+), 4 deletions(-)
>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>> index 69f1140..4ae83f9 100644
>> --- a/arch/powerpc/kvm/booke.c
>> +++ b/arch/powerpc/kvm/booke.c
>> @@ -554,14 +554,16 @@ static void update_timer_ints(struct kvm_vcpu *vcpu)
>> kvmppc_core_dequeue_watchdog(vcpu);
>> }
>> -static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
>> +static int kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
>> {
>> unsigned long *pending = &vcpu->arch.pending_exceptions;
>> unsigned int priority;
>> + int r = 0;
>> priority = __ffs(*pending);
>> while (priority < BOOKE_IRQPRIO_MAX) {
>> - if (kvmppc_booke_irqprio_deliver(vcpu, priority))
>> + r = kvmppc_booke_irqprio_deliver(vcpu, priority);
>> + if (r)
>> break;
>> priority = find_next_bit(pending,
>> @@ -571,15 +573,19 @@ static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu)
>> /* Tell the guest about our interrupt status */
>> vcpu->arch.shared->int_pending = !!*pending;
>> +
>> + return r > 1;
>> }
>
> It isn't until patch 3/4 that the reason for "r > 1" appears...
>
> Why not just:
>
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 69f1140..964f447 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -581,6 +581,11 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>
> kvmppc_core_check_exceptions(vcpu);
>
> + if (vcpu->requests) {
The main reason I didn't do it this way was that until now, we keep the guest runnable with requests pending that we never check for. With this applied, we would end up in an endless loop during guest entry.
I'm not sure which way is better though. It does have a certain appeal to fail hard when we're doing something stupid :).
Alex
> + /* Exception delivery raised request; start over */
> + return 1;
> + }
> +
> if (vcpu->arch.shared->msr & MSR_WE) {
> local_irq_enable();
> kvm_vcpu_block(vcpu);
>
> ...or if you really want to do it through return values, make it an enum rather than a magic "2".
>
> -Scott
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
