Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Sat, 2013-05-11 at 07:49 +1000, Benjamin Herrenschmidt wrote: > I would keep the EE_EDGE bit definition. I have no objection to a gradual > approach however for the other one where we apply it as is now to enable > coreint while you do a rework to make it better :-) Note also that I generally don't apply FSL related patches directly, I rely on Kumar Gala picking them up so he's the one ultimately making that choice. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, 2013-05-10 at 22:12 +0800, Kevin Hao wrote: > So I would assume you will not pick up these two patches, right? > http://patchwork.ozlabs.org/patch/235530/ > http://patchwork.ozlabs.org/patch/235532/ > > Anyway it is more easier to enable the external proxy by using this method. > But if you insist, I can respin a patch to use the method you suggested > since it will definitely reduce the window where the irq is hard disabled. I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, May 10, 2013 at 08:07:00AM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2013-05-09 at 16:27 -0500, Scott Wood wrote: > > On 05/09/2013 07:37:42 AM, Benjamin Herrenschmidt wrote: > > > On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: > > > > > > > > Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur > > > as I > > > > recall. > > > > > > Only if directed to the hypervisor. > > > > This is always the case with KVM, right? At least on booke... > > Hrm, on A2 we could choose iirc. Well not DEC but EXT at least, I don't > remember about DBELL. > > > > > > Case 1) > > > > > -> Local_irq_disable() will set soft_enabled = 0 > > > > > -> Now Externel interrupt happens, there we set PACA_IRQ_EE in > > > > irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard > > > > disabled. No more other interrupt gated by MSR.EE can happen. Looks > > > > like the idea here is to not let a device keep on inserting > > > interrupt > > > > till the interrupt condition on device is cleared, right? > > > > > > The external interrupt line is level sensitive normally, so we have to > > > mask MSR:EE in that case or the interrupt would keep re-occurring > > > (note > > > that FSL has this concept of auto-acked interrupts via the on die MPIC > > > for which you can potentially use PACA_IRQ_EE_EDGE instead and avoid > > > having to mask MSR:EE). > > > > Note that if we do this, we can no longer leave the interrupt vector in > > EPR, and would have to track (potentially multiple different) pending > > external interrupts in software. > > Right, you can have a little queue in the PACA and leave EE disabled if > it's full. You can probably get away with a queue of 1 though :-) So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. Thanks, Kevin > > Cheers, > Ben. > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html pgprrSZaX_six.pgp Description: PGP signature
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, 2013-05-09 at 16:27 -0500, Scott Wood wrote: > On 05/09/2013 07:37:42 AM, Benjamin Herrenschmidt wrote: > > On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: > > > > > > Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur > > as I > > > recall. > > > > Only if directed to the hypervisor. > > This is always the case with KVM, right? At least on booke... Hrm, on A2 we could choose iirc. Well not DEC but EXT at least, I don't remember about DBELL. > > > > Case 1) > > > > -> Local_irq_disable() will set soft_enabled = 0 > > > > -> Now Externel interrupt happens, there we set PACA_IRQ_EE in > > > irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard > > > disabled. No more other interrupt gated by MSR.EE can happen. Looks > > > like the idea here is to not let a device keep on inserting > > interrupt > > > till the interrupt condition on device is cleared, right? > > > > The external interrupt line is level sensitive normally, so we have to > > mask MSR:EE in that case or the interrupt would keep re-occurring > > (note > > that FSL has this concept of auto-acked interrupts via the on die MPIC > > for which you can potentially use PACA_IRQ_EE_EDGE instead and avoid > > having to mask MSR:EE). > > Note that if we do this, we can no longer leave the interrupt vector in > EPR, and would have to track (potentially multiple different) pending > external interrupts in software. Right, you can have a little queue in the PACA and leave EE disabled if it's full. You can probably get away with a queue of 1 though :-) Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, 2013-05-09 at 14:28 +0100, David Laight wrote: > That will happen if the IRQ goes away while the cpu is performing > the IACK sequence. > If the IRQ goes away while the cpu has interrupts masked then > the cpu won't start the interrupt sequence and then try to > read a vector when no interrupt is pending. Right, and get a spurrious vector which shouldn't be a big deal. We tend to call that a "short interrupt". There have been many other cases of short interrupts in the past, for example on some MPICs, when distribution is enabled, they occasionally shoot an interrupt to more than one CPU at once :-) Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
> -Original Message- > From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] > Sent: Thursday, May 09, 2013 8:38 PM > To: Chen, Tiejun > Cc: Bhushan Bharat-R65777; Caraman Mihai Claudiu-B02008; Wood > Scott-B07421; linuxppc-...@lists.ozlabs.org; ag...@suse.de; > kvm-...@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: > soft-disable interrupts > > On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: > > > > Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still > occur as I > > recall. > > Only if directed to the hypervisor. Yes, this is our current booehv design with EPCR[EXTGS] = 0. > > > > Case 1) > > > -> Local_irq_disable() will set soft_enabled = 0 > > > -> Now Externel interrupt happens, there we set PACA_IRQ_EE in > > irq_happened, Also clears EE in SRR1 and rfi. So interrupts > are hard > > disabled. No more other interrupt gated by MSR.EE can happen. Looks > > like the idea here is to not let a device keep on inserting > interrupt > > till the interrupt condition on device is cleared, right? > > The external interrupt line is level sensitive normally, so > we have to mask MSR:EE in that case or the interrupt would > keep re-occurring (note that FSL has this concept of > auto-acked interrupts via the on die MPIC for which you can > potentially use PACA_IRQ_EE_EDGE instead and avoid having to > mask MSR:EE). > > > I don't understand "the interrupt condition on device is cleared" > > here. > > Well, the external interrupt line will remain asserted until > the device (via the PIC) stops interrupting the processor :-) Yes, the device keeps on interrupting the interrupt until the device clear its interrupted condition. > Either because we mask at the PIC level (or raise the > priority) or because the condition goes away. > > > I think regardless if you clear the device interrupt status, the > > system still receive a pending interrupt once EE or GS = 1. > > Why ? Depends on your PIC actually but if it's a sane one, it > won't "remember" a level interrupt that is gone. An edge > interrupt is a different matter and is latched. But the interrupt controller like MPIC should leave this irq as pending if we don't ACK/EOI this irq at all if we just clear the device, right? > > Some MPIC implementations tend to generate a spurrious IRQ in > the case of level IRQs going away. IE. they still remember an > event occurred and interrupt the processor, but on IACK > return the spurious vector. However that isn't guaranteed to Yes, this is just what I mean. No matter if this vector is still valid, the external interrupt exception always occur once EE = 1 again. > be the case and it is perfectly ok (and a good > idea) for the PIC to just stop asserting the external > interrupt line if the original device interrupt goes away > (and it's configured as level sensitive). I don't remember MPIC can work with this way. So I'd like to look the manual again :) Tiejun-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
> Some MPIC implementations tend to generate a spurrious IRQ in the case > of level IRQs going away. IE. they still remember an event occurred and > interrupt the processor, but on IACK return the spurious vector. However > that isn't guaranteed to be the case and it is perfectly ok (and a good > idea) for the PIC to just stop asserting the external interrupt line if > the original device interrupt goes away (and it's configured as level > sensitive). That will happen if the IRQ goes away while the cpu is performing the IACK sequence. If the IRQ goes away while the cpu has interrupts masked then the cpu won't start the interrupt sequence and then try to read a vector when no interrupt is pending. David -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: > > Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I > recall. Only if directed to the hypervisor. > > Case 1) > > -> Local_irq_disable() will set soft_enabled = 0 > > -> Now Externel interrupt happens, there we set PACA_IRQ_EE in > irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard > disabled. No more other interrupt gated by MSR.EE can happen. Looks > like the idea here is to not let a device keep on inserting interrupt > till the interrupt condition on device is cleared, right? The external interrupt line is level sensitive normally, so we have to mask MSR:EE in that case or the interrupt would keep re-occurring (note that FSL has this concept of auto-acked interrupts via the on die MPIC for which you can potentially use PACA_IRQ_EE_EDGE instead and avoid having to mask MSR:EE). > I don't understand "the interrupt condition on device is cleared" > here. Well, the external interrupt line will remain asserted until the device (via the PIC) stops interrupting the processor :-) Either because we mask at the PIC level (or raise the priority) or because the condition goes away. > I think regardless if you clear the device interrupt status, the > system still receive a pending interrupt once EE or GS = 1. Why ? Depends on your PIC actually but if it's a sane one, it won't "remember" a level interrupt that is gone. An edge interrupt is a different matter and is latched. Some MPIC implementations tend to generate a spurrious IRQ in the case of level IRQs going away. IE. they still remember an event occurred and interrupt the processor, but on IACK return the spurious vector. However that isn't guaranteed to be the case and it is perfectly ok (and a good idea) for the PIC to just stop asserting the external interrupt line if the original device interrupt goes away (and it's configured as level sensitive). You might notice that we try to re-hard-enable (while still soft disabled) as soon as we have gone get_irq in do_IRQ. This is because fetching the interrupt normally also masks it (either by masking at the source or by raisin the processor interrupt priority depending on the specific PIC) so we know we can re-hard enable. > > -> local_irq_enable() - This checks that irq_happened is set, and > replays > > ret_from_except also check to replay. ret_from_except checks because it essentially can act as an implicit local_irq_enable. > > Now the case 2) > > Case 2) > > -> Local_irq_disable() will set soft_enabled = 0 > > -> Now DEC interrupt happens. We set PACA_IRQ_DEC in irq_happened, > But do not clear EE in SRR1 and rfi. So interrupts are not hard > disabled. Right. We move the decrementer to its maximum value however since on most implementations, it will continue interrupting the processor while it's top bit is set (and effectively act as a level sensitive interrupt). Again, the goal here is to run as much as possible with interrupts hard enabled which allow better perf sampling. > > -> Now say EE interrupt happens, there we set PACA_IRQ_EE in > irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard > disabled. > > -> local_irq_enable() - This checks that irq_happened is set. > > IIUC, it replays only one interrupt? is not it? It replays one interrupt, but interrupt are still disabled during the replay. Upon return from the replayed interrupt, the ret_from_except will see the other one and replay it too. > After anyone is replayed in arch_local_irq_restore(), we will set > soft/hard > interrupt there: > > set_soft_enabled(1); > __hard_irq_enable(); > > Then any pending interrupt can be executed now. > > Additionally, ret_from_except probably check to replay all. > > Tiejun > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, 2013-05-09 at 16:21 +0800, Kevin Hao wrote: > > Is it because that we cannot afford to lose perfmon interrupt for > more accurate capturing of data ? > > Yes, I think this will definitely improve the perf sample quality. This is one of the primary reason why we implemented lazy disabling in the first place and why I recently reworked it to decrease the periods where we are hard disabled. The other reasons are that storing bytes to the PACA is faster than manipulating EE on many processors. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On 05/09/2013 07:21 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: tiejun.chen [mailto:tiejun.c...@windriver.com] Sent: Thursday, May 09, 2013 3:48 PM To: Bhushan Bharat-R65777 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc- d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts On 05/09/2013 06:00 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: tiejun.chen [mailto:tiejun.c...@windriver.com] Sent: Thursday, May 09, 2013 3:15 PM To: Bhushan Bharat-R65777 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc- d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts On 05/09/2013 04:23 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of bounces+Caraman Mihai Claudiu-B02008 Sent: Wednesday, May 08, 2013 6:44 PM To: Wood Scott-B07421; tiejun.chen Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts 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. 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? I think Tiejun was saying that host has flags and replays only EE/DEC/DBELL interrupts. There is special macro masked_interrupt_book3e in those exception handlers that sets paca- irq_happened. The list of replied interrupts is limited to asynchronous noncritical interrupts which can be masked by MSR[EE] (therefore no TLB miss). Now on KVM book3e we don't want to put them in the irq_happened lazy state but rather to execute them directly, so there is no reason for exception handling symmetry between host and guest. Another Question: The case is: Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I recall. Case 1) -> Local_irq_disable() will set soft_enabled = 0 -> Now Externel interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. No more other interrupt gated by MSR.EE can happen. Looks like the idea here is to not let a device keep on inserting interrupt till the interrupt condition on device is cleared, right? I don't understand "the interrupt condition on device is cleared" here. I think regardless if you clear the device interrupt status, the system still receive a pending interrupt once EE or GS = 1. Once yes, but I think to avoid flood of device interrupt we disable MSR.EE when soft-disabled. But we neither ACK nor send EOI to that irq in the interrupt controller, so that should be in pending state. -> local_irq_enable() - This checks that irq_happened is set, and replays ret_from_except also check to replay. Now the case 2) Case 2) -> Local_irq_disable() will set soft_enabled = 0 -> Now DEC interrupt happens. We set PACA_IRQ_DEC in irq_happened, But do not clear EE in SRR1 and rfi. So interrupts are not hard disabled. -> Now say EE interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. -> local_irq_enable() - This checks that irq_happened is set. IIUC, it replays only one interrupt? is not it? After anyone is replayed in arch_local_irq_restore(), we will set soft/hard interrupt there: set_soft_enabled(1); __hard_irq_enable(); Then any pending interrupt can be executed now. Do you mean that the interrupt should fire again? I means the pending exception including external interrupt, the decrementer exception and the doorbell exception, can trap CPU once EE=1 with __hard_irq_enable() here. Then the kernel can handle those exception since soft enable is also 1 now. Additionally, ret_from_except probably check to replay all. Local_irq_enable() will not take us to ret_from_except. Yes. I just say ret_from_except can provide an approach to replay all :) __replay_interrupt() from arch_local_irq_enable() will ta
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On 05/09/2013 06:00 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: tiejun.chen [mailto:tiejun.c...@windriver.com] Sent: Thursday, May 09, 2013 3:15 PM To: Bhushan Bharat-R65777 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc- d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts On 05/09/2013 04:23 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of bounces+Caraman Mihai Claudiu-B02008 Sent: Wednesday, May 08, 2013 6:44 PM To: Wood Scott-B07421; tiejun.chen Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts 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. 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? I think Tiejun was saying that host has flags and replays only EE/DEC/DBELL interrupts. There is special macro masked_interrupt_book3e in those exception handlers that sets paca- irq_happened. The list of replied interrupts is limited to asynchronous noncritical interrupts which can be masked by MSR[EE] (therefore no TLB miss). Now on KVM book3e we don't want to put them in the irq_happened lazy state but rather to execute them directly, so there is no reason for exception handling symmetry between host and guest. Another Question: The case is: Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I recall. Case 1) -> Local_irq_disable() will set soft_enabled = 0 -> Now Externel interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. No more other interrupt gated by MSR.EE can happen. Looks like the idea here is to not let a device keep on inserting interrupt till the interrupt condition on device is cleared, right? I don't understand "the interrupt condition on device is cleared" here. I think regardless if you clear the device interrupt status, the system still receive a pending interrupt once EE or GS = 1. Once yes, but I think to avoid flood of device interrupt we disable MSR.EE when soft-disabled. But we neither ACK nor send EOI to that irq in the interrupt controller, so that should be in pending state. -> local_irq_enable() - This checks that irq_happened is set, and replays ret_from_except also check to replay. Now the case 2) Case 2) -> Local_irq_disable() will set soft_enabled = 0 -> Now DEC interrupt happens. We set PACA_IRQ_DEC in irq_happened, But do not clear EE in SRR1 and rfi. So interrupts are not hard disabled. -> Now say EE interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. -> local_irq_enable() - This checks that irq_happened is set. IIUC, it replays only one interrupt? is not it? After anyone is replayed in arch_local_irq_restore(), we will set soft/hard interrupt there: set_soft_enabled(1); __hard_irq_enable(); Then any pending interrupt can be executed now. Do you mean that the interrupt should fire again? I means the pending exception including external interrupt, the decrementer exception and the doorbell exception, can trap CPU once EE=1 with __hard_irq_enable() here. Then the kernel can handle those exception since soft enable is also 1 now. Additionally, ret_from_except probably check to replay all. Local_irq_enable() will not take us to ret_from_except. Yes. I just say ret_from_except can provide an approach to replay all :) Tiejun -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On 05/09/2013 04:23 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of Caraman Mihai Claudiu-B02008 Sent: Wednesday, May 08, 2013 6:44 PM To: Wood Scott-B07421; tiejun.chen Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts 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. 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? I think Tiejun was saying that host has flags and replays only EE/DEC/DBELL interrupts. There is special macro masked_interrupt_book3e in those exception handlers that sets paca->irq_happened. The list of replied interrupts is limited to asynchronous noncritical interrupts which can be masked by MSR[EE] (therefore no TLB miss). Now on KVM book3e we don't want to put them in the irq_happened lazy state but rather to execute them directly, so there is no reason for exception handling symmetry between host and guest. Another Question: The case is: Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I recall. Case 1) -> Local_irq_disable() will set soft_enabled = 0 -> Now Externel interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. No more other interrupt gated by MSR.EE can happen. Looks like the idea here is to not let a device keep on inserting interrupt till the interrupt condition on device is cleared, right? I don't understand "the interrupt condition on device is cleared" here. I think regardless if you clear the device interrupt status, the system still receive a pending interrupt once EE or GS = 1. -> local_irq_enable() - This checks that irq_happened is set, and replays ret_from_except also check to replay. Now the case 2) Case 2) -> Local_irq_disable() will set soft_enabled = 0 -> Now DEC interrupt happens. We set PACA_IRQ_DEC in irq_happened, But do not clear EE in SRR1 and rfi. So interrupts are not hard disabled. -> Now say EE interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. -> local_irq_enable() - This checks that irq_happened is set. IIUC, it replays only one interrupt? is not it? After anyone is replayed in arch_local_irq_restore(), we will set soft/hard interrupt there: set_soft_enabled(1); __hard_irq_enable(); Then any pending interrupt can be executed now. Additionally, ret_from_except probably check to replay all. Tiejun -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, May 09, 2013 at 08:12:51AM +, Bhushan Bharat-R65777 wrote: > > > > -Original Message- > > From: Kevin Hao [mailto:haoke...@gmail.com] > > Sent: Thursday, May 09, 2013 1:38 PM > > To: Bhushan Bharat-R65777 > > Cc: tiejun.chen; Caraman Mihai Claudiu-B02008; kvm@vger.kernel.org; Wood > > Scott- > > B07421; ag...@suse.de; kvm-...@vger.kernel.org; > > linuxppc-...@lists.ozlabs.org > > Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts > > > > On Thu, May 09, 2013 at 07:51:09AM +, Bhushan Bharat-R65777 wrote: > > > > > > > > > > -Original Message- > > > > From: tiejun.chen [mailto:tiejun.c...@windriver.com] > > > > Sent: Thursday, May 09, 2013 1:18 PM > > > > To: Bhushan Bharat-R65777 > > > > Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc- > > > > d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; > > > > kvm@vger.kernel.org > > > > Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable > > > > interrupts > > > > > > > > On 05/09/2013 03:33 PM, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > > >> -Original Message- > > > > >> From: Linuxppc-dev [mailto:linuxppc-dev- > > > > >> bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf > > > > >> bounces+Of Caraman > > > > >> Mihai Claudiu-B02008 > > > > >> Sent: Wednesday, May 08, 2013 6:44 PM > > > > >> To: Wood Scott-B07421; tiejun.chen > > > > >> Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de; > > > > >> kvm-...@vger.kernel.org; kvm@vger.kernel.org > > > > >> Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable > > > > >> interrupts > > > > >> > > > > >>>> 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. > > > > >>> > > > > >>>> 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? > > > > >> > > > > >> I think Tiejun was saying that host has flags and replays only > > > > >> EE/DEC/DBELL interrupts. There is special macro > > > > >> masked_interrupt_book3e in those exception handlers that sets > > > > >> paca- > > > > >irq_happened. > > > > >> > > > > >> The list of replied interrupts is limited to asynchronous > > > > >> noncritical interrupts which can be masked by MSR[EE] (therefore no > > > > >> TLB > > miss). > > > > > > > > > > Embedded Perfmon interrupt is also asynchronous, Why that is not > > > > > in the list > > > > of masked interruts. > > > > > > > > Are you saying perfmon? If so, its also in that list: > > > > > > > > START_EXCEPTION(perfmon); > > > > NORMAL_EXCEPTION_PROLOG(0x260, > > > > BOOKE_INTERRUPT_PERFORMANCE_MONITOR, > > > > PROLOG_ADDITION_NONE) > > > >
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On 05/09/2013 04:12 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Kevin Hao [mailto:haoke...@gmail.com]
Sent: Thursday, May 09, 2013 1:38 PM
To: Bhushan Bharat-R65777
Cc: tiejun.chen; Caraman Mihai Claudiu-B02008; kvm@vger.kernel.org; Wood Scott-
B07421; ag...@suse.de; kvm-...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, May 09, 2013 at 07:51:09AM +, Bhushan Bharat-R65777 wrote:
-Original Message-
From: tiejun.chen [mailto:tiejun.c...@windriver.com]
Sent: Thursday, May 09, 2013 1:18 PM
To: Bhushan Bharat-R65777
Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc-
d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org;
kvm@vger.kernel.org
Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable
interrupts
On 05/09/2013 03:33 PM, Bhushan Bharat-R65777 wrote:
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf
bounces+Of Caraman
Mihai Claudiu-B02008
Sent: Wednesday, May 08, 2013 6:44 PM
To: Wood Scott-B07421; tiejun.chen
Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de;
kvm-...@vger.kernel.org; kvm@vger.kernel.org
Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable
interrupts
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.
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?
I think Tiejun was saying that host has flags and replays only
EE/DEC/DBELL interrupts. There is special macro
masked_interrupt_book3e in those exception handlers that sets
paca-
irq_happened.
The list of replied interrupts is limited to asynchronous
noncritical interrupts which can be masked by MSR[EE] (therefore no TLB
miss).
Embedded Perfmon interrupt is also asynchronous, Why that is not
in the list
of masked interruts.
Are you saying perfmon? If so, its also in that list:
START_EXCEPTION(perfmon);
NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
PROLOG_ADDITION_NONE)
EXCEPTION_COMMON(0x260, PACA_EXGEN, INTS_DISABLE)
Where it is recorded in paca->irq_happned to be replayed later ?
Actually we don't want replay the perfmon interrupt later. We would run it even
soft irq is disabled and just treat it as NMI. Please see the following function
quoted from arch/powerpc/perf/core-fsl-emb.c:
/*
* If interrupts were soft-disabled when a PMU interrupt occurs, treat
* it as an NMI.
*/
static inline int perf_intr_is_nmi(struct pt_regs *regs)
{
#ifdef __powerpc64__
return !regs->softe;
#else
return 0;
#endif
}
Is it because that we cannot afford to lose perfmon interrupt for more accurate
capturing of data ?
powerpc/perf: e500 support
This implements perf_event support for the Freescale embedded performance
monitor, based on the existing perf_event.c that supports server/classic
chips.
Some limitations:
- Performance monitor interrupts are regular EE interrupts, and thus you
can't profile places with interrupts disabled. We may want to implement
soft IRQ-disabling, with perfmon interrupts exempted and treated as NMIs.
Tiejun
-Bharat
Thanks,
Kevin
Tiejun
-Bharat
Now on KVM book3e we
don't want to put them in the irq_happened lazy state but rather
to execute them directly, so there is no reason for exception
handling symmetry between host and guest.
-Mike
___
Linuxppc-dev mailing list
linuxppc-...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Thu, May 09, 2013 at 07:51:09AM +, Bhushan Bharat-R65777 wrote:
>
>
> > -Original Message-
> > From: tiejun.chen [mailto:tiejun.c...@windriver.com]
> > Sent: Thursday, May 09, 2013 1:18 PM
> > To: Bhushan Bharat-R65777
> > Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc-
> > d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org;
> > kvm@vger.kernel.org
> > Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
> >
> > On 05/09/2013 03:33 PM, Bhushan Bharat-R65777 wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Linuxppc-dev [mailto:linuxppc-dev-
> > >> bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of
> > >> bounces+Caraman
> > >> Mihai Claudiu-B02008
> > >> Sent: Wednesday, May 08, 2013 6:44 PM
> > >> To: Wood Scott-B07421; tiejun.chen
> > >> Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de;
> > >> kvm-...@vger.kernel.org; kvm@vger.kernel.org
> > >> Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable
> > >> interrupts
> > >>
> > >>>> 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.
> > >>>
> > >>>> 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?
> > >>
> > >> I think Tiejun was saying that host has flags and replays only
> > >> EE/DEC/DBELL interrupts. There is special macro
> > >> masked_interrupt_book3e in those exception handlers that sets paca-
> > >irq_happened.
> > >>
> > >> The list of replied interrupts is limited to asynchronous noncritical
> > >> interrupts which can be masked by MSR[EE] (therefore no TLB miss).
> > >
> > > Embedded Perfmon interrupt is also asynchronous, Why that is not in the
> > > list
> > of masked interruts.
> >
> > Are you saying perfmon? If so, its also in that list:
> >
> > START_EXCEPTION(perfmon);
> > NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
> > PROLOG_ADDITION_NONE)
> > EXCEPTION_COMMON(0x260, PACA_EXGEN, INTS_DISABLE)
>
> Where it is recorded in paca->irq_happned to be replayed later ?
Actually we don't want replay the perfmon interrupt later. We would run it
even soft irq is disabled and just treat it as NMI. Please see the following
function quoted from arch/powerpc/perf/core-fsl-emb.c:
/*
* If interrupts were soft-disabled when a PMU interrupt occurs, treat
* it as an NMI.
*/
static inline int perf_intr_is_nmi(struct pt_regs *regs)
{
#ifdef __powerpc64__
return !regs->softe;
#else
return 0;
#endif
}
Thanks,
Kevin
>
> >
> > Tiejun
> >
> > >
> > > -Bharat
> > >
> > >> Now on KVM book3e we
> > >> don't want to put them in the irq_happened lazy state but rather to
> > >> execute them directly, so there is no reason for exception handling
> > >> symmetry between host and guest.
> > >>
> > >> -Mike
> >
>
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
pgpz87iGt9B8Z.pgp
Description: PGP signature
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On 05/09/2013 03:51 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: tiejun.chen [mailto:tiejun.c...@windriver.com] Sent: Thursday, May 09, 2013 1:18 PM To: Bhushan Bharat-R65777 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; linuxppc- d...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts On 05/09/2013 03:33 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of bounces+Caraman Mihai Claudiu-B02008 Sent: Wednesday, May 08, 2013 6:44 PM To: Wood Scott-B07421; tiejun.chen Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts 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. 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? I think Tiejun was saying that host has flags and replays only EE/DEC/DBELL interrupts. There is special macro masked_interrupt_book3e in those exception handlers that sets paca- irq_happened. The list of replied interrupts is limited to asynchronous noncritical interrupts which can be masked by MSR[EE] (therefore no TLB miss). Embedded Perfmon interrupt is also asynchronous, Why that is not in the list of masked interruts. Are you saying perfmon? If so, its also in that list: START_EXCEPTION(perfmon); NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR, PROLOG_ADDITION_NONE) EXCEPTION_COMMON(0x260, PACA_EXGEN, INTS_DISABLE) Where it is recorded in paca->irq_happned to be replayed later ? In stead of PROLOG_ADDITION_MASKABLE, actually we have nothing to do with PROLOG_ADDITION_NONE for perfmon, so perfmon can be executed without lazy state. Tiejun Tiejun -Bharat Now on KVM book3e we don't want to put them in the irq_happened lazy state but rather to execute them directly, so there is no reason for exception handling symmetry between host and guest. -Mike -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On 05/09/2013 03:33 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+bharat.bhushan=freescale@lists.ozlabs.org] On Behalf Of Caraman Mihai Claudiu-B02008 Sent: Wednesday, May 08, 2013 6:44 PM To: Wood Scott-B07421; tiejun.chen Cc: linuxppc-...@lists.ozlabs.org; ag...@suse.de; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts 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. 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? I think Tiejun was saying that host has flags and replays only EE/DEC/DBELL interrupts. There is special macro masked_interrupt_book3e in those exception handlers that sets paca->irq_happened. The list of replied interrupts is limited to asynchronous noncritical interrupts which can be masked by MSR[EE] (therefore no TLB miss). Embedded Perfmon interrupt is also asynchronous, Why that is not in the list of masked interruts. Are you saying perfmon? If so, its also in that list: START_EXCEPTION(perfmon); NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR, PROLOG_ADDITION_NONE) EXCEPTION_COMMON(0x260, PACA_EXGEN, INTS_DISABLE) Tiejun -Bharat Now on KVM book3e we don't want to put them in the irq_happened lazy state but rather to execute them directly, so there is no reason for exception handling symmetry between host and guest. -Mike -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
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());
Signed-off-by: Tiejun Chen
---
arch/powerpc/kvm/bookehv_interrupts.S |9 +
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
b/arch/powerpc/kvm/bookehv_interrupts.S
index e8ed7d6..2fd62bf 100644
--- a/arch/powerpc/kvm/bookehv_interrupts.S
+++ b/arch/powerpc/kvm/bookehv_interrupts.S
@@ -33,6 +33,8 @@
#ifdef CONFIG_64BIT
#include
+#include
+#include
#else
#include "../kernel/head_booke.h" /* for THREAD_NORMSAVE() */
#endif
@@ -469,6 +471,13 @@ _GLOBAL(kvmppc_resume_host)
PPC_LLr3, HOST_RUN(r1)
mrr5, r14 /* intno */
mrr14, r4 /* Save vcpu pointer. */
+#ifdef CONFIG_64BIT
+/* Should we soft-disable interrupts? */
+andi.r6, r5, BOOKE_INTERRUPT_EXTERNAL | BOOKE_INTERRUPT_DECREMENTER
| BOOKE_INTERRUPT_DOORBELL
+beqskip_soft_dis
+SOFT_DISABLE_INTS(r7,r8)
+skip_soft_dis:
+#endif
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
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*) \
...
So I think this should be reasonable :)
Tiejun
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
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). Sorry, miss to send Ben. Tiejun Signed-off-by: Tiejun Chen --- arch/powerpc/kvm/bookehv_interrupts.S |9 + 1 file changed, 9 insertions(+) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..2fd62bf 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -33,6 +33,8 @@ #ifdef CONFIG_64BIT #include +#include +#include #else #include "../kernel/head_booke.h" /* for THREAD_NORMSAVE() */ #endif @@ -469,6 +471,13 @@ _GLOBAL(kvmppc_resume_host) PPC_LL r3, HOST_RUN(r1) mr r5, r14 /* intno */ mr r14, r4 /* Save vcpu pointer. */ +#ifdef CONFIG_64BIT + /* Should we soft-disable interrupts? */ + andi. r6, r5, BOOKE_INTERRUPT_EXTERNAL | BOOKE_INTERRUPT_DECREMENTER | BOOKE_INTERRUPT_DOORBELL + beq skip_soft_dis + SOFT_DISABLE_INTS(r7,r8) +skip_soft_dis: +#endif bl kvmppc_handle_exit /* Restore vcpu pointer and the nonvolatiles we used. */ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
