RE: [PATCH v2 1/4] nitro_enclaves: merge contiguous physical memory regions

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> Sent: Tuesday, September 21, 2021 11:20 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpeng2@xxxxxxxxxx>
> Cc: andraprs@xxxxxxxxxx; lexnv@xxxxxxxxxx; alcioa@xxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Gonglei (Arei) <arei.gonglei@xxxxxxxxxx>;
> kamal@xxxxxxxxxxxxx; pbonzini@xxxxxxxxxx; sgarzare@xxxxxxxxxx;
> stefanha@xxxxxxxxxx; vkuznets@xxxxxxxxxx; ne-devel-upstream@xxxxxxxxxx
> Subject: Re: [PATCH v2 1/4] nitro_enclaves: merge contiguous physical memory
> regions
> 
> On Tue, Sep 21, 2021 at 11:10:36PM +0800, Longpeng(Mike) wrote:
> > There can be cases when there are more memory regions that need to be
> > set for an enclave than the maximum supported number of memory regions
> > per enclave. One example can be when the memory regions are backed by 2
> > MiB hugepages (the minimum supported hugepage size).
> >
> > Let's merge the adjacent regions if they are physical contiguous. This
> > way the final number of memory regions is less than before merging and
> > could potentially avoid reaching maximum.
> >
> > Signed-off-by: Longpeng(Mike) <longpeng2@xxxxxxxxxx>
> 
> I need a real (i.e. legal) name for a signed-off-by line please.
> 

Okay.

> > ---
> >  drivers/virt/nitro_enclaves/ne_misc_dev.c | 87
> ++++++++++++++++++++-----------
> >  1 file changed, 58 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> > index e21e1e8..a4776fc 100644
> > --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> > +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> > @@ -126,6 +126,26 @@ struct ne_cpu_pool {
> >  static struct ne_cpu_pool ne_cpu_pool;
> >
> >  /**
> > + * struct phys_mem_region - Physical memory region
> > + * @paddr:	The start physical address of the region.
> > + * @size:	The sizeof of the region.
> > + */
> > +struct phys_mem_region {
> > +	u64 paddr;
> > +	u64 size;
> > +};
> > +
> > +/**
> > + * struct phys_contig_mem_region - Physical contiguous memory regions
> > + * @num:	The number of regions that currently has.
> > + * @region:	The array of physical memory regions.
> > + */
> > +struct phys_contig_mem_region {
> > +	unsigned long num;
> > +	struct phys_mem_region region[0];
> > +};
> 
> Why create your own structures and not use the in-kernel ones for stuff
> like this?  What is wrong with the existing memory region or resource
> handling logic?
> 

This patch only optimizes the physical memory regions handling logic of 
the Nitro Enclaves driver, not the common resource management in kernel. 
The physical memory regions are need to send to the hardware later.

Thanks for your suggestion, maybe we can use 'struct range' instead of 
'struct phys_mem_region'.

> thanks,
> 
> greg k-h