Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI testing ZCU102 SMP #72

Closed
wants to merge 4 commits into from

Conversation

axel-h
Copy link
Member

@axel-h axel-h commented Jan 31, 2024

Test with: seL4/seL4_projects_libs#124, seL4/camkes-vm#54

actual PR is seL4/seL4_projects_libs#84, but we need some debug infos

testing seL4/ci-actions#327

@axel-h axel-h added hw-build hardware builds for this PR hw-test hardware builds + runs for this PR labels Jan 31, 2024
@axel-h axel-h force-pushed the patch-axel-ci-base branch from 92b4184 to 41f413f Compare January 31, 2024 23:39
@axel-h axel-h force-pushed the patch-axel-ci-test branch from 3c2cade to 1c73f1f Compare January 31, 2024 23:40
@axel-h axel-h removed the hw-build hardware builds for this PR label Jan 31, 2024
@axel-h axel-h force-pushed the patch-axel-ci-test branch from 1c73f1f to cf958e4 Compare February 1, 2024 02:40
@axel-h axel-h force-pushed the patch-axel-ci-base branch from 41f413f to 46ae3c5 Compare February 1, 2024 02:42
@axel-h axel-h force-pushed the patch-axel-ci-test branch 3 times, most recently from 5ca316e to 8aaf716 Compare February 1, 2024 15:17
@chrisguikema
Copy link
Contributor

chrisguikema commented Feb 1, 2024

Can you add this change to really test the multicore changes?

diff --git a/apps/Arm/vm_minimal/zcu102/2022_1/devices.camkes b/apps/Arm/vm_minimal/zcu102/2022_1/devices.camkes
index 5450e81..7d64d8e 100644
--- a/apps/Arm/vm_minimal/zcu102/2022_1/devices.camkes
+++ b/apps/Arm/vm_minimal/zcu102/2022_1/devices.camkes
@@ -27,7 +27,8 @@ assembly {
             "initrd_addr" : VAR_STRINGIZE(VM_INITRD_ADDR),
             "kernel_entry_addr" : VAR_STRINGIZE(VM_ENTRY_ADDR),
         };
-        vm0.num_vcpus = 4;
+        vm0.num_vcpus = 2;
+        vm0.pcpus = [2, 3];

@axel-h axel-h force-pushed the patch-axel-ci-test branch from 8aaf716 to da0617f Compare February 1, 2024 15:42
@chrisguikema
Copy link
Contributor

Sorry, I had a typo in the original diff. It needs to be pcpus. I edited it, but you must have caught it before the edit.

@axel-h axel-h force-pushed the patch-axel-ci-test branch from da0617f to b91fda5 Compare February 1, 2024 15:57
@axel-h
Copy link
Member Author

axel-h commented Feb 1, 2024

Sorry, I had a typo in the original diff. It needs to be pcpus. I edited it, but you must have caught it before the edit.

Thanks, I should have spotted this instead of blindly copying the block.

@chrisguikema
Copy link
Contributor

  [    0.056212] psci: failed to boot CPU1 (-22)
  [    0.060054] CPU1: failed to boot: -22

Looks like its failing?

This is what I get locally:

[    0.051504] smp: Bringing up secondary CPUs ...
[    0.056266] Detected VIPT I-cache on CPU1
[    0.056597] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
[    0.056738] smp: Brought up 1 node, 2 CPUs
[    0.070429] SMP: Total of 2 processors activated.

@axel-h
Copy link
Member Author

axel-h commented Feb 1, 2024

Can you add some more debug output, so we can see what happens?

@axel-h axel-h marked this pull request as ready for review February 1, 2024 16:20
@axel-h axel-h marked this pull request as draft February 1, 2024 16:21
@chrisguikema
Copy link
Contributor

Here's a seL4_projects_libs PR that adds some debug statements: seL4/seL4_projects_libs#124

And here is what I get with those debug prints:

