Hi Sami,
Thanks for quick response.
The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved
(in addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?
For the current implementation, initrd has been considered and placed on the top of memory in Cloud Hypervisor. And for kernel image, in the current design, is placed under initrd image.
As to kernel parameters, it is just a string in “chosen node” and will not occupy memory. So, I think, in general, it won’t happen of accidental overwriting. But I need to set to memory region for initrd image to read-only like kernel does to avoid the memory
being allocated.
Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?
Generally, kernel stack locates at the lowest part of memory. I think it’s safe to place Kernel and Initrd image on the top of memory, unless the memory size is too small.
WDYT?
Thanks
Jianyong
From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Wednesday, January 11, 2023 4:35 PM
To: Jianyong Wu <Jianyong.Wu@arm.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
Hi Jianyong,
Thank you for your kind wishes and happy new year to you.
I think linking with a NULL lib implementation of
QemuBootOrderLib would be good.
The other comment (probably for patch 2/3 in this series) was with regards to the memory reservation for the Initrd and Kernel arguments. I think these regions should be reserved (in
addition to the kernel region) to avoid any accidental overwriting. I believe if you start reducing the System Memory size at some point you may find that the Initrd and Kernel arguments are overwritten. Can you check this, please?
Can you also check if there is a possibility that these regions could overlap the CPU stack, etc. and if so, how do we make sure this never happens?
Regards,
Sami Mujawar
From: Jianyong Wu <Jianyong.Wu@arm.com>
Date: Wednesday, 11 January 2023 at 08:12
To: "devel@edk2.groups.io" <devel@edk2.groups.io>, Jianyong Wu <Jianyong.Wu@arm.com>, Sami Mujawar <Sami.Mujawar@arm.com>
Cc: "ardb+tianocore@kernel.org" <ardb+tianocore@kernel.org>, Justin He <Justin.He@arm.com>, nd <nd@arm.com>
Subject: RE: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
Hi Sami,
Happy new year and hope you are well.
Following up last discussion. I try to add null lib instead of QemuBootOrderLib and it works. If there are no other comments, I will post the change in the next version.
Regards,
Jianyong
From: devel@edk2.groups.io <devel@edk2.groups.io>
On Behalf Of Jianyong Wu via groups.io
Sent: Wednesday, November 23, 2022 1:44 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>;
devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
Hi Sami,
Inline reply
From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Tuesday, November 22, 2022 11:48 PM
To: Jianyong Wu <Jianyong.Wu@arm.com>;
devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; Justin He <Justin.He@arm.com>; nd <nd@arm.com>
Subject: Re: [PATCH 3/3] CloudHv:arm: add kernel load driver into dsc/fdf
Hi Jianyong,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 16/09/2022 03:46 am, Jianyong Wu wrote: