diff mbox series

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

Message ID 20230302044421.136068-2-jiamei.xie@arm.com (mailing list archive)
State Superseded
Headers show
Series automation: introduce static heap and shared memory tests | expand

Commit Message

Jiamei Xie March 2, 2023, 4:44 a.m. UTC
From: Jiamei Xie <jiamei.xie@arm.com>

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@arm.com>
---
 automation/gitlab-ci/test.yaml                 | 16 ++++++++++++++++
 .../scripts/qemu-smoke-dom0less-arm64.sh       | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Stefano Stabellini March 3, 2023, 1:50 a.m. UTC | #1
On Thu, 2 Mar 2023, jiamei.xie wrote:
> From: Jiamei Xie <jiamei.xie@arm.com>
> 
> 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@arm.com>

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.


> +    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
>
Jiamei Xie March 3, 2023, 6:49 a.m. UTC | #2
Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: Friday, March 3, 2023 9:51 AM
> To: Jiamei Xie <Jiamei.Xie@arm.com>
> Cc: xen-devel@lists.xenproject.org; Wei Chen <Wei.Chen@arm.com>;
> sstabellini@kernel.org; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> Doug Goldstein <cardoe@cardoe.com>
> 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@arm.com>
> >
> > 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@arm.com>
> 
> 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
> >
Michal Orzel March 3, 2023, 7:47 a.m. UTC | #3
Hi Jiamei,

On 03/03/2023 07:49, Jiamei Xie wrote:
> 
> 
> Hi Stefano,
> 
>> -----Original Message-----
>> From: Stefano Stabellini <sstabellini@kernel.org>
>> Sent: Friday, March 3, 2023 9:51 AM
>> To: Jiamei Xie <Jiamei.Xie@arm.com>
>> Cc: xen-devel@lists.xenproject.org; Wei Chen <Wei.Chen@arm.com>;
>> sstabellini@kernel.org; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>> Doug Goldstein <cardoe@cardoe.com>
>> 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@arm.com>
>>>
>>> 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@arm.com>
>>
>> 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.

~Michal
Jiamei Xie March 3, 2023, 9:13 a.m. UTC | #4
Hi Michal,

> -----Original Message-----
> From: Michal Orzel <michal.orzel@amd.com>
> Sent: Friday, March 3, 2023 3:47 PM
> To: Jiamei Xie <Jiamei.Xie@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org; Wei Chen <Wei.Chen@arm.com>;
> Bertrand Marquis <Bertrand.Marquis@arm.com>; Doug Goldstein
> <cardoe@cardoe.com>
> 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@kernel.org>
> >> Sent: Friday, March 3, 2023 9:51 AM
> >> To: Jiamei Xie <Jiamei.Xie@arm.com>
> >> Cc: xen-devel@lists.xenproject.org; Wei Chen <Wei.Chen@arm.com>;
> >> sstabellini@kernel.org; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> >> Doug Goldstein <cardoe@cardoe.com>
> >> 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@arm.com>
> >>>
> >>> 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@arm.com>
> >>
> >> 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
Michal Orzel March 3, 2023, 9:52 a.m. UTC | #5
Hi Jiamei,

On 03/03/2023 10:13, Jiamei Xie wrote:
> 
> 
> Hi Michal,
> 
>> -----Original Message-----
>> From: Michal Orzel <michal.orzel@amd.com>
>> Sent: Friday, March 3, 2023 3:47 PM
>> To: Jiamei Xie <Jiamei.Xie@arm.com>; Stefano Stabellini
>> <sstabellini@kernel.org>
>> Cc: xen-devel@lists.xenproject.org; Wei Chen <Wei.Chen@arm.com>;
>> Bertrand Marquis <Bertrand.Marquis@arm.com>; Doug Goldstein
>> <cardoe@cardoe.com>
>> 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@kernel.org>
>>>> Sent: Friday, March 3, 2023 9:51 AM
>>>> To: Jiamei Xie <Jiamei.Xie@arm.com>
>>>> Cc: xen-devel@lists.xenproject.org; Wei Chen <Wei.Chen@arm.com>;
>>>> sstabellini@kernel.org; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>>>> Doug Goldstein <cardoe@cardoe.com>
>>>> 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@arm.com>
>>>>>
>>>>> 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@arm.com>
>>>>
>>>> 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.
You can set it globally as it will be helpful also for static-mem and other test cases should not be impacted.
Before pushing any commit related to CI, the maintainer shall check that it does not cause any issues
by running the pipeline on his account before committing a patch.

~Michal
diff mbox series

Patch

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.
+    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"