From patchwork Sun Dec 20 21:13:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxime Coquelin X-Patchwork-Id: 85544 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DB3FDA09FD; Sun, 20 Dec 2020 22:20:54 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B6904CDC4; Sun, 20 Dec 2020 22:15:41 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by dpdk.org (Postfix) with ESMTP id 35AF9CD9A for ; Sun, 20 Dec 2020 22:15:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608498935; 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=uSvFX9UBNfJcOyMFaaciG9eO0Xle6E0UC8gRUIIg5Go=; b=h9POIyO8QNgrNY7WbrzO7DdVjwaGOozkuwnSaXK+4uZuSUqzFmRjAS7AkU+G6PqD8482F5 TSZjJFM6VLlygdHojUaLfcdie7s6ZRflLX8wfyw07asOgaFN2uzE4WcWfJZwayEdkq255Z ytNMpjZQZlC+ySeuSxjFI9YkVLWWQG8= 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-441-M-oh9q5bPSau4EUl-whPvg-1; Sun, 20 Dec 2020 16:15:33 -0500 X-MC-Unique: M-oh9q5bPSau4EUl-whPvg-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C624D19251A1; Sun, 20 Dec 2020 21:15:32 +0000 (UTC) Received: from max-t490s.redhat.com (unknown [10.36.110.43]) by smtp.corp.redhat.com (Postfix) with ESMTP id EDBDB60C43; Sun, 20 Dec 2020 21:15:30 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, chenbo.xia@intel.com, olivier.matz@6wind.com, amorenoz@redhat.com, david.marchand@redhat.com Cc: Maxime Coquelin Date: Sun, 20 Dec 2020 22:13:45 +0100 Message-Id: <20201220211405.313012-21-maxime.coquelin@redhat.com> In-Reply-To: <20201220211405.313012-1-maxime.coquelin@redhat.com> References: <20201220211405.313012-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 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 20/40] net/virtio: make interrupt handling more generic X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" This patch aims at isolating MSIX notion into PCI layer. Signed-off-by: Maxime Coquelin Reviewed-by: David Marchand --- drivers/net/virtio/virtio.c | 6 ++ drivers/net/virtio/virtio.h | 11 +- drivers/net/virtio/virtio_ethdev.c | 7 +- drivers/net/virtio/virtio_pci.c | 131 ++++++++++++------------ drivers/net/virtio/virtio_pci.h | 25 ++--- drivers/net/virtio/virtio_user_ethdev.c | 4 +- 6 files changed, 90 insertions(+), 94 deletions(-) diff --git a/drivers/net/virtio/virtio.c b/drivers/net/virtio/virtio.c index ba3203e68b..7e1e77797f 100644 --- a/drivers/net/virtio/virtio.c +++ b/drivers/net/virtio/virtio.c @@ -63,3 +63,9 @@ virtio_get_status(struct virtio_hw *hw) { return VIRTIO_OPS(hw)->get_status(hw); } + +uint8_t +virtio_get_isr(struct virtio_hw *hw) +{ + return VIRTIO_OPS(hw)->get_isr(hw); +} diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h index 5169436c9f..f44125f48a 100644 --- a/drivers/net/virtio/virtio.h +++ b/drivers/net/virtio/virtio.h @@ -124,6 +124,13 @@ #define VIRTIO_CONFIG_STATUS_DEV_NEED_RESET 0x40 #define VIRTIO_CONFIG_STATUS_FAILED 0x80 +/* The bit of the ISR which indicates a device has an interrupt. */ +#define VIRTIO_ISR_INTR 0x1 +/* The bit of the ISR which indicates a device configuration change. */ +#define VIRTIO_ISR_CONFIG 0x2 +/* Vector value used to disable MSI for queue. */ +#define VIRTIO_MSI_NO_VECTOR 0xFFFF + /* * This structure is just a reference to read * net device specific config space; it just a chodu structure @@ -168,7 +175,7 @@ struct virtio_hw { uint8_t mac_addr[RTE_ETHER_ADDR_LEN]; uint32_t speed; /* link speed in MB */ uint8_t duplex; - uint8_t use_msix; + uint8_t intr_lsc; uint16_t max_mtu; /* * App management thread and virtio interrupt handler thread @@ -233,5 +240,5 @@ void virtio_write_dev_config(struct virtio_hw *hw, size_t offset, const void *sr void virtio_read_dev_config(struct virtio_hw *hw, size_t offset, void *dst, int length); void virtio_reset(struct virtio_hw *hw); void virtio_reinit_complete(struct virtio_hw *hw); - +uint8_t virtio_get_isr(struct virtio_hw *hw); #endif /* _VIRTIO_H_ */ diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 560647f11b..99a2dd24c4 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -1442,13 +1442,13 @@ virtio_interrupt_handler(void *param) uint16_t status; /* Read interrupt status which clears interrupt */ - isr = vtpci_isr(hw); + isr = virtio_get_isr(hw); PMD_DRV_LOG(INFO, "interrupt status = %#x", isr); if (virtio_intr_unmask(dev) < 0) PMD_DRV_LOG(ERR, "interrupt enable failed"); - if (isr & VIRTIO_PCI_ISR_CONFIG) { + if (isr & VIRTIO_ISR_CONFIG) { if (virtio_dev_link_update(dev, 0) == 0) rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, @@ -1655,8 +1655,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features) hw->weak_barriers = !virtio_with_feature(hw, VIRTIO_F_ORDER_PLATFORM); /* If host does not support both status and MSI-X then disable LSC */ - if (virtio_with_feature(hw, VIRTIO_NET_F_STATUS) && - hw->use_msix != VIRTIO_MSIX_NONE) + if (virtio_with_feature(hw, VIRTIO_NET_F_STATUS) && hw->intr_lsc) eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; else eth_dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC; diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c index 29dd84888f..01a437a1b4 100644 --- a/drivers/net/virtio/virtio_pci.c +++ b/drivers/net/virtio/virtio_pci.c @@ -28,8 +28,8 @@ * The remaining space is defined by each driver as the per-driver * configuration space. */ -#define VIRTIO_PCI_CONFIG(hw) \ - (((hw)->use_msix == VIRTIO_MSIX_ENABLED) ? 24 : 20) +#define VIRTIO_PCI_CONFIG(dev) \ + (((dev)->msix_status == VIRTIO_MSIX_ENABLED) ? 24 : 20) struct virtio_pci_internal { @@ -71,6 +71,7 @@ static void legacy_read_dev_config(struct virtio_hw *hw, size_t offset, void *dst, int length) { + struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); #ifdef RTE_ARCH_PPC_64 int size; @@ -78,17 +79,17 @@ legacy_read_dev_config(struct virtio_hw *hw, size_t offset, if (length >= 4) { size = 4; rte_pci_ioport_read(VTPCI_IO(hw), dst, size, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); *(uint32_t *)dst = rte_be_to_cpu_32(*(uint32_t *)dst); } else if (length >= 2) { size = 2; rte_pci_ioport_read(VTPCI_IO(hw), dst, size, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); *(uint16_t *)dst = rte_be_to_cpu_16(*(uint16_t *)dst); } else { size = 1; rte_pci_ioport_read(VTPCI_IO(hw), dst, size, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); } dst = (char *)dst + size; @@ -97,7 +98,7 @@ legacy_read_dev_config(struct virtio_hw *hw, size_t offset, } #else rte_pci_ioport_read(VTPCI_IO(hw), dst, length, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); #endif } @@ -105,6 +106,7 @@ static void legacy_write_dev_config(struct virtio_hw *hw, size_t offset, const void *src, int length) { + struct virtio_pci_dev *dev = virtio_pci_get_dev(hw); #ifdef RTE_ARCH_PPC_64 union { uint32_t u32; @@ -117,16 +119,16 @@ legacy_write_dev_config(struct virtio_hw *hw, size_t offset, size = 4; tmp.u32 = rte_cpu_to_be_32(*(const uint32_t *)src); rte_pci_ioport_write(VTPCI_IO(hw), &tmp.u32, size, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); } else if (length >= 2) { size = 2; tmp.u16 = rte_cpu_to_be_16(*(const uint16_t *)src); rte_pci_ioport_write(VTPCI_IO(hw), &tmp.u16, size, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); } else { size = 1; rte_pci_ioport_write(VTPCI_IO(hw), src, size, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); } src = (const char *)src + size; @@ -135,7 +137,7 @@ legacy_write_dev_config(struct virtio_hw *hw, size_t offset, } #else rte_pci_ioport_write(VTPCI_IO(hw), src, length, - VIRTIO_PCI_CONFIG(hw) + offset); + VIRTIO_PCI_CONFIG(dev) + offset); #endif } @@ -533,12 +535,6 @@ const struct virtio_ops modern_ops = { .dev_close = modern_dev_close, }; -uint8_t -vtpci_isr(struct virtio_hw *hw) -{ - return VIRTIO_OPS(hw)->get_isr(hw); -} - static void * get_cfg_addr(struct rte_pci_device *dev, struct virtio_pci_cap *cap) { @@ -623,9 +619,9 @@ virtio_read_caps(struct rte_pci_device *pci_dev, struct virtio_hw *hw) } if (flags & PCI_MSIX_ENABLE) - hw->use_msix = VIRTIO_MSIX_ENABLED; + dev->msix_status = VIRTIO_MSIX_ENABLED; else - hw->use_msix = VIRTIO_MSIX_DISABLED; + dev->msix_status = VIRTIO_MSIX_DISABLED; } if (cap.cap_vndr != PCI_CAP_ID_VNDR) { @@ -691,6 +687,54 @@ virtio_read_caps(struct rte_pci_device *pci_dev, struct virtio_hw *hw) return 0; } +static enum virtio_msix_status +vtpci_msix_detect(struct rte_pci_device *dev) +{ + uint8_t pos; + int ret; + + ret = rte_pci_read_config(dev, &pos, 1, PCI_CAPABILITY_LIST); + if (ret != 1) { + PMD_INIT_LOG(DEBUG, + "failed to read pci capability list, ret %d", ret); + return VIRTIO_MSIX_NONE; + } + + while (pos) { + uint8_t cap[2]; + + ret = rte_pci_read_config(dev, cap, sizeof(cap), pos); + if (ret != sizeof(cap)) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos: %x ret %d", + pos, ret); + break; + } + + if (cap[0] == PCI_CAP_ID_MSIX) { + uint16_t flags; + + ret = rte_pci_read_config(dev, &flags, sizeof(flags), + pos + sizeof(cap)); + if (ret != sizeof(flags)) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos:" + " %x ret %d", pos + 2, ret); + break; + } + + if (flags & PCI_MSIX_ENABLE) + return VIRTIO_MSIX_ENABLED; + else + return VIRTIO_MSIX_DISABLED; + } + + pos = cap[1]; + } + + return VIRTIO_MSIX_NONE; +} + /* * Return -1: * if there is error mapping with VFIO/UIO. @@ -736,59 +780,12 @@ vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev) dev->modern = false; msix_detect: - hw->use_msix = vtpci_msix_detect(pci_dev); + dev->msix_status = vtpci_msix_detect(pci_dev); + hw->intr_lsc = !!dev->msix_status; return 0; } -enum virtio_msix_status -vtpci_msix_detect(struct rte_pci_device *dev) -{ - uint8_t pos; - int ret; - - ret = rte_pci_read_config(dev, &pos, 1, PCI_CAPABILITY_LIST); - if (ret != 1) { - PMD_INIT_LOG(DEBUG, - "failed to read pci capability list, ret %d", ret); - return VIRTIO_MSIX_NONE; - } - - while (pos) { - uint8_t cap[2]; - - ret = rte_pci_read_config(dev, cap, sizeof(cap), pos); - if (ret != sizeof(cap)) { - PMD_INIT_LOG(DEBUG, - "failed to read pci cap at pos: %x ret %d", - pos, ret); - break; - } - - if (cap[0] == PCI_CAP_ID_MSIX) { - uint16_t flags; - - ret = rte_pci_read_config(dev, &flags, sizeof(flags), - pos + sizeof(cap)); - if (ret != sizeof(flags)) { - PMD_INIT_LOG(DEBUG, - "failed to read pci cap at pos:" - " %x ret %d", pos + 2, ret); - break; - } - - if (flags & PCI_MSIX_ENABLE) - return VIRTIO_MSIX_ENABLED; - else - return VIRTIO_MSIX_DISABLED; - } - - pos = cap[1]; - } - - return VIRTIO_MSIX_NONE; -} - void vtpci_legacy_ioport_unmap(struct virtio_hw *hw) { rte_pci_ioport_unmap(VTPCI_IO(hw)); diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h index 19d56c920c..4ee890ffda 100644 --- a/drivers/net/virtio/virtio_pci.h +++ b/drivers/net/virtio/virtio_pci.h @@ -42,13 +42,6 @@ struct virtnet_ctl; #define VIRTIO_MSI_QUEUE_VECTOR 22 /* vector for selected VQ notifications (16, RW) */ -/* The bit of the ISR which indicates a device has an interrupt. */ -#define VIRTIO_PCI_ISR_INTR 0x1 -/* The bit of the ISR which indicates a device configuration change. */ -#define VIRTIO_PCI_ISR_CONFIG 0x2 -/* Vector value used to disable MSI for queue. */ -#define VIRTIO_MSI_NO_VECTOR 0xFFFF - /* Common configuration */ #define VIRTIO_PCI_CAP_COMMON_CFG 1 /* Notifications */ @@ -103,12 +96,18 @@ struct virtio_pci_common_cfg { uint32_t queue_used_hi; /* read-write */ }; +enum virtio_msix_status { + VIRTIO_MSIX_NONE = 0, + VIRTIO_MSIX_DISABLED = 1, + VIRTIO_MSIX_ENABLED = 2 +}; 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; uint8_t *isr; uint16_t *notify_base; uint32_t notify_off_multiplier; @@ -126,22 +125,10 @@ struct virtio_pci_dev { /* The alignment to use between consumer and producer parts of vring. */ #define VIRTIO_PCI_VRING_ALIGN 4096 -enum virtio_msix_status { - VIRTIO_MSIX_NONE = 0, - VIRTIO_MSIX_DISABLED = 1, - VIRTIO_MSIX_ENABLED = 2 -}; - - /* * Function declaration from virtio_pci.c */ int vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev); - -uint8_t vtpci_isr(struct virtio_hw *); - -enum virtio_msix_status vtpci_msix_detect(struct rte_pci_device *dev); - void vtpci_legacy_ioport_unmap(struct virtio_hw *hw); int vtpci_legacy_ioport_map(struct virtio_hw *hw); diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 0f252c0732..eacb268297 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -339,7 +339,7 @@ virtio_user_get_isr(struct virtio_hw *hw __rte_unused) /* rxq interrupts and config interrupt are separated in virtio-user, * here we only report config change. */ - return VIRTIO_PCI_ISR_CONFIG; + return VIRTIO_ISR_CONFIG; } static uint16_t @@ -640,7 +640,7 @@ virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev) * MSIX is required to enable LSC (see virtio_init_device). * Here just pretend that we support msix. */ - hw->use_msix = 1; + hw->intr_lsc = 1; hw->use_vec_rx = 0; hw->use_vec_tx = 0; hw->use_inorder_rx = 0;