[    0.051503] smp: Bringing up secondary CPUs ...
[email protected]:50 target_cpu: 1
[email protected]:53 Null VCPU
[email protected]:55 Found free unassigned VCPU
[email protected]:57 Started found VCPU
[    0.071825] Detected VIPT I-cache on CPU1
[    0.075670] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
[    0.075809] smp: Brought up 1 node, 2 CPUs

@axel-h axel-h force-pushed the patch-axel-ci-test branch from b91fda5 to 76c811a Compare February 1, 2024 16:45
@axel-h
Copy link
Member Author

axel-h commented Feb 1, 2024

This is what CI shows. Needs more debug output to see what fails ;)

  [    0.051503] smp: Bringing up secondary CPUs ...
  [email protected]:50 target_cpu: 1
  [email protected]:53 Null VCPU
  [email protected]:55 Found free unassigned VCPU
  [email protected]:60 Failed to start found VCPU
  [    0.076080] psci: failed to boot CPU1 (-22)
  [    0.076120] CPU1: failed to boot: -22
  [    0.078426] smp: Brought up 1 node, 1 CPU

@chrisguikema
Copy link
Contributor

My guess is that the target_vcpu is returning NULL. All of the function calls in start_vcpu print errors if they return failures. I pushed up another commit to the seL4_projects_libs draft PR.

[    0.051504] smp: Bringing up secondary CPUs ...
[email protected]:53 target_cpu: 1
[email protected]:56 Null VCPU
[email protected]:58 Found free unassigned VCPU: 0x857a60
[email protected]:192 Calling TBC Resume
[email protected]:60 Started found VCPU
[    0.076150] Detected VIPT I-cache on CPU1
[    0.079997] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
[    0.080138] smp: Brought up 1 node, 2 CPUs

@axel-h axel-h force-pushed the patch-axel-ci-test branch from 76c811a to 9875016 Compare February 1, 2024 17:43
@chrisguikema
Copy link
Contributor

So guess who forgot to change the typo in his own config....

I've replicated the error. Checking into it now.

@chrisguikema
Copy link
Contributor

Okay, I see what's happening now:

The PCPU list is 2 and 3. PSCI is trying to enable (what it thinks it) CPU 1, so we search the PCPU list for 1. When that's not found, we error out.

[email protected]:53 target_cpu: 1
vm_vcpu_for_target_cpu@guest_vm_util.h:51 Finding vcpu for target 1
vm_vcpu_for_target_cpu@guest_vm_util.h:53 vcpu 0: target cpu: 2
vm_vcpu_for_target_cpu@guest_vm_util.h:53 vcpu 1: target cpu: 3
[email protected]:56 Null VCPU
[email protected]:58 Found free unassigned VCPU: 0
[email protected]:63 Failed to start found VCPU

@chrisguikema
Copy link
Contributor

@Apave24 did make this PR which I think handles this exact situation: seL4/seL4_projects_libs@c95179f

@axel-h
Copy link
Member Author

axel-h commented Feb 1, 2024

This seems the PR: seL4/seL4_projects_libs#85

@axel-h axel-h force-pushed the patch-axel-ci-test branch from 9875016 to 9edb302 Compare February 20, 2024 19:55
The QEMU/aarch32 platform is not supported actively at the moment for
virtualization.

Signed-off-by: Axel Heider <[email protected]>
axel-h and others added 3 commits March 16, 2024 23:42
Co-authored-by: Chris Guikema <[email protected]>
Signed-off-by: Axel Heider <[email protected]>
Signed-off-by: Axel Heider <[email protected]>
@axel-h axel-h force-pushed the patch-axel-ci-test branch from 9edb302 to 0ea6601 Compare March 16, 2024 22:42
@axel-h axel-h deleted the branch seL4:patch-axel-ci-base March 16, 2024 22:45
@axel-h axel-h closed this Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hw-test hardware builds + runs for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants