> -----Original Message-----
> From: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Sent: Wednesday, June 12, 2024 10:22 AM
> To: Michael Kelley <mhklinux@xxxxxxxxxxx>; linux-hyperv@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; Paul Rosswurm <paulros@xxxxxxxxxxxxx>
> Cc: Dexuan Cui <decui@xxxxxxxxxxxxx>; stephen@xxxxxxxxxxxxxxxxxx; KY
> Srinivasan <kys@xxxxxxxxxxxxx>; olaf@xxxxxxxxx; vkuznets
> <vkuznets@xxxxxxxxxx>; davem@xxxxxxxxxxxxx; wei.liu@xxxxxxxxxx;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; leon@xxxxxxxxxx;
> Long Li <longli@xxxxxxxxxxxxx>; ssengar@xxxxxxxxxxxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx; daniel@xxxxxxxxxxxxx; john.fastabend@xxxxxxxxx;
> bpf@xxxxxxxxxxxxxxx; ast@xxxxxxxxxx; hawk@xxxxxxxxxx; tglx@xxxxxxxxxxxxx;
> shradhagupta@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH net-next] net: mana: Add support for variable page
> sizes of ARM64
>
>
>
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> > Sent: Tuesday, June 11, 2024 10:42 PM
> > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; linux-
> hyperv@xxxxxxxxxxxxxxx;
> > netdev@xxxxxxxxxxxxxxx; Paul Rosswurm <paulros@xxxxxxxxxxxxx>
> > Cc: Dexuan Cui <decui@xxxxxxxxxxxxx>; stephen@xxxxxxxxxxxxxxxxxx; KY
> > Srinivasan <kys@xxxxxxxxxxxxx>; olaf@xxxxxxxxx; vkuznets
> > <vkuznets@xxxxxxxxxx>; davem@xxxxxxxxxxxxx; wei.liu@xxxxxxxxxx;
> > edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> leon@xxxxxxxxxx;
> > Long Li <longli@xxxxxxxxxxxxx>; ssengar@xxxxxxxxxxxxxxxxxxx; linux-
> > rdma@xxxxxxxxxxxxxxx; daniel@xxxxxxxxxxxxx; john.fastabend@xxxxxxxxx;
> > bpf@xxxxxxxxxxxxxxx; ast@xxxxxxxxxx; hawk@xxxxxxxxxx;
> tglx@xxxxxxxxxxxxx;
> > shradhagupta@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > 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
