On 14.11.2011, at 18:22, Scott Wood <scottwood@xxxxxxxxxxxxx> wrote:
> On 11/14/2011 07:13 AM, Alexander Graf wrote:
>> On 11/09/2011 01:23 AM, Scott Wood wrote:
>>> +/* Check pending exceptions and deliver one, if possible. */
>>> +void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>>> +{
>>> + WARN_ON_ONCE(!irqs_disabled());
>>> +
>>> + kvmppc_core_check_exceptions(vcpu);
>>> +
>>> + if (vcpu->arch.shared->msr& MSR_WE) {
>>> + local_irq_enable();
>>> + kvm_vcpu_block(vcpu);
>>> + local_irq_disable();
>>
>> Hrm. This specific irq enable/disable part isn't pretty but I can't
>> think of a cleaner way either. Unless you move it out of the prepare
>> function, since I don't see a way it could race with an interrupt.
>
> kvmppc_core_check_exceptions can clear MSR_WE, so we need to check after
> that. We can't enable interrupts after kvmppc_core_check_exceptions (or
> rather, if we do, we need to check again once interrupts are
> re-disabled, as in the MSR_WE case) because otherwise we could have an
> exception delivered afterward and receive the resched IPI at just the
> wrong time to take any action on it (just like the signal check).
Well, but if you enable interrupts here you're basically rendering code that runs earlier in disabled-interrupt mode racy.
How does this align with the signal handling? We could receive a signal here and not handle it the same as the race you fixed earlier, no?
Alex
>
>>> +
>>> + kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
>>> + kvmppc_core_check_exceptions(vcpu);
>>
>> Shouldn't
>>
>> if (msr & MSR_WE) {
>> ...
>> }
>>
>> core_check_exceptions(vcpu);
>>
>>
>> work just as well?
>
> That would have us pointlessly checking the exceptions twice in the
> non-WE case -- unless you mean only check after, which as described
> above means you'll fail to wake up.
>
> -Scott
>
--
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
