| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 06 Feb 2023 13:36:38 -0800
From: Alexander H Duyck <alexander.duyck@...il.com>
To: Shannon Nelson <shannon.nelson@....com>, netdev@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org
Cc: drivers@...sando.io
Subject: Re: [PATCH v2 net-next 3/3] ionic: add support for device Component
Memory Buffers
On Mon, 2023-02-06 at 10:16 -0800, Shannon Nelson wrote:
> The ionic device has on-board memory (CMB) that can be used
> for descriptors as a way to speed descriptor access for faster
> traffic processing. It imposes a couple of restrictions so
> is not on by default, but can be enabled through the ethtool
> priv-flags.
For the purposes of patch review it might be convinent to call out what
those restrictions are as you enable the code below. I'm assuming it is
mostly just the amount of space you can use, but if there is something
else it would be useful to have that noted.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
> ---
> .../ethernet/pensando/ionic/ionic_bus_pci.c | 3 +
> .../net/ethernet/pensando/ionic/ionic_dev.c | 71 ++++++++
> .../net/ethernet/pensando/ionic/ionic_dev.h | 13 ++
> .../ethernet/pensando/ionic/ionic_ethtool.c | 128 +++++++++++++-
> .../ethernet/pensando/ionic/ionic_ethtool.h | 1 +
> .../net/ethernet/pensando/ionic/ionic_if.h | 3 +-
> .../net/ethernet/pensando/ionic/ionic_lif.c | 160 ++++++++++++++++--
> .../net/ethernet/pensando/ionic/ionic_lif.h | 32 +++-
> .../net/ethernet/pensando/ionic/ionic_txrx.c | 22 ++-
> 9 files changed, 413 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index 0eff78fa0565..f62f11ba21c0 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -352,6 +352,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> err_out_reset:
> ionic_reset(ionic);
> err_out_teardown:
> + ionic_dev_teardown(ionic);
> pci_clear_master(pdev);
> /* Don't fail the probe for these errors, keep
> * the hw interface around for inspection
> @@ -369,6 +370,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> err_out_clear_drvdata:
> mutex_destroy(&ionic->dev_cmd_lock);
> ionic_devlink_free(ionic);
> + pci_set_drvdata(pdev, NULL);
>
> return err;
> }
This piece almost seems like it is unrelated. Perhaps it bled through
from some other work or cleanup?
> @@ -390,6 +392,7 @@ static void ionic_remove(struct pci_dev *pdev)
>
> ionic_port_reset(ionic);
> ionic_reset(ionic);
> + ionic_dev_teardown(ionic);
> pci_clear_master(pdev);
> ionic_unmap_bars(ionic);
> pci_release_regions(pdev);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
> index 626b9113e7c4..9b4bba2279ab 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
> @@ -92,6 +92,7 @@ int ionic_dev_setup(struct ionic *ionic)
> unsigned int num_bars = ionic->num_bars;
> struct ionic_dev *idev = &ionic->idev;
> struct device *dev = ionic->dev;
> + int size;
> u32 sig;
>
> /* BAR0: dev_cmd and interrupts */
> @@ -133,9 +134,40 @@ int ionic_dev_setup(struct ionic *ionic)
> idev->db_pages = bar->vaddr;
> idev->phy_db_pages = bar->bus_addr;
>
> + /* BAR2: optional controller memory mapping */
> + bar++;
> + mutex_init(&idev->cmb_inuse_lock);
> + if (num_bars < 3 || !ionic->bars[IONIC_PCI_BAR_CMB].len) {
> + idev->cmb_inuse = NULL;
> + idev->phy_cmb_pages = 0;
> + idev->cmb_npages = 0;
> + return 0;
> + }
> +
> + idev->phy_cmb_pages = bar->bus_addr;
> + idev->cmb_npages = bar->len / PAGE_SIZE;
> + size = BITS_TO_LONGS(idev->cmb_npages) * sizeof(long);
> + idev->cmb_inuse = kzalloc(size, GFP_KERNEL);
> + if (!idev->cmb_inuse) {
> + idev->phy_cmb_pages = 0;
> + idev->cmb_npages = 0;
> + }
> +
Why not hold of on setting phy_cmb_pages and cmb_npages until after you
have allocated the pages rather then resetting them in the event of
failure?
Also is it really acceptable for this to fail silently?
> return 0;
> }
>
> +void ionic_dev_teardown(struct ionic *ionic)
> +{
> + struct ionic_dev *idev = &ionic->idev;
> +
> + kfree(idev->cmb_inuse);
> + idev->cmb_inuse = NULL;
> + idev->phy_cmb_pages = 0;
> + idev->cmb_npages = 0;
> +
> + mutex_destroy(&idev->cmb_inuse_lock);
> +}
> +
> /* Devcmd Interface */
> bool ionic_is_fw_running(struct ionic_dev *idev)
> {
> @@ -571,6 +603,33 @@ int ionic_db_page_num(struct ionic_lif *lif, int pid)
> return (lif->hw_index * lif->dbid_count) + pid;
> }
>
> +int ionic_get_cmb(struct ionic_lif *lif, u32 *pgid, phys_addr_t *pgaddr, int order)
> +{
> + struct ionic_dev *idev = &lif->ionic->idev;
> + int ret;
> +
> + mutex_lock(&idev->cmb_inuse_lock);
> + ret = bitmap_find_free_region(idev->cmb_inuse, idev->cmb_npages, order);
> + mutex_unlock(&idev->cmb_inuse_lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + *pgid = (u32)ret;
I'm not sure if the cast is necessary since we are guaranteed ret will
be unsigned based on the earlier check.
> + *pgaddr = idev->phy_cmb_pages + ret * PAGE_SIZE;
> +
> + return 0;
> +}
> +
> +void ionic_put_cmb(struct ionic_lif *lif, u32 pgid, int order)
> +{
> + struct ionic_dev *idev = &lif->ionic->idev;
> +
> + mutex_lock(&idev->cmb_inuse_lock);
> + bitmap_release_region(idev->cmb_inuse, pgid, order);
> + mutex_unlock(&idev->cmb_inuse_lock);
> +}
> +
> int ionic_cq_init(struct ionic_lif *lif, struct ionic_cq *cq,
> struct ionic_intr_info *intr,
> unsigned int num_descs, size_t desc_size)
> @@ -679,6 +738,18 @@ void ionic_q_map(struct ionic_queue *q, void *base, dma_addr_t base_pa)
> cur->desc = base + (i * q->desc_size);
> }
>
> +void ionic_q_cmb_map(struct ionic_queue *q, void __iomem *base, dma_addr_t base_pa)
> +{
> + struct ionic_desc_info *cur;
> + unsigned int i;
> +
> + q->cmb_base = base;
> + q->cmb_base_pa = base_pa;
> +
> + for (i = 0, cur = q->info; i < q->num_descs; i++, cur++)
> + cur->cmb_desc = base + (i * q->desc_size);
> +}
> +
> void ionic_q_sg_map(struct ionic_queue *q, void *base, dma_addr_t base_pa)
> {
> struct ionic_desc_info *cur;
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> index 2a1d7b9c07e7..a4a8802f3771 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> @@ -153,6 +153,11 @@ struct ionic_dev {
> struct ionic_intr __iomem *intr_ctrl;
> u64 __iomem *intr_status;
>
> + struct mutex cmb_inuse_lock; /* for cmb_inuse */
> + unsigned long *cmb_inuse;
> + dma_addr_t phy_cmb_pages;
> + u32 cmb_npages;
> +
> u32 port_info_sz;
> struct ionic_port_info *port_info;
> dma_addr_t port_info_pa;
> @@ -197,6 +202,7 @@ struct ionic_desc_info {
> struct ionic_rxq_desc *rxq_desc;
> struct ionic_admin_cmd *adminq_desc;
> };
> + void __iomem *cmb_desc;
> union {
> void *sg_desc;
> struct ionic_txq_sg_desc *txq_sg_desc;
> @@ -233,12 +239,14 @@ struct ionic_queue {
> struct ionic_rxq_desc *rxq;
> struct ionic_admin_cmd *adminq;
> };
> + void __iomem *cmb_base;
> union {
> void *sg_base;
> struct ionic_txq_sg_desc *txq_sgl;
> struct ionic_rxq_sg_desc *rxq_sgl;
> };
> dma_addr_t base_pa;
> + dma_addr_t cmb_base_pa;
> dma_addr_t sg_base_pa;
> unsigned int desc_size;
> unsigned int sg_desc_size;
> @@ -301,6 +309,7 @@ static inline bool ionic_q_has_space(struct ionic_queue *q, unsigned int want)
>
> void ionic_init_devinfo(struct ionic *ionic);
> int ionic_dev_setup(struct ionic *ionic);
> +void ionic_dev_teardown(struct ionic *ionic);
>
> void ionic_dev_cmd_go(struct ionic_dev *idev, union ionic_dev_cmd *cmd);
> u8 ionic_dev_cmd_status(struct ionic_dev *idev);
> @@ -336,6 +345,9 @@ void ionic_dev_cmd_adminq_init(struct ionic_dev *idev, struct ionic_qcq *qcq,
>
> int ionic_db_page_num(struct ionic_lif *lif, int pid);
>
> +int ionic_get_cmb(struct ionic_lif *lif, u32 *pgid, phys_addr_t *pgaddr, int order);
> +void ionic_put_cmb(struct ionic_lif *lif, u32 pgid, int order);
> +
> int ionic_cq_init(struct ionic_lif *lif, struct ionic_cq *cq,
> struct ionic_intr_info *intr,
> unsigned int num_descs, size_t desc_size);
> @@ -352,6 +364,7 @@ int ionic_q_init(struct ionic_lif *lif, struct ionic_dev *idev,
> unsigned int num_descs, size_t desc_size,
> size_t sg_desc_size, unsigned int pid);
> void ionic_q_map(struct ionic_queue *q, void *base, dma_addr_t base_pa);
> +void ionic_q_cmb_map(struct ionic_queue *q, void __iomem *base, dma_addr_t base_pa);
> void ionic_q_sg_map(struct ionic_queue *q, void *base, dma_addr_t base_pa);
> void ionic_q_post(struct ionic_queue *q, bool ring_doorbell, ionic_desc_cb cb,
> void *cb_arg);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 01c22701482d..45e5dc400095 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -11,6 +11,38 @@
> #include "ionic_ethtool.h"
> #include "ionic_stats.h"
>
> +static const char ionic_priv_flags_strings[][ETH_GSTRING_LEN] = {
> +#define IONIC_PRIV_F_CMB_RINGS BIT(0)
> + "cmb-rings",
> +};
> +
> +#define IONIC_PRIV_FLAGS_COUNT ARRAY_SIZE(ionic_priv_flags_strings)
> +
> +static int ionic_validate_cmb_config(struct ionic_lif *lif,
> + struct ionic_queue_params *qparam)
> +{
> + int pages_have, pages_required = 0;
> + unsigned long sz;
> +
> + if (!qparam->cmb_enabled)
> + return 0;
> +
> + sz = sizeof(struct ionic_txq_desc) * qparam->ntxq_descs * qparam->nxqs;
> + pages_required += ALIGN(sz, PAGE_SIZE) / PAGE_SIZE;
> +
> + sz = sizeof(struct ionic_rxq_desc) * qparam->nrxq_descs * qparam->nxqs;
> + pages_required += ALIGN(sz, PAGE_SIZE) / PAGE_SIZE;
> +
> + pages_have = lif->ionic->bars[IONIC_PCI_BAR_CMB].len / PAGE_SIZE;
> + if (pages_required > pages_have) {
> + netdev_info(lif->netdev, "Not enough CMB pages for number of queues and size of descriptor rings, need %d have %d",
> + pages_required, pages_have);
> + return -ENOMEM;
> + }
> +
> + return pages_required;
> +}
> +
> static void ionic_get_stats_strings(struct ionic_lif *lif, u8 *buf)
> {
> u32 i;
> @@ -52,6 +84,9 @@ static int ionic_get_sset_count(struct net_device *netdev, int sset)
> case ETH_SS_STATS:
> count = ionic_get_stats_count(lif);
> break;
> + case ETH_SS_PRIV_FLAGS:
> + count = IONIC_PRIV_FLAGS_COUNT;
> + break;
> }
> return count;
> }
> @@ -65,6 +100,10 @@ static void ionic_get_strings(struct net_device *netdev,
> case ETH_SS_STATS:
> ionic_get_stats_strings(lif, buf);
> break;
> + case ETH_SS_PRIV_FLAGS:
> + memcpy(buf, ionic_priv_flags_strings,
> + IONIC_PRIV_FLAGS_COUNT * ETH_GSTRING_LEN);
> + break;
> }
> }
>
> @@ -554,6 +593,13 @@ static int ionic_set_ringparam(struct net_device *netdev,
> ring->rx_pending == lif->nrxq_descs)
> return 0;
>
> + qparam.ntxq_descs = ring->tx_pending;
> + qparam.nrxq_descs = ring->rx_pending;
> +
> + err = ionic_validate_cmb_config(lif, &qparam);
> + if (err < 0)
> + return err;
> +
> if (ring->tx_pending != lif->ntxq_descs)
> netdev_info(netdev, "Changing Tx ring size from %d to %d\n",
> lif->ntxq_descs, ring->tx_pending);
> @@ -569,9 +615,6 @@ static int ionic_set_ringparam(struct net_device *netdev,
> return 0;
> }
>
> - qparam.ntxq_descs = ring->tx_pending;
> - qparam.nrxq_descs = ring->rx_pending;
> -
> mutex_lock(&lif->queue_lock);
> err = ionic_reconfigure_queues(lif, &qparam);
> mutex_unlock(&lif->queue_lock);
> @@ -638,7 +681,7 @@ static int ionic_set_channels(struct net_device *netdev,
> lif->nxqs, ch->combined_count);
>
> qparam.nxqs = ch->combined_count;
> - qparam.intr_split = 0;
> + qparam.intr_split = false;
> } else {
> max_cnt /= 2;
> if (ch->rx_count > max_cnt)
> @@ -654,9 +697,13 @@ static int ionic_set_channels(struct net_device *netdev,
> lif->nxqs, ch->rx_count);
>
> qparam.nxqs = ch->rx_count;
> - qparam.intr_split = 1;
> + qparam.intr_split = true;
> }
>
> + err = ionic_validate_cmb_config(lif, &qparam);
> + if (err < 0)
> + return err;
> +
> /* if we're not running, just set the values and return */
> if (!netif_running(lif->netdev)) {
> lif->nxqs = qparam.nxqs;
> @@ -699,6 +746,75 @@ static int ionic_get_rxnfc(struct net_device *netdev,
> return err;
> }
>
> +int ionic_cmb_pages_in_use(struct ionic_lif *lif)
> +{
> + struct ionic_queue_params qparam;
> +
> + ionic_init_queue_params(lif, &qparam);
> + return ionic_validate_cmb_config(lif, &qparam);
> +}
> +
> +static int ionic_cmb_rings_toggle(struct ionic_lif *lif, bool cmb_enable)
> +{
> + struct ionic_queue_params qparam;
> + int pages_used;
> +
> + if (!(lif->qtype_info[IONIC_QTYPE_TXQ].features & IONIC_QIDENT_F_CMB) ||
> + !(lif->qtype_info[IONIC_QTYPE_RXQ].features & IONIC_QIDENT_F_CMB) ||
> + !lif->ionic->idev.cmb_npages) {
> + netdev_info(lif->netdev, "CMB rings are not supported on this device\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (netif_running(lif->netdev))
> + return -EBUSY;
> +
> + ionic_init_queue_params(lif, &qparam);
> + qparam.cmb_enabled = cmb_enable;
> + pages_used = ionic_validate_cmb_config(lif, &qparam);
> + if (pages_used < 0)
> + return pages_used;
> +
> + if (cmb_enable) {
> + netdev_info(lif->netdev, "Enabling CMB rings - %d pages\n",
> + pages_used);
> + set_bit(IONIC_LIF_F_CMB_RINGS, lif->state);
> + } else {
> + netdev_info(lif->netdev, "Disabling CMB rings\n");
> + clear_bit(IONIC_LIF_F_CMB_RINGS, lif->state);
> + }
> +
> + return 0;
> +}
> +
> +static u32 ionic_get_priv_flags(struct net_device *netdev)
> +{
> + struct ionic_lif *lif = netdev_priv(netdev);
> + u32 priv_flags = 0;
> +
> + if (test_bit(IONIC_LIF_F_CMB_RINGS, lif->state))
> + priv_flags |= IONIC_PRIV_F_CMB_RINGS;
> +
> + return priv_flags;
> +}
> +
> +static int ionic_set_priv_flags(struct net_device *netdev, u32 priv_flags)
> +{
> + struct ionic_lif *lif = netdev_priv(netdev);
> + bool cmb_now, cmb_req;
> + int ret;
> +
> + cmb_now = test_bit(IONIC_LIF_F_CMB_RINGS, lif->state);
> + cmb_req = !!(priv_flags & IONIC_PRIV_F_CMB_RINGS);
> + if (cmb_now != cmb_req) {
> + ret = ionic_cmb_rings_toggle(lif, cmb_req);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static u32 ionic_get_rxfh_indir_size(struct net_device *netdev)
> {
> struct ionic_lif *lif = netdev_priv(netdev);
> @@ -980,6 +1096,8 @@ static const struct ethtool_ops ionic_ethtool_ops = {
> .get_strings = ionic_get_strings,
> .get_ethtool_stats = ionic_get_stats,
> .get_sset_count = ionic_get_sset_count,
> + .get_priv_flags = ionic_get_priv_flags,
> + .set_priv_flags = ionic_set_priv_flags,
> .get_rxnfc = ionic_get_rxnfc,
> .get_rxfh_indir_size = ionic_get_rxfh_indir_size,
> .get_rxfh_key_size = ionic_get_rxfh_key_size,
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.h b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.h
> index 38b91b1d70ae..6bc9c177d14b 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.h
> @@ -4,6 +4,7 @@
> #ifndef _IONIC_ETHTOOL_H_
> #define _IONIC_ETHTOOL_H_
>
> +int ionic_cmb_pages_in_use(struct ionic_lif *lif);
> void ionic_ethtool_set_ops(struct net_device *netdev);
>
> #endif /* _IONIC_ETHTOOL_H_ */
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
> index eac09b2375b8..9a1825edf0d0 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
> @@ -3073,9 +3073,10 @@ union ionic_adminq_comp {
>
> #define IONIC_BARS_MAX 6
> #define IONIC_PCI_BAR_DBELL 1
> +#define IONIC_PCI_BAR_CMB 2
>
> -/* BAR0 */
> #define IONIC_BAR0_SIZE 0x8000
> +#define IONIC_BAR2_SIZE 0x800000
>
> #define IONIC_BAR0_DEV_INFO_REGS_OFFSET 0x0000
> #define IONIC_BAR0_DEV_CMD_REGS_OFFSET 0x0800
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 8499165b1563..e8884de83474 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -25,9 +25,12 @@
> static const u8 ionic_qtype_versions[IONIC_QTYPE_MAX] = {
> [IONIC_QTYPE_ADMINQ] = 0, /* 0 = Base version with CQ support */
> [IONIC_QTYPE_NOTIFYQ] = 0, /* 0 = Base version */
> - [IONIC_QTYPE_RXQ] = 0, /* 0 = Base version with CQ+SG support */
> - [IONIC_QTYPE_TXQ] = 1, /* 0 = Base version with CQ+SG support
> - * 1 = ... with Tx SG version 1
> + [IONIC_QTYPE_RXQ] = 2, /* 0 = Base version with CQ+SG support
> + * 2 = ... with CMB rings
> + */
> + [IONIC_QTYPE_TXQ] = 3, /* 0 = Base version with CQ+SG support
> + * 1 = ... with Tx SG version 1
> + * 3 = ... with CMB rings
> */
> };
>
> @@ -379,6 +382,15 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
> qcq->q_base_pa = 0;
> }
>
> + if (qcq->cmb_q_base) {
> + iounmap(qcq->cmb_q_base);
> + ionic_put_cmb(lif, qcq->cmb_pgid, qcq->cmb_order);
> + qcq->cmb_pgid = 0;
> + qcq->cmb_order = 0;
> + qcq->cmb_q_base = NULL;
> + qcq->cmb_q_base_pa = 0;
> + }
> +
> if (qcq->cq_base) {
> dma_free_coherent(dev, qcq->cq_size, qcq->cq_base, qcq->cq_base_pa);
> qcq->cq_base = NULL;
> @@ -587,6 +599,7 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> ionic_cq_map(&new->cq, cq_base, cq_base_pa);
> ionic_cq_bind(&new->cq, &new->q);
> } else {
> + /* regular DMA q descriptors */
> new->q_size = PAGE_SIZE + (num_descs * desc_size);
> new->q_base = dma_alloc_coherent(dev, new->q_size, &new->q_base_pa,
> GFP_KERNEL);
> @@ -599,6 +612,33 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> q_base_pa = ALIGN(new->q_base_pa, PAGE_SIZE);
> ionic_q_map(&new->q, q_base, q_base_pa);
>
> + if (flags & IONIC_QCQ_F_CMB_RINGS) {
> + /* on-chip CMB q descriptors */
> + new->cmb_q_size = num_descs * desc_size;
> + new->cmb_order = order_base_2(new->cmb_q_size / PAGE_SIZE);
> +
> + err = ionic_get_cmb(lif, &new->cmb_pgid, &new->cmb_q_base_pa,
> + new->cmb_order);
> + if (err) {
> + netdev_err(lif->netdev,
> + "Cannot allocate queue order %d from cmb: err %d\n",
> + new->cmb_order, err);
> + goto err_out_free_q;
> + }
> +
> + new->cmb_q_base = ioremap_wc(new->cmb_q_base_pa, new->cmb_q_size);
> + if (!new->cmb_q_base) {
> + netdev_err(lif->netdev, "Cannot map queue from cmb\n");
> + ionic_put_cmb(lif, new->cmb_pgid, new->cmb_order);
> + err = -ENOMEM;
> + goto err_out_free_q;
> + }
> +
> + new->cmb_q_base_pa -= idev->phy_cmb_pages;
> + ionic_q_cmb_map(&new->q, new->cmb_q_base, new->cmb_q_base_pa);
> + }
> +
> + /* cq DMA descriptors */
> new->cq_size = PAGE_SIZE + (num_descs * cq_desc_size);
> new->cq_base = dma_alloc_coherent(dev, new->cq_size, &new->cq_base_pa,
> GFP_KERNEL);
> @@ -637,6 +677,10 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
> err_out_free_cq:
> dma_free_coherent(dev, new->cq_size, new->cq_base, new->cq_base_pa);
> err_out_free_q:
> + if (new->cmb_q_base) {
> + iounmap(new->cmb_q_base);
> + ionic_put_cmb(lif, new->cmb_pgid, new->cmb_order);
> + }
> dma_free_coherent(dev, new->q_size, new->q_base, new->q_base_pa);
> err_out_free_cq_info:
> vfree(new->cq.info);
> @@ -718,6 +762,8 @@ static void ionic_qcq_sanitize(struct ionic_qcq *qcq)
> qcq->cq.tail_idx = 0;
> qcq->cq.done_color = 1;
> memset(qcq->q_base, 0, qcq->q_size);
> + if (qcq->cmb_q_base)
> + memset_io(qcq->cmb_q_base, 0, qcq->cmb_q_size);
> memset(qcq->cq_base, 0, qcq->cq_size);
> memset(qcq->sg_base, 0, qcq->sg_size);
> }
> @@ -737,6 +783,7 @@ static int ionic_lif_txq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
> .index = cpu_to_le32(q->index),
> .flags = cpu_to_le16(IONIC_QINIT_F_IRQ |
> IONIC_QINIT_F_SG),
> + .intr_index = cpu_to_le16(qcq->intr.index),
> .pid = cpu_to_le16(q->pid),
> .ring_size = ilog2(q->num_descs),
> .ring_base = cpu_to_le64(q->base_pa),
> @@ -745,17 +792,19 @@ static int ionic_lif_txq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
> .features = cpu_to_le64(q->features),
> },
> };
> - unsigned int intr_index;
> int err;
>
> - intr_index = qcq->intr.index;
> -
> - ctx.cmd.q_init.intr_index = cpu_to_le16(intr_index);
> + if (qcq->flags & IONIC_QCQ_F_CMB_RINGS) {
> + ctx.cmd.q_init.flags |= cpu_to_le16(IONIC_QINIT_F_CMB);
> + ctx.cmd.q_init.ring_base = cpu_to_le64(qcq->cmb_q_base_pa);
> + }
>
> dev_dbg(dev, "txq_init.pid %d\n", ctx.cmd.q_init.pid);
> dev_dbg(dev, "txq_init.index %d\n", ctx.cmd.q_init.index);
> dev_dbg(dev, "txq_init.ring_base 0x%llx\n", ctx.cmd.q_init.ring_base);
> dev_dbg(dev, "txq_init.ring_size %d\n", ctx.cmd.q_init.ring_size);
> + dev_dbg(dev, "txq_init.cq_ring_base 0x%llx\n", ctx.cmd.q_init.cq_ring_base);
> + dev_dbg(dev, "txq_init.sg_ring_base 0x%llx\n", ctx.cmd.q_init.sg_ring_base);
> dev_dbg(dev, "txq_init.flags 0x%x\n", ctx.cmd.q_init.flags);
> dev_dbg(dev, "txq_init.ver %d\n", ctx.cmd.q_init.ver);
> dev_dbg(dev, "txq_init.intr_index %d\n", ctx.cmd.q_init.intr_index);
> @@ -807,6 +856,11 @@ static int ionic_lif_rxq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
> };
> int err;
>
> + if (qcq->flags & IONIC_QCQ_F_CMB_RINGS) {
> + ctx.cmd.q_init.flags |= cpu_to_le16(IONIC_QINIT_F_CMB);
> + ctx.cmd.q_init.ring_base = cpu_to_le64(qcq->cmb_q_base_pa);
> + }
> +
> dev_dbg(dev, "rxq_init.pid %d\n", ctx.cmd.q_init.pid);
> dev_dbg(dev, "rxq_init.index %d\n", ctx.cmd.q_init.index);
> dev_dbg(dev, "rxq_init.ring_base 0x%llx\n", ctx.cmd.q_init.ring_base);
> @@ -1966,8 +2020,13 @@ static int ionic_txrx_alloc(struct ionic_lif *lif)
> sg_desc_sz = sizeof(struct ionic_txq_sg_desc);
>
> flags = IONIC_QCQ_F_TX_STATS | IONIC_QCQ_F_SG;
> +
> + if (test_bit(IONIC_LIF_F_CMB_RINGS, lif->state))
> + flags |= IONIC_QCQ_F_CMB_RINGS;
> +
> if (test_bit(IONIC_LIF_F_SPLIT_INTR, lif->state))
> flags |= IONIC_QCQ_F_INTR;
> +
> for (i = 0; i < lif->nxqs; i++) {
> err = ionic_qcq_alloc(lif, IONIC_QTYPE_TXQ, i, "tx", flags,
> num_desc, desc_sz, comp_sz, sg_desc_sz,
> @@ -1988,6 +2047,9 @@ static int ionic_txrx_alloc(struct ionic_lif *lif)
>
> flags = IONIC_QCQ_F_RX_STATS | IONIC_QCQ_F_SG | IONIC_QCQ_F_INTR;
>
> + if (test_bit(IONIC_LIF_F_CMB_RINGS, lif->state))
> + flags |= IONIC_QCQ_F_CMB_RINGS;
> +
> num_desc = lif->nrxq_descs;
> desc_sz = sizeof(struct ionic_rxq_desc);
> comp_sz = sizeof(struct ionic_rxq_comp);
> @@ -2663,6 +2725,55 @@ static const struct net_device_ops ionic_netdev_ops = {
> .ndo_get_vf_stats = ionic_get_vf_stats,
> };
>
> +static int ionic_cmb_reconfig(struct ionic_lif *lif,
> + struct ionic_queue_params *qparam)
> +{
> + struct ionic_queue_params start_qparams;
> + int err = 0;
> +
> + /* When changing CMB queue parameters, we're using limited
> + * on-device memory and don't have extra memory to use for
> + * duplicate allocations, so we free it all first then
> + * re-allocate with the new parameters.
> + */
> +
> + /* Checkpoint for possible unwind */
> + ionic_init_queue_params(lif, &start_qparams);
> +
> + /* Stop and free the queues */
> + ionic_stop_queues_reconfig(lif);
> + ionic_txrx_free(lif);
> +
> + /* Set up new qparams */
> + ionic_set_queue_params(lif, qparam);
> +
> + if (netif_running(lif->netdev)) {
> + /* Alloc and start the new configuration */
> + err = ionic_txrx_alloc(lif);
> + if (err) {
> + dev_warn(lif->ionic->dev,
> + "CMB reconfig failed, restoring values: %d\n", err);
> +
> + /* Back out the changes */
> + ionic_set_queue_params(lif, &start_qparams);
> + err = ionic_txrx_alloc(lif);
> + if (err) {
> + dev_err(lif->ionic->dev,
> + "CMB restore failed: %d\n", err);
> + goto errout;
> + }
> + }
> +
> + ionic_start_queues_reconfig(lif);
> + } else {
> + /* This was detached in ionic_stop_queues_reconfig() */
> + netif_device_attach(lif->netdev);
> + }
> +
> +errout:
> + return err;
> +}
> +
> static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
> {
> /* only swapping the queues, not the napi, flags, or other stuff */
> @@ -2705,6 +2816,10 @@ int ionic_reconfigure_queues(struct ionic_lif *lif,
> unsigned int flags, i;
> int err = 0;
>
> + /* Are we changing q params while CMB is on */
> + if (test_bit(IONIC_LIF_F_CMB_RINGS, lif->state) && qparam->cmb_enabled)
> + return ionic_cmb_reconfig(lif, qparam);
> +
> /* allocate temporary qcq arrays to hold new queue structs */
> if (qparam->nxqs != lif->nxqs || qparam->ntxq_descs != lif->ntxq_descs) {
> tx_qcqs = devm_kcalloc(lif->ionic->dev, lif->ionic->ntxqs_per_lif,
> @@ -2741,6 +2856,16 @@ int ionic_reconfigure_queues(struct ionic_lif *lif,
> sg_desc_sz = sizeof(struct ionic_txq_sg_desc);
>
> for (i = 0; i < qparam->nxqs; i++) {
> + /* If missing, short placeholder qcq needed for swap */
> + if (!lif->txqcqs[i]) {
> + flags = IONIC_QCQ_F_TX_STATS | IONIC_QCQ_F_SG;
> + err = ionic_qcq_alloc(lif, IONIC_QTYPE_TXQ, i, "tx", flags,
> + 4, desc_sz, comp_sz, sg_desc_sz,
> + lif->kern_pid, &lif->txqcqs[i]);
> + if (err)
> + goto err_out;
> + }
> +
> flags = lif->txqcqs[i]->flags & ~IONIC_QCQ_F_INTR;
> err = ionic_qcq_alloc(lif, IONIC_QTYPE_TXQ, i, "tx", flags,
> num_desc, desc_sz, comp_sz, sg_desc_sz,
> @@ -2760,6 +2885,16 @@ int ionic_reconfigure_queues(struct ionic_lif *lif,
> comp_sz *= 2;
>
> for (i = 0; i < qparam->nxqs; i++) {
> + /* If missing, short placeholder qcq needed for swap */
> + if (!lif->rxqcqs[i]) {
> + flags = IONIC_QCQ_F_RX_STATS | IONIC_QCQ_F_SG;
> + err = ionic_qcq_alloc(lif, IONIC_QTYPE_RXQ, i, "rx", flags,
> + 4, desc_sz, comp_sz, sg_desc_sz,
> + lif->kern_pid, &lif->rxqcqs[i]);
> + if (err)
> + goto err_out;
> + }
> +
> flags = lif->rxqcqs[i]->flags & ~IONIC_QCQ_F_INTR;
> err = ionic_qcq_alloc(lif, IONIC_QTYPE_RXQ, i, "rx", flags,
> num_desc, desc_sz, comp_sz, sg_desc_sz,
> @@ -2809,10 +2944,15 @@ int ionic_reconfigure_queues(struct ionic_lif *lif,
> lif->tx_coalesce_hw = lif->rx_coalesce_hw;
> }
>
> - /* clear existing interrupt assignments */
> + /* Clear existing interrupt assignments. We check for NULL here
> + * because we're checking the whole array for potential qcqs, not
> + * just those qcqs that have just been set up.
> + */
> for (i = 0; i < lif->ionic->ntxqs_per_lif; i++) {
> - ionic_qcq_intr_free(lif, lif->txqcqs[i]);
> - ionic_qcq_intr_free(lif, lif->rxqcqs[i]);
> + if (lif->txqcqs[i])
> + ionic_qcq_intr_free(lif, lif->txqcqs[i]);
> + if (lif->rxqcqs[i])
> + ionic_qcq_intr_free(lif, lif->rxqcqs[i]);
> }
>
> /* re-assign the interrupts */
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> index a53984bf3544..5425a8983ae0 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> @@ -59,6 +59,7 @@ struct ionic_rx_stats {
> #define IONIC_QCQ_F_TX_STATS BIT(3)
> #define IONIC_QCQ_F_RX_STATS BIT(4)
> #define IONIC_QCQ_F_NOTIFYQ BIT(5)
> +#define IONIC_QCQ_F_CMB_RINGS BIT(6)
>
> struct ionic_qcq {
> void *q_base;
> @@ -70,6 +71,11 @@ struct ionic_qcq {
> void *sg_base;
> dma_addr_t sg_base_pa;
> u32 sg_size;
> + void __iomem *cmb_q_base;
> + phys_addr_t cmb_q_base_pa;
> + u32 cmb_q_size;
> + u32 cmb_pgid;
> + u32 cmb_order;
> struct dim dim;
> struct ionic_queue q;
> struct ionic_cq cq;
> @@ -140,6 +146,7 @@ enum ionic_lif_state_flags {
> IONIC_LIF_F_BROKEN,
> IONIC_LIF_F_TX_DIM_INTR,
> IONIC_LIF_F_RX_DIM_INTR,
> + IONIC_LIF_F_CMB_RINGS,
>
> /* leave this as last */
> IONIC_LIF_F_STATE_SIZE
> @@ -243,8 +250,9 @@ struct ionic_queue_params {
> unsigned int nxqs;
> unsigned int ntxq_descs;
> unsigned int nrxq_descs;
> - unsigned int intr_split;
> u64 rxq_features;
> + bool intr_split;
> + bool cmb_enabled;
Use of bool in structs is frowned upon since bool doesn't have a
strictly defined size. It is usually preferred to use u8.
> };
>
> static inline void ionic_init_queue_params(struct ionic_lif *lif,
> @@ -253,8 +261,28 @@ static inline void ionic_init_queue_params(struct ionic_lif *lif,
> qparam->nxqs = lif->nxqs;
> qparam->ntxq_descs = lif->ntxq_descs;
> qparam->nrxq_descs = lif->nrxq_descs;
> - qparam->intr_split = test_bit(IONIC_LIF_F_SPLIT_INTR, lif->state);
> qparam->rxq_features = lif->rxq_features;
> + qparam->intr_split = test_bit(IONIC_LIF_F_SPLIT_INTR, lif->state);
> + qparam->cmb_enabled = test_bit(IONIC_LIF_F_CMB_RINGS, lif->state);
> +}
> +
> +static inline void ionic_set_queue_params(struct ionic_lif *lif,
> + struct ionic_queue_params *qparam)
> +{
> + lif->nxqs = qparam->nxqs;
> + lif->ntxq_descs = qparam->ntxq_descs;
> + lif->nrxq_descs = qparam->nrxq_descs;
> + lif->rxq_features = qparam->rxq_features;
> +
> + if (qparam->intr_split)
> + set_bit(IONIC_LIF_F_SPLIT_INTR, lif->state);
> + else
> + clear_bit(IONIC_LIF_F_SPLIT_INTR, lif->state);
> +
> + if (qparam->cmb_enabled)
> + set_bit(IONIC_LIF_F_CMB_RINGS, lif->state);
> + else
> + clear_bit(IONIC_LIF_F_CMB_RINGS, lif->state);
> }
>
> static inline u32 ionic_coal_usec_to_hw(struct ionic *ionic, u32 usecs)
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index 0c3977416cd1..0cb464931d3d 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -419,6 +419,10 @@ void ionic_rx_fill(struct ionic_queue *q)
> IONIC_RXQ_DESC_OPCODE_SIMPLE;
> desc_info->nbufs = nfrags;
>
> + /* commit CMB descriptor contents in one shot */
> + if (q_to_qcq(q)->flags & IONIC_QCQ_F_CMB_RINGS)
> + memcpy_toio(desc_info->cmb_desc, desc, q->desc_size);
> +
> ionic_rxq_post(q, false, ionic_rx_clean, NULL);
> }
>
So this bit is repeated below and the comment is somewhat confusing as
I am assuming you are using some combination of write combining and the
memcpy to generate the "one shot" update you are referring to here.
My suggestion would be to look at creating an inline function somewhere
above here that takes the q, desc_info and desc as arguments and can do
all this as a single function call. Then in that function you can add a
comment explaining how this is a "one shot" write.
> @@ -860,7 +864,8 @@ static int ionic_tx_tcp_pseudo_csum(struct sk_buff *skb)
> return 0;
> }
>
> -static void ionic_tx_tso_post(struct ionic_queue *q, struct ionic_txq_desc *desc,
> +static void ionic_tx_tso_post(struct ionic_queue *q,
> + struct ionic_desc_info *desc_info,
> struct sk_buff *skb,
> dma_addr_t addr, u8 nsge, u16 len,
> unsigned int hdrlen, unsigned int mss,
> @@ -868,6 +873,7 @@ static void ionic_tx_tso_post(struct ionic_queue *q, struct ionic_txq_desc *desc
> u16 vlan_tci, bool has_vlan,
> bool start, bool done)
> {
> + struct ionic_txq_desc *desc = desc_info->desc;
> u8 flags = 0;
> u64 cmd;
>
> @@ -883,6 +889,10 @@ static void ionic_tx_tso_post(struct ionic_queue *q, struct ionic_txq_desc *desc
> desc->hdr_len = cpu_to_le16(hdrlen);
> desc->mss = cpu_to_le16(mss);
>
> + /* commit CMB descriptor contents in one shot */
> + if (q_to_qcq(q)->flags & IONIC_QCQ_F_CMB_RINGS)
> + memcpy_toio(desc_info->cmb_desc, desc, q->desc_size);
> +
> if (start) {
> skb_tx_timestamp(skb);
> if (!unlikely(q->features & IONIC_TXQ_F_HWSTAMP))
> @@ -1001,7 +1011,7 @@ static int ionic_tx_tso(struct ionic_queue *q, struct sk_buff *skb)
> seg_rem = min(tso_rem, mss);
> done = (tso_rem == 0);
> /* post descriptor */
> - ionic_tx_tso_post(q, desc, skb,
> + ionic_tx_tso_post(q, desc_info, skb,
> desc_addr, desc_nsge, desc_len,
> hdrlen, mss, outer_csum, vlan_tci, has_vlan,
> start, done);
> @@ -1050,6 +1060,10 @@ static void ionic_tx_calc_csum(struct ionic_queue *q, struct sk_buff *skb,
> desc->csum_start = cpu_to_le16(skb_checksum_start_offset(skb));
> desc->csum_offset = cpu_to_le16(skb->csum_offset);
>
> + /* commit descriptor contents in one shot */
> + if (q_to_qcq(q)->flags & IONIC_QCQ_F_CMB_RINGS)
> + memcpy_toio(desc_info->cmb_desc, desc, q->desc_size);
> +
> if (skb_csum_is_sctp(skb))
> stats->crc32_csum++;
> else
> @@ -1087,6 +1101,10 @@ static void ionic_tx_calc_no_csum(struct ionic_queue *q, struct sk_buff *skb,
> desc->csum_start = 0;
> desc->csum_offset = 0;
>
> + /* commit descriptor contents in one shot */
> + if (q_to_qcq(q)->flags & IONIC_QCQ_F_CMB_RINGS)
> + memcpy_toio(desc_info->cmb_desc, desc, q->desc_size);
> +
> stats->csum_none++;
> }
>
Powered by blists - more mailing lists