RE: [PATCH 1/2] automation: arm64: Create a test job for testing static heap on qemu

[Jiamei Xie <Jiamei.Xie@xxxxxxx>]

Hi Michal,

> -----Original Message-----
> From: Michal Orzel <michal.orzel@xxxxxxx>
> Sent: Friday, March 3, 2023 3:47 PM
> To: Jiamei Xie <Jiamei.Xie@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Chen <Wei.Chen@xxxxxxx>;
> Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Doug Goldstein
> <cardoe@xxxxxxxxxx>
> Subject: Re: [PATCH 1/2] automation: arm64: Create a test job for testing
> static heap on qemu
> 
> Hi Jiamei,
> 
> On 03/03/2023 07:49, Jiamei Xie wrote:
> >
> >
> > Hi Stefano,
> >
> >> -----Original Message-----
> >> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >> Sent: Friday, March 3, 2023 9:51 AM
> >> To: Jiamei Xie <Jiamei.Xie@xxxxxxx>
> >> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Chen <Wei.Chen@xxxxxxx>;
> >> sstabellini@xxxxxxxxxx; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>;
> >> Doug Goldstein <cardoe@xxxxxxxxxx>
> >> Subject: Re: [PATCH 1/2] automation: arm64: Create a test job for testing
> >> static heap on qemu
> >>
> >> On Thu, 2 Mar 2023, jiamei.xie wrote:
> >>> From: Jiamei Xie <jiamei.xie@xxxxxxx>
> >>>
> >>> Create a new test job, called qemu-smoke-dom0less-arm64-gcc-
> staticheap.
> >>>
> >>> Add property "xen,static-heap" under /chosen node to enable static-heap.
> >>> If the domU can start successfully with static-heap enabled, then this
> >>> test pass.
> >>>
> >>> Signed-off-by: Jiamei Xie <jiamei.xie@xxxxxxx>
> >>
> >> Hi Jiamei, thanks for the patch!
> >>
> >>
> >>> ---
> >>>  automation/gitlab-ci/test.yaml                 | 16 ++++++++++++++++
> >>>  .../scripts/qemu-smoke-dom0less-arm64.sh       | 18
> >> ++++++++++++++++++
> >>>  2 files changed, 34 insertions(+)
> >>>
> >>> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-
> ci/test.yaml
> >>> index 1c5f400b68..5a9b88477a 100644
> >>> --- a/automation/gitlab-ci/test.yaml
> >>> +++ b/automation/gitlab-ci/test.yaml
> >>> @@ -133,6 +133,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-
> >> staticmem:
> >>>      - *arm64-test-needs
> >>>      - alpine-3.12-gcc-debug-arm64-staticmem
> >>>
> >>> +qemu-smoke-dom0less-arm64-gcc-staticheap:
> >>> + extends: .qemu-arm64
> >>> + script:
> >>> +   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap
> >> 2>&1 | tee ${LOGFILE}
> >>> + needs:
> >>> +   - *arm64-test-needs
> >>> +   - alpine-3.12-gcc-arm64
> >>> +
> >>> +qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
> >>> + extends: .qemu-arm64
> >>> + script:
> >>> +   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap
> >> 2>&1 | tee ${LOGFILE}
> >>> + needs:
> >>> +   - *arm64-test-needs
> >>> +   - alpine-3.12-gcc-debug-arm64
> >>> +
> >>>  qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
> >>>    extends: .qemu-arm64
> >>>    script:
> >>> diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >>> index 182a4b6c18..4e73857199 100755
> >>> --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >>> +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >>> @@ -27,6 +27,11 @@ fi
> >>>  "
> >>>  fi
> >>>
> >>> +if [[ "${test_variant}" == "static-heap" ]]; then
> >>> +    passed="${test_variant} test passed"
> >>> +    domU_check="echo \"${passed}\""
> >>> +fi
> >>> +
> >>>  if [[ "${test_variant}" == "boot-cpupools" ]]; then
> >>>      # Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
> >>>      passed="${test_variant} test passed"
> >>> @@ -128,6 +133,19 @@ if [[ "${test_variant}" == "static-mem" ]]; then
> >>>      echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base}
> ${domu_size}\"" >>
> >> binaries/config
> >>>  fi
> >>>
> >>> +if [[ "${test_variant}" == "static-heap" ]]; then
> >>> +    # ImageBuilder uses the config file to create the uboot script.
> Devicetree
> >>> +    # will be set via the generated uboot script.
> >>> +    # The valid memory range is 0x40000000 to 0x80000000 as defined
> >> before.
> >>> +    # ImageBuillder sets the kernel and ramdisk range based on the file
> size.
> >>> +    # It will use the memory range between 0x45600000 to 0x47AED1E8,
> so
> >> set
> >>> +    # memory range between 0x50000000 and 0x80000000 as static
> heap.
> >>
> >> I think this is OK. One suggestion to make things more reliable would be
> >> to change MEMORY_END to be 0x50000000 so that you can be sure that
> >> ImageBuilder won't go over the limit. You could do it just for this
> >> test, which would be safer, but to be honest you could limit MEMORY_END
> >> to 0x50000000 for all tests in qemu-smoke-dom0less-arm64.sh because it
> >> shouldn't really cause any problems.
> >>
> > [Jiamei Xie]
> > Thanks for your comments. I am a little confused about " to change
> MEMORY_END to be 0x50000000".
> >  I set 0STATIC_HEAP="0x50000000 0x30000000" where is the start address.
> Why change MEMORY_END
> >  to be 0x50000000?
> Let me answer to that question so that you do not need to wait another day
> for Stefano.
> ImageBuilder uses MEMORY_START and MEMORY_END from the cfg file as a
> range in which it can instruct
> u-boot where to place the images. It is safer to set MEMORY_END to
> 0x50000000 rather than to 0xC0000000
> because you will be sure that no image will be placed in a region that you
> defined as static heap.

Got it, thanks!  Another question,  set MEMORY_END=0x50000000 for all test 
cases or just this test cases ? Currently I run the test case manually without 
xen gitlab ci enviroment . If set this for all test cases, all test cases 
should be tested.

Best wishes
Jiamei Xie
> 
> ~Michal