Page MenuHomeFreeBSD
Differential D3297

Release memory for CPUs that fail to init on ARM64
ClosedPublic
Actions

Authored by zbb on Aug 4 2015, 7:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 5:53 PM
Unknown Object (File)
Thu, Nov 7, 8:50 AM
Unknown Object (File)
Oct 26 2024, 6:06 AM
Unknown Object (File)
Oct 7 2024, 10:32 PM
Unknown Object (File)
Oct 4 2024, 11:51 PM
Unknown Object (File)
Oct 4 2024, 9:03 PM
Unknown Object (File)
Oct 4 2024, 2:56 PM
Unknown Object (File)
Sep 28 2024, 2:02 AM
View All Files
Subscribers
andrew
emaste
imp

Details

Reviewers
wma_semihalf.com
Group Reviewers
arm64
Commits
rS287883: Release memory for CPUs that fail to init on ARM64
Summary
cpu_init_fdt will now release memory allocated for structures serving
CPUs that have failed to init.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

wma_semihalf.com updated this revision to Diff 7644.Aug 4 2015, 7:31 AM
wma_semihalf.com retitled this revision from to Add memory for CPUs that fail to init on ARM64.
wma_semihalf.com updated this object.
wma_semihalf.com edited the test plan for this revision. (Show Details)
wma_semihalf.com added a reviewer: arm64.
wma_semihalf.com set the repository for this revision to rS FreeBSD src repository - subversion.
Herald added subscribers: emaste, andrew, imp. · View Herald TranscriptAug 4 2015, 7:31 AM
wma_semihalf.com retitled this revision from Add memory for CPUs that fail to init on ARM64 to Release memory for CPUs that fail to init on ARM64.Aug 4 2015, 7:31 AM
emaste added a comment.Aug 15 2015, 12:10 AM

Seems fine - did you actually encounter CPUs failing to start, or just found by code inspection?

wma_semihalf.com added a comment.Aug 17 2015, 11:44 AM

We found some troubles with CPU starting on one of our non-cavm arm64 boards. If you have no objections I'd like to submit this patch tomorrow.

andrew added a comment.Aug 18 2015, 8:21 AM

This should only be the case when there is broken firmware, an incorrect dtb, or missing psci. In the latter we could check if psci is enabled in the dtb before attempting to start secondary cores, otherwise I feel the correct response is to panic.

zbb commandeered this revision.Aug 19 2015, 12:21 PM
zbb updated this revision to Diff 8057.
zbb added a reviewer: wma_semihalf.com.
zbb removed rS FreeBSD src repository - subversion as the repository for this revision.
zbb added a comment.Aug 19 2015, 12:25 PM
In D3297#69573, @andrew wrote:

This should only be the case when there is broken firmware, an incorrect dtb, or missing psci. In the latter we could check if psci is enabled in the dtb before attempting to start secondary cores, otherwise I feel the correct response is to panic.

Let's split the difference and put KASSERT in there that will panic when INVARIANTS are enabled. We assume that in most cases if CPU fails to start it is not harmful for the system (by that I mean that CPU is in power-off or etc.). What do you think about that?

andrew added a comment.Aug 19 2015, 3:57 PM
In D3297#69857, @zbb wrote:

Let's split the difference and put KASSERT in there that will panic when INVARIANTS are enabled. We assume that in most cases if CPU fails to start it is not harmful for the system (by that I mean that CPU is in power-off or etc.). What do you think about that?

I'm not sure how useful that is. In what cases should failing to start a core not be fatal? All the cases I can think of I would want the kernel to fail as it indicates a larger problem.

zbb added a comment.Aug 19 2015, 5:08 PM
In D3297#69900, @andrew wrote:
In D3297#69857, @zbb wrote:

Let's split the difference and put KASSERT in there that will panic when INVARIANTS are enabled. We assume that in most cases if CPU fails to start it is not harmful for the system (by that I mean that CPU is in power-off or etc.). What do you think about that?

I'm not sure how useful that is. In what cases should failing to start a core not be fatal? All the cases I can think of I would want the kernel to fail as it indicates a larger problem.

Depends on CPU wake-up method. If we just release the CPU core and it does not notify his presence it will probably be fatal for the system. If we have a firmware that wakes CPUs and we send a request to our monitor (or whatever) and it fails to start the CPU (because it limits the number of CPUs or has some arbitrary settings ), we end up having 1 CPU less in our system.
Still it's a political dispute and we are not going to oppose if you really prefer good old panic there.

Closed by commit rS287883: Release memory for CPUs that fail to init on ARM64 (authored by zbb). · Explain WhySep 16 2015, 11:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
arm64/
arm64/
8 lines

Diff 7644

View Options

sys/arm64/arm64/mp_machdep.c

Loading...