From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Thu, 04 Apr 2013 18:41:44 +0000 Subject: Re: [RFC PATCH v3 6/6] kvm/ppc/mpic: add KVM_CAP_IRQ_MPIC Message-Id: <1365100904.14772.12@snotra> List-Id: References: <37191669-BF8E-4DAC-B3A0-98F0820C3208@suse.de> In-Reply-To: <37191669-BF8E-4DAC-B3A0-98F0820C3208@suse.de> (from agraf@suse.de on Thu Apr 4 07:54:20 2013) MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alexander Graf Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, paulus@samba.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