Add support for setting up `SMP` on `ARM64` through `ACPI` #912

mogasergiu · 2023-05-25T12:54:30Z

Prerequisite checklist

Read the contribution guidelines regarding submitting new changes to the project;
Tested your changes against relevant architectures and platforms;
Ran the checkpatch.pl on your commit series before opening this PR;
Updated relevant documentation.

Base target

Architecture(s): arm64
Platform(s): kvm
Application(s): N/A

Additional configuration

Description of changes

Add support for setting up SMP on ARM64 through ACPI.

This PR depends on #772 #848 #908 #909 #910 and #911, therefore it includes.

NOTE

This is part of a larger set of Pull Requests. To make things easier and to ensure that things build as expected, it is recommended that testing is done based on #912, which includes everything. The splitting has been done to ease the review process.

To make testing easier, use the script from #910 to build the Unikraft images. You will need an OVMF BIOS image first.

razvanvirtan · 2023-08-11T14:18:10Z

@mogasergiu I looked on the code and tested with nginx, things look good to me 👍

One minor comment: in the 9801546 commit message, you have one repetition: "fetch the GIC Distributor's GIC Distributor's physical base"

razvanvirtan

This looks good to me, thanks for the great work @mogasergiu!

Reviewed-by: Razvan Virtan virtanrazvan@gmail.com

Split secondary cores enumeration in two methods: - if CONFIG_UKPLAT_ACPI is enabled then get the information through the `MADT`'s `GICC` structures - else rely on the `Devicetree`'s `cpu@` nodes Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

Allow the existence of two ways to enumerate secondary cores: `ACPI` and `Devicetree`. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

Add the alternative of fetching the `PSCI` method from `ACPI`'s `FADT`'s `ARM Boot Flags` field. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

Since this applies for both `GICv2` and `GICv3`, implement a generic method of fetching the unique `GICD` structure from the `MADT`. Unlike `Devicetree`'s `reg` property, `ACPI` does not inform us of the length of the `GIC Distributor`'s address space and therefore one must assume the page-aligned default size from the specification: - for GICv2: ARM Generic Interrupt Controller Architecture version 2.0 Issue B.b - for GICv3: ARM Generic Interrupt Controller Architecture version 3 and version 4 Issue H Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

If `ACPI` is enabled, rely on it for probing the `GICv2`. Thus, we need to take two steps for this: - fetch the GIC CPU interface's base address and default length from the `GICC` `MADT` entry. Since there is only one `Redistributor`, all `GICC`'s must have the same physical base address. - fetch the `GIC Distributor`'s physical base address from `MADT`'s `GICD` structure and check that the size is compliant with that of `GICv2`. `GICR Base Address` must be set to 0, otherwise this is `GICv3/4`. Furthermore, separate the two ways of probing `GICv2` in two: one for `ACPI` and one for `Devicetree`. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

If `ACPI` is enabled, rely on it for probing the `GICv3`. Thus, we need to take two steps for this: - fetch the GIC Redistributor's base address and address range length from the `GICR` `MADT` entry. Normally we should check first if there exists a Redistributor. If there is none then we must get its address from the `GICC` structure, since that means that the Redistributors are not in an always-on power domain. Otherwise there could be a `GICR` for each Redistributor region and the `DT` probe version does not support multiple regions anyway. So, until there is need for that (not the case for QEMU Virt), align `ACPI` probe with `DT` probe and assume we only have one Redistributor region. - fetch the `GIC Distributor`'s physical base address from `MADT`'s `GICD` structure and check that the size is compliant with that of `GICv3`. Furthermore, separate the two ways of probing `GICv3` in two: one for `ACPI` and one for `Devicetree`. As a reminder, add a `TODO` in the `DT` probe for when someone decides to actually improve this driver. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

Now that we have separated the ways we probe the `GIC` (`Devicetree` vs `ACPI`) and the `Devicetree` parts can fetch the `Devicetree` on their own, remove the no longer necessary `fdt` function parameters from the related functions. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

michpappas

Reviewed-by: Michalis Pappas michalis@unikraft.io

razvand

Approved-by: Razvan Deaconescu razvand@unikraft.io

Allow the existence of two ways to enumerate secondary cores: `ACPI` and `Devicetree`. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #912

Add the alternative of fetching the `PSCI` method from `ACPI`'s `FADT`'s `ARM Boot Flags` field. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #912

Since this applies for both `GICv2` and `GICv3`, implement a generic method of fetching the unique `GICD` structure from the `MADT`. Unlike `Devicetree`'s `reg` property, `ACPI` does not inform us of the length of the `GIC Distributor`'s address space and therefore one must assume the page-aligned default size from the specification: - for GICv2: ARM Generic Interrupt Controller Architecture version 2.0 Issue B.b - for GICv3: ARM Generic Interrupt Controller Architecture version 3 and version 4 Issue H Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #912

If `ACPI` is enabled, rely on it for probing the `GICv2`. Thus, we need to take two steps for this: - fetch the GIC CPU interface's base address and default length from the `GICC` `MADT` entry. Since there is only one `Redistributor`, all `GICC`'s must have the same physical base address. - fetch the `GIC Distributor`'s physical base address from `MADT`'s `GICD` structure and check that the size is compliant with that of `GICv2`. `GICR Base Address` must be set to 0, otherwise this is `GICv3/4`. Furthermore, separate the two ways of probing `GICv2` in two: one for `ACPI` and one for `Devicetree`. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #912

