From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Subject: Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
Date: Mon, 6 May 2013 21:06:30 -0500
Message-ID: <1367892390.3398.12@snotra>
References: <51885F49.6060605@windriver.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="Flowed"; DelSp="Yes"
Content-Transfer-Encoding: 7bit
Cc: linuxppc-dev@lists.ozlabs.org, agraf@suse.de, kvm-ppc@vger.kernel.org,
 kvm@vger.kernel.org
To: tiejun.chen <tiejun.chen@windriver.com>
Return-path: <linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org>
In-Reply-To: <51885F49.6060605@windriver.com> (from
 tiejun.chen@windriver.com on Mon May  6 20:56:25 2013)
Content-Disposition: inline
List-Unsubscribe: <https://lists.ozlabs.org/options/linuxppc-dev>,
 <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=unsubscribe>
List-Archive: <http://lists.ozlabs.org/pipermail/linuxppc-dev/>
List-Post: <mailto:linuxppc-dev@lists.ozlabs.org>
List-Help: <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=help>
List-Subscribe: <https://lists.ozlabs.org/listinfo/linuxppc-dev>,
 <mailto:linuxppc-dev-request@lists.ozlabs.org?subject=subscribe>
Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org
Sender: "Linuxppc-dev"
 <linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org>
List-Id: kvm.vger.kernel.org

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?

>> 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.

And I'd rather see any fix for this problem stay out of the asm code.

> 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?

-Scott