From patchwork Wed Feb 3 15:58:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxime Coquelin X-Patchwork-Id: 87699 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7D35BA0A0E; Wed, 3 Feb 2021 16:58:27 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 03FA524063E; Wed, 3 Feb 2021 16:58:27 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mails.dpdk.org (Postfix) with ESMTP id 74A5D240636 for ; Wed, 3 Feb 2021 16:58:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1612367904; 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; bh=JEcC00fB7ZrKT26IeaJfqw0ECpIY/gw3V4YDt2ucF/g=; b=UNi9hz4W27Sy8aHJE2VS3+zzUOLjUz/Fhe8N4OD+/J7CNC8w2OthzsQGQEMWj2Gs8PNSTl oEvoieg3OmBANS5drEzYB4TlMaY8QXW6EjUurwcrOgNvyqx0UEDT1McCQGiWD2Yiw3wUno bksA/wzk5FpcZ9660iI1U1NPVuWVO2c= 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-244-Z0ixqBKyO3i3JwEYTrvsEg-1; Wed, 03 Feb 2021 10:58:23 -0500 X-MC-Unique: Z0ixqBKyO3i3JwEYTrvsEg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EF340CE646; Wed, 3 Feb 2021 15:58:21 +0000 (UTC) Received: from max-t490s.redhat.com (unknown [10.36.110.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2499A72161; Wed, 3 Feb 2021 15:58:13 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, yinan.wang@intel.com, chenbo.xia@intel.com, amorenoz@redhat.com, david.marchand@redhat.com, weix.ling@intel.com, yux.jiang@intel.com, anatoly.burakov@intel.com Cc: Maxime Coquelin Date: Wed, 3 Feb 2021 16:58:11 +0100 Message-Id: <20210203155811.53005-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [dpdk-dev] [PATCH] net/virtio: fix secondary process crash with PCI devices X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" The Virtio rework series mistakenly moved the rte_pci_device pointer to struct virtio_hw, which is shared between the two processes. But this structure is per-process, so this change made secondary process to try accessing primary process-only memory, leading to a crash This patch reverts to proper behavior, by storing the rte_pci_device pointer into the pre-process virtio_pci_internal struct. It provides also helper to get the pointer from the virtio_hw struct pointer. Bugzilla ID: 633 Fixes: c8d4b02f72ae ("net/virtio: move legacy IO to virtio PCI") Reported-by: Anatoly Burakov Signed-off-by: Maxime Coquelin Reviewed-by: David Marchand --- drivers/net/virtio/virtio_pci.c | 25 +++++-------------------- drivers/net/virtio/virtio_pci.h | 12 +++++++++++- drivers/net/virtio/virtio_pci_ethdev.c | 2 ++ 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c index d27e9eb515..c14d1339c9 100644 --- a/drivers/net/virtio/virtio_pci.c +++ b/drivers/net/virtio/virtio_pci.c @@ -31,13 +31,6 @@ #define VIRTIO_PCI_CONFIG(dev) \ (((dev)->msix_status == VIRTIO_MSIX_ENABLED) ? 24 : 20) - -struct virtio_pci_internal { - struct rte_pci_ioport io; -}; - -#define VTPCI_IO(hw) (&virtio_pci_internal[(hw)->port_id].io) - struct virtio_pci_internal virtio_pci_internal[RTE_MAX_ETHPORTS]; static inline int @@ -313,16 +306,14 @@ legacy_intr_detect(struct virtio_hw *hw) { struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); - dev->msix_status = vtpci_msix_detect(dev->pci_dev); + dev->msix_status = vtpci_msix_detect(VTPCI_DEV(hw)); hw->intr_lsc = !!dev->msix_status; } static int legacy_dev_close(struct virtio_hw *hw) { - struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); - - rte_pci_unmap_device(dev->pci_dev); + rte_pci_unmap_device(VTPCI_DEV(hw)); rte_pci_ioport_unmap(VTPCI_IO(hw)); return 0; @@ -574,16 +565,14 @@ modern_intr_detect(struct virtio_hw *hw) { struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); - dev->msix_status = vtpci_msix_detect(dev->pci_dev); + dev->msix_status = vtpci_msix_detect(VTPCI_DEV(hw)); hw->intr_lsc = !!dev->msix_status; } static int modern_dev_close(struct virtio_hw *hw) { - struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); - - rte_pci_unmap_device(dev->pci_dev); + rte_pci_unmap_device(VTPCI_DEV(hw)); return 0; } @@ -772,8 +761,6 @@ vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev) RTE_BUILD_BUG_ON(offsetof(struct virtio_pci_dev, hw) != 0); - dev->pci_dev = pci_dev; - /* * Try if we can succeed reading virtio pci caps, which exists * only on modern pci device. If failed, we fallback to legacy @@ -816,7 +803,5 @@ void vtpci_legacy_ioport_unmap(struct virtio_hw *hw) int vtpci_legacy_ioport_map(struct virtio_hw *hw) { - struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); - - return rte_pci_ioport_map(dev->pci_dev, 0, VTPCI_IO(hw)); + return rte_pci_ioport_map(VTPCI_DEV(hw), 0, VTPCI_IO(hw)); } diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h index 57596e471f..11e25a0142 100644 --- a/drivers/net/virtio/virtio_pci.h +++ b/drivers/net/virtio/virtio_pci.h @@ -104,7 +104,6 @@ enum virtio_msix_status { struct virtio_pci_dev { struct virtio_hw hw; - struct rte_pci_device *pci_dev; struct virtio_pci_common_cfg *common_cfg; struct virtio_net_config *dev_cfg; enum virtio_msix_status msix_status; @@ -116,6 +115,17 @@ struct virtio_pci_dev { #define virtio_pci_get_dev(hwp) container_of(hwp, struct virtio_pci_dev, hw) +struct virtio_pci_internal { + struct rte_pci_ioport io; + struct rte_pci_device *dev; +}; + +extern struct virtio_pci_internal virtio_pci_internal[RTE_MAX_ETHPORTS]; + +#define VTPCI_IO(hw) (&virtio_pci_internal[(hw)->port_id].io) +#define VTPCI_DEV(hw) (virtio_pci_internal[(hw)->port_id].dev) + + /* * How many bits to shift physical queue address written to QUEUE_PFN. * 12 is historical, and due to x86 page size. diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c index 3bb5c6268b..4083853c48 100644 --- a/drivers/net/virtio/virtio_pci_ethdev.c +++ b/drivers/net/virtio/virtio_pci_ethdev.c @@ -78,12 +78,14 @@ eth_virtio_pci_init(struct rte_eth_dev *eth_dev) if (rte_eal_process_type() == RTE_PROC_PRIMARY) { hw->port_id = eth_dev->data->port_id; + VTPCI_DEV(hw) = pci_dev; ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), dev); if (ret) { PMD_INIT_LOG(ERR, "Failed to init PCI device\n"); return -1; } } else { + VTPCI_DEV(hw) = pci_dev; if (dev->modern) VIRTIO_OPS(hw) = &modern_ops; else