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

vm_arm: add pcpu list for core pinning #54

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Apave24
Copy link

@Apave24 Apave24 commented Nov 11, 2022

Adds a pcpu list to the VM_Arm component to allow pinning of vcpus to specific physical cpus within a CAmkES configuration.

Test with: seL4/seL4_projects_libs#84

Copy link
Member

@axel-h axel-h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@axel-h axel-h requested a review from yyshen November 17, 2022 21:24
@axel-h
Copy link
Member

axel-h commented Dec 10, 2022

Please add the DCO.

@wom-bat
Copy link
Member

wom-bat commented Feb 21, 2023

Please rebase this, and add the signed-off-by: line in your commit message.

@Apave24
Copy link
Author

Apave24 commented Mar 8, 2023

Bumping this PR. I believe everything has been addressed.

@axel-h
Copy link
Member

axel-h commented Mar 9, 2023

The CI build is failing with include errors, but I wonder if this is related the the change here or a general probelm?

@lsf37
Copy link
Member

lsf37 commented Mar 9, 2023

The CI build is failing with include errors, but I wonder if this is related the the change here or a general probelm?

It's at least not a CI glitch, one of the recently merged PRs must have introduced this problem (or it is introduced here).

@lsf37
Copy link
Member

lsf37 commented Mar 9, 2023

The current status of camkes-vm-examples is green, with the most recent tested manifest being this one:

2023-03-08T21:53:03.0758265Z Manifest summary:
2023-03-08T21:53:03.0758827Z -----------------
2023-03-08T21:53:03.0759355Z camkes-vm-examples-manifest: ee34902d Updating default.xml                           (origin/master)
2023-03-08T21:53:03.0759859Z                        seL4: f8b2440b Implemented the vm fault fastpath on aarch64   (seL4/master)
2023-03-08T21:53:03.0760375Z                 camkes-tool: d7d37df9 cmake: improve maintainability                 (seL4/master)
2023-03-08T21:53:03.0760912Z            camkes-vm-images: 69af5316 github: trigger main test on push to master .. (seL4/master)
2023-03-08T21:53:03.0761354Z                       capdl: 3fa3a1b5 trivial: fix style for changed file            (seL4/master)
2023-03-08T21:53:03.0761867Z           global-components: 908195f6 SerialServer: add ^C and ^Z to switching logic (seL4/master)
2023-03-08T21:53:03.0762315Z                      libzmq: d062edd8 Finalise changelog for 4.2.5                   (tags/v4.2.5^0)
2023-03-08T21:53:03.0762844Z                        lwip: 159e31b6 Prepare 2.1.2 release                          (tags/STABLE-2_1_2_RELEASE^0)
2023-03-08T21:53:03.0763272Z                    musllibc: 3d6b939e aarch64_sel4: Use more supported asm directive (seL4/sel4)
2023-03-08T21:53:03.0764145Z                     picotcp: 13c00a06 minor README.md text improvements              (tags/v1.7.0)
2023-03-08T21:53:03.0764587Z               projects_libs: daa7e44e ringbuffer: add "has_data" helper function     (seL4/master)
2023-03-08T21:53:03.0765118Z                   seL4_libs: 2ca52542 libsel4bench: add support for ARM Cortex-A55   (seL4/master)
2023-03-08T21:53:03.0765554Z          seL4_projects_libs: 58dc2820 libsel4vm: add entries to vm structure         (seL4/master)
2023-03-08T21:53:03.0765976Z                 sel4runtime: d935dd05 aarch32: Rename __aeabi_read_tp file (#15)     (seL4/master)
2023-03-08T21:53:03.0766469Z                   util_libs: e8e30909 libcpio: fix conversion of -1 to ulong         (seL4/master)
2023-03-08T21:53:03.0766953Z                   camkes-vm: a577f206 vm_arm: remove more linux references           (seL4/master)
2023-03-08T21:53:03.0767465Z          camkes-vm-examples: 9f0a5344 Add documentation of package structure         (seL4/master)
2023-03-08T21:53:03.0768009Z             camkes-vm-linux: 46fb8b6e vm-linux-helpers: symbolic links in overlays   (seL4/master)
2023-03-08T21:53:03.0768435Z                       polly: ef7e79c2 README.md                                      (polly/master)
2023-03-08T21:53:03.0768829Z                  seL4_tools: 074a54ae Add Quartz64 support                           (seL4/master)

@axel-h
Copy link
Member

axel-h commented Mar 9, 2023

@Apave24 could you do a rebase, so we get another CI run here?

arm_vm_helpers.cmake Outdated Show resolved Hide resolved
@chrisguikema chrisguikema force-pushed the core-pinning branch 2 times, most recently from 5d4a348 to 727df13 Compare February 1, 2024 15:11
Adds a pcpu list to the VM_Arm component to allow pinning of vcpus to
specific physical cpus within a CAmkES configuration.

Signed-off-by: Alex Pavey <[email protected]>
/*- endfor -*/
/*- endif -*/

int get_instance_size_pcpus_list(void) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int get_instance_size_pcpus_list(void) {
unsigned int get_instance_size_pcpus_list(void) {

@@ -1270,8 +1271,26 @@ static int main_continued(void)
}
}

int pcpu_assignments = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid assigning 0 and then overwriting this, it gives compiler warnings at the picky level.

    unsigned int pcpu_assignments = get_instance_size_pcpus_list();
    if (pcpu_assignments > NUM_VCPUS) {
        pcpu_assignments = NUM_VCPUS;
    }

Copy link
Member

@axel-h axel-h Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or well, we could even merge this nicely into the CPU node creation at line 1260:

    /* Create CPUs and DTB node */
    unsigned int pcpu_list_size = get_instance_size_pcpus_list();
    for (int i = 0; i < NUM_VCPUS; i++) {
        vm_vcpu_t *new_vcpu = create_vmm_plat_vcpu(&vm, VM_PRIO - 1);
        assert(new_vcpu);
        if (i < pcpu_list_size) {
            /* Assign VCPU to explicit physical CPU */
            new_vcpu->target_cpu = pcpus[i];
        }
    }

    // ... and now generate the device tree VCPU nodes

@@ -42,3 +42,5 @@ typedef struct {
} vm_config_t;

extern const vm_config_t vm_config;

int get_instance_size_pcpus_list(void);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int get_instance_size_pcpus_list(void);
unsigned int get_instance_size_pcpus_list(void);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants