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

Alexander Graf
2013-04-04T22:30:06Z

Am 04.04.2013 um 20:41 schrieb 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.

Ok, please add a comment about this here then :).

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

Is there any way to break heavily if user space attempts this?

Alex

>
> -Scott