netdev - RE: [PATCH net-next] net: mana: Add support for variable page sizes of ARM64

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <DM6PR21MB14813BDF16ABED9C9AADE3F2CAC22@DM6PR21MB1481.namprd21.prod.outlook.com>
Date: Fri, 14 Jun 2024 18:46:12 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>, Michael Kelley
	<mhklinux@...look.com>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, Paul Rosswurm <paulros@...rosoft.com>
CC: Dexuan Cui <decui@...rosoft.com>, "stephen@...workplumber.org"
	<stephen@...workplumber.org>, KY Srinivasan <kys@...rosoft.com>,
	"olaf@...fle.de" <olaf@...fle.de>, vkuznets <vkuznets@...hat.com>,
	"davem@...emloft.net" <davem@...emloft.net>, "wei.liu@...nel.org"
	<wei.liu@...nel.org>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"leon@...nel.org" <leon@...nel.org>, Long Li <longli@...rosoft.com>,
	"ssengar@...ux.microsoft.com" <ssengar@...ux.microsoft.com>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"daniel@...earbox.net" <daniel@...earbox.net>, "john.fastabend@...il.com"
	<john.fastabend@...il.com>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
	"ast@...nel.org" <ast@...nel.org>, "hawk@...nel.org" <hawk@...nel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "shradhagupta@...ux.microsoft.com"
	<shradhagupta@...ux.microsoft.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next] net: mana: Add support for variable page sizes
 of ARM64



> -----Original Message-----
> From: Haiyang Zhang <haiyangz@...rosoft.com>
> Sent: Wednesday, June 12, 2024 10:22 AM
> To: Michael Kelley <mhklinux@...look.com>; linux-hyperv@...r.kernel.org;
> netdev@...r.kernel.org; Paul Rosswurm <paulros@...rosoft.com>
> Cc: Dexuan Cui <decui@...rosoft.com>; stephen@...workplumber.org; KY
> Srinivasan <kys@...rosoft.com>; olaf@...fle.de; vkuznets
> <vkuznets@...hat.com>; davem@...emloft.net; wei.liu@...nel.org;
> edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com; leon@...nel.org;
> Long Li <longli@...rosoft.com>; ssengar@...ux.microsoft.com; linux-
> rdma@...r.kernel.org; daniel@...earbox.net; john.fastabend@...il.com;
> bpf@...r.kernel.org; ast@...nel.org; hawk@...nel.org; tglx@...utronix.de;
> shradhagupta@...ux.microsoft.com; linux-kernel@...r.kernel.org
> Subject: RE: [PATCH net-next] net: mana: Add support for variable page
> sizes of ARM64
> 
> 
> 
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@...look.com>
> > Sent: Tuesday, June 11, 2024 10:42 PM
> > To: Haiyang Zhang <haiyangz@...rosoft.com>; linux-
> hyperv@...r.kernel.org;
> > netdev@...r.kernel.org; Paul Rosswurm <paulros@...rosoft.com>
> > Cc: Dexuan Cui <decui@...rosoft.com>; stephen@...workplumber.org; KY
> > Srinivasan <kys@...rosoft.com>; olaf@...fle.de; vkuznets
> > <vkuznets@...hat.com>; davem@...emloft.net; wei.liu@...nel.org;
> > edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
> leon@...nel.org;
> > Long Li <longli@...rosoft.com>; ssengar@...ux.microsoft.com; linux-
> > rdma@...r.kernel.org; daniel@...earbox.net; john.fastabend@...il.com;
> > bpf@...r.kernel.org; ast@...nel.org; hawk@...nel.org;
> tglx@...utronix.de;
> > shradhagupta@...ux.microsoft.com; linux-kernel@...r.kernel.org
> > Subject: RE: [PATCH net-next] net: mana: Add support for variable page
> > sizes of ARM64
> 
> > > > > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > > > b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > > > index 1332db9a08eb..c9df942d0d02 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;
> > > > >
> > > > >  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_MIN_QSIZE;
> > > >
> > > > This calculation seems a bit weird when using MANA_MIN_QSIZE. The
> > > > number of pages, and the construction of the page_addr_list array
> > > > a few lines later, seem unrelated to the concept of a minimum queue
> > > > size. Is the right concept really a "mapping chunk", and num_page
> > > > would conceptually be "num_chunks", or something like that?  Then
> > > > a queue must be at least one chunk in size, but that's derived from
> > the
> > > > chunk size, and is not the core concept.
> > >
> > > I think calling it "num_chunks" is fine.
> > > May I use "num_chunks" in next version?
> > >
> >
> > I think first is the decision on what to use for MANA_MIN_QSIZE. I'm
> > admittedly not familiar with mana and gdma, but the function
> > mana_gd_create_dma_region() seems fairly typical in defining a
> > logical region that spans multiple 4K chunks that may or may not
> > be physically contiguous.  So you set up an array of physical
> > addresses that identify the physical memory location of each chunk.
> > It seems very similar to a Hyper-V GPADL. I typically *do* see such
> > chunks called "pages", but a "mapping chunk" or "mapping unit"
> > is probably OK too.  So mana_gd_create_dma_region() would use
> > MANA_CHUNK_SIZE instead of MANA_MIN_QSIZE.  Then you could
> > also define MANA_MIN_QSIZE to be MANA_CHUNK_SIZE, for use
> > specifically when checking the size of a queue.
> >
> > Then if you are using MANA_CHUNK_SIZE, the local variable
> > would be "num_chunks".  The use of "page_count", "page_addr_list",
> > and "offset_in_page" field names in struct
> > gdma_create_dma_region_req should then be changed as well.
> 
> I'm fine with these names. I will check with Paul too.
> 
> I'd define just one macro, with a code comment like this. It
> will be used for chunk calculation and q len checking:
> /* Chunk size of MANA DMA, which is also the min queue size */
> #define MANA_CHUNK_SIZE 4096
> 
> 

After further discussion with Paul, and reading documents, we 
decided to use MANA_PAGE_SIZE for DMA unit calculations etc.
And use another macro MANA_MIN_QSIZE for queue length checking. 
This is also what Michael initially suggested. 

Thanks,
- Haiyang