| Message ID | 20220902070905.1262-4-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | GitLab CI cleanup and boot time cpupools test | expand |
> On 2 Sep 2022, at 08:09, Michal Orzel <michal.orzel@amd.com> wrote: > > Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute > script qemu-smoke-arm64.sh to test boot time cpupools feature. > Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test > case in qemu-smoke-arm64.sh that if selected will: > - create a device tree cpupool node with cpu@1 > - assign created cpupool to domU0 > - add a check in dom0 xen.start to see if domU is assigned a Pool-1 > > Take the opportunity to refactor the qemu-smoke-arm64.sh script as > follows: > - use domU_check to store the test's commands to be run from domU > - use dom0_check to store the test's commands to be run from dom0 > - use fdtput instead of sed to perform dtb modifications > - use more meaningful messages for "passed" variable. This way we can > grep for messages reported either by domU or dom0 and get rid of > assumption that tests can only be run from domU > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
On Fri, 2 Sep 2022, Michal Orzel wrote: > Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute > script qemu-smoke-arm64.sh to test boot time cpupools feature. > Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test > case in qemu-smoke-arm64.sh that if selected will: > - create a device tree cpupool node with cpu@1 > - assign created cpupool to domU0 > - add a check in dom0 xen.start to see if domU is assigned a Pool-1 > > Take the opportunity to refactor the qemu-smoke-arm64.sh script as > follows: > - use domU_check to store the test's commands to be run from domU > - use dom0_check to store the test's commands to be run from dom0 > - use fdtput instead of sed to perform dtb modifications > - use more meaningful messages for "passed" variable. This way we can > grep for messages reported either by domU or dom0 and get rid of > assumption that tests can only be run from domU > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > --- > automation/gitlab-ci/test.yaml | 19 +++++++++++++++ > automation/scripts/build | 3 ++- > automation/scripts/qemu-smoke-arm64.sh | 33 +++++++++++++++++++------- > 3 files changed, 45 insertions(+), 10 deletions(-) > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index 07209820b474..d899b3bdbf7a 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -100,6 +100,25 @@ qemu-smoke-arm64-gcc-staticmem: > tags: > - arm64 > > +qemu-smoke-arm64-gcc-boot-cpupools: > + extends: .test-jobs-common > + variables: > + CONTAINER: debian:unstable-arm64v8 > + script: > + - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log > + needs: > + - alpine-3.12-gcc-arm64 > + - alpine-3.12-arm64-rootfs-export > + - kernel-5.19-arm64-export > + - qemu-system-aarch64-6.0.0-arm64-export > + artifacts: > + paths: > + - smoke.serial > + - '*.log' > + when: always > + tags: > + - arm64 > + > qemu-smoke-arm32-gcc: > extends: .test-jobs-common > variables: > diff --git a/automation/scripts/build b/automation/scripts/build > index 2b9f2d2b541a..2f15ab3198e6 100755 > --- a/automation/scripts/build > +++ b/automation/scripts/build > @@ -19,7 +19,8 @@ else > echo " > CONFIG_EXPERT=y > CONFIG_UNSUPPORTED=y > -CONFIG_STATIC_MEMORY=y" > xen/.config > +CONFIG_STATIC_MEMORY=y > +CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config > make -j$(nproc) -C xen olddefconfig > else > make -j$(nproc) -C xen defconfig > diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh > index 7ac96027760d..c2184850293c 100755 > --- a/automation/scripts/qemu-smoke-arm64.sh > +++ b/automation/scripts/qemu-smoke-arm64.sh > @@ -4,20 +4,22 @@ set -ex > > test_variant=$1 > > -passed="passed" > -check=" > +if [ -z "${test_variant}" ]; then > + passed="ping test passed" > + domU_check=" > until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do > sleep 30 > done > echo \"${passed}\" > " > +fi > > if [[ "${test_variant}" == "static-mem" ]]; then > # Memory range that is statically allocated to DOM1 > domu_base="50000000" > domu_size="10000000" > passed="${test_variant} test passed" > - check=" > + domU_check=" > current=\$(hexdump -e '16/1 \"%02x\"' /proc/device-tree/memory@${domu_base}/reg 2>/dev/null) > expected=$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size}) > if [[ \"\${expected}\" == \"\${current}\" ]]; then > @@ -42,11 +44,23 @@ curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom > -cpu cortex-a57 -machine type=virt \ > -m 1024 -smp 2 -display none \ > -machine dumpdtb=binaries/virt-gicv2.dtb > -# XXX disable pl061 to avoid Linux crash > -dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts > -sed 's/compatible = "arm,pl061.*/status = "disabled";/g' binaries/virt-gicv2.dts > binaries/virt-gicv2-edited.dts > -dtc -I dts -O dtb binaries/virt-gicv2-edited.dts > binaries/virt-gicv2.dtb > > +# XXX disable pl061 to avoid Linux crash > +fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled Currently this test fails with: + fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled + [[ boot-cpupools == \b\o\o\t\-\c\p\u\p\o\o\l\s ]] ++ fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle Error at 'phandle': FDT_ERR_NOTFOUND Given my other comment below, I would leave this code as is. > +if [[ "${test_variant}" == "boot-cpupools" ]]; then > + # Create cpupool node and assign it to domU0 > + cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" > + cpupool_phandle="0xffffff" > + fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool > + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle > + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle > + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool $cpupool_phandle > + > + # Check if domU0 (id=1) is assigned to Pool-1 > + passed="${test_variant} test passed" > + dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi" > +fi I would prefer to keep the device tree editing here to a minimum and instead add boot-cpupool support in ImageBuilder and add CPUPOOL* config options to the existing config file for ImageBuilder created in this file below. This way, we keep this test cleaner and we help more the user by proving a way to set boot-cpupools more easily in general, also useful outside gitlab-ci.
Hi Stefano, On 03/09/2022 01:49, Stefano Stabellini wrote: > > > Currently this test fails with: > > + fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled > + [[ boot-cpupools == \b\o\o\t\-\c\p\u\p\o\o\l\s ]] > ++ fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle > Error at 'phandle': FDT_ERR_NOTFOUND My bad. The qemu version used by CI does not generate phandles for cpus. So the fix is very straightforward and requires putting custom phandle for cpu@1. diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh index c2184850293c..158d5665d71d 100755 --- a/automation/scripts/qemu-smoke-arm64.sh +++ b/automation/scripts/qemu-smoke-arm64.sh @@ -50,8 +50,9 @@ fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled if [[ "${test_variant}" == "boot-cpupools" ]]; then # Create cpupool node and assign it to domU0 - cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" + cpu_phandle="0xfffffe" cpupool_phandle="0xffffff" + fdtput binaries/virt-gicv2.dtb -p -t x /cpus/cpu@1 phandle $cpu_phandle fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle > > Given my other comment below, I would leave this code as is. > > >> +if [[ "${test_variant}" == "boot-cpupools" ]]; then >> + # Create cpupool node and assign it to domU0 >> + cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" >> + cpupool_phandle="0xffffff" >> + fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool >> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle >> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle >> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool $cpupool_phandle >> + >> + # Check if domU0 (id=1) is assigned to Pool-1 >> + passed="${test_variant} test passed" >> + dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi" >> +fi > > I would prefer to keep the device tree editing here to a minimum and > instead add boot-cpupool support in ImageBuilder and add CPUPOOL* config > options to the existing config file for ImageBuilder created in this > file below. This way, we keep this test cleaner and we help more the > user by proving a way to set boot-cpupools more easily in general, also > useful outside gitlab-ci. I agree that ImageBuilder is a great tool. However, I would opt for keeping what I did because of the following: - current release schedule (we could benefit from having a test for 4.17 feature instead of waiting for the corresponding change to be done in ImageBuilder first and tested), - test is already prepared and requires just a trivial fix, - we should not enforce users willing to add tests to gitlab-ci to always prepare the ImageBuilder changes first. ImageBuilder is not meant to support all the features strictly because some of them require too much end-user knowledge and digging into device tree (it should stay as simple as possible), - all in all we need to have a way to modify the dtb and fdtput is certainly better than sed as it does not require additional steps for decompilation/compilation and its commands look more clean than using sed transformation. Let me know what you think. On a side note, I can add boot-time cpupools support in ImageBuilder to my TODO list so that we can check if this is something ImageBuilder should support. If yes, we can modify this test after the release. I can already think of the following IB config options that would need to be introduced to properly support boot-time cpupools: CPUPOOL[number] = "<list_of_cpus> <scheduler>" - to create cpupools NUM_CPUPOOLS = "<number>" - to keep the number of created cpupools DOMU_CPUPOOL[number] = "CPUPOOL[number]" - to assing domU to one of the created cpupools So we already have 3 new options and the number of required sanity checks I can think of is significant. Even then, we could easily trigger a failure e.g. if user assigns cpus of different type and does not pass hmp-unsafe=1. ~Michal
On Mon, 5 Sep 2022, Michal Orzel wrote: > Hi Stefano, > > On 03/09/2022 01:49, Stefano Stabellini wrote: > > > > > > Currently this test fails with: > > > > + fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled > > + [[ boot-cpupools == \b\o\o\t\-\c\p\u\p\o\o\l\s ]] > > ++ fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle > > Error at 'phandle': FDT_ERR_NOTFOUND > My bad. The qemu version used by CI does not generate phandles for cpus. > So the fix is very straightforward and requires putting custom phandle for cpu@1. > > diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh > index c2184850293c..158d5665d71d 100755 > --- a/automation/scripts/qemu-smoke-arm64.sh > +++ b/automation/scripts/qemu-smoke-arm64.sh > @@ -50,8 +50,9 @@ fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled > > if [[ "${test_variant}" == "boot-cpupools" ]]; then > # Create cpupool node and assign it to domU0 > - cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" > + cpu_phandle="0xfffffe" > cpupool_phandle="0xffffff" > + fdtput binaries/virt-gicv2.dtb -p -t x /cpus/cpu@1 phandle $cpu_phandle > fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool > fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle > fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle > > Given my other comment below, I would leave this code as is. > > > > > >> +if [[ "${test_variant}" == "boot-cpupools" ]]; then > >> + # Create cpupool node and assign it to domU0 > >> + cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" > >> + cpupool_phandle="0xffffff" > >> + fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool > >> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle > >> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle > >> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool $cpupool_phandle > >> + > >> + # Check if domU0 (id=1) is assigned to Pool-1 > >> + passed="${test_variant} test passed" > >> + dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi" > >> +fi > > > > I would prefer to keep the device tree editing here to a minimum and > > instead add boot-cpupool support in ImageBuilder and add CPUPOOL* config > > options to the existing config file for ImageBuilder created in this > > file below. This way, we keep this test cleaner and we help more the > > user by proving a way to set boot-cpupools more easily in general, also > > useful outside gitlab-ci. > > I agree that ImageBuilder is a great tool. However, I would opt for keeping what I did because of the following: > - current release schedule (we could benefit from having a test for 4.17 feature instead of waiting for the corresponding > change to be done in ImageBuilder first and tested), > - test is already prepared and requires just a trivial fix, > - we should not enforce users willing to add tests to gitlab-ci to always prepare the ImageBuilder changes first. > ImageBuilder is not meant to support all the features strictly because some of them require too much > end-user knowledge and digging into device tree (it should stay as simple as possible), > - all in all we need to have a way to modify the dtb and fdtput is certainly better than sed as it does not > require additional steps for decompilation/compilation and its commands look more clean than using sed transformation. > > Let me know what you think. > > On a side note, I can add boot-time cpupools support in ImageBuilder to my TODO list so that we can check if this is something > ImageBuilder should support. If yes, we can modify this test after the release. Yeah, ImageBuilder doesn't necessarely need to support every feature. However, a tool (if not ImageBuilder, Lopper, or a new ImageBuilder script) should support CPUPOOLs to enable the user. You are right that ImageBuilder is not necessarely tied with gitlab-ci. This is especially true once we start doing more interface-level testing, such as hypercalls fuzzing with XTF. We are not going to be able to use ImageBuilder to trigger every possible device tree boot time combination, especially the ones that are invalid. We want to be able to test Xen with invalid device tree input as well. In addition to interface-level testing, we need user-level testing to test features the way we expect a user to use them. This is what ImageBuilder is for and that is why it has been used today in gitlab-ci. On ARM today we only have user-level testing in gitlab-ci, but I'd love to have more interface-level testing, which will surely require more device tree manipulations outside of ImageBuilder. - user-level tests -> ImageBuilder, common valid configurations - interface-level tests -> not ImageBuilder, various valid and invalid configurations, maybe automatically generated? Device tree manipulations expected in gitlab-ci. In my view, this test belongs to the "user-level test" category, this is why I would prefer if it was done using the same tool that we expect the user to use. Ideally, it would be ImageBuilder because that is the tool that we have used so far (but it could be a new script under ImageBuilder or Lopper). But I understand deadlines, release schedule, etc., so if you think it cannot be done properly using ImageBuilder in 2-3 days, then I would take this patch as is, and we can revisit it in the future as you suggested. I am OK with that too. > I can already think of the following IB config options that would need to be introduced to properly support boot-time cpupools: > CPUPOOL[number] = "<list_of_cpus> <scheduler>" - to create cpupools > NUM_CPUPOOLS = "<number>" - to keep the number of created cpupools > DOMU_CPUPOOL[number] = "CPUPOOL[number]" - to assing domU to one of the created cpupools > So we already have 3 new options and the number of required sanity checks I can think of is significant. > Even then, we could easily trigger a failure e.g. if user assigns cpus of different type and does not pass hmp-unsafe=1. As we both wrote above, ImageBuilder doesn't need to support every feature and every possible way a feature can be used. Only the most common ones. I don't think we want to support the case where a user assigns cpus of different type. I think you are right that these are the interesting options at the ImageBuilder level: NUM_CPUPOOLS="<number>" CPUPOOL[number] = "<list_of_cpus> <scheduler>" DOMU_CPUPOOL[number] = "CPUPOOL[number]" The last one could also be: DOMU_CPUPOOL[number] = "CPUPOOL<number>" # e.g. DOMU_CPUPOOL[1] = "CPUPOOL2"
Hi Stefano, On 06/09/2022 01:21, Stefano Stabellini wrote: > > On Mon, 5 Sep 2022, Michal Orzel wrote: >> Hi Stefano, >> >> On 03/09/2022 01:49, Stefano Stabellini wrote: >>> >>> >>> Currently this test fails with: >>> >>> + fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled >>> + [[ boot-cpupools == \b\o\o\t\-\c\p\u\p\o\o\l\s ]] >>> ++ fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle >>> Error at 'phandle': FDT_ERR_NOTFOUND >> My bad. The qemu version used by CI does not generate phandles for cpus. >> So the fix is very straightforward and requires putting custom phandle for cpu@1. >> >> diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh >> index c2184850293c..158d5665d71d 100755 >> --- a/automation/scripts/qemu-smoke-arm64.sh >> +++ b/automation/scripts/qemu-smoke-arm64.sh >> @@ -50,8 +50,9 @@ fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled >> >> if [[ "${test_variant}" == "boot-cpupools" ]]; then >> # Create cpupool node and assign it to domU0 >> - cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" >> + cpu_phandle="0xfffffe" >> cpupool_phandle="0xffffff" >> + fdtput binaries/virt-gicv2.dtb -p -t x /cpus/cpu@1 phandle $cpu_phandle >> fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool >> fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle >> fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle > > >>> Given my other comment below, I would leave this code as is. >>> >>> >>>> +if [[ "${test_variant}" == "boot-cpupools" ]]; then >>>> + # Create cpupool node and assign it to domU0 >>>> + cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" >>>> + cpupool_phandle="0xffffff" >>>> + fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool >>>> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle >>>> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle >>>> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool $cpupool_phandle >>>> + >>>> + # Check if domU0 (id=1) is assigned to Pool-1 >>>> + passed="${test_variant} test passed" >>>> + dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi" >>>> +fi >>> >>> I would prefer to keep the device tree editing here to a minimum and >>> instead add boot-cpupool support in ImageBuilder and add CPUPOOL* config >>> options to the existing config file for ImageBuilder created in this >>> file below. This way, we keep this test cleaner and we help more the >>> user by proving a way to set boot-cpupools more easily in general, also >>> useful outside gitlab-ci. >> >> I agree that ImageBuilder is a great tool. However, I would opt for keeping what I did because of the following: >> - current release schedule (we could benefit from having a test for 4.17 feature instead of waiting for the corresponding >> change to be done in ImageBuilder first and tested), >> - test is already prepared and requires just a trivial fix, >> - we should not enforce users willing to add tests to gitlab-ci to always prepare the ImageBuilder changes first. >> ImageBuilder is not meant to support all the features strictly because some of them require too much >> end-user knowledge and digging into device tree (it should stay as simple as possible), >> - all in all we need to have a way to modify the dtb and fdtput is certainly better than sed as it does not >> require additional steps for decompilation/compilation and its commands look more clean than using sed transformation. >> >> Let me know what you think. >> >> On a side note, I can add boot-time cpupools support in ImageBuilder to my TODO list so that we can check if this is something >> ImageBuilder should support. If yes, we can modify this test after the release. > > > Yeah, ImageBuilder doesn't necessarely need to support every feature. > However, a tool (if not ImageBuilder, Lopper, or a new ImageBuilder > script) should support CPUPOOLs to enable the user. > > You are right that ImageBuilder is not necessarely tied with gitlab-ci. > This is especially true once we start doing more interface-level > testing, such as hypercalls fuzzing with XTF. We are not going to be > able to use ImageBuilder to trigger every possible device tree boot time > combination, especially the ones that are invalid. We want to be able to > test Xen with invalid device tree input as well. > > In addition to interface-level testing, we need user-level testing to > test features the way we expect a user to use them. This is what > ImageBuilder is for and that is why it has been used today in gitlab-ci. > On ARM today we only have user-level testing in gitlab-ci, but I'd love > to have more interface-level testing, which will surely require more > device tree manipulations outside of ImageBuilder. > > - user-level tests -> ImageBuilder, common valid configurations > - interface-level tests -> not ImageBuilder, various valid and invalid > configurations, maybe automatically generated? > Device tree manipulations expected in gitlab-ci. > > > In my view, this test belongs to the "user-level test" category, this is > why I would prefer if it was done using the same tool that we expect the > user to use. Ideally, it would be ImageBuilder because that is the tool > that we have used so far (but it could be a new script under > ImageBuilder or Lopper). > > But I understand deadlines, release schedule, etc., so if you think it > cannot be done properly using ImageBuilder in 2-3 days, then I would > take this patch as is, and we can revisit it in the future as you > suggested. I am OK with that too. Ok, let me see if I can come up with a clean solution in the ImageBuilder. ~Michal
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 07209820b474..d899b3bdbf7a 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -100,6 +100,25 @@ qemu-smoke-arm64-gcc-staticmem: tags: - arm64 +qemu-smoke-arm64-gcc-boot-cpupools: + extends: .test-jobs-common + variables: + CONTAINER: debian:unstable-arm64v8 + script: + - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log + needs: + - alpine-3.12-gcc-arm64 + - alpine-3.12-arm64-rootfs-export + - kernel-5.19-arm64-export + - qemu-system-aarch64-6.0.0-arm64-export + artifacts: + paths: + - smoke.serial + - '*.log' + when: always + tags: + - arm64 + qemu-smoke-arm32-gcc: extends: .test-jobs-common variables: diff --git a/automation/scripts/build b/automation/scripts/build index 2b9f2d2b541a..2f15ab3198e6 100755 --- a/automation/scripts/build +++ b/automation/scripts/build @@ -19,7 +19,8 @@ else echo " CONFIG_EXPERT=y CONFIG_UNSUPPORTED=y -CONFIG_STATIC_MEMORY=y" > xen/.config +CONFIG_STATIC_MEMORY=y +CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config make -j$(nproc) -C xen olddefconfig else make -j$(nproc) -C xen defconfig diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh index 7ac96027760d..c2184850293c 100755 --- a/automation/scripts/qemu-smoke-arm64.sh +++ b/automation/scripts/qemu-smoke-arm64.sh @@ -4,20 +4,22 @@ set -ex test_variant=$1 -passed="passed" -check=" +if [ -z "${test_variant}" ]; then + passed="ping test passed" + domU_check=" until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do sleep 30 done echo \"${passed}\" " +fi if [[ "${test_variant}" == "static-mem" ]]; then # Memory range that is statically allocated to DOM1 domu_base="50000000" domu_size="10000000" passed="${test_variant} test passed" - check=" + domU_check=" current=\$(hexdump -e '16/1 \"%02x\"' /proc/device-tree/memory@${domu_base}/reg 2>/dev/null) expected=$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size}) if [[ \"\${expected}\" == \"\${current}\" ]]; then @@ -42,11 +44,23 @@ curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom -cpu cortex-a57 -machine type=virt \ -m 1024 -smp 2 -display none \ -machine dumpdtb=binaries/virt-gicv2.dtb -# XXX disable pl061 to avoid Linux crash -dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts -sed 's/compatible = "arm,pl061.*/status = "disabled";/g' binaries/virt-gicv2.dts > binaries/virt-gicv2-edited.dts -dtc -I dts -O dtb binaries/virt-gicv2-edited.dts > binaries/virt-gicv2.dtb +# XXX disable pl061 to avoid Linux crash +fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled + +if [[ "${test_variant}" == "boot-cpupools" ]]; then + # Create cpupool node and assign it to domU0 + cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)" + cpupool_phandle="0xffffff" + fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible xen,cpupool + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus $cpu_phandle + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle $cpupool_phandle + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool $cpupool_phandle + + # Check if domU0 (id=1) is assigned to Pool-1 + passed="${test_variant} test passed" + dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi" +fi # Busybox mkdir -p initrd @@ -66,7 +80,7 @@ echo "#!/bin/sh mount -t proc proc /proc mount -t sysfs sysfs /sys mount -t devtmpfs devtmpfs /dev -${check} +${domU_check} /bin/sh" > initrd/init chmod +x initrd/init cd initrd @@ -98,6 +112,7 @@ ifconfig xenbr0 up ifconfig xenbr0 192.168.0.1 xl network-attach 1 type=vif +${dom0_check} " > etc/local.d/xen.start chmod +x etc/local.d/xen.start echo "rc_verbose=yes" >> etc/rc.conf @@ -148,5 +163,5 @@ timeout -k 1 240 \ -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& tee smoke.serial set -e -(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "DOM1: ${passed}" smoke.serial) || exit 1 +(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1 exit 0
Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute script qemu-smoke-arm64.sh to test boot time cpupools feature. Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test case in qemu-smoke-arm64.sh that if selected will: - create a device tree cpupool node with cpu@1 - assign created cpupool to domU0 - add a check in dom0 xen.start to see if domU is assigned a Pool-1 Take the opportunity to refactor the qemu-smoke-arm64.sh script as follows: - use domU_check to store the test's commands to be run from domU - use dom0_check to store the test's commands to be run from dom0 - use fdtput instead of sed to perform dtb modifications - use more meaningful messages for "passed" variable. This way we can grep for messages reported either by domU or dom0 and get rid of assumption that tests can only be run from domU Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- automation/gitlab-ci/test.yaml | 19 +++++++++++++++ automation/scripts/build | 3 ++- automation/scripts/qemu-smoke-arm64.sh | 33 +++++++++++++++++++------- 3 files changed, 45 insertions(+), 10 deletions(-)