> -----Original Message-----
> From: Julien Grall <[email protected]>
> Sent: Monday, January 9, 2023 6:58 PM
> To: Penny Zheng <[email protected]>; [email protected]
> Cc: Wei Chen <[email protected]>; Stefano Stabellini
> <[email protected]>; Bertrand Marquis <[email protected]>;
> Volodymyr Babchuk <[email protected]>
> Subject: Re: [PATCH v1 06/13] xen/arm: assign shared memory to owner
> when host address not provided
>
>
>
> On 09/01/2023 07:49, Penny Zheng wrote:
> > Hi Julien
>
> Hi Penny,
>
> > Happy new year~~~~
>
> Happy new year too!
>
> >> -----Original Message-----
> >> From: Julien Grall <[email protected]>
> >> Sent: Sunday, January 8, 2023 8:53 PM
> >> To: Penny Zheng <[email protected]>; xen-
> [email protected]
> >> Cc: Wei Chen <[email protected]>; Stefano Stabellini
> >> <[email protected]>; Bertrand Marquis
> >> <[email protected]>; Volodymyr Babchuk
> >> <[email protected]>
> >> Subject: Re: [PATCH v1 06/13] xen/arm: assign shared memory to owner
> >> when host address not provided
> >>
> >> Hi,
> >>
> >
> > A few concerns explained why I didn't choose "struct meminfo" over two
> > pointers "struct membank*" and "struct meminfo*".
> > 1) memory usage is the main reason.
> > If we use "struct meminfo" over the current "struct membank*" and
> > "struct meminfo*", "struct shm_meminfo" will become a array of 256
> > "struct shm_membank", with "struct shm_membank" being also an 256-
> item
> > array, that is 256 * 256, too big for a structure and If I remembered
> > clearly,
> it will lead to "more than PAGE_SIZE" compiling error.
>
> I am not aware of any place where we would restrict the size of kinfo in
> upstream. Can you give me a pointer?
>
If I remembered correctly, my first version of "struct shm_meminfo" is this
"big"(256 * 256) structure, and it leads to the whole xen binary is bigger than
2MB. ;\
> > FWIT, either reworking meminfo or using a different structure, are
> > both leading to sizing down the array, hmmm, I don't know which size
> > is suitable. That's why I prefer pointer and dynamic allocation.
>
> I would expect that in most cases, you will need only one bank when the host
> address is not provided. So I think this is a bit odd to me to impose a
> "large"
> allocation for them.
>
Only if user is not defining size as something like (2^a + 2^b + 2^c + ...). ;\
So maybe 8 or 16 is enough?
struct new_meminfo {
unsigned int nr_banks;
struct membank bank[8];
};
Correct me if I'm wrong:
The "struct shm_membank" you are suggesting is looking like this, right?
struct shm_membank {
char shm_id[MAX_SHM_ID_LENGTH];
unsigned int nr_shm_borrowers;
struct new_meminfo shm_banks;
unsigned long total_size;
};
Let me try~
> > 2) If we use "struct meminfo*" over the current "struct membank*" and
> > "struct meminfo*", we will need a new flag to differentiate two
> > scenarios(host address provided or not), As the special case "struct
> membank*" is already helping us differentiate.
> > And since when host address is provided, the related "struct membank"
> > also needs to be reserved in "bootinfo.reserved_mem". That's why I
> > used pointer " struct membank*" to avoid memory waste.
>
> I am confused... Today you are defining as:
>
> struct {
> struct membank *;
> struct {
> struct meminfo *;
> unsigned long total_size;
> }
> }
>
> So on arm64 host, you would use 24 bytes. If we were using an union instead.
> We would use 16 bytes. Adding a 32-bit fields, would bring to
> 20 bytes (assuming we can re-use a padding).
>
> Therefore, there is no memory waste here with a flag...
>
> Cheers,
>
> --
> Julien Grall
