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

Scott Wood
2013-04-04T18:41:56Z

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