Re: [bug report] GICv4.1: multiple vpus execute vgic_v4_load at the same time will greatly increase the time consumption

[Kunkun Jiang]

Hi Marc,

On 2024/8/21 18:59, Marc Zyngier wrote:
> On Wed, 21 Aug 2024 10:51:27 +0100,
> Kunkun Jiang <jiangkunkun@xxxxxxxxxx> wrote:
> > Hi all,
> >
> > Recently I discovered a problem about GICv4.1, the scenario is as follows:
> > 1. Enable GICv4.1
> > 2. Create multiple VMs.For example, 50 VMs(4U8G)

s/4U8G/8U16G/, sorry..

> I don't know what 4U8G means. On how many physical CPUs are you
> running 50 VMs? Direct injection of interrupts and over-subscription
> are fundamentally incompatible.

Each VM is configured with 8 vcpus and 16G memory. The number of
physical CPUs is 320.

> > 3. The business running in VMs has a frequent mmio access and need to exit
> >    to qemu for processing.
> > 4. Or modify the kvm code so that wfi must trap to kvm
> > 5. Then the utilization of pcpu where the vcpu is located will be 100%,and
> >    basically all in sys.
>
> What did you expect? If you trap all the time, your performance will
> suck.  Don't do that.
>
> > 6. This problem does not exist in GICv3.
>
> Because GICv3 doesn't have the same constraints.
>
> > According to analysis, this problem is due to the execution of vgic_v4_load.
> > vcpu_load or kvm_sched_in
> >      kvm_arch_vcpu_load
> >      ...
> >          vgic_v4_load
> >              irq_set_affinity
> >              ...
> >                  irq_do_set_affinity
> >                      raw_spin_lock(&tmp_mask_lock)
> >                      chip->irq_set_affinity
> >                      ...
> >                        its_vpe_set_affinity
> >
> > The tmp_mask_lock is the key. This is a global lock. I don't quite
> > understand
> > why tmp_mask_lock is needed here. I think there are two possible
> > solutions here:
> > 1. Remove this tmp_mask_lock
>
> Maybe you could have a look at 33de0aa4bae98 (and 11ea68f553e24)? It
> would allow you to understand the nature of the problem.
>
> This can probably be replaced with a per-CPU cpumask, which would
> avoid the locking, but potentially result in a larger memory usage.

Thanks, I will try it.

> > 2. Modify the gicv4 driver,do not perfrom VMOVP via
> > irq_set_affinity.
>
> Sure. You could also not use KVM at all if don't care about interrupts
> being delivered to your VM. We do not send a VMOVP just for fun. We
> send it because your vcpu has moved to a different CPU, and the ITS
> needs to know about that.

When a vcpu is moved to a different CPU, of course VMOVP has to be sent.
I mean is it possible to call its_vpe_set_affinity() to send VMOVP by
other means (instead of by calling the irq_set_affinity() API). So we
can bypass this tmp_mask_lock.

> You seem to be misunderstanding the use case for GICv4: a partitioned
> system, without any over-subscription, no vcpu migration between CPUs.
> If that's not your setup, then GICv4 will always be a net loss
> compared to SW injection with GICv3 (additional HW interaction,
> doorbell interrupts).

Thanks for the explanation. The key to the problem is not vcpu migration
between CPUs. The key point is that many vcpus execute vgic_v4_load() at
the same time. Even if it is not migrated to another CPU, there may be a
large number of vcpus executing vgic_v4_load() at the same time. For
example, the service running in VMs has a large number of MMIO accesses
and need to return to userspace for emulation. Due to the competition of
tmp_mask_lock, performance will deteriorate.

When the target CPU is the same CPU as the last run, there seems to be
no need to call irq_set_affinity() in this case. I did a test and it was
indeed able to alleviate the problem described above.

I feel it might be better to remove tmp_mask_lock or call
its_vpe_set_affinity() in another way. So I mentioned these two ideas
above.

Thanks,
Kunkun Jiang