| Message ID | 20241005050155.61103-2-CFSworks@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bcm4908: Fix secondary CPU initialization | expand |
On 10/4/24 22:01, Sam Edwards wrote: > The CFE bootloader places a stub program in the first page of physical > memory to hold the secondary CPUs until the boot CPU writes the release > address, but does not splice a /reserved-memory node into the FDT to > protect it. If Linux overwrites this program before execution reaches > smp_prepare_cpus(), the secondary CPUs may become inaccessible. > > This is only a problem with CFE, and then only until the secondary CPUs > are brought online. Ideally, there would be some hypothetical mechanism > we could use to indicate that this area of memory is sensitive only > during boot. But as there is none, and since it is such a small amount > of memory, it is easiest to reserve it unconditionally. If we supported CPU hotplug on those platforms (do we?) then it actually does matter that this memory remains protected, and it cannot be reclaimed. This does not invalidate the commit message and I will take it as-is, but it it is not memory that we can necessarily reclaim that easily, if we did things properly. > > Therefore, add a /reserved-memory node to bcm4908.dtsi to protect the > first 4KiB of physical memory. > > Signed-off-by: Sam Edwards <CFSworks@gmail.com> > --- > arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > index 8b924812322c..c51b92387fad 100644 > --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > @@ -68,6 +68,16 @@ l2: l2-cache0 { > }; > }; > > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + cfe-stub@0 { > + reg = <0x0 0x0 0x0 0x1000>; > + }; > + }; > + > axi@81000000 { > compatible = "simple-bus"; > #address-cells = <1>;
On Thu, Oct 10, 2024 at 3:57 PM Florian Fainelli <florian.fainelli@broadcom.com> wrote: > > On 10/4/24 22:01, Sam Edwards wrote: > > The CFE bootloader places a stub program in the first page of physical > > memory to hold the secondary CPUs until the boot CPU writes the release > > address, but does not splice a /reserved-memory node into the FDT to > > protect it. If Linux overwrites this program before execution reaches > > smp_prepare_cpus(), the secondary CPUs may become inaccessible. > > > > This is only a problem with CFE, and then only until the secondary CPUs > > are brought online. Ideally, there would be some hypothetical mechanism > > we could use to indicate that this area of memory is sensitive only > > during boot. But as there is none, and since it is such a small amount > > of memory, it is easiest to reserve it unconditionally. > > If we supported CPU hotplug on those platforms (do we?) then it actually > does matter that this memory remains protected, and it cannot be > reclaimed. This does not invalidate the commit message and I will take > it as-is, but it it is not memory that we can necessarily reclaim that > easily, if we did things properly. I am looking at only one build of CFE, so don't take what I'm saying as gospel, but as I understand it: CFE implements only the spin-table method, which isn't dynamic. Once the kernel calls for the secondary CPUs to be released, it cannot shut them down and bring them up again, so they have no need to reenter the spin stub. With U-Boot+ATF, it's of course a different story: ATF needs to stay resident in memory to implement PSCI (which _is_ dynamic), but U-Boot inserts/overwrites the necessary DT structures to protect ATF and tell Linux to use PSCI, so we don't need to be thinking about that here. But, again, that's just my understanding. If there's a version of CFE out there that loads ATF/similar or implements PSCI itself, then, well, that'd invalidate this patch('s commit message). I doubt such a variant of CFE exists, but you're much better equipped to confirm or refute that possibility, given your familiarity with the platform and internal details. Best wishes, Sam > > > > > Therefore, add a /reserved-memory node to bcm4908.dtsi to protect the > > first 4KiB of physical memory. > > > > Signed-off-by: Sam Edwards <CFSworks@gmail.com> > > --- > > arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > > index 8b924812322c..c51b92387fad 100644 > > --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > > +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi > > @@ -68,6 +68,16 @@ l2: l2-cache0 { > > }; > > }; > > > > + reserved-memory { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + cfe-stub@0 { > > + reg = <0x0 0x0 0x0 0x1000>; > > + }; > > + }; > > + > > axi@81000000 { > > compatible = "simple-bus"; > > #address-cells = <1>; > > > -- > Florian
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi index 8b924812322c..c51b92387fad 100644 --- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi @@ -68,6 +68,16 @@ l2: l2-cache0 { }; }; + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + cfe-stub@0 { + reg = <0x0 0x0 0x0 0x1000>; + }; + }; + axi@81000000 { compatible = "simple-bus"; #address-cells = <1>;
The CFE bootloader places a stub program in the first page of physical memory to hold the secondary CPUs until the boot CPU writes the release address, but does not splice a /reserved-memory node into the FDT to protect it. If Linux overwrites this program before execution reaches smp_prepare_cpus(), the secondary CPUs may become inaccessible. This is only a problem with CFE, and then only until the secondary CPUs are brought online. Ideally, there would be some hypothetical mechanism we could use to indicate that this area of memory is sensitive only during boot. But as there is none, and since it is such a small amount of memory, it is easiest to reserve it unconditionally. Therefore, add a /reserved-memory node to bcm4908.dtsi to protect the first 4KiB of physical memory. Signed-off-by: Sam Edwards <CFSworks@gmail.com> --- arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+)