From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Subject: Re: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC
Date: Thu, 4 Apr 2013 13:41:44 -0500
Message-ID: <1365100904.14772.12@snotra>
References: <37191669-BF8E-4DAC-B3A0-98F0820C3208@suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed
Content-Transfer-Encoding: 8BIT
Cc: <kvm-ppc@vger.kernel.org>, <kvm@vger.kernel.org>,
	<paulus@samba.org>
To: Alexander Graf <agraf@suse.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from co9ehsobe004.messaging.microsoft.com ([207.46.163.27]:24899
	"EHLO co9outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1762534Ab3DDSl4 convert rfc822-to-8bit
	(ORCPT <rfc822;kvm@vger.kernel.org>); Thu, 4 Apr 2013 14:41:56 -0400
In-Reply-To: <37191669-BF8E-4DAC-B3A0-98F0820C3208@suse.de> (from
	agraf@suse.de on Thu Apr  4 07:54:20 2013)
Content-Disposition: inline
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 04/04/2013 07:54:20 AM, Alexander Graf wrote:
> 
> On 03.04.2013, at 03:57, Scott Wood wrote:
> 
> > +	if (opp->mpic_mode_mask == GCR_MODE_PROXY)
> 
> Shouldn't this be an &?

The way the mode field was originally documented was a two-bit field,  
where 0b11 was external proxy, and 0b10 was reserved.  If we use & it  
would have to be:

	if ((opp->mpic_mode_mask & GCR_MODE_PROXY) == GCR_MODE_PROXY)
		...

Simply testing "opp->mpic_mode_mask & GCR_MODE_PROXY" would return true  
in the case of GCR_MODE_MIXED.

In MPIC 4.3 external proxy is defined as a separate bit (GCR[CI]) that  
is ignored if the mixed-mode bit (GCR[M]) is not set, which makes it a  
bit more legitimate to view it as a bitmap.  Still, I doubt we'll see  
new mode bits.

> > @@ -460,6 +464,13 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
> > 	tasklet_kill(&vcpu->arch.tasklet);
> >
> > 	kvmppc_remove_vcpu_debugfs(vcpu);
> > +
> > +	switch (vcpu->arch.irq_type) {
> > +	case KVMPPC_IRQ_MPIC:
> > +		kvmppc_mpic_put(vcpu->arch.mpic);
> 
> This doesn't tell the MPIC that this exact CPU is getting killed.  
> What if we hotplug remove just a single CPU? Don't we have to  
> deregister the CPU with the MPIC?

If we ever support hot vcpu removal, yes.  We'd probably need some MPIC  
code changes to accommodate that, and we wouldn't currently have a way  
to test it, so I'd rather make it obviously not supported for now.

-Scott