> -----Original Message-----
> From: Stewart Hildebrand <stewart.hildebr...@amd.com>
> Sent: Wednesday, February 8, 2023 4:55 AM
> To: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <wei.c...@arm.com>; Stefano Stabellini
> <sstabell...@kernel.org>; Julien Grall <jul...@xen.org>; Bertrand Marquis
> <bertrand.marq...@arm.com>; Volodymyr Babchuk
> <volodymyr_babc...@epam.com>
> Subject: Re: [PATCH v1 01/13] xen/arm: re-arrange the static shared memory
> region
>
> Hi Penny,
>
Hi, Stewart
Sorry for the late response, got sidetracked by a few internal projects
> On 11/14/22 21:52, Penny Zheng wrote:
> > ...
> > diff --git a/xen/arch/arm/include/asm/setup.h
> > b/xen/arch/arm/include/asm/setup.h
> > index fdbf68aadc..2d4ae0f00a 100644
> > --- a/xen/arch/arm/include/asm/setup.h
> > +++ b/xen/arch/arm/include/asm/setup.h
> > @@ -50,10 +50,6 @@ struct membank {
> > paddr_t start;
> > paddr_t size;
> > enum membank_type type;
> > -#ifdef CONFIG_STATIC_SHM
> > - char shm_id[MAX_SHM_ID_LENGTH];
> > - unsigned int nr_shm_borrowers;
> > -#endif
> > };
> >
> > struct meminfo {
> > @@ -61,6 +57,17 @@ struct meminfo {
> > struct membank bank[NR_MEM_BANKS]; };
> >
> > +struct shm_membank {
> > + char shm_id[MAX_SHM_ID_LENGTH];
> > + unsigned int nr_shm_borrowers;
> > + struct membank *membank;
> > +};
> > +
> > +struct shm_meminfo {
> > + unsigned int nr_banks;
> > + struct shm_membank bank[NR_MEM_BANKS]; };
> > +
> > /*
> > * The domU flag is set for kernels and ramdisks of "xen,domain" nodes.
> > * The purpose of the domU flag is to avoid getting confused in @@
> > -105,6 +112,7 @@ struct bootinfo {
> > struct meminfo acpi;
> > #endif
> > bool static_heap;
> > + struct shm_meminfo shm_mem;
> > };
> >
> > struct map_range_data
>
> The reduction in the sizeof struct membank is a welcome improvement, in
> my opinion, because it is multiplied by NR_MEM_BANKS, and IIRC we only
> have 32k of boot stack to play with.
>
> Before this patch:
> sizeof(struct kernel_info): 20648
> sizeof(struct meminfo): 10248
> sizeof(struct shm_meminfo): 10248
>
> When building with EXTRA_CFLAGS_XEN_CORE="Wstack-usage=4096 -Wno-
> error=stack-usage=":
Learnt! Thx!
> arch/arm/domain_build.c: In function ‘construct_domU’:
> arch/arm/domain_build.c:3747:19: warning: stack usage is 20720 bytes [-
> Wstack-usage=]
> 3747 | static int __init construct_domU(struct domain *d,
> | ^~~~~~~~~~~~~~
> arch/arm/domain_build.c: In function ‘construct_dom0’:
> arch/arm/domain_build.c:3979:19: warning: stack usage is 20688 bytes [-
> Wstack-usage=]
> 3979 | static int __init construct_dom0(struct domain *d)
> | ^~~~~~~~~~~~~~
>
>
>
> After this patch:
> sizeof(struct kernel_info): 14504
> sizeof(struct meminfo): 6152
> sizeof(struct shm_meminfo): 8200
>
> arch/arm/domain_build.c: In function ‘construct_domU’:
> arch/arm/domain_build.c:3754:19: warning: stack usage is 14576 bytes [-
> Wstack-usage=]
> 3754 | static int __init construct_domU(struct domain *d,
> | ^~~~~~~~~~~~~~
> arch/arm/domain_build.c: In function ‘construct_dom0’:
> arch/arm/domain_build.c:3986:19: warning: stack usage is 14544 bytes [-
> Wstack-usage=]
> 3986 | static int __init construct_dom0(struct domain *d)
> | ^~~~~~~~~~~~~~
>
> A later patch in this series will increase it again slightly. Note that I'm
> not
> expecting this series to address these particular warnings, I'm merely
> pointing out the (positive) impact of the change.
I agreed that NR_MEM_BANKS could be a large multiplier, and if we make
"struct shm_meminfo" like "struct meminfo", to have a array of NR_MEM_BANKS
items, it will make Xen binary exceed 20MB... We have discussed it here[1].
However, I'm afraid that dynamic allocation is also not a preferred option here,
where to free the data could be a problem.
So in next serie, which will come very soon, I'll introduce:
```
#define NR_SHM_BANKS 16
/*
* A static shared memory node could be banked with a statically
* configured host memory bank, or a set of arbitrary host memory
* banks allocated from heap.
*/
struct shm_meminfo {
unsigned int nr_banks;
struct membank bank[NR_SHM_BANKS];
paddr_t tot_size;
};
```
Taking your previous instructions, compiling with
"EXTRA_CFLAGS_XEN_CORE="-Wstack-usage=4096 -Wno-error=stack-usage=",
boot stack usage for "construct_domU" and " construct_dom0" will be like:
"
arch/arm/domain_build.c: In function ‘construct_domU’:
arch/arm/domain_build.c:4127:19: warning: stack usage is 16800 bytes
[-Wstack-usage=]
4127 | static int __init construct_domU(struct domain *d,
| ^~~~~~~~~~~~~~
arch/arm/domain_build.c: In function ‘construct_dom0’:
arch/arm/domain_build.c:4359:19: warning: stack usage is 16640 bytes
[-Wstack-usage=]
4359 | static int __init construct_dom0(struct domain *d)
| ^~~~~~~~~~~~~~
"
[1] https://lists.xenproject.org/archives/html/xen-devel/2023-01/msg00449.html
Cheers,
---
Penny Zheng
