Received-SPF: pass (google.com: domain of linux-kernel+bounces-26211-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240112091128.3868059-1-foxywang@tencent.com> <ZaFor2Lvdm4O2NWa@google.com>
In-Reply-To: <ZaFor2Lvdm4O2NWa@google.com>
From: Yi Wang <up2wing@gmail.com>
Date: Tue, 16 Jan 2024 00:01:03 +0800
Message-ID: <CAN35MuSkQf0XmBZ5ZXGhcpUCGD-kKoyTv9G7ya4QVD1xiqOxLg@mail.gmail.com>
Subject: Re: [PATCH] KVM: irqchip: synchronize srcu only if needed
To: Sean Christopherson <seanjc@google.com>
Cc: pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, 
	dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, 
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org, wanpengli@tencent.com, 
	Yi Wang <foxywang@tencent.com>, Oliver Upton <oliver.upton@linux.dev>, 
	Marc Zyngier <maz@kernel.org>, Anup Patel <anup@brainfault.org>, 
	Atish Patra <atishp@atishpatra.org>, Christian Borntraeger <borntraeger@linux.ibm.com>, 
	Janosch Frank <frankja@linux.ibm.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Many thanks for your such kind and detailed reply, Sean!

On Sat, Jan 13, 2024 at 12:28=E2=80=AFAM Sean Christopherson <seanjc@google=
com> wrote:
>
> +other KVM maintainers
>
> On Fri, Jan 12, 2024, Yi Wang wrote:
> > From: Yi Wang <foxywang@tencent.com>
> >
> > We found that it may cost more than 20 milliseconds very accidentally
> > to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
> > already.
> >
> > The reason is that when vmm(qemu/CloudHypervisor) invokes
> > KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
> > might_sleep and kworker of srcu may cost some delay during this period.
>
> might_sleep() yielding is not justification for changing KVM.  That's mor=
e or
> less saying "my task got preempted and took longer to run".  Well, yeah.

Agree. But I suppose it may be one of the reasons that makes  time of
KVM_CAP_SPLIT_IRQCHIP delayed, of course, the kworker has the biggest
suspicion :)

>
> > Since this happens during creating vm, it's no need to synchronize srcu
> > now 'cause everything is not ready(vcpu/irqfd) and none uses irq_srcu n=
ow.

...

> And on x86, I'm pretty sure as of commit 654f1f13ea56 ("kvm: Check irqchi=
p mode
> before assign irqfd"), which added kvm_arch_irqfd_allowed(), it's impossi=
ble for
> kvm_irq_map_gsi() to encounter a NULL irq_routing _on x86_.
>
> But I strongly suspect other architectures can reach kvm_irq_map_gsi() wi=
th a
> NULL irq_routing, e.g. RISC-V dynamically configures its interrupt contro=
ller,
> yet doesn't implement kvm_arch_intc_initialized().
>
> So instead of special casing x86, what if we instead have KVM setup an em=
pty
> IRQ routing table during kvm_create_vm(), and then avoid this mess entire=
ly?
> That way x86 and s390 no longer need to set empty/dummy routing when crea=
ting
> an IRQCHIP, and the worst case scenario of userspace misusing an ioctl() =
is no
> longer a NULL pointer deref.

To setup an empty IRQ routing table during kvm_create_vm() sounds a good id=
ea,
at this time vCPU have not been created and kvm->lock is held so skipping
synchronization is safe here.

However, there is one drawback, if vmm wants to emulate irqchip
itself, e.g. qemu
with command line '-machine kernel-irqchip=3Doff' may not need irqchip
in kernel. How
do we handle this issue?


---
Best wishes
Yi Wang