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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 35DB5C47094 for ; Thu, 10 Jun 2021 17:16:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1950E613E9 for ; Thu, 10 Jun 2021 17:16:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230220AbhFJRSb (ORCPT ); Thu, 10 Jun 2021 13:18:31 -0400 Received: from foss.arm.com ([217.140.110.172]:37410 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230179AbhFJRSa (ORCPT ); Thu, 10 Jun 2021 13:18:30 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4AB1F1396; Thu, 10 Jun 2021 10:16:33 -0700 (PDT) Received: from [192.168.0.110] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4A48C3F719; Thu, 10 Jun 2021 10:16:32 -0700 (PDT) Subject: Re: [PATCH kvmtool 4/4] arm/arm64: vfio: Add PCI Express Capability Structure To: Andre Przywara Cc: will@kernel.org, julien.thierry.kdev@gmail.com, kvm@vger.kernel.org, sami.mujawar@arm.com, lorenzo.pieralisi@arm.com, maz@kernel.org References: <20210609183812.29596-1-alexandru.elisei@arm.com> <20210609183812.29596-5-alexandru.elisei@arm.com> <20210610171427.09ee5108@slackpad.fritz.box> From: Alexandru Elisei Message-ID: Date: Thu, 10 Jun 2021 18:17:24 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210610171427.09ee5108@slackpad.fritz.box> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Andre, On 6/10/21 5:14 PM, Andre Przywara wrote: > On Wed, 9 Jun 2021 19:38:12 +0100 > Alexandru Elisei wrote: > >> It turns out that some Linux drivers (like Realtek R8169) fall back to a >> device-specific configuration method if the device is not PCI Express >> capable: >> >> [ 1.433825] r8169 0000:00:00.0 enp0s0: No native access to PCI extended config space, falling back to CSI >> > Nice discovery, thanks! > >> Add the PCI Express Capability Structure and populate it for assigned >> devices, as this is how the Linux PCI driver determines if a device is PCI >> Express capable. >> >> Because we don't emulate a PCI Express link, a root complex or any slot >> related properties, the PCI Express capability is kept as small as possible >> by ignoring those fields. >> >> Signed-off-by: Alexandru Elisei >> --- >> include/kvm/pci.h | 24 ++++++++++++++++++++++++ >> vfio/pci.c | 13 +++++++++++++ >> 2 files changed, 37 insertions(+) >> >> diff --git a/include/kvm/pci.h b/include/kvm/pci.h >> index 42d9e1c5645f..0f2d5bbabdc3 100644 >> --- a/include/kvm/pci.h >> +++ b/include/kvm/pci.h >> @@ -46,6 +46,8 @@ struct kvm; >> #define PCI_DEV_CFG_SIZE_EXTENDED 4096 >> >> #ifdef ARCH_HAS_PCI_EXP >> +#define arch_has_pci_exp() (true) >> + >> #define PCI_CFG_SIZE PCI_CFG_SIZE_EXTENDED >> #define PCI_DEV_CFG_SIZE PCI_DEV_CFG_SIZE_EXTENDED >> >> @@ -73,6 +75,8 @@ union pci_config_address { >> }; >> >> #else >> +#define arch_has_pci_exp() (false) >> + >> #define PCI_CFG_SIZE PCI_CFG_SIZE_LEGACY >> #define PCI_DEV_CFG_SIZE PCI_DEV_CFG_SIZE_LEGACY >> >> @@ -143,6 +147,24 @@ struct pci_cap_hdr { >> u8 next; >> }; >> >> +struct pci_exp_cap { >> + u8 cap; >> + u8 next; >> + u16 cap_reg; >> + u32 dev_cap; >> + u16 dev_ctrl; >> + u16 dev_status; >> + u32 link_cap; >> + u16 link_ctrl; >> + u16 link_status; >> + u32 slot_cap; >> + u16 slot_ctrl; >> + u16 slot_status; >> + u16 root_ctrl; >> + u16 root_cap; >> + u32 root_status; >> +}; >> + >> struct pci_device_header; >> >> typedef int (*bar_activate_fn_t)(struct kvm *kvm, >> @@ -188,6 +210,8 @@ struct pci_device_header { >> u8 min_gnt; >> u8 max_lat; >> struct msix_cap msix; >> + /* Used only by architectures which support PCIE */ >> + struct pci_exp_cap pci_exp; >> } __attribute__((packed)); >> /* Pad to PCI config space size */ >> u8 __pad[PCI_DEV_CFG_SIZE]; >> diff --git a/vfio/pci.c b/vfio/pci.c >> index 6a4204634e71..5c9bec6db710 100644 >> --- a/vfio/pci.c >> +++ b/vfio/pci.c >> @@ -623,6 +623,12 @@ static ssize_t vfio_pci_cap_size(struct pci_cap_hdr *cap_hdr) >> return PCI_CAP_MSIX_SIZEOF; >> case PCI_CAP_ID_MSI: >> return vfio_pci_msi_cap_size((void *)cap_hdr); >> + case PCI_CAP_ID_EXP: >> + /* >> + * We don't emulate any of the link, slot and root complex >> + * properties, so ignore them. >> + */ >> + return PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V1; > My (admittedly) older distro kernel headers don't carry this symbol. > Can we have a "#ifndef ... #define ... #endif" at the beginning of > this file? Good catch, we'll fix in the next iteration. Thanks, Alex > If "Slackware" isn't convincing enough, RHEL 7.4 doesn't include it > either. And this issue hits x86 as well, even though we don't use PCIe > there. Also we do something similar in patch 3/4. > > Rest looks alright. > > Cheers, > Andre > >> default: >> pr_err("unknown PCI capability 0x%x", cap_hdr->type); >> return 0; >> @@ -696,6 +702,13 @@ static int vfio_pci_parse_caps(struct vfio_device *vdev) >> pdev->msi.pos = pos; >> pdev->irq_modes |= VFIO_PCI_IRQ_MODE_MSI; >> break; >> + case PCI_CAP_ID_EXP: >> + if (!arch_has_pci_exp()) >> + continue; >> + ret = vfio_pci_add_cap(vdev, virt_hdr, cap, pos); >> + if (ret) >> + return ret; >> + break; >> } >> } >>