On 05/07/2013 10:06 AM, Scott Wood wrote:
On 05/06/2013 08:56:25 PM, tiejun.chen wrote:
On 05/07/2013 07:50 AM, Scott Wood wrote:
On 05/05/2013 10:13:17 PM, tiejun.chen wrote:
On 05/06/2013 11:10 AM, Tiejun Chen wrote:
For the external interrupt, the decrementer exception and the doorbell
excpetion, we also need to soft-disable interrupts while doing as host
interrupt handlers since the DO_KVM hook is always performed to skip
EXCEPTION_COMMON then miss this original chance with the 'ints'
(INTS_DISABLE).
http://patchwork.ozlabs.org/patch/241344/
http://patchwork.ozlabs.org/patch/241412/
:-)
I'm observing the same behaviour as well:
WARN_ON_ONCE(!irqs_disabled());
So, could you explain the benefits of your approach over what's being discussed
in those threads?
They're a long thread so I think I need to take time to see :)
Why wouldn't we always disable them? kvmppc_handle_exit() will enable
interrupts when it's ready.
This only disable soft interrupt for kvmppc_restart_interrupt() that restarts
interrupts if they were meant for the host:
a. SOFT_DISABLE_INTS() only for BOOKE_INTERRUPT_EXTERNAL |
BOOKE_INTERRUPT_DECREMENTER | BOOKE_INTERRUPT_DOORBELL
Those aren't the only exceptions that can end up going to the host. We could
get a TLB miss that results in a heavyweight MMIO exit, etc.
This is like host handler, so I'm just disabling soft interrupt during
kvmppc_restart_interrupt() for Doorbell interrupt/Decrementer Interrupt/External
Input Interrupt.
I don't see anything should be disabled for any TLB exception in host handler.
And I'd rather see any fix for this problem stay out of the asm code.
We already have an appropriate SOFT_DISABLE_INTS so I think we can take this
easily :)
b. bl kvmppc_handle_exit
c. kvmppc_handle_exit()
{
int r = RESUME_HOST;
int s;
/* update before a new last_exit_type is rewritten */
kvmppc_update_timing_stats(vcpu);
/* restart interrupts if they were meant for the host */
kvmppc_restart_interrupt(vcpu, exit_nr);
local_irq_enable(); ==> Enable again.
....
And shouldn't we handle kvmppc_restart_interrupt() like the original HOST flow?
#define MASKABLE_EXCEPTION(trapnum, intnum, label, hdlr, ack) \
START_EXCEPTION(label); \
NORMAL_EXCEPTION_PROLOG(trapnum, intnum, PROLOG_ADDITION_MASKABLE)\
EXCEPTION_COMMON(trapnum, PACA_EXGEN, *INTS_DISABLE*) \
...
Could you elaborate on what you mean?
In host handler, we always use MASKABLE_EXCEPTION() to define-to-handle some
exceptions: Doorbell interrupt/Decrementer Interrupt/External Input Interrupt:
#define MASKABLE_EXCEPTION(trapnum, intnum, label, hdlr, ack) \
START_EXCEPTION(label); \
NORMAL_EXCEPTION_PROLOG(trapnum, intnum, PROLOG_ADDITION_MASKABLE)\
EXCEPTION_COMMON(trapnum, PACA_EXGEN, *INTS_DISABLE*) \
This would call INTS_DISABLE, which is equal to SOFT_DISABLE_INTS(), to disable
soft interrupt before call all associated handlers:
do_IRQ()/timer_interrupt()/doorbell_exception().
But DO_KVM hook always skips INTS_DISABLE.
So I think we also need to do INTS_DISABLE for kvmppc_restart_interrupt() since
actually that restarts interrupts for the host with a similar way as they are
called by host.
Tiejun
--
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
