On Thu, Dec 06, 2012 at 02:55:16AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2012-12-05:
> > On Wed, Dec 05, 2012 at 01:51:36PM +0000, Zhang, Yang Z wrote:
> >> Gleb Natapov wrote on 2012-12-05:
> >>> On Wed, Dec 05, 2012 at 06:02:59AM +0000, Zhang, Yang Z wrote:
> >>>> Gleb Natapov wrote on 2012-12-05:
> >>>>> On Wed, Dec 05, 2012 at 01:55:17AM +0000, Zhang, Yang Z wrote:
> >>>>>> Gleb Natapov wrote on 2012-12-04:
> >>>>>>> On Tue, Dec 04, 2012 at 06:39:50AM +0000, Zhang, Yang Z wrote:
> >>>>>>>> Gleb Natapov wrote on 2012-12-03:
> >>>>>>>>> On Mon, Dec 03, 2012 at 03:01:03PM +0800, Yang Zhang wrote:
> >>>>>>>>>> Virtual interrupt delivery avoids KVM to inject vAPIC interrupts
> >>>>>>>>>> manually, which is fully taken care of by the hardware. This needs
> >>>>>>>>>> some special awareness into existing interrupr injection path:
> >>>>>>>>>>
> >>>>>>>>>> - for pending interrupt, instead of direct injection, we may need
> >>>>>>>>>> update architecture specific indicators before resuming to
> >>>>>>>>>> guest. - A pending interrupt, which is masked by ISR, should
> >>>>>>>>>> be also considered in above update action, since hardware
> >>>>>>>>>> will decide when to inject it at right time. Current
> >>>>>>>>>> has_interrupt and get_interrupt only returns a valid vector
> >>>>>>>>>> from injection p.o.v.
> >>>>>>>>> Most of my previous comments still apply.
> >>>>>>>>>
> >>>>>>>>>> +void kvm_set_eoi_exitmap(struct kvm_vcpu *vcpu, int vector,
> >>>>>>>>>> + int trig_mode, int always_set)
> >>>>>>>>>> +{
> >>>>>>>>>> + if (kvm_x86_ops->set_eoi_exitmap)
> >>>>>>>>>> + kvm_x86_ops->set_eoi_exitmap(vcpu, vector,
> >>>>>>>>>> + trig_mode, always_set);
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>> /*
> >>>>>>>>>> * Add a pending IRQ into lapic.
> >>>>>>>>>> * Return 1 if successfully added and 0 if discarded.
> >>>>>>>>>> @@ -661,6 +669,7 @@ static int __apic_accept_irq(struct kvm_lapic
> >>>>> *apic,
> >>>>>>> int
> >>>>>>>>> delivery_mode,
> >>>>>>>>>> if (unlikely(!apic_enabled(apic)))
> >>>>>>>>>> break;
> >>>>>>>>>> + kvm_set_eoi_exitmap(vcpu, vector, trig_mode, 0);
> >>>>>>>>> As I said in the last review rebuild the bitmap when ioapic or irq
> >>>>>>>>> notifier configuration changes, user request bit to notify vcpus to
> >>>>>>>>> reload the bitmap.
> >>>>>>>> It is too complicated. When program ioapic entry, we cannot get the
> >>> target
> >>>>> vcpu
> >>>>>>> easily. We need to read destination format register and logical
> >>>>>>> destination register to find out target vcpu if using logical mode.
> >>>>>>> Also, we must trap every modification to the two registers to update
> >>>>>>> eoi bitmap. No need to check target vcpu. Enable exit on all vcpus
> >>>>>>> for the vector
> >>>>>> This is wrong. As we known, modern OS uses per VCPU vector. We cannot
> >>>>> ensure all vectors have same trigger mode. And what's worse, the
> >>>>> vector in another vcpu is used to handle high frequency
> >>>>> interrupts(like 10G NIC), then it will hurt performance.
> >>>>>>
> >>>>> I never saw OSes reuse vector used by ioapic, as far as I see this
> >>>> Could you point out which code does this check in Linux kernel? I don't
> >>>> see any special checks when Linux kernel allocates a vector.
> >>>>
> >>> arch/x86/kernel/apic/io_apic.c:create_irq_nr(). It uses
> >>> apic->target_cpus() to get cpu mask. target_cpus() return mask of all
> >>> online cpus. Actually you wrote arch_pi_create_irq() in PI patches to
> >>> workaround this behaviour and allocated vector per cpu, no?
> >> Yes, when create an new irq, it will allocate vector from all online cpus. But after
> > user changes the irq affinity, then the vector will reallocate with new cpumask.
> > And this will leave the vector available on other cpus.
> >>
> > Since during vector allocation all cpus are checked vector will not be
> > reused if it is allocated on any cpu.
> Sorry, I still cannot find this check in kernel. Can you point me out to it?
You pointed to it above by yourself:
"Yes, when create an new irq, it will allocate vector from all online
cpus"
So if vector is allocated by at least one online cpu it cannot be reused
during allocation.
> Also, I do see the vector reused by MSI and IOAPIC in my system.
>
What is your system? What is your qemu command line? We only care if
MSI uses the same vector as IOAPICs level interrupt but on different
cpu. If this happens we can use apic_map to calculate per cpu eoi exit
bitmap.
> >>> Are you aware of any guest that I can run, examine ioapic/apic
> >>> configuration and see that the same vector is used on different vcpus
> >>> for different devices? Can you point me to it?
> >>>
> > Can you answer this?
> I am sure Xen will reused the IOAPIC vector.
>
What configuration it creates exactly? Also if Xen running as KVM guest
will takes small performance hit it is not a big problem. Remember there
is not correctness issue here.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
