From: Michael Kelley <mhklinux@outlook.com>
To: Haiyang Zhang <haiyangz@microsoft.com>,
"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: "decui@microsoft.com" <decui@microsoft.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"kys@microsoft.com" <kys@microsoft.com>,
"paulros@microsoft.com" <paulros@microsoft.com>,
"olaf@aepfle.de" <olaf@aepfle.de>,
"vkuznets@redhat.com" <vkuznets@redhat.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"wei.liu@kernel.org" <wei.liu@kernel.org>,
"edumazet@google.com" <edumazet@google.com>,
"kuba@kernel.org" <kuba@kernel.org>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"leon@kernel.org" <leon@kernel.org>,
"longli@microsoft.com" <longli@microsoft.com>,
"ssengar@linux.microsoft.com" <ssengar@linux.microsoft.com>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"john.fastabend@gmail.com" <john.fastabend@gmail.com>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
"ast@kernel.org" <ast@kernel.org>,
"hawk@kernel.org" <hawk@kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"shradhagupta@linux.microsoft.com"
<shradhagupta@linux.microsoft.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2, net-next] net: mana: Add support for page sizes other than 4KB on ARM64
Date: Sun, 16 Jun 2024 04:04:11 +0000 [thread overview]
Message-ID: <SN6PR02MB41573C486E03FA07162F7CA7D4CC2@SN6PR02MB4157.namprd02.prod.outlook.com> (raw)
In-Reply-To: <1718390136-25954-1-git-send-email-haiyangz@microsoft.com>
From: LKML haiyangz <lkmlhyz@microsoft.com> On Behalf Of Haiyang Zhang Sent: Friday, June 14, 2024 11:36 AM
>
> As defined by the MANA Hardware spec, the queue size for DMA is 4KB
> minimal, and power of 2. And, the HWC queue size has to be exactly
> 4KB.
>
> To support page sizes other than 4KB on ARM64, define the minimal
> queue size as a macro separately from the PAGE_SIZE, which we always
> assumed it to be 4KB before supporting ARM64.
>
> Also, add MANA specific macros and update code related to size
> alignment, DMA region calculations, etc.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> v2: Updated alignments, naming as suggested by Michael and Paul.
>
> ---
> drivers/net/ethernet/microsoft/Kconfig | 2 +-
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 10 +++++-----
> drivers/net/ethernet/microsoft/mana/hw_channel.c | 14 +++++++-------
> drivers/net/ethernet/microsoft/mana/mana_en.c | 8 ++++----
> drivers/net/ethernet/microsoft/mana/shm_channel.c | 13 +++++++------
> include/net/mana/gdma.h | 10 +++++++++-
> include/net/mana/mana.h | 3 ++-
> 7 files changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/Kconfig
> b/drivers/net/ethernet/microsoft/Kconfig
> index 286f0d5697a1..901fbffbf718 100644
> --- a/drivers/net/ethernet/microsoft/Kconfig
> +++ b/drivers/net/ethernet/microsoft/Kconfig
> @@ -18,7 +18,7 @@ if NET_VENDOR_MICROSOFT
> config MICROSOFT_MANA
> tristate "Microsoft Azure Network Adapter (MANA) support"
> depends on PCI_MSI
> - depends on X86_64 || (ARM64 && !CPU_BIG_ENDIAN && ARM64_4K_PAGES)
> + depends on X86_64 || (ARM64 && !CPU_BIG_ENDIAN)
> depends on PCI_HYPERV
> select AUXILIARY_BUS
> select PAGE_POOL
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 1332db9a08eb..aa215e2e9606 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -182,7 +182,7 @@ int mana_gd_alloc_memory(struct gdma_context *gc, unsigned int length,
> dma_addr_t dma_handle;
> void *buf;
>
> - if (length < PAGE_SIZE || !is_power_of_2(length))
> + if (length < MANA_MIN_QSIZE || !is_power_of_2(length))
> return -EINVAL;
Since mana_gd_alloc_memory() is a somewhat generic function that wraps
dma_alloc_coherent(), checking the length against MANA_MIN_QSIZE is
unexpected. In looking at the call graph, I see that mana_gd_alloc_memory()
is used in creating queues, but all the callers already ensure that the minimum
size requirement is met. For robustness, having a check here seems OK, but
I would have expected checking against MANA_PAGE_SIZE, since that's the
DMA-related concept.
>
> gmi->dev = gc->dev;
> @@ -717,7 +717,7 @@ EXPORT_SYMBOL_NS(mana_gd_destroy_dma_region, NET_MANA);
> static int mana_gd_create_dma_region(struct gdma_dev *gd,
> struct gdma_mem_info *gmi)
> {
> - unsigned int num_page = gmi->length / PAGE_SIZE;
> + unsigned int num_page = gmi->length / MANA_PAGE_SIZE;
> struct gdma_create_dma_region_req *req = NULL;
> struct gdma_create_dma_region_resp resp = {};
> struct gdma_context *gc = gd->gdma_context;
> @@ -727,10 +727,10 @@ static int mana_gd_create_dma_region(struct gdma_dev *gd,
> int err;
> int i;
>
> - if (length < PAGE_SIZE || !is_power_of_2(length))
> + if (length < MANA_MIN_QSIZE || !is_power_of_2(length))
Same here. Checking against MANA_MIN_QSIZE is unexpected in a function devoted
to DMA functionality. Callers higher in the call graph already ensure that the
MANA_MIN_QSIZE requirement is met. Again, for robustness, checking against
MANA_PAGE_SIZE would be OK.
> return -EINVAL;
>
> - if (offset_in_page(gmi->virt_addr) != 0)
> + if (!MANA_PAGE_ALIGNED(gmi->virt_addr))
> return -EINVAL;
>
> hwc = gc->hwc.driver_data;
> @@ -751,7 +751,7 @@ static int mana_gd_create_dma_region(struct gdma_dev *gd,
> req->page_addr_list_len = num_page;
>
> for (i = 0; i < num_page; i++)
> - req->page_addr_list[i] = gmi->dma_handle + i * PAGE_SIZE;
> + req->page_addr_list[i] = gmi->dma_handle + i * MANA_PAGE_SIZE;
>
> err = mana_gd_send_request(gc, req_msg_size, req, sizeof(resp), &resp);
> if (err)
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index bbc4f9e16c98..cafded2f9382 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -362,12 +362,12 @@ static int mana_hwc_create_cq(struct hw_channel_context *hwc, u16 q_depth,
> int err;
>
> eq_size = roundup_pow_of_two(GDMA_EQE_SIZE * q_depth);
> - if (eq_size < MINIMUM_SUPPORTED_PAGE_SIZE)
> - eq_size = MINIMUM_SUPPORTED_PAGE_SIZE;
> + if (eq_size < MANA_MIN_QSIZE)
> + eq_size = MANA_MIN_QSIZE;
>
> cq_size = roundup_pow_of_two(GDMA_CQE_SIZE * q_depth);
> - if (cq_size < MINIMUM_SUPPORTED_PAGE_SIZE)
> - cq_size = MINIMUM_SUPPORTED_PAGE_SIZE;
> + if (cq_size < MANA_MIN_QSIZE)
> + cq_size = MANA_MIN_QSIZE;
>
> hwc_cq = kzalloc(sizeof(*hwc_cq), GFP_KERNEL);
> if (!hwc_cq)
> @@ -429,7 +429,7 @@ static int mana_hwc_alloc_dma_buf(struct hw_channel_context *hwc, u16 q_depth,
>
> dma_buf->num_reqs = q_depth;
>
> - buf_size = PAGE_ALIGN(q_depth * max_msg_size);
> + buf_size = MANA_PAGE_ALIGN(q_depth * max_msg_size);
>
> gmi = &dma_buf->mem_info;
> err = mana_gd_alloc_memory(gc, buf_size, gmi);
> @@ -497,8 +497,8 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
> else
> queue_size = roundup_pow_of_two(GDMA_MAX_SQE_SIZE * q_depth);
>
> - if (queue_size < MINIMUM_SUPPORTED_PAGE_SIZE)
> - queue_size = MINIMUM_SUPPORTED_PAGE_SIZE;
> + if (queue_size < MANA_MIN_QSIZE)
> + queue_size = MANA_MIN_QSIZE;
>
> hwc_wq = kzalloc(sizeof(*hwc_wq), GFP_KERNEL);
> if (!hwc_wq)
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b89ad4afd66e..1381de866b2e 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1904,10 +1904,10 @@ static int mana_create_txq(struct mana_port_context *apc,
> * to prevent overflow.
> */
> txq_size = MAX_SEND_BUFFERS_PER_QUEUE * 32;
> - BUILD_BUG_ON(!PAGE_ALIGNED(txq_size));
> + BUILD_BUG_ON(!MANA_PAGE_ALIGNED(txq_size));
>
> cq_size = MAX_SEND_BUFFERS_PER_QUEUE * COMP_ENTRY_SIZE;
> - cq_size = PAGE_ALIGN(cq_size);
> + cq_size = MANA_PAGE_ALIGN(cq_size);
>
> gc = gd->gdma_context;
>
> @@ -2204,8 +2204,8 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
> if (err)
> goto out;
>
> - rq_size = PAGE_ALIGN(rq_size);
> - cq_size = PAGE_ALIGN(cq_size);
> + rq_size = MANA_PAGE_ALIGN(rq_size);
> + cq_size = MANA_PAGE_ALIGN(cq_size);
>
> /* Create RQ */
> memset(&spec, 0, sizeof(spec));
> diff --git a/drivers/net/ethernet/microsoft/mana/shm_channel.c
> b/drivers/net/ethernet/microsoft/mana/shm_channel.c
> index 5553af9c8085..0f1679ebad96 100644
> --- a/drivers/net/ethernet/microsoft/mana/shm_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/shm_channel.c
> @@ -6,6 +6,7 @@
> #include <linux/io.h>
> #include <linux/mm.h>
>
> +#include <net/mana/gdma.h>
> #include <net/mana/shm_channel.h>
>
> #define PAGE_FRAME_L48_WIDTH_BYTES 6
> @@ -155,8 +156,8 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
> return err;
> }
>
> - if (!PAGE_ALIGNED(eq_addr) || !PAGE_ALIGNED(cq_addr) ||
> - !PAGE_ALIGNED(rq_addr) || !PAGE_ALIGNED(sq_addr))
> + if (!MANA_PAGE_ALIGNED(eq_addr) || !MANA_PAGE_ALIGNED(cq_addr) ||
> + !MANA_PAGE_ALIGNED(rq_addr) || !MANA_PAGE_ALIGNED(sq_addr))
> return -EINVAL;
>
> if ((eq_msix_index & VECTOR_MASK) != eq_msix_index)
> @@ -183,7 +184,7 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
>
> /* EQ addr: low 48 bits of frame address */
> shmem = (u64 *)ptr;
> - frame_addr = PHYS_PFN(eq_addr);
> + frame_addr = MANA_PFN(eq_addr);
> *shmem = frame_addr & PAGE_FRAME_L48_MASK;
> all_addr_h4bits |= (frame_addr >> PAGE_FRAME_L48_WIDTH_BITS) <<
> (frame_addr_seq++ * PAGE_FRAME_H4_WIDTH_BITS);
> @@ -191,7 +192,7 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
>
> /* CQ addr: low 48 bits of frame address */
> shmem = (u64 *)ptr;
> - frame_addr = PHYS_PFN(cq_addr);
> + frame_addr = MANA_PFN(cq_addr);
> *shmem = frame_addr & PAGE_FRAME_L48_MASK;
> all_addr_h4bits |= (frame_addr >> PAGE_FRAME_L48_WIDTH_BITS) <<
> (frame_addr_seq++ * PAGE_FRAME_H4_WIDTH_BITS);
> @@ -199,7 +200,7 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
>
> /* RQ addr: low 48 bits of frame address */
> shmem = (u64 *)ptr;
> - frame_addr = PHYS_PFN(rq_addr);
> + frame_addr = MANA_PFN(rq_addr);
> *shmem = frame_addr & PAGE_FRAME_L48_MASK;
> all_addr_h4bits |= (frame_addr >> PAGE_FRAME_L48_WIDTH_BITS) <<
> (frame_addr_seq++ * PAGE_FRAME_H4_WIDTH_BITS);
> @@ -207,7 +208,7 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
>
> /* SQ addr: low 48 bits of frame address */
> shmem = (u64 *)ptr;
> - frame_addr = PHYS_PFN(sq_addr);
> + frame_addr = MANA_PFN(sq_addr);
> *shmem = frame_addr & PAGE_FRAME_L48_MASK;
> all_addr_h4bits |= (frame_addr >> PAGE_FRAME_L48_WIDTH_BITS) <<
> (frame_addr_seq++ * PAGE_FRAME_H4_WIDTH_BITS);
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index c547756c4284..83963d9e804d 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -224,7 +224,15 @@ struct gdma_dev {
> struct auxiliary_device *adev;
> };
>
> -#define MINIMUM_SUPPORTED_PAGE_SIZE PAGE_SIZE
> +/* MANA_PAGE_SIZE is the DMA unit */
> +#define MANA_PAGE_SHIFT 12
> +#define MANA_PAGE_SIZE BIT(MANA_PAGE_SHIFT)
> +#define MANA_PAGE_ALIGN(x) ALIGN((x), MANA_PAGE_SIZE)
> +#define MANA_PAGE_ALIGNED(addr) IS_ALIGNED((unsigned long)(addr),
> MANA_PAGE_SIZE)
> +#define MANA_PFN(a) ((a) >> MANA_PAGE_SHIFT)
> +
> +/* Required by HW */
> +#define MANA_MIN_QSIZE MANA_PAGE_SIZE
>
> #define GDMA_CQE_SIZE 64
> #define GDMA_EQE_SIZE 16
> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index 59823901b74f..e39b8676fe54 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h
> @@ -42,7 +42,8 @@ enum TRI_STATE {
>
> #define MAX_SEND_BUFFERS_PER_QUEUE 256
>
> -#define EQ_SIZE (8 * PAGE_SIZE)
> +#define EQ_SIZE (8 * MANA_PAGE_SIZE)
> +
> #define LOG2_EQ_THROTTLE 3
>
> #define MAX_PORTS_IN_MANA_DEV 256
> --
> 2.34.1
>
The rest of the changes here look good to me.
Michael
next prev parent reply other threads:[~2024-06-16 4:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-14 18:35 Haiyang Zhang
2024-06-16 4:04 ` Michael Kelley [this message]
2024-06-17 12:56 ` Haiyang Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SN6PR02MB41573C486E03FA07162F7CA7D4CC2@SN6PR02MB4157.namprd02.prod.outlook.com \
--to=mhklinux@outlook.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=decui@microsoft.com \
--cc=edumazet@google.com \
--cc=haiyangz@microsoft.com \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=kys@microsoft.com \
--cc=leon@kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=longli@microsoft.com \
--cc=netdev@vger.kernel.org \
--cc=olaf@aepfle.de \
--cc=pabeni@redhat.com \
--cc=paulros@microsoft.com \
--cc=shradhagupta@linux.microsoft.com \
--cc=ssengar@linux.microsoft.com \
--cc=stephen@networkplumber.org \
--cc=tglx@linutronix.de \
--cc=vkuznets@redhat.com \
--cc=wei.liu@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®