Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b,emulation/qemu: Drop select SMP ......................................................................
mb/asus/p2b,emulation/qemu: Drop select SMP
With MAX_CPUS > 1, SMP remains selected.
With MAX_CPUS==1, this has the effect of removing spinlock implementation. But since is_smp_boot() evaluates false and SMM uses separate smi_semaphore, there is no concurrency to protect against with a spinlock.
Change-Id: I9048a4821f19d90e1489b09e294d2551941abf10 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/qemu-x86/Kconfig M src/mainboard/asus/p2b/Kconfig 2 files changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/43809/1
diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 21ada02..3136162 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -6,7 +6,6 @@ select ARCH_VERSTAGE_X86_32 select ARCH_ROMSTAGE_X86_32 select ARCH_RAMSTAGE_X86_32 - select SMP select UDELAY_TSC select TSC_MONOTONIC_TIMER select UNKNOWN_TSC_RATE diff --git a/src/mainboard/asus/p2b/Kconfig b/src/mainboard/asus/p2b/Kconfig index 00295be..1d3d3b9 100644 --- a/src/mainboard/asus/p2b/Kconfig +++ b/src/mainboard/asus/p2b/Kconfig @@ -7,7 +7,6 @@ select SDRAMPWR_4DIMM select HAVE_MP_TABLE select IOAPIC - select SMP
config BOARD_SPECIFIC_OPTIONS def_bool y
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b,emulation/qemu: Drop select SMP ......................................................................
Patch Set 2: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b,emulation/qemu: Drop select SMP ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/2//COMMIT_MSG@7 PS2, Line 7: mb/asus/p2b,emulation/qemu: Drop select SMP I can easily ack the asus/p2b change. Maybe do the QEMU change separately?
Hello Keith Hui, build bot (Jenkins), Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43809
to look at the new patch set (#3).
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
mb/asus/p2b: Drop select SMP
With MAX_CPUS > 1, SMP remains selected.
With MAX_CPUS==1, this has the effect of removing spinlock implementation. But since is_smp_boot() evaluates false and SMM uses separate smi_semaphore, there is no concurrency to protect against with a spinlock.
Change-Id: I9048a4821f19d90e1489b09e294d2551941abf10 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/asus/p2b/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/43809/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock. This doesn't apply (SMP is selected when MAX_CPUS = 2)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock.
This doesn't apply (SMP is selected when MAX_CPUS = 2)
But line above says SMP remains selected with MAX_CPUS > 1 ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock.
But line above says SMP remains selected with MAX_CPUS > 1 ?
on asus/p2b, SMP isn't selected when MAX_CPUS==1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock.
on asus/p2b, SMP isn't selected when MAX_CPUS==1
Edit the commit message in-situ. I have no clue what you want this to say.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock.
Edit the commit message in-situ. I have no clue what you want this to say.
Oh, sorry for not being clear. Just drop this paragraph.
I don't want to do it myself because I will then need someone else's +2.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock.
Oh, sorry for not being clear. Just drop this paragraph. […]
My expectation was MAX_CPUS==1,SMP=n would have been identical to MAX_CPUS==1,SMP=y, but it is not. I thought it's worth commenting this odd case.
Hello Keith Hui, build bot (Jenkins), Paul Menzel, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43809
to look at the new patch set (#4).
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
mb/asus/p2b: Drop select SMP
Variants that select BASE_ASUS_P2B_D will also get MAX_CPUS==2 below, so this was redundant.
Change-Id: I9048a4821f19d90e1489b09e294d2551941abf10 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/asus/p2b/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/43809/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43809/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/2//COMMIT_MSG@7 PS2, Line 7: mb/asus/p2b,emulation/qemu: Drop select SMP
I can easily ack the asus/p2b change. […]
Done
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43809/3//COMMIT_MSG@11 PS3, Line 11: With MAX_CPUS==1, this has the effect of removing spinlock : implementation. But since is_smp_boot() evaluates false and : SMM uses separate smi_semaphore, there is no concurrency to : protect against with a spinlock.
My expectation was MAX_CPUS==1,SMP=n would have been identical to MAX_CPUS==1,SMP=y, but it is not. […]
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43809 )
Change subject: mb/asus/p2b: Drop select SMP ......................................................................
mb/asus/p2b: Drop select SMP
Variants that select BASE_ASUS_P2B_D will also get MAX_CPUS==2 below, so this was redundant.
Change-Id: I9048a4821f19d90e1489b09e294d2551941abf10 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43809 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/asus/p2b/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/asus/p2b/Kconfig b/src/mainboard/asus/p2b/Kconfig index 00295be..1d3d3b9 100644 --- a/src/mainboard/asus/p2b/Kconfig +++ b/src/mainboard/asus/p2b/Kconfig @@ -7,7 +7,6 @@ select SDRAMPWR_4DIMM select HAVE_MP_TABLE select IOAPIC - select SMP
config BOARD_SPECIFIC_OPTIONS def_bool y
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)