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=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 21D07C10F11 for ; Wed, 10 Apr 2019 12:42:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E497720818 for ; Wed, 10 Apr 2019 12:42:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732031AbfDJMm6 (ORCPT ); Wed, 10 Apr 2019 08:42:58 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:54080 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732028AbfDJMm5 (ORCPT ); Wed, 10 Apr 2019 08:42:57 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 29532EBD; Wed, 10 Apr 2019 05:42:57 -0700 (PDT) Received: from [10.1.196.129] (ostrya.cambridge.arm.com [10.1.196.129]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E97403F59C; Wed, 10 Apr 2019 05:42:55 -0700 (PDT) Subject: Re: [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking To: Leo Yan , "kvm@vger.kernel.org" , "kvmarm@lists.cs.columbia.edu" , Will Deacon , Marc Zyngier , Eric Auger , Robin Murphy References: <20190322052308.28254-1-leo.yan@linaro.org> From: Jean-Philippe Brucker Message-ID: Date: Wed, 10 Apr 2019 13:42:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.3 MIME-Version: 1.0 In-Reply-To: <20190322052308.28254-1-leo.yan@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Leo, On 22/03/2019 05:23, Leo Yan wrote: > If MSI doesn't support per-vector masking capability and > PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function > vfio_pci_msi_vector_write() will directly bail out for this case and > every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED. > > This results in the state maintained in 'virt_state' cannot really > reflect the MSI hardware state; finally it will mislead the function > vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow: > > vfio_pci_update_msi_entry() { > > [...] > > if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state)) > return 0; ==> skip IRQ forwarding > > [...] > } > > To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the > message control field, this patch simply clears bit > VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end > vfio_pci_update_msi_entry() can forward MSI IRQ successfully. > > Signed-off-by: Leo Yan > --- > vfio/pci.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/vfio/pci.c b/vfio/pci.c > index ba971eb..4fd24ac 100644 > --- a/vfio/pci.c > +++ b/vfio/pci.c > @@ -363,8 +363,18 @@ static int vfio_pci_msi_vector_write(struct kvm *kvm, struct vfio_device *vdev, > struct vfio_pci_device *pdev = &vdev->pci; > struct msi_cap_64 *msi_cap_64 = PCI_CAP(&pdev->hdr, pdev->msi.pos); > > - if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT)) > + if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT)) { > + /* > + * If MSI doesn't support per-vector masking capability, > + * simply unmask for all vectors. > + */ > + for (i = 0; i < pdev->msi.nr_entries; i++) { > + entry = &pdev->msi.entries[i]; > + msi_set_masked(entry->virt_state, false); > + } > + This seems like the wrong place for this fix. vfio_pci_msi_vector_write() is called every time the guest pokes the MSI capability, and checks whether the access was on the Mask Bits Register. If the function doesn't support per-vector masking, then the register isn't implemented and we shouldn't do anything here. To fix the problem I think we need to set masked(virt_state) properly at init time, instead of blindly setting it to true. In fact from the guest's point of view, MSIs and MSI-X are unmasked (and disabled) at boot, so you could always set masked(virt_state) to false, but it may be safer to copy the actual state of the MSI's Mask Bits into virt_state, since for MSI, it could be non zero. If per-vector masking isn't supported, then the virt state should be false. Thanks, Jean