From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B65B7C4332F for ; Sat, 26 Nov 2022 10:58:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229506AbiKZK6d (ORCPT ); Sat, 26 Nov 2022 05:58:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41806 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229450AbiKZK6b (ORCPT ); Sat, 26 Nov 2022 05:58:31 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 35C2B5FA2 for ; Sat, 26 Nov 2022 02:58:30 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B5C5360A4E for ; Sat, 26 Nov 2022 10:58:29 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 143DBC433D6; Sat, 26 Nov 2022 10:58:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1669460309; bh=ePwaGRkSZtZmjVC4kNQl3xw5+xIRod5wCUJ5FF0oE1s=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=rkNN6RsUW7XPCgLA9J5eReGCDouT687rTTt4vEnQRIOtJnNUxJRpNwneLtzp2pM7E p59IQNcEWjAill62YkiwEpI+W5HsPQh2yfEQW13/XE79JOJPMMwc3RZXbaDaOGoKrD hcV0j9XUY5jW2jnHgMzw+sYnv1OHi5Z+eqdkyyWDmSNArHx3onRau6k7m+YT9Eej7y 5CulM/cpD438++ZItf+x0aeYh1IRNAK7pqcfLU28YtUKFRB7QWaER4cxvz59dyFT41 6OuozcDIj4SPH1zhpAbZMfptIEjaA57bBSzq64E614VxhpQ7HiHlZ0yx1hixlYTLDK 1lhWEYGj1Labg== Received: from sofa.misterjones.org ([185.219.108.64] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oyst8-008hi8-QF; Sat, 26 Nov 2022 10:58:26 +0000 Date: Sat, 26 Nov 2022 10:58:18 +0000 Message-ID: <8735a6ugmt.wl-maz@kernel.org> From: Marc Zyngier To: "chenxiang (M)" Cc: , , , Subject: Re: [PATCH v2] vfio/pci: Verify each MSI vector to avoid invalid MSI vectors In-Reply-To: References: <1669167756-196788-1-git-send-email-chenxiang66@hisilicon.com> <86k03loouy.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: chenxiang66@hisilicon.com, alex.williamson@redhat.com, kvm@vger.kernel.org, qemu-devel@nongnu.org, linuxarm@huawei.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Sat, 26 Nov 2022 06:33:15 +0000, "chenxiang (M)" wrote: >=20 >=20 > =E5=9C=A8 2022/11/23 20:08, Marc Zyngier =E5=86=99=E9=81=93: > > On Wed, 23 Nov 2022 01:42:36 +0000, > > chenxiang wrote: > >> From: Xiang Chen > >>=20 > >> Currently the number of MSI vectors comes from register PCI_MSI_FLAGS > >> which should be power-of-2 in qemu, in some scenaries it is not the sa= me as > >> the number that driver requires in guest, for example, a PCI driver wa= nts > >> to allocate 6 MSI vecotrs in guest, but as the limitation, it will all= ocate > >> 8 MSI vectors. So it requires 8 MSI vectors in qemu while the driver in > >> guest only wants to allocate 6 MSI vectors. > >>=20 > >> When GICv4.1 is enabled, it iterates over all possible MSIs and enable= the > >> forwarding while the guest has only created some of mappings in the vi= rtual > >> ITS, so some calls fail. The exception print is as following: > >> vfio-pci 0000:3a:00.1: irq bypass producer (token 000000008f08224d) re= gistration > >> fails:66311 > >>=20 > >> To avoid the issue, verify each MSI vector, skip some operations such = as > >> request_irq() and irq_bypass_register_producer() for those invalid MSI= vectors. > >>=20 > >> Signed-off-by: Xiang Chen > >> --- > >> I reported the issue at the link: > >> https://lkml.kernel.org/lkml/87cze9lcut.wl-maz@kernel.org/T/ > >>=20 > >> Change Log: > >> v1 -> v2: > >> Verify each MSI vector in kernel instead of adding systemcall accordin= g to > >> Mar's suggestion > >> --- > >> arch/arm64/kvm/vgic/vgic-irqfd.c | 13 +++++++++++++ > >> arch/arm64/kvm/vgic/vgic-its.c | 36 +++++++++++++++++++++++++++++= +++++++ > >> arch/arm64/kvm/vgic/vgic.h | 1 + > >> drivers/vfio/pci/vfio_pci_intrs.c | 33 +++++++++++++++++++++++++++++= ++++ > >> include/linux/kvm_host.h | 2 ++ > >> 5 files changed, 85 insertions(+) > >>=20 > >> diff --git a/arch/arm64/kvm/vgic/vgic-irqfd.c b/arch/arm64/kvm/vgic/vg= ic-irqfd.c > >> index 475059b..71f6af57 100644 > >> --- a/arch/arm64/kvm/vgic/vgic-irqfd.c > >> +++ b/arch/arm64/kvm/vgic/vgic-irqfd.c > >> @@ -98,6 +98,19 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry= *e, > >> return vgic_its_inject_msi(kvm, &msi); > >> } > >> +int kvm_verify_msi(struct kvm *kvm, > >> + struct kvm_kernel_irq_routing_entry *irq_entry) > >> +{ > >> + struct kvm_msi msi; > >> + > >> + if (!vgic_has_its(kvm)) > >> + return -ENODEV; > >> + > >> + kvm_populate_msi(irq_entry, &msi); > >> + > >> + return vgic_its_verify_msi(kvm, &msi); > >> +} > >> + > >> /** > >> * kvm_arch_set_irq_inatomic: fast-path for irqfd injection > >> */ > >> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic= -its.c > >> index 94a666d..8312a4a 100644 > >> --- a/arch/arm64/kvm/vgic/vgic-its.c > >> +++ b/arch/arm64/kvm/vgic/vgic-its.c > >> @@ -767,6 +767,42 @@ int vgic_its_inject_cached_translation(struct kvm= *kvm, struct kvm_msi *msi) > >> return 0; > >> } > >> +int vgic_its_verify_msi(struct kvm *kvm, struct kvm_msi *msi) > >> +{ > >> + struct vgic_its *its; > >> + struct its_ite *ite; > >> + struct kvm_vcpu *vcpu; > >> + int ret =3D 0; > >> + > >> + if (!irqchip_in_kernel(kvm) || (msi->flags & ~KVM_MSI_VALID_DEVID)) > >> + return -EINVAL; > >> + > >> + if (!vgic_has_its(kvm)) > >> + return -ENODEV; > >> + > >> + its =3D vgic_msi_to_its(kvm, msi); > >> + if (IS_ERR(its)) > >> + return PTR_ERR(its); > >> + > >> + mutex_lock(&its->its_lock); > >> + if (!its->enabled) { > >> + ret =3D -EBUSY; > >> + goto unlock; > >> + } > >> + ite =3D find_ite(its, msi->devid, msi->data); > >> + if (!ite || !its_is_collection_mapped(ite->collection)) { > >> + ret =3D E_ITS_INT_UNMAPPED_INTERRUPT; > >> + goto unlock; > >> + } > >> + > >> + vcpu =3D kvm_get_vcpu(kvm, ite->collection->target_addr); > >> + if (!vcpu) > >> + ret =3D E_ITS_INT_UNMAPPED_INTERRUPT; > > I'm sorry, but what does this mean to the caller? This should never > > leak outside of the ITS code. >=20 > Actually it is already leak outside of ITS code, and please see the > exception printk (E_ITS_INT_UNMAPPED_INTERRUPT is 0x10307 which is > equal to 66311): >=20 > vfio-pci 0000:3a:00.1: irq bypass producer (token 000000008f08224d) regis= tration fails:66311 >=20 But that's hardly interpreted, which is the whole point. Only zero is considered a success value. > > Honestly, the whole things seems really complicated to avoid something > > that is only a harmless warning . >=20 > It seems also waste some interrupts. Allocating and requesting some > interrupts but not used. What makes you think they are not used? A guest can install a mapping for those at any point. They won't be directly injected, but they will be delivered to the guest via the normal SW injection mechanism. M. --=20 Without deviation from the norm, progress is not possible.