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

[Jiamei Xie <Jiamei.Xie@xxxxxxx>]

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?


Best wishes
Jiamei Xie


> 
> > +    echo  '
> > +STATIC_HEAP="0x50000000 0x30000000"
> > +# The size of static heap should be greater than the guest memory
> > +DOMU_MEM[0]="128"' >> binaries/config
> > +fi
> > +
> >  if [[ "${test_variant}" == "boot-cpupools" ]]; then
> >      echo '
> >  CPUPOOL[0]="cpu@1 null"
> > --
> > 2.25.1
> >