On 21/06/12 21:58, Christoffer Dall wrote:
>>>> +
>>>> +static void handle_mmio_priority_reg(struct kvm_vcpu *vcpu,
>>>> + struct kvm_run *run, u32 offset)
>>>> +{
>>>> + u32 *reg = vgic_bytemap_get_reg(&vcpu->kvm->arch.vgic.irq_priority,
>>>> + vcpu->vcpu_id, offset);
>>>> + mmio_do_copy(run, reg, offset,
>>>> + ACCESS_READ_VALUE | ACCESS_WRITE_VALUE);
>>>
>>> shouldn't this call vgic_update_state(vcpu->kvm) to check the running
>>> priority of the virtual cpu interface?
>>
>> No, we do not take priority into account to deliver an interrupt, as
>> Linux doesn't use this. Can be introduced later if necessary.
>>
>
> then at least a TODO, or NOTE comment or something should be placed here.
Sure.
>>>> +}
>>>> +
>>>> +static void update_spi_target(struct kvm *kvm)
>>>> +{
>>>> + struct vgic_dist *dist = &kvm->arch.vgic;
>>>> + int c, i, nrcpus = atomic_read(&kvm->online_vcpus);
>>>> + u8 targ;
>>>> + unsigned long *bmap;
>>>> +
>>>> + for (i = 32; i < VGIC_NR_IRQS; i++) {
>>>> + targ = vgic_bytemap_get_irq_val(&dist->irq_target, 0, i);
>>>> +
>>>
>>> ah, I see, cpu=0 is because we're always using i >= 32.
>>
>> Yeah, it's an ugly hack.
>>
>>>> + for (c = 0; c < nrcpus; c++) {
>>>> + bmap = dist->irq_spi_target[c].global.reg_ul;
>>>> +
>>>> + if (targ & (1 << c))
>>>> + set_bit(i - 32, bmap);
>>>> + else
>>>> + clear_bit(i - 32, bmap);
>>>> + }
>>>> + }
>>>> +}
>>>
>>> I'm reading this as an optimization so you can easily determine if a
>>> specific VCPU should get the SPI? Or am I getting this entirely wrong?
>>
>> It is a lot more than just an optimization. We really need to know which
>> CPU is getting targeted by a specific interrupt.
>>
>
> yes, but can't we just look in the irq_target map when we need this information?
Sure, but having a full bitmap of what you handle on a particular CPU
means that (pending & enabled & irq_spi_target) gives you a nice bitmap
of the SPIs for this CPU.
Otherwise, you have to compute (pending & enabled), and then lookup each
bit into irq_target, which has a very different format.
Another possibility could be to get rid of irq_target altogether, and
just have the MMIO accessors to populate the per-CPU state. Actually, I
might just do that.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
