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 X-Spam-Level: X-Spam-Status: No, score=-10.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ED99CC433ED for ; Sun, 9 May 2021 17:02:07 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 638C6610A5 for ; Sun, 9 May 2021 17:02:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 638C6610A5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ZxC+ehzeMN8Sm59h3aTdkiaeIzK9ZIPc1sem5W+rqf0=; b=ncHEr713SdEk646tCHpfAnwTT sXJ323hbcCcEphcbwU60hRCMyanGFfN9Ba6gJ2feDWmZOp+hlo9v9iJQ8OMUhxLSlxya2IslqJSqc NttQnO88Y+qhewcGYwm1pvGKCxy/L5cNbQQO4petwJdc2+gNLmGLT51dANJftKPV/D1TKjnV3+W65 wfCdfo7rLzuvUXr6fEeK2XMROGdUPnBpL7YVTpGERMZSSRklk/01uXexWB2YzzzTT1VFME04rkcXO XrnmIlwX6WFyZFpyxqR28dVulrwvfBNvfqO8z42ti6OsC6MpeboGWa1CbZ4Gj18Q9D0VkVkvgs4iY mO/JPH98w==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lfmn7-00BxeI-Qa; Sun, 09 May 2021 17:00:30 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lfmn4-00Bxe6-D4 for linux-arm-kernel@desiato.infradead.org; Sun, 09 May 2021 17:00:27 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=nvRrwIvL//wqR4eXLZSclNxQvYlBf9vfJ7b18pGt9jI=; b=4qd1eGJVPFqcCqJwJPqBgODyCE uvtRGgOpknr15H7KBqks7dqgKEEkWAWby0a1YiY7GtWN2w1X8HNNtJXyGVDw3M2yGkw+VOUKMAIAl N17aUVpyP/HMQknXqrjYn20IQS0d+4RiwA+gBjmGWq95RXogot9MqctKvO2+ydyu8Z37GbR4wh7f6 ApGGi8h7k4yEQ3aG+9quMUqsS7ghtuggwlzEt4EcBYVnA71SVLdAx7yy677looCx1qjV1h4ZB35JN GP8+XPu4iD+35NVwk7OrsxMr/WWjh6JUW+Y1SA3TKGcWcsvQnIhqKC07mEGZqVShPJEnJF7jt1hid m5b4z5Pw==; Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lfmn0-00839t-RB for linux-arm-kernel@lists.infradead.org; Sun, 09 May 2021 17:00:25 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620579619; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nvRrwIvL//wqR4eXLZSclNxQvYlBf9vfJ7b18pGt9jI=; b=Sgyi0MIBT2BFoJCrxHC47pxcwLmjaGC6d/s9SEDRmQdIx1GxJ48OHgBIlp4t9LvXNOCfeg RhGzv7IHD+MVwPaKMgmGdJWTrM+oJYI4wTfT4iXcKMTx+RpZg9oew4uTcqSmuO41A/KsZR YR78hzylNSiri/8rtOX8ecyZ5k2hulY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-84-mo1rzEFOPWmzXXIL-DVA0w-1; Sun, 09 May 2021 13:00:14 -0400 X-MC-Unique: mo1rzEFOPWmzXXIL-DVA0w-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 562DE8015A8; Sun, 9 May 2021 17:00:13 +0000 (UTC) Received: from [10.36.113.168] (ovpn-113-168.ams2.redhat.com [10.36.113.168]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D5E856249C; Sun, 9 May 2021 17:00:06 +0000 (UTC) Subject: Re: Question on guest enable msi fail when using GICv4/4.1 To: Marc Zyngier , Shaokun Zhang Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, linux-pci@vger.kernel.org, Alex Williamson , Cornelia Huck , Nianyao Tang , Bjorn Helgaas References: <3a2c66d6-6ca0-8478-d24b-61e8e3241b20@hisilicon.com> <87k0oaq5jf.wl-maz@kernel.org> <878s4qq00u.wl-maz@kernel.org> From: Auger Eric Message-ID: Date: Sun, 9 May 2021 19:00:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <878s4qq00u.wl-maz@kernel.org> Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210509_100022_994765_AF1C9CFD X-CRM114-Status: GOOD ( 37.59 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On 5/7/21 1:02 PM, Marc Zyngier wrote: > On Fri, 07 May 2021 10:58:23 +0100, > Shaokun Zhang wrote: >> >> Hi Marc, >> >> Thanks for your quick reply. >> >> On 2021/5/7 17:03, Marc Zyngier wrote: >>> On Fri, 07 May 2021 06:57:04 +0100, >>> Shaokun Zhang wrote: >>>> >>>> [This letter comes from Nianyao Tang] >>>> >>>> Hi, >>>> >>>> Using GICv4/4.1 and msi capability, guest vf driver requires 3 >>>> vectors and enable msi, will lead to guest stuck. >>> >>> Stuck how? >> >> Guest serial does not response anymore and guest network shutdown. >> >>> >>>> Qemu gets number of interrupts from Multiple Message Capable field >>>> set by guest. This field is aligned to a power of 2(if a function >>>> requires 3 vectors, it initializes it to 2). >>> >>> So I guess this is a MultiMSI device with 4 vectors, right? >>> >> >> Yes, it can support maximum of 32 msi interrupts, and vf driver only use 3 msi. >> >>>> However, guest driver just sends 3 mapi-cmd to vits and 3 ite >>>> entries is recorded in host. Vfio initializes msi interrupts using >>>> the number of interrupts 4 provide by qemu. When it comes to the >>>> 4th msi without ite in vits, in irq_bypass_register_producer, >>>> producer and consumer will __connect fail, due to find_ite fail, and >>>> do not resume guest. >>> >>> Let me rephrase this to check that I understand it: >>> - The device has 4 vectors >>> - The guest only create mappings for 3 of them >>> - VFIO calls kvm_vgic_v4_set_forwarding() for each vector >>> - KVM doesn't have a mapping for the 4th vector and returns an error >>> - VFIO disable this 4th vector >>> >>> Is that correct? If yes, I don't understand why that impacts the guest >>> at all. From what I can see, vfio_msi_set_vector_signal() just prints >>> a message on the console and carries on. >>> >> >> function calls: >> --> vfio_msi_set_vector_signal >> --> irq_bypass_register_producer >> -->__connect >> >> in __connect, add_producer finally calls kvm_vgic_v4_set_forwarding >> and fails to get the 4th mapping. When add_producer fail, it does >> not call cons->start, calls kvm_arch_irq_bypass_start and then >> kvm_arm_resume_guest. > > [+Eric, who wrote the irq_bypass infrastructure.] > > Ah, so the guest is actually paused, not in a livelock situation > (which is how I interpreted "stuck"). > > I think we should handle this case gracefully, as there should be no > expectation that the guest will be using this interrupt. Given that > VFIO seems to be pretty unfazed when a producer fails, I'm temped to > do the same thing and restart the guest. > > Also, __disconnect doesn't care about errors, so why should __connect > have this odd behaviour? _disconnect() does not care as we should always succeed tearing off things. del_* ops are void functions. On the opposite we can fail setting up the bypass. Effectively a979a6aa009f ("irqbypass: do not start cons/prod when failed connect") needs to be reverted. I agree the kerneldoc comments in linux/irqbypass.h may be improved to better explain the role of stop/start cbs and warn about their potential global impact. wrt the case above, "in __connect, add_producer finally calls kvm_vgic_v4_set_forwarding and fails to get the 4th mapping", shouldn't we succeed in that case? Thanks Eric > > Can you please try this? It is completely untested (and I think the > del_consumer call is odd, which is why I've also dropped it). > > Eric, what do you think? > > Thanks, > > M. > > diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c > index c9bb3957f58a..7e1865e15668 100644 > --- a/virt/lib/irqbypass.c > +++ b/virt/lib/irqbypass.c > @@ -40,21 +40,14 @@ static int __connect(struct irq_bypass_producer *prod, > if (prod->add_consumer) > ret = prod->add_consumer(prod, cons); > > - if (ret) > - goto err_add_consumer; > - > - ret = cons->add_producer(cons, prod); > - if (ret) > - goto err_add_producer; > + if (!ret) > + ret = cons->add_producer(cons, prod); > > if (cons->start) > cons->start(cons); > if (prod->start) > prod->start(prod); > -err_add_producer: > - if (prod->del_consumer) > - prod->del_consumer(prod, cons); > -err_add_consumer: > + > return ret; > } > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel