Re: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC

[Scott Wood <scottwood@freescale.com>]

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