If `ACPI` is enabled, rely on it for probing the `GICv3`. Thus, we need to take two steps for this: - fetch the GIC Redistributor's base address and address range length from the `GICR` `MADT` entry. Normally we should check first if there exists a Redistributor. If there is none then we must get its address from the `GICC` structure, since that means that the Redistributors are not in an always-on power domain. Otherwise there could be a `GICR` for each Redistributor region and the `DT` probe version does not support multiple regions anyway. So, until there is need for that (not the case for QEMU Virt), align `ACPI` probe with `DT` probe and assume we only have one Redistributor region. - fetch the `GIC Distributor`'s physical base address from `MADT`'s `GICD` structure and check that the size is compliant with that of `GICv3`. Furthermore, separate the two ways of probing `GICv3` in two: one for `ACPI` and one for `Devicetree`. As a reminder, add a `TODO` in the `DT` probe for when someone decides to actually improve this driver. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #912

Now that we have separated the ways we probe the `GIC` (`Devicetree` vs `ACPI`) and the `Devicetree` parts can fetch the `Devicetree` on their own, remove the no longer necessary `fdt` function parameters from the related functions. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Tested-by: Unikraft CI <monkey@unikraft.io> GitHub-Closes: #912

unikraft-bot · 2023-08-11T16:51:02Z

⚠️ Checkpatch passed with warnings

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request but it encountered warnings:

SHA	commit	checkpatch
`61b580f`	plat: Enable secondary cores allocation through `ACPI` on `ARM64`	⚠️
`89572a0`	plat/kvm/arm: Enable enumerating secondary cores through `ACPI`	✅
`2d2bab5`	plat/kvm/arm: Enable fetching of the `PSCI` method through `ACPI`	✅
`f6c2fba`	plat/drivers/gic: Add a method for fetching the `GICD` from `MADT`	✅
`9406cbe`	plat/drivers/gic: Enable `GICv2` probing through `ACPI`	✅
`498c8bd`	plat/drivers/gic: Enable `GICv3` probing through `ACPI`	✅
`519a44f`	plat/drivers/gic: Remove `fdt` argument from initialization methods	✅

Truncated logs starting from first warning 61b580f:

WARNING:LINE_SPACING: Missing a blank line after declarations
#48: FILE: plat/common/arm/lcpu.c:100:
+	__lcpuid bsp_cpu_id = lcpu_get(0)->id;
+	int bsp_found __maybe_unused = 0;

total: 0 errors, 1 warnings, 121 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/build/a53d4c5b/patches/0001-plat-Enable-secondary-cores-allocation-through-ACPI-.patch has style problems, please review.

NOTE: Ignored message types: ASSIGN_IN_IF FILE_PATH_CHANGES NEW_TYPEDEFS OBSOLETE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

View complete logs | Learn more about Unikraft's coding style and contribution guidelines.

mogasergiu requested review from a team as code owners May 25, 2023 12:54

mogasergiu force-pushed the acpi-smp-aa64 branch from 8da9dbe to d3c88bb Compare May 25, 2023 13:19

razvand assigned michpappas May 31, 2023

razvand added this to the v0.14.0 (Prometheus) milestone May 31, 2023

razvand added area/plat Unikraft Patform arch/arm64 topic/smp PR or issues related to SMP support. labels May 31, 2023

razvand requested review from razvanvirtan and mariasfiraiala and removed request for a team May 31, 2023 04:40

unikraft-bot mentioned this pull request Aug 7, 2023

plat: Migrate bus drivers to drivers/ukbus #1024

Closed

4 tasks

mogasergiu force-pushed the acpi-smp-aa64 branch 3 times, most recently from aee5e14 to 6f11e49 Compare August 10, 2023 18:20

razvand requested a review from michpappas August 11, 2023 08:25

razvand assigned razvand and unassigned michpappas Aug 11, 2023

razvand self-requested a review August 11, 2023 08:26

razvanvirtan approved these changes Aug 11, 2023

View reviewed changes

mogasergiu added 7 commits August 11, 2023 19:08

plat/kvm/arm: Enable enumerating secondary cores through ACPI

89572a0

Allow the existence of two ways to enumerate secondary cores: `ACPI` and `Devicetree`. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

plat/kvm/arm: Enable fetching of the PSCI method through ACPI

2d2bab5

Add the alternative of fetching the `PSCI` method from `ACPI`'s `FADT`'s `ARM Boot Flags` field. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>

mogasergiu force-pushed the acpi-smp-aa64 branch from 6f11e49 to 519a44f Compare August 11, 2023 16:09

michpappas approved these changes Aug 11, 2023

View reviewed changes

razvand approved these changes Aug 11, 2023

View reviewed changes

unikraft-bot closed this in 0b02ff3 Aug 11, 2023

unikraft-bot added the ci/merged Merged by CI label Aug 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for setting up `SMP` on `ARM64` through `ACPI` #912

Add support for setting up `SMP` on `ARM64` through `ACPI` #912

mogasergiu commented May 25, 2023 •

edited

Loading

razvanvirtan commented Aug 11, 2023

razvanvirtan left a comment

michpappas left a comment

razvand left a comment

unikraft-bot commented Aug 11, 2023

Add support for setting up SMP on ARM64 through ACPI #912

Add support for setting up SMP on ARM64 through ACPI #912

Conversation

mogasergiu commented May 25, 2023 • edited Loading

Prerequisite checklist

Base target

Additional configuration

Description of changes

NOTE

razvanvirtan commented Aug 11, 2023

razvanvirtan left a comment

Choose a reason for hiding this comment

michpappas left a comment

Choose a reason for hiding this comment

razvand left a comment

Choose a reason for hiding this comment

unikraft-bot commented Aug 11, 2023

Add support for setting up `SMP` on `ARM64` through `ACPI` #912

Add support for setting up `SMP` on `ARM64` through `ACPI` #912

mogasergiu commented May 25, 2023 •

edited

Loading