* [PATCH 3/3] powerpc/kprobes: Check return value of patch_instruction()
From: Naveen N. Rao @ 2020-04-23 15:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Steven Rostedt
In-Reply-To: <cover.1587654213.git.naveen.n.rao@linux.vnet.ibm.com>
patch_instruction() can fail in some scenarios. Add appropriate error
checking so that such failures are caught and logged, and suitable error
code is returned.
Fixes: d07df82c43be8 ("powerpc/kprobes: Move kprobes over to patch_instruction()")
Fixes: f3eca95638931 ("powerpc/kprobes/optprobes: Use patch_instruction()")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/kprobes.c | 10 +++-
arch/powerpc/kernel/optprobes.c | 99 ++++++++++++++++++++++++++-------
2 files changed, 87 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 81efb605113e..4a297ae2bd87 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -138,13 +138,19 @@ NOKPROBE_SYMBOL(arch_prepare_kprobe);
void arch_arm_kprobe(struct kprobe *p)
{
- patch_instruction(p->addr, BREAKPOINT_INSTRUCTION);
+ int rc = patch_instruction(p->addr, BREAKPOINT_INSTRUCTION);
+
+ if (rc)
+ WARN("Failed to patch trap at 0x%pK: %d\n", (void *)p->addr, rc);
}
NOKPROBE_SYMBOL(arch_arm_kprobe);
void arch_disarm_kprobe(struct kprobe *p)
{
- patch_instruction(p->addr, p->opcode);
+ int rc = patch_instruction(p->addr, p->opcode);
+
+ if (rc)
+ WARN("Failed to remove trap at 0x%pK: %d\n", (void *)p->addr, rc);
}
NOKPROBE_SYMBOL(arch_disarm_kprobe);
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 024f7aad1952..046485bb0a52 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -139,52 +139,67 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
}
}
+#define PATCH_INSN(addr, instr) \
+do { \
+ int rc = patch_instruction((unsigned int *)(addr), instr); \
+ if (rc) { \
+ pr_err("%s:%d Error patching instruction at 0x%pK (%pS): %d\n", \
+ __func__, __LINE__, \
+ (void *)(addr), (void *)(addr), rc); \
+ return rc; \
+ } \
+} while (0)
+
/*
* emulate_step() requires insn to be emulated as
* second parameter. Load register 'r4' with the
* instruction.
*/
-void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
+static int patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
{
/* addis r4,0,(insn)@h */
- patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(4) |
+ PATCH_INSN(addr, PPC_INST_ADDIS | ___PPC_RT(4) |
((val >> 16) & 0xffff));
addr++;
/* ori r4,r4,(insn)@l */
- patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(4) |
+ PATCH_INSN(addr, PPC_INST_ORI | ___PPC_RA(4) |
___PPC_RS(4) | (val & 0xffff));
+
+ return 0;
}
/*
* Generate instructions to load provided immediate 64-bit value
* to register 'r3' and patch these instructions at 'addr'.
*/
-void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
+static int patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
{
/* lis r3,(op)@highest */
- patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(3) |
+ PATCH_INSN(addr, PPC_INST_ADDIS | ___PPC_RT(3) |
((val >> 48) & 0xffff));
addr++;
/* ori r3,r3,(op)@higher */
- patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(3) |
+ PATCH_INSN(addr, PPC_INST_ORI | ___PPC_RA(3) |
___PPC_RS(3) | ((val >> 32) & 0xffff));
addr++;
/* rldicr r3,r3,32,31 */
- patch_instruction(addr, PPC_INST_RLDICR | ___PPC_RA(3) |
+ PATCH_INSN(addr, PPC_INST_RLDICR | ___PPC_RA(3) |
___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31));
addr++;
/* oris r3,r3,(op)@h */
- patch_instruction(addr, PPC_INST_ORIS | ___PPC_RA(3) |
+ PATCH_INSN(addr, PPC_INST_ORIS | ___PPC_RA(3) |
___PPC_RS(3) | ((val >> 16) & 0xffff));
addr++;
/* ori r3,r3,(op)@l */
- patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(3) |
+ PATCH_INSN(addr, PPC_INST_ORI | ___PPC_RA(3) |
___PPC_RS(3) | (val & 0xffff));
+
+ return 0;
}
int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
@@ -216,14 +231,18 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
* be within 32MB on either side of the current instruction.
*/
b_offset = (unsigned long)buff - (unsigned long)p->addr;
- if (!is_offset_in_branch_range(b_offset))
+ if (!is_offset_in_branch_range(b_offset)) {
+ rc = -ERANGE;
goto error;
+ }
/* Check if the return address is also within 32MB range */
b_offset = (unsigned long)(buff + TMPL_RET_IDX) -
(unsigned long)nip;
- if (!is_offset_in_branch_range(b_offset))
+ if (!is_offset_in_branch_range(b_offset)) {
+ rc = -ERANGE;
goto error;
+ }
/* Setup template */
/* We can optimize this via patch_instruction_window later */
@@ -231,15 +250,22 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
pr_devel("Copying template to %p, size %lu\n", buff, size);
for (i = 0; i < size; i++) {
rc = patch_instruction(buff + i, *(optprobe_template_entry + i));
- if (rc < 0)
+ if (rc) {
+ pr_err("%s: Error copying optprobe template to 0x%pK: %d\n",
+ __func__, (void *)(buff + i), rc);
+ rc = -EFAULT;
goto error;
+ }
}
/*
* Fixup the template with instructions to:
* 1. load the address of the actual probepoint
*/
- patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
+ if (patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX)) {
+ rc = -EFAULT;
+ goto error;
+ }
/*
* 2. branch to optimized_callback() and emulate_step()
@@ -248,6 +274,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
emulate_step_addr = (kprobe_opcode_t *)ppc_kallsyms_lookup_name("emulate_step");
if (!op_callback_addr || !emulate_step_addr) {
WARN(1, "Unable to lookup optimized_callback()/emulate_step()\n");
+ rc = -ERANGE;
goto error;
}
@@ -259,21 +286,48 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
(unsigned long)emulate_step_addr,
BRANCH_SET_LINK);
- if (!branch_op_callback || !branch_emulate_step)
+ if (!branch_op_callback || !branch_emulate_step) {
+ rc = -ERANGE;
goto error;
+ }
- patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
- patch_instruction(buff + TMPL_EMULATE_IDX, branch_emulate_step);
+ rc = patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
+ if (rc) {
+ pr_err("%s:%d: Error patching instruction at 0x%pK: %d\n",
+ __func__, __LINE__,
+ (void *)(buff + TMPL_CALL_HDLR_IDX), rc);
+ rc = -EFAULT;
+ goto error;
+ }
+
+ rc = patch_instruction(buff + TMPL_EMULATE_IDX, branch_emulate_step);
+ if (rc) {
+ pr_err("%s:%d: Error patching instruction at 0x%pK: %d\n",
+ __func__, __LINE__,
+ (void *)(buff + TMPL_EMULATE_IDX), rc);
+ rc = -EFAULT;
+ goto error;
+ }
/*
* 3. load instruction to be emulated into relevant register, and
*/
- patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
+ if (patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX)) {
+ rc = -EFAULT;
+ goto error;
+ }
/*
* 4. branch back from trampoline
*/
- patch_branch(buff + TMPL_RET_IDX, (unsigned long)nip, 0);
+ rc = patch_branch(buff + TMPL_RET_IDX, (unsigned long)nip, 0);
+ if (rc) {
+ pr_err("%s:%d: Error patching instruction at 0x%pK: %d\n",
+ __func__, __LINE__,
+ (void *)(buff + TMPL_RET_IDX), rc);
+ rc = -EFAULT;
+ goto error;
+ }
flush_icache_range((unsigned long)buff,
(unsigned long)(&buff[TMPL_END_IDX]));
@@ -284,7 +338,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
error:
free_ppc_optinsn_slot(buff, 0);
- return -ERANGE;
+ return rc;
}
@@ -307,6 +361,7 @@ void arch_optimize_kprobes(struct list_head *oplist)
{
struct optimized_kprobe *op;
struct optimized_kprobe *tmp;
+ int rc;
list_for_each_entry_safe(op, tmp, oplist, list) {
/*
@@ -315,9 +370,13 @@ void arch_optimize_kprobes(struct list_head *oplist)
*/
memcpy(op->optinsn.copied_insn, op->kp.addr,
RELATIVEJUMP_SIZE);
- patch_instruction(op->kp.addr,
+ rc = patch_instruction(op->kp.addr,
create_branch((unsigned int *)op->kp.addr,
(unsigned long)op->optinsn.insn, 0));
+ if (rc)
+ pr_err("%s:%d: Error patching instruction at 0x%pK: %d\n",
+ __func__, __LINE__,
+ (void *)(op->kp.addr), rc);
list_del_init(&op->list);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH 0/3] powerpc: Enhance error handling with patch_instruction()
From: Naveen N. Rao @ 2020-04-23 15:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Steven Rostedt
This patchset updates error handling with patch_instruction(). The first
patch fixes an issue with do_patch_instruction() with STRICT_KERNEL_RWX
wherein errors were not being returned back. The second and third
patches update users of patch_instruction() in ftrace and kprobes code
to properly validate return value from patch_instruction() and to notify
errors.
- Naveen
Naveen N. Rao (3):
powerpc: Properly return error code from do_patch_instruction()
powerpc/ftrace: Simplify error checking when patching instructions
powerpc/kprobes: Check return value of patch_instruction()
arch/powerpc/kernel/kprobes.c | 10 ++-
arch/powerpc/kernel/optprobes.c | 99 ++++++++++++++++++++++++------
arch/powerpc/kernel/trace/ftrace.c | 69 ++++++++++-----------
arch/powerpc/lib/code-patching.c | 6 +-
4 files changed, 123 insertions(+), 61 deletions(-)
base-commit: 8299da600ad05b8aa0f15ec0f5f03bd40e37d6f0
--
2.25.1
^ permalink raw reply
* Re: [PATCH][next] ASoC: fsl_easrc: fix spelling mistake "prefitler" -> "prefilter"
From: Mark Brown @ 2020-04-23 14:45 UTC (permalink / raw)
To: alsa-devel, linuxppc-dev, Timur Tabi, Liam Girdwood, Nicolin Chen,
Jaroslav Kysela, Xiubo Li, Fabio Estevam, Takashi Iwai,
Colin King
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20200423083922.8159-1-colin.king@canonical.com>
On Thu, 23 Apr 2020 09:39:22 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a spelling mistake in a deb_dbg message, fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> sound/soc/fsl/fsl_easrc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.8
Thanks!
[1/1] ASoC: fsl_easrc: fix spelling mistake "prefitler" -> "prefilter"
commit: 76ec4aea9fd8117f064caa63ee6f7fbcb70eeb2c
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH 04/29] staging: media: ipu3: use vmap instead of reimplementing it
From: Sakari Ailus @ 2020-04-23 10:32 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-hyperv, David Airlie, dri-devel, linux-mm, K. Y. Srinivasan,
Sumit Semwal, linux-arch, linux-s390, Wei Liu, Stephen Hemminger,
x86, Peter Zijlstra, Laura Abbott, Nitin Gupta, Haiyang Zhang,
linaro-mm-sig, bpf, linux-arm-kernel, Robin Murphy, linux-kernel,
Minchan Kim, iommu, Daniel Vetter, Andrew Morton, linuxppc-dev
In-Reply-To: <20200414131348.444715-5-hch@lst.de>
On Tue, Apr 14, 2020 at 03:13:23PM +0200, Christoph Hellwig wrote:
> Just use vmap instead of messing with vmalloc internals.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Thanks!
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
^ permalink raw reply
* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Adhemerval Zanella @ 2020-04-23 12:13 UTC (permalink / raw)
To: Rich Felker, Nicholas Piggin; +Cc: libc-dev, libc-alpha, linuxppc-dev, musl
In-Reply-To: <20200423023642.GP11469@brightrain.aerifal.cx>
On 22/04/2020 23:36, Rich Felker wrote:
> On Wed, Apr 22, 2020 at 04:18:36PM +1000, Nicholas Piggin wrote:
>> Yeah I had a bit of a play around with musl (which is very nice code I
>> must say). The powerpc64 syscall asm is missing ctr clobber by the way.
>> Fortunately adding it doesn't change code generation for me, but it
>> should be fixed. glibc had the same bug at one point I think (probably
>> due to syscall ABI documentation not existing -- something now lives in
>> linux/Documentation/powerpc/syscall64-abi.rst).
>
> Do you know anywhere I can read about the ctr issue, possibly the
> relevant glibc bug report? I'm not particularly familiar with ppc
> register file (at least I have to refamiliarize myself every time I
> work on this stuff) so it'd be nice to understand what's
> potentially-wrong now.
My understanding is the ctr issue only happens for vDSO calls where it
fallback to a syscall in case an error (invalid argument, etc. and
assuming if vDSO does not fallback to a syscall it always succeed).
This makes the vDSO call on powerpc to have same same ABI constraint
as a syscall, where it clobbers CR0.
On glibc we handle by simulating a function call and analysing the CR0
result:
__asm__ __volatile__
("mtctr %0\n\t"
"bctrl\n\t"
"mfcr %0\n\t"
"0:"
: "+r" (r0), "+r" (r3), "+r" (r4), "+r" (r5), "+r" (r6),
"+r" (r7), "+r" (r8)
: : "r9", "r10", "r11", "r12", "cr0", "ctr", "lr", "memory");
__asm__ __volatile__ ("" : "=r" (rval) : "r" (r3));
On musl you don't have this issue because it does not enable vDSO
support on powerpc. And if it eventually does it with the VDSO_*
macros the only issue I see is on when vDSO fallbacks to the syscall
and it also fails (the return code won't be negated since on musl it
uses a default C function pointer issue which does not model the CR0
kernel abi).
So I think the extra ctr constraint on glibc powerpc syscall code is
not really required. I think I have some patches to optimize this a
bit based on previous discussions.
^ permalink raw reply
* Re: [PATCH v2 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Tianjia Zhang @ 2020-04-23 11:11 UTC (permalink / raw)
To: Christian Borntraeger, Cornelia Huck
Cc: wanpengli, kvm, david, heiko.carstens, peterx, linux-mips, hpa,
kvmarm, linux-s390, frankja, maz, joro, x86, mingo,
julien.thierry.kdev, thuth, gor, suzuki.poulose, kvm-ppc, bp,
tglx, linux-arm-kernel, jmattson, tsbogend, christoffer.dall,
sean.j.christopherson, linux-kernel, james.morse, pbonzini,
vkuznets, linuxppc-dev
In-Reply-To: <1d73b700-4a20-3d7a-66d1-29b5afa03f4d@de.ibm.com>
On 2020/4/23 19:00, Christian Borntraeger wrote:
>
>
> On 23.04.20 12:58, Tianjia Zhang wrote:
>>
>>
>> On 2020/4/23 18:39, Cornelia Huck wrote:
>>> On Thu, 23 Apr 2020 11:01:43 +0800
>>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>>
>>>> On 2020/4/23 0:04, Cornelia Huck wrote:
>>>>> On Wed, 22 Apr 2020 17:58:04 +0200
>>>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>>>
>>>>>> On 22.04.20 15:45, Cornelia Huck wrote:
>>>>>>> On Wed, 22 Apr 2020 20:58:04 +0800
>>>>>>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>>>>>>
>>>>>>>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>>>>>>>> structure. Earlier than historical reasons, many kvm-related function
>>>>>>>
>>>>>>> s/Earlier than/For/ ?
>>>>>>>
>>>>>>>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>>>>>>>> This patch does a unified cleanup of these remaining redundant parameters.
>>>>>>>>
>>>>>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>>>>>> ---
>>>>>>>> arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++---------------
>>>>>>>> 1 file changed, 22 insertions(+), 15 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>>>> index e335a7e5ead7..d7bb2e7a07ff 100644
>>>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>>>> @@ -4176,8 +4176,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>>>>>>>> return rc;
>>>>>>>> }
>>>>>>>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>>>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
>>>>>>>> {
>>>>>>>> + struct kvm_run *kvm_run = vcpu->run;
>>>>>>>> struct runtime_instr_cb *riccb;
>>>>>>>> struct gs_cb *gscb;
>>>>>>>> @@ -4235,7 +4236,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>>>> }
>>>>>>>> if (vcpu->arch.gs_enabled) {
>>>>>>>> current->thread.gs_cb = (struct gs_cb *)
>>>>>>>> - &vcpu->run->s.regs.gscb;
>>>>>>>> + &kvm_run->s.regs.gscb;
>>>>>>>
>>>>>>> Not sure if these changes (vcpu->run-> => kvm_run->) are really worth
>>>>>>> it. (It seems they amount to at least as much as the changes advertised
>>>>>>> in the patch description.)
>>>>>>>
>>>>>>> Other opinions?
>>>>>>
>>>>>> Agreed. It feels kind of random. Maybe just do the first line (move kvm_run from the
>>>>>> function parameter list into the variable declaration)? Not sure if this is better.
>>>>>>
>>>>>
>>>>> There's more in this patch that I cut... but I think just moving
>>>>> kvm_run from the parameter list would be much less disruptive.
>>>>>
>>>>
>>>> I think there are two kinds of code(`vcpu->run->` and `kvm_run->`), but
>>>> there will be more disruptive, not less.
>>>
>>> I just fail to see the benefit; sure, kvm_run-> is convenient, but the
>>> current code is just fine, and any rework should be balanced against
>>> the cost (e.g. cluttering git annotate).
>>>
>>
>> cluttering git annotate ? Does it mean Fix xxxx ("comment"). Is it possible to solve this problem by splitting this patch?
>
> No its about breaking git blame (and bugfix backports) for just a cosmetic improvement.
> And I agree with Conny: the cost is higher than the benefit.
>
I will make a fix in the v3 version. Help to see if there are problems
with the next few patches.
Thanks,
Tianjia
^ permalink raw reply
* Re: [PATCH v2 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Christian Borntraeger @ 2020-04-23 11:00 UTC (permalink / raw)
To: Tianjia Zhang, Cornelia Huck
Cc: wanpengli, kvm, david, heiko.carstens, peterx, linux-mips, hpa,
kvmarm, linux-s390, frankja, maz, joro, x86, mingo,
julien.thierry.kdev, thuth, gor, suzuki.poulose, kvm-ppc, bp,
tglx, linux-arm-kernel, jmattson, tsbogend, christoffer.dall,
sean.j.christopherson, linux-kernel, james.morse, pbonzini,
vkuznets, linuxppc-dev
In-Reply-To: <71344f73-c34f-a373-49d1-5d839c6be5f6@linux.alibaba.com>
On 23.04.20 12:58, Tianjia Zhang wrote:
>
>
> On 2020/4/23 18:39, Cornelia Huck wrote:
>> On Thu, 23 Apr 2020 11:01:43 +0800
>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>
>>> On 2020/4/23 0:04, Cornelia Huck wrote:
>>>> On Wed, 22 Apr 2020 17:58:04 +0200
>>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>>
>>>>> On 22.04.20 15:45, Cornelia Huck wrote:
>>>>>> On Wed, 22 Apr 2020 20:58:04 +0800
>>>>>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>>>>>
>>>>>>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>>>>>>> structure. Earlier than historical reasons, many kvm-related function
>>>>>>
>>>>>> s/Earlier than/For/ ?
>>>>>>
>>>>>>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>>>>>>> This patch does a unified cleanup of these remaining redundant parameters.
>>>>>>>
>>>>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>>>>> ---
>>>>>>> arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++---------------
>>>>>>> 1 file changed, 22 insertions(+), 15 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>>> index e335a7e5ead7..d7bb2e7a07ff 100644
>>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>>> @@ -4176,8 +4176,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>>>>>>> return rc;
>>>>>>> }
>>>>>>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
>>>>>>> {
>>>>>>> + struct kvm_run *kvm_run = vcpu->run;
>>>>>>> struct runtime_instr_cb *riccb;
>>>>>>> struct gs_cb *gscb;
>>>>>>> @@ -4235,7 +4236,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>>> }
>>>>>>> if (vcpu->arch.gs_enabled) {
>>>>>>> current->thread.gs_cb = (struct gs_cb *)
>>>>>>> - &vcpu->run->s.regs.gscb;
>>>>>>> + &kvm_run->s.regs.gscb;
>>>>>>
>>>>>> Not sure if these changes (vcpu->run-> => kvm_run->) are really worth
>>>>>> it. (It seems they amount to at least as much as the changes advertised
>>>>>> in the patch description.)
>>>>>>
>>>>>> Other opinions?
>>>>>
>>>>> Agreed. It feels kind of random. Maybe just do the first line (move kvm_run from the
>>>>> function parameter list into the variable declaration)? Not sure if this is better.
>>>>>
>>>>
>>>> There's more in this patch that I cut... but I think just moving
>>>> kvm_run from the parameter list would be much less disruptive.
>>>>
>>>
>>> I think there are two kinds of code(`vcpu->run->` and `kvm_run->`), but
>>> there will be more disruptive, not less.
>>
>> I just fail to see the benefit; sure, kvm_run-> is convenient, but the
>> current code is just fine, and any rework should be balanced against
>> the cost (e.g. cluttering git annotate).
>>
>
> cluttering git annotate ? Does it mean Fix xxxx ("comment"). Is it possible to solve this problem by splitting this patch?
No its about breaking git blame (and bugfix backports) for just a cosmetic improvement.
And I agree with Conny: the cost is higher than the benefit.
^ permalink raw reply
* Re: [PATCH v2 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Tianjia Zhang @ 2020-04-23 10:58 UTC (permalink / raw)
To: Cornelia Huck
Cc: wanpengli, kvm, david, heiko.carstens, peterx, linux-mips, hpa,
kvmarm, linux-s390, frankja, maz, joro, x86,
Christian Borntraeger, mingo, julien.thierry.kdev, thuth, gor,
suzuki.poulose, kvm-ppc, bp, tglx, linux-arm-kernel, jmattson,
tsbogend, christoffer.dall, sean.j.christopherson, linux-kernel,
james.morse, pbonzini, vkuznets, linuxppc-dev
In-Reply-To: <20200423123901.72a4c6a4.cohuck@redhat.com>
On 2020/4/23 18:39, Cornelia Huck wrote:
> On Thu, 23 Apr 2020 11:01:43 +0800
> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>
>> On 2020/4/23 0:04, Cornelia Huck wrote:
>>> On Wed, 22 Apr 2020 17:58:04 +0200
>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>
>>>> On 22.04.20 15:45, Cornelia Huck wrote:
>>>>> On Wed, 22 Apr 2020 20:58:04 +0800
>>>>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
>>>>>
>>>>>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>>>>>> structure. Earlier than historical reasons, many kvm-related function
>>>>>
>>>>> s/Earlier than/For/ ?
>>>>>
>>>>>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>>>>>> This patch does a unified cleanup of these remaining redundant parameters.
>>>>>>
>>>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>>>> ---
>>>>>> arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++---------------
>>>>>> 1 file changed, 22 insertions(+), 15 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>> index e335a7e5ead7..d7bb2e7a07ff 100644
>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>> @@ -4176,8 +4176,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>>>>>> return rc;
>>>>>> }
>>>>>>
>>>>>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
>>>>>> {
>>>>>> + struct kvm_run *kvm_run = vcpu->run;
>>>>>> struct runtime_instr_cb *riccb;
>>>>>> struct gs_cb *gscb;
>>>>>>
>>>>>> @@ -4235,7 +4236,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>>>>> }
>>>>>> if (vcpu->arch.gs_enabled) {
>>>>>> current->thread.gs_cb = (struct gs_cb *)
>>>>>> - &vcpu->run->s.regs.gscb;
>>>>>> + &kvm_run->s.regs.gscb;
>>>>>
>>>>> Not sure if these changes (vcpu->run-> => kvm_run->) are really worth
>>>>> it. (It seems they amount to at least as much as the changes advertised
>>>>> in the patch description.)
>>>>>
>>>>> Other opinions?
>>>>
>>>> Agreed. It feels kind of random. Maybe just do the first line (move kvm_run from the
>>>> function parameter list into the variable declaration)? Not sure if this is better.
>>>>
>>>
>>> There's more in this patch that I cut... but I think just moving
>>> kvm_run from the parameter list would be much less disruptive.
>>>
>>
>> I think there are two kinds of code(`vcpu->run->` and `kvm_run->`), but
>> there will be more disruptive, not less.
>
> I just fail to see the benefit; sure, kvm_run-> is convenient, but the
> current code is just fine, and any rework should be balanced against
> the cost (e.g. cluttering git annotate).
>
cluttering git annotate ? Does it mean Fix xxxx ("comment"). Is it
possible to solve this problem by splitting this patch?
^ permalink raw reply
* [PATCH v8 3/3] Self save API integration
From: Pratik Rajesh Sampat @ 2020-04-23 10:54 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
pratik.r.sampat, psampat
In-Reply-To: <20200423105438.29034-1-psampat@linux.ibm.com>
The commit makes the self save API available outside the firmware by defining
an OPAL wrapper.
This wrapper has a similar interface to that of self restore and expects the
cpu pir, SPR number, minus the value of that SPR to be passed in its
paramters and returns OPAL_SUCCESS on success. It adds a device-tree
node signifying support for self-save after verifying the stop API
version compatibility.
The commit also documents both the self-save and the self-restore API
calls along with their working and usage.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
doc/opal-api/opal-slw-self-save-reg-181.rst | 51 ++++++++++
doc/opal-api/opal-slw-set-reg-100.rst | 5 +
doc/power-management.rst | 48 +++++++++
hw/slw.c | 106 ++++++++++++++++++++
include/opal-api.h | 3 +-
include/p9_stop_api.H | 18 ++++
include/skiboot.h | 3 +
7 files changed, 233 insertions(+), 1 deletion(-)
create mode 100644 doc/opal-api/opal-slw-self-save-reg-181.rst
diff --git a/doc/opal-api/opal-slw-self-save-reg-181.rst b/doc/opal-api/opal-slw-self-save-reg-181.rst
new file mode 100644
index 00000000..f20e9b81
--- /dev/null
+++ b/doc/opal-api/opal-slw-self-save-reg-181.rst
@@ -0,0 +1,51 @@
+.. OPAL_SLW_SELF_SAVE_REG:
+
+OPAL_SLW_SELF_SAVE_REG
+======================
+
+.. code-block:: c
+
+ #define OPAL_SLW_SELF_SAVE_REG 181
+
+ int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
+
+:ref:`OPAL_SLW_SELF_SAVE_REG` is used to inform low-level firmware to save
+the current contents of the SPR before entering a state of loss and
+also restore the content back on waking up from a deep stop state.
+
+An OPAL call `OPAL_SLW_SET_REG` exists which is similar in function as
+saving and restoring the SPR, with one difference being that the value of the
+SPR must also be supplied in the parameters.
+Complete reference: doc/opal-api/opal-slw-set-reg-100.rst
+
+Parameters
+----------
+
+``uint64_t cpu_pir``
+ This parameter specifies the pir of the cpu for which the call is being made.
+``uint64_t sprn``
+ This parameter specifies the spr number as mentioned in p9_stop_api.H
+ The list of SPRs supported is as follows.
+ P9_STOP_SPR_DAWR,
+ P9_STOP_SPR_HSPRG0,
+ P9_STOP_SPR_LDBAR,
+ P9_STOP_SPR_LPCR,
+ P9_STOP_SPR_PSSCR,
+ P9_STOP_SPR_MSR,
+ P9_STOP_SPR_HRMOR,
+ P9_STOP_SPR_HMEER,
+ P9_STOP_SPR_PMCR,
+ P9_STOP_SPR_PTCR
+
+ The property "ibm,opal-self-save" is supplied to the device tree to advterise
+ support.
+
+Returns
+-------
+
+:ref:`OPAL_UNSUPPORTED`
+ If spr restore is not supported by pore engine.
+:ref:`OPAL_PARAMETER`
+ Invalid handle for the pir/chip
+:ref:`OPAL_SUCCESS`
+ On success
diff --git a/doc/opal-api/opal-slw-set-reg-100.rst b/doc/opal-api/opal-slw-set-reg-100.rst
index 2e8f1bd6..ee3e68ce 100644
--- a/doc/opal-api/opal-slw-set-reg-100.rst
+++ b/doc/opal-api/opal-slw-set-reg-100.rst
@@ -21,6 +21,11 @@ In Power 9, it uses p9_stop_save_cpureg(), api provided by self restore code,
to inform the spr with their corresponding values with which they
must be restored.
+An OPAL call `OPAL_SLW_SELF_SAVE_REG` exists which is similar in function
+saving and restoring the SPR, with one difference being that the value of the
+SPR doesn't need to be passed in the parameters, only with the SPR number
+the firmware can identify, save and restore the values for the same.
+Complete reference: doc/opal-api/opal-slw-self-save-reg-181.rst
Parameters
----------
diff --git a/doc/power-management.rst b/doc/power-management.rst
index 76491a71..d6bd5358 100644
--- a/doc/power-management.rst
+++ b/doc/power-management.rst
@@ -15,3 +15,51 @@ On boot, specific stop states can be disabled via setting a mask. For example,
to disable all but stop 0,1,2, use ~0xE0000000. ::
nvram -p ibm,skiboot --update-config opal-stop-state-disable-mask=0x1FFFFFFF
+
+Saving and restoring Special Purpose Registers(SPRs)
+----------------------------------------------------
+
+When a CPU wakes up from a deep stop state which can result in
+hypervisor state loss, all the SPRs are lost. The Linux Kernel expects
+a small set of SPRs to contain an expected value when the CPU wakes up
+from such a deep stop state. The microcode firmware provides the
+following two APIs, collectively known as the stop-APIs, to allow the
+kernel/OPAL to specify this set of SPRs and the value that they need
+to be restored with on waking up from a deep stop state.
+
+Self-restore:
+int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
+The SPR number and the value of the that SPR must be restored with on
+wakeup from the deep-stop state must be specified. When this call is
+made, the microcode inserts instruction into the HOMER region to
+restore the content of the SPR to the specified value on wakeup from a
+deep-stop state. These instructions are executed by the CPU as soon as
+it wakes up from a deep stop state. The call is to be made once per
+SPR.
+
+Self-Save:
+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
+Only the SPR number needs to be specified. When this call is made, the
+microcode inserts instructions into the HOMER region to save the
+current value of the SPR before the CPU goes to a deep stop state, and
+restores the value back when the CPU wakes up from a deep stop state.
+These instructions are correspondingly executed just before and after
+the CPU goes/comes out of a deep stop state. This call can be made
+once per SPR.
+
+The key difference between self-save and self-restore is the
+use-case. If the Kernel expects the SPR to contain a particular value
+on waking up from a deep-stop state, that wasn't the value of that SPR
+before entering deep stop-state, then self-restore is preferable.
+Also in a case where SPR does not change across the lifetime
+self-restore is more efficient as when the value is same the memeory location
+is not updated.
+
+When deep stop states are to be supported in an Ultravisor
+environment, since HOMER is in a secure region, the stop-api cannot
+update the HOMER if invoked from a context when the OPAL/Kernel is
+executing without the ultravisor privilege. In this scenario, at the
+time of early OPAL boot, while OPAL has ultravisor privileges, it can
+make the self-save stop-api call for all the supported SPRs, so that
+the microcode in the HOMER will always save and restore all the
+supported SPRs during entry/exit from a deep stop state.
diff --git a/hw/slw.c b/hw/slw.c
index beb129a8..8423eeaf 100644
--- a/hw/slw.c
+++ b/hw/slw.c
@@ -35,6 +35,43 @@ static bool slw_current_le = false;
enum wakeup_engine_states wakeup_engine_state = WAKEUP_ENGINE_NOT_PRESENT;
bool has_deep_states = false;
+/**
+ * The struct and SPR list is partially consistent with libpore/p9_stop_api.c
+ */
+/**
+ * @brief summarizes attributes associated with a SPR register.
+ */
+typedef struct
+{
+ uint32_t iv_sprId;
+ bool iv_isThreadScope;
+ uint32_t iv_saveMaskPos;
+
+} StopSprReg_t;
+
+/**
+ * @brief a true in the table below means register is of scope thread
+ * whereas a false meanse register is of scope core.
+ * The number is the bit position on a uint32_t mask
+ */
+
+static const StopSprReg_t g_sprRegister[] =
+{
+ { P9_STOP_SPR_DAWR, true, 1 },
+ { P9_STOP_SPR_HSPRG0, true, 3 },
+ { P9_STOP_SPR_LDBAR, true, 4, },
+ { P9_STOP_SPR_LPCR, true, 5 },
+ { P9_STOP_SPR_PSSCR, true, 6 },
+ { P9_STOP_SPR_MSR, true, 7 },
+ { P9_STOP_SPR_HRMOR, false, 255 },
+ { P9_STOP_SPR_HID, false, 21 },
+ { P9_STOP_SPR_HMEER, false, 22 },
+ { P9_STOP_SPR_PMCR, false, 23 },
+ { P9_STOP_SPR_PTCR, false, 24 },
+};
+
+static const uint32_t MAX_SPR_SUPPORTED = ARRAY_SIZE(g_sprRegister);
+
DEFINE_LOG_ENTRY(OPAL_RC_SLW_INIT, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
OPAL_PLATFORM_FIRMWARE, OPAL_PREDICTIVE_ERR_GENERAL,
OPAL_NA);
@@ -720,11 +757,14 @@ void add_cpu_idle_state_properties(void)
struct cpu_idle_states *states;
struct proc_chip *chip;
int nr_states;
+ int rc;
bool can_sleep = true;
bool has_stop_inst = false;
+ bool has_self_save = true;
u8 i;
+ u64 compVector = -1;
fdt64_t *pm_ctrl_reg_val_buf;
fdt64_t *pm_ctrl_reg_mask_buf;
u32 supported_states_mask;
@@ -766,6 +806,20 @@ void add_cpu_idle_state_properties(void)
*/
chip = next_chip(NULL);
assert(chip);
+ rc = proc_stop_api_discover_capability((void *) chip->homer_base,
+ &compVector);
+ if (rc == STOP_SAVE_ARG_INVALID_IMG) {
+ prlog(PR_DEBUG, "HOMER BASE INVALID\n");
+ return;
+ } else if (rc == STOP_SAVE_API_IMG_INCOMPATIBLE) {
+ prlog(PR_DEBUG, "STOP API running incompatible versions\n");
+ if ((compVector & SELF_RESTORE_VER_MISMATCH) == 0) {
+ prlog(PR_DEBUG, "Self-save API unsupported\n");
+ has_self_save = false;
+ }
+ }
+ if (has_self_save)
+ dt_add_property(power_mgt, "ibm,opal-self-save", NULL, 0);
if (chip->type == PROC_CHIP_P9_NIMBUS ||
chip->type == PROC_CHIP_P9_CUMULUS ||
chip->type == PROC_CHIP_P9P) {
@@ -1446,6 +1500,58 @@ int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)
opal_call(OPAL_SLW_SET_REG, opal_slw_set_reg, 3);
+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn)
+{
+ struct cpu_thread * c = find_cpu_by_pir(cpu_pir);
+ uint32_t save_reg_vector = 0;
+ struct proc_chip * chip;
+ int rc;
+ int index;
+
+ if (!c) {
+ prlog(PR_DEBUG, "SLW: Unknown thread with pir %x\n",
+ (u32) cpu_pir);
+ return OPAL_PARAMETER;
+ }
+
+ chip = get_chip(c->chip_id);
+ if (!chip) {
+ prlog(PR_DEBUG, "SLW: Unknown chip for thread with pir %x\n",
+ (u32) cpu_pir);
+ return OPAL_PARAMETER;
+ }
+ if (proc_gen != proc_gen_p9 || !has_deep_states) {
+ prlog(PR_DEBUG, "SLW: Self-save feature unsupported\n");
+ return OPAL_UNSUPPORTED;
+ }
+ if (wakeup_engine_state != WAKEUP_ENGINE_PRESENT) {
+ log_simple_error(&e_info(OPAL_RC_SLW_REG),
+ "SLW: wakeup_engine in bad state=%d chip=%x\n",
+ wakeup_engine_state, chip->id);
+ return OPAL_INTERNAL_ERROR;
+ }
+ for (index = 0; index < MAX_SPR_SUPPORTED; ++index) {
+ if (sprn == (CpuReg_t) g_sprRegister[index].iv_sprId) {
+ save_reg_vector = PPC_BIT32(
+ g_sprRegister[index].iv_saveMaskPos);
+ break;
+ }
+ }
+ if (save_reg_vector == 0)
+ return OPAL_INTERNAL_ERROR;
+ rc = p9_stop_save_cpureg_control((void *) chip->homer_base,
+ cpu_pir, save_reg_vector);
+
+ if (rc) {
+ log_simple_error(&e_info(OPAL_RC_SLW_REG),
+ "SLW: Failed to save vector %x for CPU %x\n",
+ save_reg_vector, c->pir);
+ return OPAL_INTERNAL_ERROR;
+ }
+ return OPAL_SUCCESS;
+}
+opal_call(OPAL_SLW_SELF_SAVE_REG, opal_slw_self_save_reg, 2);
+
void slw_init(void)
{
struct proc_chip *chip;
diff --git a/include/opal-api.h b/include/opal-api.h
index e90cab1e..1607a89b 100644
--- a/include/opal-api.h
+++ b/include/opal-api.h
@@ -227,7 +227,8 @@
#define OPAL_SECVAR_ENQUEUE_UPDATE 178
#define OPAL_PHB_SET_OPTION 179
#define OPAL_PHB_GET_OPTION 180
-#define OPAL_LAST 180
+#define OPAL_SLW_SELF_SAVE_REG 181
+#define OPAL_LAST 181
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
diff --git a/include/p9_stop_api.H b/include/p9_stop_api.H
index cb5ffd6f..09ce3dc1 100644
--- a/include/p9_stop_api.H
+++ b/include/p9_stop_api.H
@@ -34,6 +34,8 @@
///
/// @file p9_stop_api.H
/// @brief describes STOP API which create/manipulate STOP image.
+/// This header need not be consistent, however is a subset of the
+/// libpore/p9_stop_api.H counterpart
///
// *HWP HW Owner : Greg Still <stillgs@us.ibm.com>
// *HWP FW Owner : Prem Shanker Jha <premjha2@in.ibm.com>
@@ -58,6 +60,7 @@ typedef enum
P9_STOP_SPR_HRMOR = 313, // core register
P9_STOP_SPR_LPCR = 318, // thread register
P9_STOP_SPR_HMEER = 337, // core register
+ P9_STOP_SPR_PTCR = 464, // core register
P9_STOP_SPR_LDBAR = 850, // thread register
P9_STOP_SPR_PSSCR = 855, // thread register
P9_STOP_SPR_PMCR = 884, // core register
@@ -247,6 +250,21 @@ StopReturnCode_t p9_stop_save_scom( void* const i_pImage,
const ScomOperation_t i_operation,
const ScomSection_t i_section );
+/**
+ * @brief Facilitates self save and restore of a list of SPRs of a thread.
+ * @param[in] i_pImage points to the start of HOMER image of P9 chip.
+ * @param[in] i_pir PIR associated with thread
+ * @param[in] i_saveRegVector bit vector representing SPRs that needs to be restored.
+ * @return STOP_SAVE_SUCCESS if API succeeds, error code otherwise.
+ * @note SPR save vector is a bit vector. For each SPR supported,
+ * there is an associated bit position in the bit vector.Refer
+ * to definition of SprBitPositionList_t to determine bit position
+ * associated with a particular SPR.
+ */
+StopReturnCode_t
+p9_stop_save_cpureg_control( void* i_pImage, const uint64_t i_pir,
+ const uint32_t i_saveRegVector );
+
/**
* @brief verifies if API is compatible of current HOMER image.
* @param[in] i_pImage points to the start of HOMER image of P9 chip.
diff --git a/include/skiboot.h b/include/skiboot.h
index 30ff500c..9ced240e 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -306,6 +306,9 @@ extern void nx_p9_rng_late_init(void);
/* SLW reinit function for switching core settings */
extern int64_t slw_reinit(uint64_t flags);
+/* Self save SPR before entering the stop state */
+extern int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
+
/* Patch SPR in SLW image */
extern int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
--
2.25.1
^ permalink raw reply related
* [PATCH v8 1/1] powerpc/powernv: Introduce support and parsing for self-save API
From: Pratik Rajesh Sampat @ 2020-04-23 10:55 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
pratik.r.sampat, psampat
In-Reply-To: <20200423105557.29108-1-psampat@linux.ibm.com>
This commit introduces and leverages the Self save API. The difference
between self-save and self-restore is that the value to be saved for the
SPR does not need to be passed to the call.
Add the new Self Save OPAL API call in the list of OPAL calls.
The device tree is parsed looking for the property "ibm,opal-self-save"
If self-save is supported then for all SPRs self-save is invoked for all
P9 supported registers. In the case self-save fails corresponding
self-restore call is invoked as a fallback.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/include/asm/opal-api.h | 3 +-
arch/powerpc/include/asm/opal.h | 1 +
arch/powerpc/platforms/powernv/idle.c | 73 ++++++++++++++++++----
arch/powerpc/platforms/powernv/opal-call.c | 1 +
4 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index 1dffa3cb16ba..7ba698369083 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -214,7 +214,8 @@
#define OPAL_SECVAR_GET 176
#define OPAL_SECVAR_GET_NEXT 177
#define OPAL_SECVAR_ENQUEUE_UPDATE 178
-#define OPAL_LAST 178
+#define OPAL_SLW_SELF_SAVE_REG 181
+#define OPAL_LAST 181
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9986ac34b8e2..a370b0e8d899 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -204,6 +204,7 @@ int64_t opal_handle_hmi2(__be64 *out_flags);
int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
int64_t opal_unregister_dump_region(uint32_t id);
int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
int64_t opal_pci_get_pbcq_tunnel_bar(uint64_t phb_id, uint64_t *addr);
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 78599bca66c2..ada7ece24521 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -32,6 +32,11 @@
#define P9_STOP_SPR_MSR 2000
#define P9_STOP_SPR_PSSCR 855
+/* Caching the self-save functionality, lpcr, ptcr support */
+DEFINE_STATIC_KEY_FALSE(self_save_available);
+DEFINE_STATIC_KEY_FALSE(is_lpcr_self_save);
+DEFINE_STATIC_KEY_FALSE(is_ptcr_self_save);
+
static u32 supported_cpuidle_states;
struct pnv_idle_states_t *pnv_idle_states;
int nr_pnv_idle_states;
@@ -61,6 +66,35 @@ static bool deepest_stop_found;
static unsigned long power7_offline_type;
+/*
+ * Cache support for SPRs that support self-save as well as kernel save restore
+ * so that kernel does not duplicate efforts in saving and restoring SPRs
+ */
+static void cache_spr_self_save_support(u64 sprn)
+{
+ switch (sprn) {
+ case SPRN_LPCR:
+ static_branch_enable(&is_lpcr_self_save);
+ break;
+ case SPRN_PTCR:
+ static_branch_enable(&is_ptcr_self_save);
+ break;
+ }
+}
+
+static int pnv_save_one_spr(u64 pir, u64 sprn, u64 val)
+{
+ if (static_branch_likely(&self_save_available)) {
+ int rc = opal_slw_self_save_reg(pir, sprn);
+
+ if (!rc) {
+ cache_spr_self_save_support(sprn);
+ return rc;
+ }
+ }
+ return opal_slw_set_reg(pir, sprn, val);
+}
+
static int pnv_save_sprs_for_deep_states(void)
{
int cpu;
@@ -72,6 +106,7 @@ static int pnv_save_sprs_for_deep_states(void)
* same across all cpus.
*/
uint64_t lpcr_val = mfspr(SPRN_LPCR);
+ uint64_t ptcr_val = mfspr(SPRN_PTCR);
uint64_t hid0_val = mfspr(SPRN_HID0);
uint64_t hid1_val = mfspr(SPRN_HID1);
uint64_t hid4_val = mfspr(SPRN_HID4);
@@ -84,30 +119,34 @@ static int pnv_save_sprs_for_deep_states(void)
uint64_t pir = get_hard_smp_processor_id(cpu);
uint64_t hsprg0_val = (uint64_t)paca_ptrs[cpu];
- rc = opal_slw_set_reg(pir, SPRN_HSPRG0, hsprg0_val);
+ rc = pnv_save_one_spr(pir, SPRN_HSPRG0, hsprg0_val);
if (rc != 0)
return rc;
- rc = opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+ rc = pnv_save_one_spr(pir, SPRN_LPCR, lpcr_val);
if (rc != 0)
return rc;
+ /*
+ * No need to check for failure, if firmware fails to save then
+ * kernel handles save-restore for PTCR
+ */
+ pnv_save_one_spr(pir, SPRN_PTCR, ptcr_val);
+
if (cpu_has_feature(CPU_FTR_ARCH_300)) {
- rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
+ rc = pnv_save_one_spr(pir, P9_STOP_SPR_MSR, msr_val);
if (rc)
return rc;
- rc = opal_slw_set_reg(pir,
+ rc = pnv_save_one_spr(pir,
P9_STOP_SPR_PSSCR, psscr_val);
-
if (rc)
return rc;
}
/* HIDs are per core registers */
if (cpu_thread_in_core(cpu) == 0) {
-
- rc = opal_slw_set_reg(pir, SPRN_HMEER, hmeer_val);
+ rc = pnv_save_one_spr(pir, SPRN_HMEER, hmeer_val);
if (rc != 0)
return rc;
@@ -658,7 +697,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
mmcr0 = mfspr(SPRN_MMCR0);
}
if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
- sprs.lpcr = mfspr(SPRN_LPCR);
+ if (!static_branch_unlikely(&is_lpcr_self_save))
+ sprs.lpcr = mfspr(SPRN_LPCR);
sprs.hfscr = mfspr(SPRN_HFSCR);
sprs.fscr = mfspr(SPRN_FSCR);
sprs.pid = mfspr(SPRN_PID);
@@ -672,7 +712,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
sprs.mmcr1 = mfspr(SPRN_MMCR1);
sprs.mmcr2 = mfspr(SPRN_MMCR2);
- sprs.ptcr = mfspr(SPRN_PTCR);
+ if (!static_branch_unlikely(&is_ptcr_self_save))
+ sprs.ptcr = mfspr(SPRN_PTCR);
sprs.rpr = mfspr(SPRN_RPR);
sprs.tscr = mfspr(SPRN_TSCR);
if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
@@ -756,7 +797,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
goto core_woken;
/* Per-core SPRs */
- mtspr(SPRN_PTCR, sprs.ptcr);
+ if (!static_branch_unlikely(&is_ptcr_self_save))
+ mtspr(SPRN_PTCR, sprs.ptcr);
mtspr(SPRN_RPR, sprs.rpr);
mtspr(SPRN_TSCR, sprs.tscr);
@@ -777,7 +819,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
atomic_unlock_and_stop_thread_idle();
/* Per-thread SPRs */
- mtspr(SPRN_LPCR, sprs.lpcr);
+ if (!static_branch_unlikely(&is_lpcr_self_save))
+ mtspr(SPRN_LPCR, sprs.lpcr);
mtspr(SPRN_HFSCR, sprs.hfscr);
mtspr(SPRN_FSCR, sprs.fscr);
mtspr(SPRN_PID, sprs.pid);
@@ -956,8 +999,10 @@ void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
* Program the LPCR via stop-api only if the deepest stop state
* can lose hypervisor context.
*/
- if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT)
- opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+ if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT) {
+ if (!static_branch_unlikely(&is_lpcr_self_save))
+ opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+ }
}
/*
@@ -1298,6 +1343,8 @@ static int pnv_parse_cpuidle_dt(void)
}
for (i = 0; i < nr_idle_states; i++)
pnv_idle_states[i].psscr_mask = temp_u64[i];
+ if (of_property_read_bool(np, "ibm,opal-self-save"))
+ static_branch_enable(&self_save_available);
}
/*
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 5cd0f52d258f..11e0ceb90de0 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -223,6 +223,7 @@ OPAL_CALL(opal_handle_hmi, OPAL_HANDLE_HMI);
OPAL_CALL(opal_handle_hmi2, OPAL_HANDLE_HMI2);
OPAL_CALL(opal_config_cpu_idle_state, OPAL_CONFIG_CPU_IDLE_STATE);
OPAL_CALL(opal_slw_set_reg, OPAL_SLW_SET_REG);
+OPAL_CALL(opal_slw_self_save_reg, OPAL_SLW_SELF_SAVE_REG);
OPAL_CALL(opal_register_dump_region, OPAL_REGISTER_DUMP_REGION);
OPAL_CALL(opal_unregister_dump_region, OPAL_UNREGISTER_DUMP_REGION);
OPAL_CALL(opal_pci_set_phb_cxl_mode, OPAL_PCI_SET_PHB_CAPI_MODE);
--
2.17.1
^ permalink raw reply related
* [PATCH v8 0/1] powerpc/powernv: Introduce support and parsing for self-save API
From: Pratik Rajesh Sampat @ 2020-04-23 10:55 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
pratik.r.sampat, psampat
v7: https://lkml.org/lkml/2020/4/16/247
Changelog
v7 --> v8
Simplified kernel design. Introducing an approach which eliminates
the need for having support and preference for SPRs that need to be
saved. Instead a simple self-save property is advertised and if it
exists then self-save is called otherwise self-restore is resorted to.
Complete specification of the approach is described below in the
cover-letter.
Background
==========
The power management framework on POWER systems include core idle
states that lose context. Deep idle states namely "winkle" on POWER8
and "stop4" and "stop5" on POWER9 can be entered by a CPU to save
different levels of power, as a consequence of which all the
hypervisor resources such as SPRs and SCOMs are lost.
For most SPRs, saving and restoration of content for SPRs and SCOMs
is handled by the hypervisor kernel prior to entering an post exit
from an idle state respectively. However, there is a small set of
critical SPRs and XSCOMs that are expected to contain sane values even
before the control is transferred to the hypervisor kernel at system
reset vector.
For this purpose, microcode firmware provides a mechanism to restore
values on certain SPRs. The communication mechanism between the
hypervisor kernel and the microcode is a standard interface called
sleep-winkle-engine (SLW) on Power8 and Stop-API on Power9 which is
abstracted by OPAL calls from the hypervisor kernel. The Stop-API
provides an interface known as the self-restore API, to which the SPR
number and a predefined value to be restored on wake-up from a deep
stop state is supplied.
Motivation to introduce a new Stop-API
======================================
The self-restore API expects not just the SPR number but also the
value with which the SPR is restored. This is good for those SPRs such
as HSPRG0 whose values do not change at runtime, since for them, the
kernel can invoke the self-restore API at boot time once the values of
these SPRs are determined.
However, there are use-cases where-in the value to be saved cannot be
known or cannot be updated in the layer it currently is.
The shortcomings and the new use-cases which cannot be served by the
existing self-restore API, serves as motivation for a new API:
Shortcoming1:
------------
In a special wakeup scenario, SPRs such as PSSCR, whose values can
change at runtime, are compelled to make the self-restore API call
every time before entering a deep-idle state rendering it to be
prohibitively expensive
Shortcoming2:
------------
The value of LPCR is dynamic based on if the CPU is entered a stop
state during cpu idle versus cpu hotplug.
Today, an additional self-restore call is made before entering
CPU-Hotplug to clear the PECE1 bit in stop-API so that if we are
woken up by a special wakeup on an offlined CPU, we go back to stop
with the the bit cleared.
There is a overhead of an extra call
New Use-case:
-------------
In the case where the hypervisor is running on an
ultravisor environment, the boot time is too late in the cycle to make
the self-restore API calls, as these cannot be invoked from an
non-secure context anymore
To address these shortcomings, the firmware provides another API known
as the self-save API. The self-save API only takes the SPR number as a
parameter and will ensure that on wakeup from a deep-stop state the
SPR is restored with the value that it contained prior to entering the
deep-stop.
Contrast between self-save and self-restore APIs
================================================
Before entering
deep idle |---------------|
------------> | HCODE A |
| |---------------|
|---------| |
| CPU |----|
|---------| |
| |---------------|
|------------>| HCODE B |
On waking up |---------------|
from deep idle
When a self-restore API is invoked, the HCODE inserts instructions
into "HCODE B" region of the above figure to restore the content of
the SPR to the said value. The "HCODE B" region gets executed soon
after the CPU wakes up from a deep idle state, thus executing the
inserted instructions, thereby restoring the contents of the SPRs to
the required values.
When a self-save API is invoked, the HCODE inserts instructions into
the "HCODE A" region of the above figure to save the content of the
SPR into some location in memory. It also inserts instructions into
the "HCODE B" region to restore the content of the SPR to the
corresponding value saved in the memory by the instructions in "HCODE
A" region.
Thus, in contrast with self-restore, the self-save API *does not* need
a value to be passed to it, since it ensures that the value of SPR
before entering deep stop is saved, and subsequently the same value is
restored.
Self-save and self-restore are complementary features since,
self-restore can help in restoring a different value in the SPR on
wakeup from a deep-idle state than what it had before entering the
deep idle state. This was used in POWER8 for HSPRG0 to distinguish a
wakeup from Winkle vs Fastsleep.
Limitations of self-save
========================
Ideally all SPRs should be available for self-save, but HID0 is very
tricky to implement in microcode due to various endianess quirks.
Couple of implementation schemes were buggy and hence HID0 was left
out to be self-restore only.
The fallout of this limitation is as follows:
* In Non PEF environment, no issue. Linux will use self-restore for
HID0 as it does today and no functional impact.
* In PEF environment, the HID0 restore value is decided by OPAL during
boot and it is setup for LE hypervisor with radix MMU. This is the
default and current working configuration of a PEF environment.
However if there is a change, then HV Linux will try to change the
HID0 value to something different than what OPAL decided, at which
time deep-stop states will be disabled under this new PEF
environment.
A simple and workable design is achieved by scoping the power
management deep-stop state support only to a known default PEF
environment. Any deviation will affect *only* deep stop-state support
(stop4,5) in that environment and not have any functional impediment
to the environment itself.
In future, if there is a need to support changing of HID0 to various
values under PEF environment and support deep-stop states, it can be
worked out via an ultravisor call or improve the microcode design to
include HID0 in self-save. These future scheme would be an extension
and does not break or make the current implementation scheme
redundant.
Design Choices
==============
Presenting the design choices in front of us:
Design-Choice 1:
----------------
A simple implementation is to just replace self-restore calls with
self-save as it is direct super-set.
Pros:
A simple design, quick to implement
Cons:
* Breaks backward compatibility. Self-restore has historically been
supported in the firmware and an old firmware running on an new
kernel will be incompatible and deep stop states will be cut.
* Furthermore, critical SPRs which need to be restored
before 0x100 vector like HID0 are not supported by self-save.
Design-Choice 2:
----------------
Advertise both self-restore and self-save from OPAL including the set
of registers that each support. The kernel can then choose which API
to go with.
For the sake of simplicity, in case both modes are supported for an
SPR by default self-save would be called for it.
Pros:
* Backwards compatible
Cons:
Overhead in parsing device tree with the SPR list
Design Choice 3:
----------------
The presence of self-save feature is indicated by the
"ibm,opal-self-save" in the device-tree.
Since self-save API supports most of the SPRs supported by
self-restore, when self-save feature is available, we first attempt
a self-save call for an SPR. If that fails, then we fallback to the
self-restore API.
Pros:
* Backwards compatible
* Minimal complexity
Cons:
* The kernel does not know which registers are supported by the
self-save API, so a try-catch model is used. Although this model is
also employed by the current self-restore API.
The patch chooses design choice 3 as an implementation.
The device tree is parsed looking for the property "ibm,opal-self-save"
If self-save is supported then for all SPRs self-save is invoked for all
P9 supported registers. In the case self-save fails corresponding
self-restore call is invoked as a fallback.
Pratik Rajesh Sampat (1):
powerpc/powernv: Introduce support and parsing for self-save API
arch/powerpc/include/asm/opal-api.h | 3 +-
arch/powerpc/include/asm/opal.h | 1 +
arch/powerpc/platforms/powernv/idle.c | 73 ++++++++++++++++++----
arch/powerpc/platforms/powernv/opal-call.c | 1 +
4 files changed, 64 insertions(+), 14 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v8 1/3] Self Save: Introducing Support for SPR Self Save
From: Pratik Rajesh Sampat @ 2020-04-23 10:54 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
pratik.r.sampat, psampat
In-Reply-To: <20200423105438.29034-1-psampat@linux.ibm.com>
From: Prem Shanker Jha <premjha2@in.ibm.com>
The commit is a merger of commits that makes the following changes:
1. Commit fixes some issues with code found during integration test
- replacement of addi with xor instruction during self save API.
- fixing instruction generation for MFMSR during self save
- data struct updates in STOP API
- error RC updates for hcode image build
- HOMER parser updates.
- removed self save support for URMOR and HRMOR
- code changes for compilation with OPAL
- populating CME Image header with unsecure HOMER address.
Key_Cronus_Test=PM_REGRESS
Change-Id: I7cedcc466267c4245255d8d75c01ed695e316720
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/66580
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: HWSV CI <hwsv-ci+hostboot@us.ibm.com>
Tested-by: PPE CI <ppe-ci+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Cronus HW CI <cronushw-ci+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Gregory S. Still <stillgs@us.ibm.com>
Reviewed-by: RAHUL BATRA <rbatra@us.ibm.com>
Reviewed-by: Jennifer A. Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/66587
Reviewed-by: Christian R. Geddes <crgeddes@us.ibm.com>
Signed-off-by: Prem Shanker Jha <premjha2@in.ibm.com>
Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
2. The commit also incorporates changes that make STOP API project
agnostic changes include defining wrapper functions which call legacy
API. It also adds duplicate enum members which start with prefix PROC
instead of P9.
Key_Cronus_Test=PM_REGRESS
Change-Id: If87970f3e8cf9b507f33eb1be249e03eb3836a5e
RTC: 201128
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/71307
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Tested-by: Cronus HW CI <cronushw-ci+hostboot@us.ibm.com>
Reviewed-by: RANGANATHPRASAD G. BRAHMASAMUDRA <prasadbgr@in.ibm.com>
Reviewed-by: Gregory S. Still <stillgs@us.ibm.com>
Reviewed-by: Jennifer A Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/71314
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Signed-off-by: Prem Shanker Jha <premjha2@in.ibm.com>
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
include/p9_stop_api.H | 79 +-
libpore/p9_cpu_reg_restore_instruction.H | 4 +
libpore/p9_stop_api.C | 954 +++++++++++++----------
libpore/p9_stop_api.H | 115 ++-
libpore/p9_stop_data_struct.H | 4 +-
libpore/p9_stop_util.H | 7 +-
6 files changed, 721 insertions(+), 442 deletions(-)
diff --git a/include/p9_stop_api.H b/include/p9_stop_api.H
index 79abd000..9d3bc1e5 100644
--- a/include/p9_stop_api.H
+++ b/include/p9_stop_api.H
@@ -63,6 +63,26 @@ typedef enum
P9_STOP_SPR_PMCR = 884, // core register
P9_STOP_SPR_HID = 1008, // core register
P9_STOP_SPR_MSR = 2000, // thread register
+
+ //enum members which are project agnostic
+ PROC_STOP_SPR_DAWR = 180, // thread register
+ PROC_STOP_SPR_CIABR = 187, // thread register
+ PROC_STOP_SPR_DAWRX = 188, // thread register
+ PROC_STOP_SPR_HSPRG0 = 304, // thread register
+ PROC_STOP_SPR_HRMOR = 313, // core register
+ PROC_STOP_SPR_LPCR = 318, // thread register
+ PROC_STOP_SPR_HMEER = 337, // core register
+ PROC_STOP_SPR_PTCR = 464, // core register
+ PROC_STOP_SPR_USPRG0 = 496, // thread register
+ PROC_STOP_SPR_USPRG1 = 497, // thread register
+ PROC_STOP_SPR_URMOR = 505, // core register
+ PROC_STOP_SPR_SMFCTRL = 511, // thread register
+ PROC_STOP_SPR_LDBAR = 850, // thread register
+ PROC_STOP_SPR_PSSCR = 855, // thread register
+ PROC_STOP_SPR_PMCR = 884, // core register
+ PROC_STOP_SPR_HID = 1008, // core register
+ PROC_STOP_SPR_MSR = 2000, // thread register
+
} CpuReg_t;
/**
@@ -85,6 +105,8 @@ typedef enum
STOP_SAVE_SCOM_ENTRY_UPDATE_FAILED = 12,
STOP_SAVE_INVALID_FUSED_CORE_STATUS = 13,
STOP_SAVE_FAIL = 14, // for internal failure within firmware.
+ STOP_SAVE_SPR_ENTRY_MISSING = 15,
+ STOP_SAVE_SPR_BIT_POS_RESERVE = 16,
} StopReturnCode_t;
/**
@@ -101,7 +123,20 @@ typedef enum
P9_STOP_SCOM_RESET = 6,
P9_STOP_SCOM_OR_APPEND = 7,
P9_STOP_SCOM_AND_APPEND = 8,
- P9_STOP_SCOM_OP_MAX = 9
+ P9_STOP_SCOM_OP_MAX = 9,
+
+ //enum members which are project agnostic
+ PROC_STOP_SCOM_OP_MIN = 0,
+ PROC_STOP_SCOM_APPEND = 1,
+ PROC_STOP_SCOM_REPLACE = 2,
+ PROC_STOP_SCOM_OR = 3,
+ PROC_STOP_SCOM_AND = 4,
+ PROC_STOP_SCOM_NOOP = 5,
+ PROC_STOP_SCOM_RESET = 6,
+ PROC_STOP_SCOM_OR_APPEND = 7,
+ PROC_STOP_SCOM_AND_APPEND = 8,
+ PROC_STOP_SCOM_OP_MAX = 9,
+
} ScomOperation_t;
/**
@@ -114,9 +149,49 @@ typedef enum
P9_STOP_SECTION_EQ_SCOM = 2,
P9_STOP_SECTION_L2 = 3,
P9_STOP_SECTION_L3 = 4,
- P9_STOP_SECTION_MAX = 5
+ P9_STOP_SECTION_MAX = 5,
+
+ //enum members which are project agnostic
+ PROC_STOP_SECTION_MIN = 0,
+ PROC_STOP_SECTION_CORE_SCOM = 1,
+ PROC_STOP_SECTION_EQ_SCOM = 2,
+ PROC_STOP_SECTION_L2 = 3,
+ PROC_STOP_SECTION_L3 = 4,
+ PROC_STOP_SECTION_MAX = 5,
+
} ScomSection_t;
+
+
+/**
+ * @brief List of major incompatibilities between API version.
+ * @note STOP APIs assumes a specific HOMER layout, certain
+ * level of CME-SGPE hcode and certain version of self-save restore
+ * binary. A mismatch can break STOP function.
+ */
+
+/**
+ * @brief Summarizes bit position allocated to SPRs in save bit mask vector.
+ */
+typedef enum
+{
+ BIT_POS_CIABR = 0,
+ BIT_POS_DAWR = 1,
+ BIT_POS_DAWRX = 2,
+ BIT_POS_HSPRG0 = 3,
+ BIT_POS_LDBAR = 4,
+ BIT_POS_LPCR = 5,
+ BIT_POS_PSSCR = 6,
+ BIT_POS_MSR = 7,
+ BIT_POS_HID = 21,
+ BIT_POS_HMEER = 22,
+ BIT_POS_PMCR = 23,
+ BIT_POS_PTCR = 24,
+ BIT_POS_SMFCTRL = 28,
+ BIT_POS_USPRG0 = 29,
+ BIT_POS_USPRG1 = 30,
+} SprBitPositionList_t;
+
#ifdef __cplusplus
extern "C" {
#endif
diff --git a/libpore/p9_cpu_reg_restore_instruction.H b/libpore/p9_cpu_reg_restore_instruction.H
index cf00ff5e..d69a4212 100644
--- a/libpore/p9_cpu_reg_restore_instruction.H
+++ b/libpore/p9_cpu_reg_restore_instruction.H
@@ -62,6 +62,10 @@ enum
MTSPR_CONST1 = 467,
MTMSRD_CONST1 = 178,
MFSPR_CONST = 339,
+ BLR_INST = 0x4e800020,
+ MTSPR_BASE_OPCODE = 0x7c0003a6,
+ MFSPR_BASE_OPCODE = 0x7c0002a6,
+ ATTN_OPCODE = 0x00000200,
OPCODE_18 = 18,
SELF_SAVE_FUNC_ADD = 0x2300,
SELF_SAVE_OFFSET = 0x180,
diff --git a/libpore/p9_stop_api.C b/libpore/p9_stop_api.C
index 33aaf788..2d9bb549 100644
--- a/libpore/p9_stop_api.C
+++ b/libpore/p9_stop_api.C
@@ -54,33 +54,33 @@ namespace stopImageSection
const StopSprReg_t g_sprRegister[] =
{
- { P9_STOP_SPR_CIABR, true, 0 },
- { P9_STOP_SPR_DAWR, true, 1 },
- { P9_STOP_SPR_DAWRX, true, 2 },
- { P9_STOP_SPR_HSPRG0, true, 3 },
- { P9_STOP_SPR_LDBAR, true, 4, },
- { P9_STOP_SPR_LPCR, true, 5 },
- { P9_STOP_SPR_PSSCR, true, 6 },
- { P9_STOP_SPR_MSR, true, 7 },
- { P9_STOP_SPR_HRMOR, false, 20 },
- { P9_STOP_SPR_HID, false, 21 },
- { P9_STOP_SPR_HMEER, false, 22 },
- { P9_STOP_SPR_PMCR, false, 23 },
- { P9_STOP_SPR_PTCR, false, 24 },
- { P9_STOP_SPR_SMFCTRL, true, 28 },
- { P9_STOP_SPR_USPRG0, true, 29 },
- { P9_STOP_SPR_USPRG1, true, 30 },
- { P9_STOP_SPR_URMOR, false, 31 },
+ { P9_STOP_SPR_CIABR, true, 0 },
+ { P9_STOP_SPR_DAWR, true, 1 },
+ { P9_STOP_SPR_DAWRX, true, 2 },
+ { P9_STOP_SPR_HSPRG0, true, 3 },
+ { P9_STOP_SPR_LDBAR, true, 4, },
+ { P9_STOP_SPR_LPCR, true, 5 },
+ { P9_STOP_SPR_PSSCR, true, 6 },
+ { P9_STOP_SPR_MSR, true, 7 },
+ { P9_STOP_SPR_HRMOR, false, 255 },
+ { P9_STOP_SPR_HID, false, 21 },
+ { P9_STOP_SPR_HMEER, false, 22 },
+ { P9_STOP_SPR_PMCR, false, 23 },
+ { P9_STOP_SPR_PTCR, false, 24 },
+ { P9_STOP_SPR_SMFCTRL, true, 28 },
+ { P9_STOP_SPR_USPRG0, true, 29 },
+ { P9_STOP_SPR_USPRG1, true, 30 },
+ { P9_STOP_SPR_URMOR, false, 255 },
};
-const uint32_t MAX_SPR_SUPPORTED = 17;
+const uint32_t MAX_SPR_SUPPORTED = 17;
const uint32_t LEGACY_CORE_SCOM_SUPPORTED = 15;
const uint32_t LEGACY_QUAD_SCOM_SUPPORTED = 63;
//-----------------------------------------------------------------------------
/**
- * @brief vaildated input arguments passed to p9_stop_save_cpureg_control.
+ * @brief validated input arguments passed to p9_stop_save_cpureg_control.
* @param[in] i_pImage point to start of HOMER
* @param[in] i_coreId id of the core
* @param[in] i_threadId id of the thread
@@ -255,7 +255,7 @@ STATIC uint32_t getOriInstruction( const uint16_t i_Rs, const uint16_t i_Ra,
*/
STATIC uint32_t genKeyForSprLookup( const CpuReg_t i_regId )
{
- return getOriInstruction( 0, 0, (uint16_t) i_regId );
+ return getOriInstruction( 24, 0, (uint16_t) i_regId );
}
//-----------------------------------------------------------------------------
@@ -330,7 +330,7 @@ STATIC uint32_t getMtsprInstruction( const uint16_t i_Rs, const uint16_t i_Spr )
*/
STATIC uint32_t getMfmsrInstruction( const uint16_t i_Rt )
{
- uint32_t mfmsrInstOpcode = ((OPCODE_31 << 26) | (i_Rt << 21) | (MFMSR_CONST));
+ uint32_t mfmsrInstOpcode = ((OPCODE_31 << 26) | (i_Rt << 21) | ((MFMSR_CONST)<< 1));
return SWIZZLE_4_BYTE(mfmsrInstOpcode);
}
@@ -361,8 +361,13 @@ STATIC uint32_t getRldicrInstruction( const uint16_t i_Ra, const uint16_t i_Rs,
STATIC uint32_t getMfsprInstruction( const uint16_t i_Rt, const uint16_t i_sprNum )
{
- uint32_t mfsprInstOpcode = 0;
- mfsprInstOpcode = (( OPCODE_31 << 26 ) | ( i_Rt << 21 ) | ( i_sprNum << 11 ) | ( MFSPR_CONST << 1 ));
+ uint32_t mfsprInstOpcode = 0;
+ uint32_t temp = (( i_sprNum & 0x03FF ) << 11);
+ mfsprInstOpcode = (uint8_t)i_Rt << 21;
+ mfsprInstOpcode |= (( temp & 0x0000F800 ) << 5);
+ mfsprInstOpcode |= (( temp & 0x001F0000 ) >> 5);
+ mfsprInstOpcode |= MFSPR_BASE_OPCODE;
+
return SWIZZLE_4_BYTE(mfsprInstOpcode);
}
@@ -615,14 +620,14 @@ STATIC StopReturnCode_t getSprRegIndexAdjustment( const uint32_t i_saveMaskPos,
do
{
- if( (( i_saveMaskPos >= SPR_BIT_POS_8 ) && ( i_saveMaskPos <= SPR_BIT_POS_19 )) ||
+ if( (( i_saveMaskPos >= SPR_BIT_POS_8 ) && ( i_saveMaskPos <= SPR_BIT_POS_20 )) ||
(( i_saveMaskPos >= SPR_BIT_POS_25 ) && ( i_saveMaskPos <= SPR_BIT_POS_27 )) )
{
l_rc = STOP_SAVE_SPR_BIT_POS_RESERVE;
break;
}
- if( (i_saveMaskPos > SPR_BIT_POS_19) && (i_saveMaskPos < SPR_BIT_POS_25 ) )
+ if( (i_saveMaskPos > SPR_BIT_POS_20) && (i_saveMaskPos < SPR_BIT_POS_25) )
{
*i_sprAdjIndex = 12;
}
@@ -646,138 +651,9 @@ StopReturnCode_t p9_stop_save_cpureg( void* const i_pImage,
const uint64_t i_regData,
const uint64_t i_pir )
{
- StopReturnCode_t l_rc = STOP_SAVE_SUCCESS; // procedure return code
- HomerSection_t* chipHomer = NULL;
- SmfHomerSection_t* smfChipHomer = NULL;
-
- do
- {
- uint32_t threadId = 0;
- uint32_t coreId = 0;
- uint32_t lookUpKey = 0;
- void* pSprEntryLocation = NULL; // an offset w.r.t. to start of image
- void* pThreadLocation = NULL;
- bool threadScopeReg = false;
- uint8_t l_urmorFix = false;
- uint64_t l_sprValue = 0;
- uint8_t l_selfRestVer = 0;
-
- MY_INF(">> p9_stop_save_cpureg" );
-
- l_rc = getCoreAndThread( i_pImage, i_pir, &coreId, &threadId );
-
- if( l_rc )
- {
- MY_ERR("Failed to determine Core Id and Thread Id from PIR 0x%016llx",
- i_pir);
- break;
- }
-
- MY_INF( " PIR 0x%016llx coreId %d threadid %d "
- " registerId %d", i_pir, coreId,
- threadId, i_regId );
-
- // First of all let us validate all input arguments.
- l_rc = validateSprImageInputs( i_pImage,
- i_regId,
- coreId,
- &threadId,
- &threadScopeReg );
-
- if( l_rc )
- {
- // Error: bad argument traces out error code
- MY_ERR("Bad input argument rc %d", l_rc );
-
- break;
- }
-
- l_urmorFix = *(uint8_t*)((uint8_t*)i_pImage + CPMR_HOMER_OFFSET + CPMR_URMOR_FIX_BYTE);
- l_selfRestVer = *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_SELF_RESTORE_VER_BYTE );
-
- if( l_selfRestVer )
- {
- smfChipHomer = ( SmfHomerSection_t*)i_pImage;
-
- if( threadScopeReg )
- {
- pThreadLocation =
- &(smfChipHomer->iv_coreThreadRestore[coreId].iv_threadRestoreArea[threadId][0]);
- }
- else
- {
- pThreadLocation =
- &(smfChipHomer->iv_coreThreadRestore[coreId].iv_coreRestoreArea[0]);
- }
- }
- else //Old fips or OPAL release that doesn't support SMF
- {
- chipHomer = (HomerSection_t*)i_pImage;
-
- if( threadScopeReg )
- {
- pThreadLocation =
- &(chipHomer->iv_coreThreadRestore[coreId][threadId].iv_threadArea[0]);
- }
- else
- {
- pThreadLocation =
- &(chipHomer->iv_coreThreadRestore[coreId][threadId].iv_coreArea[0]);
- }
- }
-
- if( ( SWIZZLE_4_BYTE(BLR_INST) == *(uint32_t*)pThreadLocation ) ||
- ( SWIZZLE_4_BYTE(ATTN_OPCODE) == *(uint32_t*) pThreadLocation ) )
- {
- // table for given core id doesn't exit. It needs to be
- // defined.
- pSprEntryLocation = pThreadLocation;
- }
- else
- {
- // an SPR restore section for given core already exists
- lookUpKey = genKeyForSprLookup( i_regId );
- l_rc = lookUpSprInImage( (uint32_t*)pThreadLocation,
- lookUpKey,
- threadScopeReg,
- &pSprEntryLocation,
- l_selfRestVer );
- }
-
- if( l_rc )
- {
- MY_ERR("Invalid or corrupt SPR entry. CoreId 0x%08x threadId ",
- "0x%08x regId 0x%08x lookUpKey 0x%08x pThreadLocation 0x%08x"
- , coreId, threadId, i_regId, lookUpKey, pThreadLocation );
- break;
- }
-
- if( ( P9_STOP_SPR_URMOR == i_regId ) && ( l_urmorFix ) )
- {
- l_sprValue = i_regData - URMOR_CORRECTION;
- }
- else
- {
- l_sprValue = i_regData;
- }
-
- l_rc = updateSprEntryInImage( (uint32_t*) pSprEntryLocation,
- i_regId,
- l_sprValue,
- UPDATE_SPR_ENTRY );
-
- if( l_rc )
- {
- MY_ERR( " Failed to update the SPR entry of PIR 0x%08x reg"
- "0x%08x", i_pir, i_regId );
- break;
- }
-
- }
- while(0);
+ MY_INF(">> p9_stop_save_cpureg" );
- MY_INF("<< p9_stop_save_cpureg" );
- return l_rc;
+ return proc_stop_save_cpureg( i_pImage, i_regId, i_regData, i_pir );
}
//-----------------------------------------------------------------------------
@@ -1003,103 +879,334 @@ StopReturnCode_t p9_stop_save_scom( void* const i_pImage,
const ScomOperation_t i_operation,
const ScomSection_t i_section )
{
- StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
- uint32_t entryLimit = 0;
- uint8_t chipletId = 0;
- uint32_t nopInst = 0;
- uint32_t index = 0;
- uint32_t imageVer = 0;
- uint32_t entrySwzHeader = 0;
- uint32_t l_maxScomRestoreEntry = 0;
- ScomEntry_t* pScomEntry = NULL;
- ScomEntry_t* pEntryLocation = NULL;
- ScomEntry_t* pNopLocation = NULL;
- ScomEntry_t* pEditScomHeader = NULL;
- StopCacheSection_t* pStopCacheScomStart = NULL;
- ScomEntry_t* pTableEndLocationtable = NULL;
- uint32_t swizzleAddr;
- uint64_t swizzleData;
- uint32_t swizzleAttn;
- uint32_t swizzleBlr = SWIZZLE_4_BYTE(BLR_INST);
- bool cacheEntry = true;
-
MY_INF(">> p9_stop_save_scom");
- //Reads SGPE image version info from QPMR Header in HOMER
- //For backward compatibility, for base version of SGPE Hcode,
- //STOP API retains default behavior but adds version specific
- //details in each entry in later versions.
- imageVer = *(uint32_t*)((uint8_t*)i_pImage + QPMR_HOMER_OFFSET + QPMR_BUILD_VER_BYTE);
- imageVer = SWIZZLE_4_BYTE(imageVer);
-
+ return proc_stop_save_scom( i_pImage, i_scomAddress,
+ i_scomData, i_operation, i_section );
+}
- do
- {
- chipletId = i_scomAddress >> 24;
- chipletId = chipletId & 0x3F;
+//-----------------------------------------------------------------------------
- l_rc = validateScomImageInputs( i_pImage, i_scomAddress, chipletId, i_operation, i_section );
+/**
+ * @brief searches a self save entry of an SPR in self-save segment.
+ * @param[in] i_sprBitPos bit position associated with SPR in save mask vector.
+ * @param[in] l_pSprSaveStart start location of SPR save segment
+ * @param[in] i_searchLength length of SPR save segment
+ * @param[in] i_pSaveSprLoc start location of save entry for a given SPR.
+ * @return STOP_SAVE_SUCCESS if look up succeeds, error code otherwise.
+ */
+STATIC StopReturnCode_t lookUpSelfSaveSpr( uint32_t i_sprBitPos, uint32_t* l_pSprSaveStart,
+ uint32_t i_searchLength, uint32_t** i_pSaveSprLoc )
+{
+ int32_t l_saveWordLength = (int32_t)(i_searchLength >> 2);
+ uint32_t l_oriInst = getOriInstruction( 0, 0, i_sprBitPos );
+ StopReturnCode_t l_rc = STOP_SAVE_FAIL;
- if( l_rc )
+ while( l_saveWordLength > 0 )
+ {
+ if( l_oriInst == *l_pSprSaveStart )
{
- MY_ERR( "invalid argument: aborting");
+ *i_pSaveSprLoc = l_pSprSaveStart;
+ l_rc = STOP_SAVE_SUCCESS;
break;
}
- if( chipletId >= CORE_CHIPLET_ID_MIN )
- {
- // chiplet is core. So, let us find the start address of SCOM area
- // pertaining to a core in STOP image.
- l_maxScomRestoreEntry =
- *(uint32_t*)((uint8_t*)i_pImage + CPMR_HOMER_OFFSET + CPMR_MAX_SCOM_REST_PER_CORE_BYTE);
- pScomEntry = CORE_ID_SCOM_START(i_pImage, chipletId )
- cacheEntry = false;
+ l_pSprSaveStart++;
+ l_saveWordLength--;
+ }
- if( !l_maxScomRestoreEntry )
- {
- //Old HB and new STOP API case. Retain legacy Number
- l_maxScomRestoreEntry = SWIZZLE_4_BYTE(LEGACY_CORE_SCOM_SUPPORTED);
- }
- }
- else
- {
- l_maxScomRestoreEntry =
- *(uint32_t*)((uint8_t*)i_pImage + QPMR_HOMER_OFFSET + QPMR_QUAD_MAX_SCOM_ENTRY_BYTE);
+ return l_rc;
+}
- if( !l_maxScomRestoreEntry )
- {
- // Incase of a bad HOMER header initialization, fall back on legacy number.
- l_maxScomRestoreEntry = SWIZZLE_4_BYTE(LEGACY_QUAD_SCOM_SUPPORTED);
- }
- // chiplet is a cache. let us find start address of cache section
- // associated with given chiplet. A cache section associated with
- // given chiplet is split in to L2, L3 and EQ area.
- pStopCacheScomStart = CACHE_SECTN_START(i_pImage,
- chipletId);
- }
+//-----------------------------------------------------------------------------
- l_maxScomRestoreEntry = SWIZZLE_4_BYTE(l_maxScomRestoreEntry);
+/**
+ * @brief searches a self save entry of an SPR in self-save segment.
+ * @param[in] i_pSaveReg start of editable location of a SPR save entry.
+ * @param[in] i_sprNum Id of the SPR for which entry needs to be edited.
+ * @return STOP_SAVE_SUCCESS if look up succeeds, error code otherwise.
+ */
+STATIC StopReturnCode_t updateSelfSaveEntry( uint32_t* i_pSaveReg, uint16_t i_sprNum )
+{
+ StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
- if(( !pStopCacheScomStart ) && ( !pScomEntry) )
+ do
+ {
+ if( !i_pSaveReg )
{
- //Error invalid pointer to SCOM entry in cache or core section
- //of STOP image.
- MY_ERR("invalid start location for chiplet %d",
- chipletId );
+ l_rc = STOP_SAVE_FAIL;
+ MY_ERR( "Failed to update self save area for SPR 0x%04x", i_sprNum );
break;
}
- switch( i_section )
+ if( P9_STOP_SPR_MSR == i_sprNum )
{
- case P9_STOP_SECTION_EQ_SCOM:
- pScomEntry = pStopCacheScomStart->nonCacheArea;
- entryLimit = MAX_EQ_SCOM_ENTRIES;
- break;
+ *i_pSaveReg = getMfmsrInstruction( 1 );
+ }
+ else
+ {
+ *i_pSaveReg = getMfsprInstruction( 1, i_sprNum );
+ }
- case P9_STOP_SECTION_L2:
- pScomEntry = pStopCacheScomStart->l2CacheArea;
- entryLimit = MAX_L2_SCOM_ENTRIES;
- break;
+ i_pSaveReg++;
+
+ *i_pSaveReg = getBranchLinkRegInstruction( );
+ }
+ while(0);
+
+ return l_rc;
+}
+
+//-----------------------------------------------------------------------------
+
+StopReturnCode_t p9_stop_save_cpureg_control( void* i_pImage,
+ const uint64_t i_pir,
+ const uint32_t i_saveRegVector )
+{
+ MY_INF( ">> p9_stop_save_cpureg_control" );
+
+ return proc_stop_save_cpureg_control( i_pImage, i_pir, i_saveRegVector );
+}
+
+//-----------------------------------------------------------------------------------------------------
+
+StopReturnCode_t p9_stop_init_cpureg( void* const i_pImage, const uint32_t i_corePos )
+{
+ MY_INF( ">> p9_stop_init_cpureg" );
+
+ return proc_stop_init_cpureg( i_pImage, i_corePos );
+}
+
+//-----------------------------------------------------------------------------------------------------
+
+StopReturnCode_t p9_stop_init_self_save( void* const i_pImage, const uint32_t i_corePos )
+{
+ MY_INF( ">> p9_stop_init_self_save" );
+
+ return proc_stop_init_self_save( i_pImage, i_corePos );
+}
+
+//-----------------------------------------------------------------------------------------------------
+
+StopReturnCode_t proc_stop_init_cpureg( void* const i_pImage, const uint32_t i_corePos )
+{
+
+ StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
+ uint32_t* l_pRestoreStart = NULL;
+ void* l_pTempLoc = NULL;
+ SmfHomerSection_t* l_pHomer = NULL;
+ uint32_t l_threadPos = 0;
+ uint32_t l_lookUpKey = 0;
+ uint32_t l_sprIndex = 0;
+ uint8_t l_selfRestVer = 0;
+
+ MY_INF( ">> proc_stop_init_cpureg" );
+
+ do
+ {
+ if( !i_pImage )
+ {
+ l_rc = STOP_SAVE_ARG_INVALID_IMG;
+ break;
+ }
+
+ if( i_corePos > MAX_CORE_ID_SUPPORTED )
+ {
+ l_rc = STOP_SAVE_ARG_INVALID_CORE;
+ break;
+ }
+
+ l_pHomer = ( SmfHomerSection_t * ) i_pImage;
+ l_selfRestVer = *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_SELF_RESTORE_VER_BYTE );
+
+ for( l_sprIndex = 0; l_sprIndex < MAX_SPR_SUPPORTED; l_sprIndex++ )
+ {
+ //Check if a given SPR needs to be self-saved each time on STOP entry
+
+ l_lookUpKey = genKeyForSprLookup( ( CpuReg_t )g_sprRegister[l_sprIndex].iv_sprId );
+
+ if( g_sprRegister[l_sprIndex].iv_isThreadScope )
+ {
+ for( l_threadPos = 0; l_threadPos < MAX_THREADS_PER_CORE; l_threadPos++ )
+ {
+ l_pRestoreStart =
+ (uint32_t*)&l_pHomer->iv_coreThreadRestore[i_corePos].iv_threadRestoreArea[l_threadPos][0];
+
+ l_rc = lookUpSprInImage( (uint32_t*)l_pRestoreStart, l_lookUpKey,
+ g_sprRegister[l_sprIndex].iv_isThreadScope,
+ &l_pTempLoc,
+ l_selfRestVer );
+
+ if( l_rc )
+ {
+ MY_ERR( "Thread SPR lookup failed in p9_stop_init_cpureg SPR %d Core %d Thread %d Index %d",
+ g_sprRegister[l_sprIndex].iv_sprId, i_corePos, l_threadPos, l_sprIndex );
+ break;
+ }
+
+ l_rc = updateSprEntryInImage( (uint32_t*) l_pTempLoc,
+ ( CpuReg_t )g_sprRegister[l_sprIndex].iv_sprId,
+ 0x00,
+ INIT_SPR_REGION );
+
+ if( l_rc )
+ {
+ MY_ERR( "Thread SPR region init failed. Core %d SPR Id %d",
+ i_corePos, g_sprRegister[l_sprIndex].iv_sprId );
+ break;
+ }
+
+ }//end for thread
+
+ if( l_rc )
+ {
+ break;
+ }
+
+ }//end if SPR threadscope
+ else
+ {
+ l_pRestoreStart = (uint32_t*)&l_pHomer->iv_coreThreadRestore[i_corePos].iv_coreRestoreArea[0];
+
+ l_rc = lookUpSprInImage( (uint32_t*)l_pRestoreStart, l_lookUpKey,
+ g_sprRegister[l_sprIndex].iv_isThreadScope,
+ &l_pTempLoc, l_selfRestVer );
+
+ if( l_rc )
+ {
+ MY_ERR( "Core SPR lookup failed in p9_stop_init_cpureg" );
+ break;
+ }
+
+ l_rc = updateSprEntryInImage( (uint32_t*) l_pTempLoc,
+ ( CpuReg_t )g_sprRegister[l_sprIndex].iv_sprId,
+ 0x00,
+ INIT_SPR_REGION );
+
+ if( l_rc )
+ {
+ MY_ERR( "Core SPR region init failed. Core %d SPR Id %d SPR Index %d",
+ i_corePos, g_sprRegister[l_sprIndex].iv_sprId, l_sprIndex );
+ break;
+ }
+
+ }// end else
+
+ }// end for l_sprIndex
+
+ }
+ while(0);
+
+ MY_INF( "<< proc_stop_init_cpureg" );
+
+ return l_rc;
+}
+
+//-----------------------------------------------------------------------------------------------------
+
+StopReturnCode_t proc_stop_save_scom( void* const i_pImage,
+ const uint32_t i_scomAddress,
+ const uint64_t i_scomData,
+ const ScomOperation_t i_operation,
+ const ScomSection_t i_section )
+{
+ StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
+ uint32_t entryLimit = 0;
+ uint8_t chipletId = 0;
+ uint32_t nopInst = 0;
+ uint32_t index = 0;
+ uint32_t imageVer = 0;
+ uint32_t entrySwzHeader = 0;
+ uint32_t l_maxScomRestoreEntry = 0;
+ ScomEntry_t* pScomEntry = NULL;
+ ScomEntry_t* pEntryLocation = NULL;
+ ScomEntry_t* pNopLocation = NULL;
+ ScomEntry_t* pEditScomHeader = NULL;
+ StopCacheSection_t* pStopCacheScomStart = NULL;
+ ScomEntry_t* pTableEndLocationtable = NULL;
+ uint32_t swizzleAddr;
+ uint64_t swizzleData;
+ uint32_t swizzleAttn;
+ uint32_t swizzleBlr = SWIZZLE_4_BYTE(BLR_INST);
+ bool cacheEntry = true;
+
+ MY_INF( ">> proc_stop_save_scom" );
+
+ //Reads SGPE image version info from QPMR Header in HOMER
+ //For backward compatibility, for base version of SGPE Hcode,
+ //STOP API retains default behavior but adds version specific
+ //details in each entry in later versions.
+ imageVer = *(uint32_t*)((uint8_t*)i_pImage + QPMR_HOMER_OFFSET + QPMR_BUILD_VER_BYTE);
+ imageVer = SWIZZLE_4_BYTE(imageVer);
+
+
+ do
+ {
+ chipletId = i_scomAddress >> 24;
+ chipletId = chipletId & 0x3F;
+
+ l_rc = validateScomImageInputs( i_pImage, i_scomAddress, chipletId, i_operation, i_section );
+
+ if( l_rc )
+ {
+ MY_ERR( "invalid argument: aborting");
+ break;
+ }
+
+ if( chipletId >= CORE_CHIPLET_ID_MIN )
+ {
+ // chiplet is core. So, let us find the start address of SCOM area
+ // pertaining to a core in STOP image.
+ l_maxScomRestoreEntry =
+ *(uint32_t*)((uint8_t*)i_pImage + CPMR_HOMER_OFFSET + CPMR_MAX_SCOM_REST_PER_CORE_BYTE);
+ pScomEntry = CORE_ID_SCOM_START(i_pImage, chipletId )
+ cacheEntry = false;
+
+ if( !l_maxScomRestoreEntry )
+ {
+ //Old HB and new STOP API case. Retain legacy Number
+ l_maxScomRestoreEntry = SWIZZLE_4_BYTE(LEGACY_CORE_SCOM_SUPPORTED);
+ }
+ }
+ else
+ {
+ l_maxScomRestoreEntry =
+ *(uint32_t*)((uint8_t*)i_pImage + QPMR_HOMER_OFFSET + QPMR_QUAD_MAX_SCOM_ENTRY_BYTE);
+
+ if( !l_maxScomRestoreEntry )
+ {
+ // Incase of a bad HOMER header initialization, fall back on legacy number.
+ l_maxScomRestoreEntry = SWIZZLE_4_BYTE(LEGACY_QUAD_SCOM_SUPPORTED);
+ }
+ // chiplet is a cache. let us find start address of cache section
+ // associated with given chiplet. A cache section associated with
+ // given chiplet is split in to L2, L3 and EQ area.
+ pStopCacheScomStart = CACHE_SECTN_START(i_pImage,
+ chipletId);
+ }
+
+ l_maxScomRestoreEntry = SWIZZLE_4_BYTE(l_maxScomRestoreEntry);
+
+ if(( !pStopCacheScomStart ) && ( !pScomEntry) )
+ {
+ //Error invalid pointer to SCOM entry in cache or core section
+ //of STOP image.
+ MY_ERR("invalid start location for chiplet %d",
+ chipletId );
+ break;
+ }
+
+ switch( i_section )
+ {
+ case P9_STOP_SECTION_EQ_SCOM:
+ pScomEntry = pStopCacheScomStart->nonCacheArea;
+ entryLimit = MAX_EQ_SCOM_ENTRIES;
+ break;
+
+ case P9_STOP_SECTION_L2:
+ pScomEntry = pStopCacheScomStart->l2CacheArea;
+ entryLimit = MAX_L2_SCOM_ENTRIES;
+ break;
case P9_STOP_SECTION_L3:
pScomEntry = pStopCacheScomStart->l3CacheArea;
@@ -1274,131 +1381,60 @@ StopReturnCode_t p9_stop_save_scom( void* const i_pImage,
if( NULL == pEntryLocation )
{
editAppend = pTableEndLocationtable;
- }
- else
- {
- editAppend = pEntryLocation;
-
- if( P9_STOP_SCOM_OR_APPEND == i_operation )
- {
- tempOperation = P9_STOP_SCOM_OR;
- }
- else
- {
- tempOperation = P9_STOP_SCOM_AND;
- }
- }
-
- l_rc = editScomEntry( swizzleAddr,
- swizzleData,
- editAppend,
- tempOperation );
-
- pEditScomHeader = editAppend;
- }
- break;
-
- default:
- l_rc = STOP_SAVE_SCOM_INVALID_OPERATION;
- break;
- }
- }
- while(0);
-
- if( l_rc )
- {
- MY_ERR("SCOM image operation 0x%08x failed for chiplet 0x%08x addr"
- "0x%08x", i_operation, chipletId ,
- i_scomAddress );
- }
- else
- {
- //Update SCOM Restore entry with version and memory layout
- //info
- updateEntryHeader( pEditScomHeader, imageVer, l_maxScomRestoreEntry );
- }
-
- MY_INF("<< p9_stop_save_scom");
- return l_rc;
-}
-
-//-----------------------------------------------------------------------------
-
-/**
- * @brief searches a self save entry of an SPR in self-save segment.
- * @param[in] i_sprBitPos bit position associated with SPR in save mask vector.
- * @param[in] l_pSprSaveStart start location of SPR save segment
- * @param[in] i_searchLength length of SPR save segment
- * @param[in] i_pSaveSprLoc start location of save entry for a given SPR.
- * @return STOP_SAVE_SUCCESS if look up succeeds, error code otherwise.
- */
-STATIC StopReturnCode_t lookUpSelfSaveSpr( uint32_t i_sprBitPos, uint32_t* l_pSprSaveStart,
- uint32_t i_searchLength, uint32_t** i_pSaveSprLoc )
-{
- int32_t l_saveWordLength = (int32_t)(i_searchLength >> 2);
- uint32_t l_oriInst = getOriInstruction( 0, 0, i_sprBitPos );
- StopReturnCode_t l_rc = STOP_SAVE_FAIL;
-
- while( l_saveWordLength > 0 )
- {
- if( l_oriInst == *l_pSprSaveStart )
- {
- *i_pSaveSprLoc = l_pSprSaveStart;
- l_rc = STOP_SAVE_SUCCESS;
- break;
- }
-
- l_pSprSaveStart++;
- l_saveWordLength--;
- }
-
- return l_rc;
-}
-
-//-----------------------------------------------------------------------------
-
-/**
- * @brief searches a self save entry of an SPR in self-save segment.
- * @param[in] i_pSaveReg start of editable location of a SPR save entry.
- * @param[in] i_sprNum Id of the SPR for which entry needs to be edited.
- * @return STOP_SAVE_SUCCESS if look up succeeds, error code otherwise.
- */
-STATIC StopReturnCode_t updateSelfSaveEntry( uint32_t* i_pSaveReg, uint16_t i_sprNum )
-{
- StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
-
- do
- {
- if( !i_pSaveReg )
- {
- l_rc = STOP_SAVE_FAIL;
- MY_ERR( "Failed to update self save area for SPR 0x%04x", i_sprNum );
- break;
- }
+ }
+ else
+ {
+ editAppend = pEntryLocation;
- if( P9_STOP_SPR_MSR == i_sprNum )
- {
- *i_pSaveReg = getMfmsrInstruction( 1 );
- }
- else
- {
- *i_pSaveReg = getMfsprInstruction( 1, i_sprNum );
- }
+ if( P9_STOP_SCOM_OR_APPEND == i_operation )
+ {
+ tempOperation = P9_STOP_SCOM_OR;
+ }
+ else
+ {
+ tempOperation = P9_STOP_SCOM_AND;
+ }
+ }
- i_pSaveReg++;
+ l_rc = editScomEntry( swizzleAddr,
+ swizzleData,
+ editAppend,
+ tempOperation );
- *i_pSaveReg = getBranchLinkRegInstruction( );
+ pEditScomHeader = editAppend;
+ }
+ break;
+
+ default:
+ l_rc = STOP_SAVE_SCOM_INVALID_OPERATION;
+ break;
+ }
}
while(0);
+ if( l_rc )
+ {
+ MY_ERR("SCOM image operation 0x%08x failed for chiplet 0x%08x addr"
+ "0x%08x", i_operation, chipletId ,
+ i_scomAddress );
+ }
+ else
+ {
+ //Update SCOM Restore entry with version and memory layout
+ //info
+ updateEntryHeader( pEditScomHeader, imageVer, l_maxScomRestoreEntry );
+ }
+
+ MY_INF( "<< proc_stop_save_scom" );
+
return l_rc;
}
-//-----------------------------------------------------------------------------
+//-----------------------------------------------------------------------------------------------------
-StopReturnCode_t p9_stop_save_cpureg_control( void* i_pImage,
- const uint64_t i_pir,
- const uint32_t i_saveRegVector )
+StopReturnCode_t proc_stop_save_cpureg_control( void* i_pImage,
+ const uint64_t i_pir,
+ const uint32_t i_saveRegVector )
{
StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
uint32_t l_coreId = 0;
@@ -1411,8 +1447,10 @@ StopReturnCode_t p9_stop_save_cpureg_control( void* i_pImage,
uint32_t* l_pRestoreStart = NULL;
uint32_t* l_pSprSave = NULL;
void* l_pTempLoc = NULL;
+ uint32_t * l_pTempWord = NULL;
SmfHomerSection_t* l_pHomer = NULL;
uint8_t l_selfRestVer = 0;
+ MY_INF(">> proc_stop_save_cpureg_control" );
do
{
@@ -1440,6 +1478,11 @@ StopReturnCode_t p9_stop_save_cpureg_control( void* i_pImage,
{
l_sprPos = g_sprRegister[l_sprIndex].iv_saveMaskPos;
+ if( l_sprPos > MAX_SPR_BIT_POS )
+ {
+ continue;
+ }
+
//Check if a given SPR needs to be self-saved each time on STOP entry
if( i_saveRegVector & ( TEST_BIT_PATTERN >> l_sprPos ) )
@@ -1493,139 +1536,187 @@ StopReturnCode_t p9_stop_save_cpureg_control( void* i_pImage,
//update specific instructions of self save region to enable saving for SPR
l_rc = updateSelfSaveEntry( l_pSprSave, g_sprRegister[l_sprIndex].iv_sprId );
+ if( l_rc )
+ {
+ MY_ERR( "Failed to update self save instructions for 0x%08x",
+ (uint32_t) g_sprRegister[l_sprIndex].iv_sprId );
+ }
+
+ if( l_pTempLoc )
+ {
+ l_pTempWord = (uint32_t *)l_pTempLoc;
+ l_pTempWord++;
+ *l_pTempWord = getXorInstruction( 0, 0, 0 );
+ }
+
}// end if( i_saveRegVector..)
}// end for
}
while(0);
+ MY_INF("<< proc_stop_save_cpureg_control" );
+
return l_rc;
+
}
//-----------------------------------------------------------------------------------------------------
-StopReturnCode_t p9_stop_init_cpureg( void* const i_pImage, const uint32_t i_corePos )
+StopReturnCode_t proc_stop_save_cpureg( void* const i_pImage,
+ const CpuReg_t i_regId,
+ const uint64_t i_regData,
+ const uint64_t i_pir )
{
- StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
- uint32_t* l_pRestoreStart = NULL;
- void* l_pTempLoc = NULL;
- SmfHomerSection_t* l_pHomer = NULL;
- uint32_t l_threadPos = 0;
- uint32_t l_lookUpKey = 0;
- uint32_t l_sprIndex = 0;
- uint8_t l_selfRestVer = 0;
- MY_INF( ">> p9_stop_init_cpureg" );
+ StopReturnCode_t l_rc = STOP_SAVE_SUCCESS; // procedure return code
+ HomerSection_t* chipHomer = NULL;
+ SmfHomerSection_t* smfChipHomer = NULL;
+
+ MY_INF(">> proc_stop_save_cpureg" );
do
{
- if( !i_pImage )
+ uint32_t threadId = 0;
+ uint32_t coreId = 0;
+ uint32_t lookUpKey = 0;
+ void* pSprEntryLocation = NULL; // an offset w.r.t. to start of image
+ void* pThreadLocation = NULL;
+ bool threadScopeReg = false;
+ uint8_t l_urmorFix = false;
+ uint64_t l_sprValue = 0;
+ uint8_t l_selfRestVer = 0;
+
+
+ l_rc = getCoreAndThread( i_pImage, i_pir, &coreId, &threadId );
+
+ if( l_rc )
{
- l_rc = STOP_SAVE_ARG_INVALID_IMG;
+ MY_ERR("Failed to determine Core Id and Thread Id from PIR 0x%016llx",
+ i_pir);
break;
}
- if( i_corePos > MAX_CORE_ID_SUPPORTED )
+ MY_INF( " PIR 0x%016llx coreId %d threadid %d "
+ " registerId %d", i_pir, coreId,
+ threadId, i_regId );
+
+ // First of all let us validate all input arguments.
+ l_rc = validateSprImageInputs( i_pImage,
+ i_regId,
+ coreId,
+ &threadId,
+ &threadScopeReg );
+
+ if( l_rc )
{
- l_rc = STOP_SAVE_ARG_INVALID_CORE;
+ // Error: bad argument traces out error code
+ MY_ERR("Bad input argument rc %d", l_rc );
+
break;
}
- l_pHomer = ( SmfHomerSection_t * ) i_pImage;
+ l_urmorFix = *(uint8_t*)((uint8_t*)i_pImage + CPMR_HOMER_OFFSET + CPMR_URMOR_FIX_BYTE);
l_selfRestVer = *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_SELF_RESTORE_VER_BYTE );
- for( l_sprIndex = 0; l_sprIndex < MAX_SPR_SUPPORTED; l_sprIndex++ )
+ if( l_selfRestVer )
{
- //Check if a given SPR needs to be self-saved each time on STOP entry
-
- l_lookUpKey = genKeyForSprLookup( ( CpuReg_t )g_sprRegister[l_sprIndex].iv_sprId );
+ smfChipHomer = ( SmfHomerSection_t*)i_pImage;
- if( g_sprRegister[l_sprIndex].iv_isThreadScope )
+ if( threadScopeReg )
{
- for( l_threadPos = 0; l_threadPos < MAX_THREADS_PER_CORE; l_threadPos++ )
- {
- l_pRestoreStart =
- (uint32_t*)&l_pHomer->iv_coreThreadRestore[i_corePos].iv_threadRestoreArea[l_threadPos][0];
-
- l_rc = lookUpSprInImage( (uint32_t*)l_pRestoreStart, l_lookUpKey,
- g_sprRegister[l_sprIndex].iv_isThreadScope,
- &l_pTempLoc,
- l_selfRestVer );
-
- if( l_rc )
- {
- MY_ERR( "Thread SPR lookup failed in p9_stop_init_cpureg SPR %d Core %d Thread %d Index %d",
- g_sprRegister[l_sprIndex].iv_sprId, i_corePos, l_threadPos, l_sprIndex );
- break;
- }
-
- l_rc = updateSprEntryInImage( (uint32_t*) l_pTempLoc,
- ( CpuReg_t )g_sprRegister[l_sprIndex].iv_sprId,
- 0x00,
- INIT_SPR_REGION );
-
- if( l_rc )
- {
- MY_ERR( "Thread SPR region init failed. Core %d SPR Id %d",
- i_corePos, g_sprRegister[l_sprIndex].iv_sprId );
- break;
- }
-
- }//end for thread
-
- if( l_rc )
- {
- break;
- }
-
- }//end if SPR threadscope
+ pThreadLocation =
+ &(smfChipHomer->iv_coreThreadRestore[coreId].iv_threadRestoreArea[threadId][0]);
+ }
else
{
- l_pRestoreStart = (uint32_t*)&l_pHomer->iv_coreThreadRestore[i_corePos].iv_coreRestoreArea[0];
+ pThreadLocation =
+ &(smfChipHomer->iv_coreThreadRestore[coreId].iv_coreRestoreArea[0]);
+ }
+ }
+ else //Old fips or OPAL release that doesn't support SMF
+ {
+ chipHomer = (HomerSection_t*)i_pImage;
- l_rc = lookUpSprInImage( (uint32_t*)l_pRestoreStart, l_lookUpKey,
- g_sprRegister[l_sprIndex].iv_isThreadScope,
- &l_pTempLoc, l_selfRestVer );
+ if( threadScopeReg )
+ {
+ pThreadLocation =
+ &(chipHomer->iv_coreThreadRestore[coreId][threadId].iv_threadArea[0]);
+ }
+ else
+ {
+ pThreadLocation =
+ &(chipHomer->iv_coreThreadRestore[coreId][threadId].iv_coreArea[0]);
+ }
+ }
- if( l_rc )
- {
- MY_ERR( "Core SPR lookup failed in p9_stop_init_cpureg" );
- break;
- }
+ if( ( SWIZZLE_4_BYTE(BLR_INST) == *(uint32_t*)pThreadLocation ) ||
+ ( SWIZZLE_4_BYTE(ATTN_OPCODE) == *(uint32_t*) pThreadLocation ) )
+ {
+ // table for given core id doesn't exit. It needs to be
+ // defined.
+ pSprEntryLocation = pThreadLocation;
+ }
+ else
+ {
+ // an SPR restore section for given core already exists
+ lookUpKey = genKeyForSprLookup( i_regId );
+ l_rc = lookUpSprInImage( (uint32_t*)pThreadLocation,
+ lookUpKey,
+ threadScopeReg,
+ &pSprEntryLocation,
+ l_selfRestVer );
+ }
- l_rc = updateSprEntryInImage( (uint32_t*) l_pTempLoc,
- ( CpuReg_t )g_sprRegister[l_sprIndex].iv_sprId,
- 0x00,
- INIT_SPR_REGION );
+ if( l_rc )
+ {
+ MY_ERR("Invalid or corrupt SPR entry. CoreId 0x%08x threadId ",
+ "0x%08x regId 0x%08x lookUpKey 0x%08x pThreadLocation 0x%08x"
+ , coreId, threadId, i_regId, lookUpKey, pThreadLocation );
+ break;
+ }
- if( l_rc )
- {
- MY_ERR( "Core SPR region init failed. Core %d SPR Id %d SPR Index %d",
- i_corePos, g_sprRegister[l_sprIndex].iv_sprId, l_sprIndex );
- break;
- }
+ if( ( P9_STOP_SPR_URMOR == i_regId ) && ( l_urmorFix ) )
+ {
+ l_sprValue = i_regData - URMOR_CORRECTION;
+ }
+ else
+ {
+ l_sprValue = i_regData;
+ }
- }// end else
+ l_rc = updateSprEntryInImage( (uint32_t*) pSprEntryLocation,
+ i_regId,
+ l_sprValue,
+ UPDATE_SPR_ENTRY );
- }// end for l_sprIndex
+ if( l_rc )
+ {
+ MY_ERR( " Failed to update the SPR entry of PIR 0x%08x reg"
+ "0x%08x", i_pir, i_regId );
+ break;
+ }
}
while(0);
- MY_INF( "<< p9_stop_init_cpureg" );
+ MY_INF("<< proc_stop_save_cpureg" );
+
return l_rc;
}
//-----------------------------------------------------------------------------------------------------
-StopReturnCode_t p9_stop_init_self_save( void* const i_pImage, const uint32_t i_corePos )
+StopReturnCode_t proc_stop_init_self_save( void* const i_pImage, const uint32_t i_corePos )
{
+
StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
uint32_t* l_pSaveStart = NULL;
SmfHomerSection_t * l_pHomer = NULL;
uint32_t l_threadPos = 0;
uint32_t l_sprBitPos = 0;
uint32_t l_sprIndexAdj = 0;
- MY_INF( ">> p9_stop_init_self_save" );
+
+ MY_INF(">> proc_stop_init_self_save" );
do
{
@@ -1732,7 +1823,8 @@ StopReturnCode_t p9_stop_init_self_save( void* const i_pImage, const uint32_t i
}
while(0);
- MY_INF( "<< p9_stop_init_self_save" );
+ MY_INF("<< proc_stop_init_self_save" );
+
return l_rc;
}
diff --git a/libpore/p9_stop_api.H b/libpore/p9_stop_api.H
index 17caedb3..3f6420ff 100644
--- a/libpore/p9_stop_api.H
+++ b/libpore/p9_stop_api.H
@@ -70,6 +70,26 @@ typedef enum
P9_STOP_SPR_PMCR = 884, // core register
P9_STOP_SPR_HID = 1008, // core register
P9_STOP_SPR_MSR = 2000, // thread register
+
+ //enum members which are project agnostic
+ PROC_STOP_SPR_DAWR = 180, // thread register
+ PROC_STOP_SPR_CIABR = 187, // thread register
+ PROC_STOP_SPR_DAWRX = 188, // thread register
+ PROC_STOP_SPR_HSPRG0 = 304, // thread register
+ PROC_STOP_SPR_HRMOR = 313, // core register
+ PROC_STOP_SPR_LPCR = 318, // thread register
+ PROC_STOP_SPR_HMEER = 337, // core register
+ PROC_STOP_SPR_PTCR = 464, // core register
+ PROC_STOP_SPR_USPRG0 = 496, // thread register
+ PROC_STOP_SPR_USPRG1 = 497, // thread register
+ PROC_STOP_SPR_URMOR = 505, // core register
+ PROC_STOP_SPR_SMFCTRL = 511, // thread register
+ PROC_STOP_SPR_LDBAR = 850, // thread register
+ PROC_STOP_SPR_PSSCR = 855, // thread register
+ PROC_STOP_SPR_PMCR = 884, // core register
+ PROC_STOP_SPR_HID = 1008, // core register
+ PROC_STOP_SPR_MSR = 2000, // thread register
+
} CpuReg_t;
/**
@@ -110,7 +130,20 @@ typedef enum
P9_STOP_SCOM_RESET = 6,
P9_STOP_SCOM_OR_APPEND = 7,
P9_STOP_SCOM_AND_APPEND = 8,
- P9_STOP_SCOM_OP_MAX = 9
+ P9_STOP_SCOM_OP_MAX = 9,
+
+ //enum members which are project agnostic
+ PROC_STOP_SCOM_OP_MIN = 0,
+ PROC_STOP_SCOM_APPEND = 1,
+ PROC_STOP_SCOM_REPLACE = 2,
+ PROC_STOP_SCOM_OR = 3,
+ PROC_STOP_SCOM_AND = 4,
+ PROC_STOP_SCOM_NOOP = 5,
+ PROC_STOP_SCOM_RESET = 6,
+ PROC_STOP_SCOM_OR_APPEND = 7,
+ PROC_STOP_SCOM_AND_APPEND = 8,
+ PROC_STOP_SCOM_OP_MAX = 9,
+
} ScomOperation_t;
/**
@@ -123,7 +156,15 @@ typedef enum
P9_STOP_SECTION_EQ_SCOM = 2,
P9_STOP_SECTION_L2 = 3,
P9_STOP_SECTION_L3 = 4,
- P9_STOP_SECTION_MAX = 5
+ P9_STOP_SECTION_MAX = 5,
+
+ //enum members which are project agnostic
+ PROC_STOP_SECTION_MIN = 0,
+ PROC_STOP_SECTION_CORE_SCOM = 1,
+ PROC_STOP_SECTION_EQ_SCOM = 2,
+ PROC_STOP_SECTION_L2 = 3,
+ PROC_STOP_SECTION_L3 = 4,
+ PROC_STOP_SECTION_MAX = 5,
} ScomSection_t;
/**
@@ -148,7 +189,6 @@ typedef enum
BIT_POS_LPCR = 5,
BIT_POS_PSSCR = 6,
BIT_POS_MSR = 7,
- BIT_POS_HRMOR = 20,
BIT_POS_HID = 21,
BIT_POS_HMEER = 22,
BIT_POS_PMCR = 23,
@@ -156,7 +196,6 @@ typedef enum
BIT_POS_SMFCTRL = 28,
BIT_POS_USPRG0 = 29,
BIT_POS_USPRG1 = 30,
- BIT_POS_URMOR = 31,
} SprBitPositionList_t;
@@ -229,13 +268,79 @@ p9_stop_save_cpureg_control( void* i_pImage, const uint64_t i_pir,
* @brief initializes self-save region with specific instruction.
* @param[in] i_pImage start address of homer image of P9 chip.
* @param[in] i_corePos physical core's relative position within processor chip.
- * @return STOP_SAVE_SUCCESS SUCCESS if self-save is initialized successfully,
+ * @return STOP_SAVE_SUCCESS if self-save is initialized successfully,
* error code otherwise.
* @note API is intended only for use case of HOMER build. There is no explicit
* effort to support any other use case.
*/
StopReturnCode_t p9_stop_init_self_save( void* const i_pImage, const uint32_t i_corePos );
+/**
+ * @brief creates SCOM restore entry for a given scom adress in HOMER.
+ * @param i_pImage points to start address of HOMER image.
+ * @param i_scomAddress address associated with SCOM restore entry.
+ * @param i_scomData data associated with SCOM restore entry.
+ * @param i_operation operation type requested for API.
+ * @param i_section section of HOMER in which restore entry needs to be created.
+ * @return STOP_SAVE_SUCCESS if API succeeds, error code otherwise.
+ * @note It is an API for creating SCOM restore entry in HOMER. It is agnostic to
+ * generation of POWER processor.
+ */
+
+StopReturnCode_t proc_stop_save_scom( void* const i_pImage,
+ const uint32_t i_scomAddress,
+ const uint64_t i_scomData,
+ const ScomOperation_t i_operation,
+ const ScomSection_t i_section );
+
+/**
+ * @brief initializes self save restore region of HOMER.
+ * @param[in] i_pImage points to base of HOMER image.
+ * @param[in] i_corePos position of the physical core.
+ * @return STOP_SAVE_SUCCESS if API succeeds, error code otherwise.
+ * @note It is an API for initializing self restore region in HOMER. It is agnostic to
+ * generation of POWER processor.
+ */
+StopReturnCode_t proc_stop_init_cpureg( void* const i_pImage, const uint32_t i_corePos );
+
+/**
+ * @brief enables self save for a given set of SPRs
+ * @param[in] i_pImage points to start address of HOMER image.
+ * @param[in] i_pir PIR value associated with core and thread.
+ * @param[in] i_saveRegVector bit vector representing the SPRs that needs to be self saved.
+ * @return STOP_SAVE_SUCCESS if API succeeds, error code otherwise.
+ * @note It is an API for enabling self save of SPRs and it is agnostic to
+ * generation of POWER processor.
+ */
+StopReturnCode_t proc_stop_save_cpureg_control( void* i_pImage,
+ const uint64_t i_pir,
+ const uint32_t i_saveRegVector );
+
+/**
+ * @brief creates an SPR restore entry in HOMER
+ * @param[in] i_pImage points to start address of HOMER image.
+ * @param[in] i_pir PIR value associated with core and thread.
+ * @param[in] i_saveRegVector bit vector representing the SPRs that needs to be self saved.
+ * @return STOP_SAVE_SUCCESS if API succeeds, error code otherwise.
+ * @note It is an API for enabling self save of SPRs and it is agnostic to
+ * generation of POWER processor.
+ */
+StopReturnCode_t proc_stop_save_cpureg( void* const i_pImage,
+ const CpuReg_t i_regId,
+ const uint64_t i_regData,
+ const uint64_t i_pir );
+
+/**
+ * @brief initializes self-save region with specific instruction.
+ * @param[in] i_pImage start address of homer image.
+ * @param[in] i_corePos physical core's relative position within processor chip.
+ * @return STOP_SAVE_SUCCESS if self-save is initialized successfully,
+ * error code otherwise.
+ * @note API is project agnostic and is intended only for use case of HOMER build.
+ * There is no explicit effort to support any other use case.
+ */
+StopReturnCode_t proc_stop_init_self_save( void* const i_pImage, const uint32_t i_corePos );
+
#ifdef __cplusplus
} // extern "C"
}; // namespace stopImageSection ends
diff --git a/libpore/p9_stop_data_struct.H b/libpore/p9_stop_data_struct.H
index 1e9721e0..4e73aab5 100644
--- a/libpore/p9_stop_data_struct.H
+++ b/libpore/p9_stop_data_struct.H
@@ -67,9 +67,9 @@ enum
SIZE_PER_SPR_RESTORE_INST = ((4 * sizeof(uint8_t)) / sizeof(uint32_t)),
MAX_THREAD_LEVEL_SPRS = 11,
MAX_CORE_LEVEL_SPRS = 6,
- MAX_SPR_BIT_POS = 31,
+ MAX_SPR_BIT_POS = 30,
SPR_BIT_POS_8 = 8,
- SPR_BIT_POS_19 = 19,
+ SPR_BIT_POS_20 = 20,
SPR_BIT_POS_25 = 25,
SPR_BIT_POS_27 = 27,
};
diff --git a/libpore/p9_stop_util.H b/libpore/p9_stop_util.H
index 3266fdef..79b4e959 100644
--- a/libpore/p9_stop_util.H
+++ b/libpore/p9_stop_util.H
@@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
-/* Contributors Listed Below - COPYRIGHT 2016,2017 */
+/* Contributors Listed Below - COPYRIGHT 2016,2018 */
/* [+] International Business Machines Corp. */
/* */
/* */
@@ -95,7 +95,10 @@ typedef struct
uint64_t cpmrMagicWord;
uint32_t buildDate;
uint32_t version;
- uint8_t reserve1[7];
+ uint8_t reserve1[4];
+ uint8_t selfRestoreVer;
+ uint8_t stopApiVer;
+ uint8_t urmorFix;
uint8_t fusedModeStatus;
uint32_t cmeImgOffset;
uint32_t cmeImgLength;
--
2.25.1
^ permalink raw reply related
* [PATCH v8 2/3] API to verify the STOP API and image compatibility
From: Pratik Rajesh Sampat @ 2020-04-23 10:54 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
pratik.r.sampat, psampat
In-Reply-To: <20200423105438.29034-1-psampat@linux.ibm.com>
From: Prem Shanker Jha <premjha2@in.ibm.com>
Commit defines a new API primarily intended for OPAL to determine
cpu register save API's compatibility with HOMER layout and
self save restore. It can help OPAL determine if version of
API integrated with OPAL is different from hostboot.
Change-Id: Ic0de45a336cfb8b6b6096a10ac1cd3ffbaa44fc0
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/77612
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: RANGANATHPRASAD G. BRAHMASAMUDRA <prasadbgr@in.ibm.com>
Reviewed-by: Gregory S Still <stillgs@us.ibm.com>
Reviewed-by: Jennifer A Stofer <stofer@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/77614
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M Crowell <dcrowell@us.ibm.com>
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
include/p9_stop_api.H | 25 ++++++++++
libpore/p9_cpu_reg_restore_instruction.H | 7 ++-
libpore/p9_hcd_memmap_base.H | 7 +++
libpore/p9_stop_api.C | 58 +++++++++++++++++++++++-
libpore/p9_stop_api.H | 26 ++++++++++-
libpore/p9_stop_util.H | 20 ++++----
6 files changed, 130 insertions(+), 13 deletions(-)
diff --git a/include/p9_stop_api.H b/include/p9_stop_api.H
index 9d3bc1e5..cb5ffd6f 100644
--- a/include/p9_stop_api.H
+++ b/include/p9_stop_api.H
@@ -107,6 +107,7 @@ typedef enum
STOP_SAVE_FAIL = 14, // for internal failure within firmware.
STOP_SAVE_SPR_ENTRY_MISSING = 15,
STOP_SAVE_SPR_BIT_POS_RESERVE = 16,
+ STOP_SAVE_API_IMG_INCOMPATIBLE = 18,
} StopReturnCode_t;
/**
@@ -161,6 +162,14 @@ typedef enum
} ScomSection_t;
+/**
+ * @brief versions pertaining relvant to STOP API.
+ */
+typedef enum
+{
+ STOP_API_VER = 0x00,
+ STOP_API_VER_CONTROL = 0x02,
+} VersionList_t;
/**
@@ -192,6 +201,14 @@ typedef enum
BIT_POS_USPRG1 = 30,
} SprBitPositionList_t;
+typedef enum
+{
+ SMF_SUPPORT_MISSING_IN_HOMER = 0x01,
+ SELF_SUPPORT_MISSING_FOR_LE_HYP = 0x02,
+ IPL_RUNTIME_CPU_SAVE_VER_MISMATCH = 0x04,
+ SELF_RESTORE_VER_MISMATCH = 0x08,
+} VersionIncompList_t;
+
#ifdef __cplusplus
extern "C" {
#endif
@@ -230,6 +247,14 @@ StopReturnCode_t p9_stop_save_scom( void* const i_pImage,
const ScomOperation_t i_operation,
const ScomSection_t i_section );
+/**
+ * @brief verifies if API is compatible of current HOMER image.
+ * @param[in] i_pImage points to the start of HOMER image of P9 chip.
+ * @param[out] o_inCompVector list of incompatibilities found.
+ * @return STOP_SAVE_SUCCESS if if API succeeds, error code otherwise.
+ */
+StopReturnCode_t proc_stop_api_discover_capability( void* const i_pImage, uint64_t* o_inCompVector );
+
#ifdef __cplusplus
} // extern "C"
}; // namespace stopImageSection ends
diff --git a/libpore/p9_cpu_reg_restore_instruction.H b/libpore/p9_cpu_reg_restore_instruction.H
index d69a4212..5f168855 100644
--- a/libpore/p9_cpu_reg_restore_instruction.H
+++ b/libpore/p9_cpu_reg_restore_instruction.H
@@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
-/* Contributors Listed Below - COPYRIGHT 2015,2018 */
+/* Contributors Listed Below - COPYRIGHT 2015,2020 */
/* [+] International Business Machines Corp. */
/* */
/* */
@@ -69,6 +69,11 @@ enum
OPCODE_18 = 18,
SELF_SAVE_FUNC_ADD = 0x2300,
SELF_SAVE_OFFSET = 0x180,
+ SKIP_SPR_REST_INST = 0x4800001c, //b . +0x01c
+ MFLR_R30 = 0x7fc802a6,
+ SKIP_SPR_SELF_SAVE = 0x3bff0020, //addi r31 r31, 0x20
+ MTLR_INST = 0x7fc803a6, //mtlr r30
+ BRANCH_BE_INST = 0x48000020,
};
#define MR_R0_TO_R10 0x7c0a0378UL //mr r10 r0
diff --git a/libpore/p9_hcd_memmap_base.H b/libpore/p9_hcd_memmap_base.H
index 000fafef..ddb56728 100644
--- a/libpore/p9_hcd_memmap_base.H
+++ b/libpore/p9_hcd_memmap_base.H
@@ -444,6 +444,13 @@ HCD_CONST(CME_QUAD_PSTATE_SIZE, HALF_KB)
HCD_CONST(CME_REGION_SIZE, (64 * ONE_KB))
+
+// HOMER compatibility
+
+HCD_CONST(STOP_API_CPU_SAVE_VER, 0x02)
+HCD_CONST(SELF_SAVE_RESTORE_VER, 0x02)
+HCD_CONST(SMF_SUPPORT_SIGNATURE_OFFSET, 0x1300)
+HCD_CONST(SMF_SELF_SIGNATURE, (0x5f534d46))
// Debug
HCD_CONST(CPMR_TRACE_REGION_OFFSET, (512 * ONE_KB))
diff --git a/libpore/p9_stop_api.C b/libpore/p9_stop_api.C
index 2d9bb549..10e050a1 100644
--- a/libpore/p9_stop_api.C
+++ b/libpore/p9_stop_api.C
@@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
-/* Contributors Listed Below - COPYRIGHT 2015,2018 */
+/* Contributors Listed Below - COPYRIGHT 2015,2020 */
/* [+] International Business Machines Corp. */
/* */
/* */
@@ -1828,6 +1828,62 @@ StopReturnCode_t proc_stop_init_self_save( void* const i_pImage, const uint32_t
return l_rc;
}
+StopReturnCode_t proc_stop_api_discover_capability( void* const i_pImage, uint64_t * o_inCompVector )
+{
+ StopReturnCode_t l_rc = STOP_SAVE_SUCCESS;
+ uint64_t l_incompVector = 0;
+ uint32_t l_tempWord = 0;
+ *o_inCompVector = 0;
+
+ do
+ {
+ if( !i_pImage )
+ {
+ l_rc = STOP_SAVE_ARG_INVALID_IMG;
+ break;
+ }
+
+ l_tempWord =
+ *(uint32_t*)((uint8_t*)i_pImage + CPMR_HOMER_OFFSET + SMF_SUPPORT_SIGNATURE_OFFSET);
+
+ if( l_tempWord != SWIZZLE_4_BYTE(SMF_SELF_SIGNATURE) )
+ {
+ l_incompVector |= SMF_SUPPORT_MISSING_IN_HOMER;
+ }
+
+ l_tempWord = *(uint32_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_HEADER_SIZE );
+
+ if( l_tempWord != SWIZZLE_4_BYTE(BRANCH_BE_INST) )
+ {
+ l_incompVector |= SELF_SUPPORT_MISSING_FOR_LE_HYP;
+ }
+
+ l_tempWord = *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_SELF_RESTORE_VER_BYTE );
+
+ if( l_tempWord < SELF_SAVE_RESTORE_VER )
+ {
+ l_incompVector |= SELF_RESTORE_VER_MISMATCH;
+ }
+
+ l_tempWord = *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_STOP_API_VER_BYTE );
+
+ if( l_tempWord < STOP_API_CPU_SAVE_VER )
+ {
+ l_incompVector |= IPL_RUNTIME_CPU_SAVE_VER_MISMATCH;
+ }
+
+ *o_inCompVector = l_incompVector;
+
+ if( l_incompVector )
+ {
+ l_rc = STOP_SAVE_API_IMG_INCOMPATIBLE;
+ }
+
+ }while(0);
+
+ return l_rc;
+}
+
#ifdef __cplusplus
} //namespace stopImageSection ends
diff --git a/libpore/p9_stop_api.H b/libpore/p9_stop_api.H
index 3f6420ff..983a3845 100644
--- a/libpore/p9_stop_api.H
+++ b/libpore/p9_stop_api.H
@@ -5,7 +5,7 @@
/* */
/* OpenPOWER HostBoot Project */
/* */
-/* Contributors Listed Below - COPYRIGHT 2015,2018 */
+/* Contributors Listed Below - COPYRIGHT 2015,2020 */
/* [+] International Business Machines Corp. */
/* */
/* */
@@ -114,6 +114,7 @@ typedef enum
STOP_SAVE_FAIL = 14, // for internal failure within firmware.
STOP_SAVE_SPR_ENTRY_MISSING = 15,
STOP_SAVE_SPR_BIT_POS_RESERVE = 16,
+ STOP_SAVE_API_IMG_INCOMPATIBLE = 18,
} StopReturnCode_t;
/**
@@ -198,6 +199,21 @@ typedef enum
BIT_POS_USPRG1 = 30,
} SprBitPositionList_t;
+/**
+ * @brief List of major incompatibilities between API version.
+ * @note STOP APIs assumes a specific HOMER layout, certain
+ * level of CME-SGPE hcode and certain version of self-save restore
+ * binary. A mismatch can break STOP function.
+ */
+
+typedef enum
+{
+ SMF_SUPPORT_MISSING_IN_HOMER = 0x01,
+ SELF_SUPPORT_MISSING_FOR_LE_HYP = 0x02,
+ IPL_RUNTIME_CPU_SAVE_VER_MISMATCH = 0x04,
+ SELF_RESTORE_VER_MISMATCH = 0x08,
+} VersionIncompList_t;
+
#ifdef __cplusplus
extern "C" {
@@ -341,6 +357,14 @@ StopReturnCode_t proc_stop_save_cpureg( void* const i_pImage,
*/
StopReturnCode_t proc_stop_init_self_save( void* const i_pImage, const uint32_t i_corePos );
+/**
+ * @brief verifies if API is compatible of current HOMER image.
+ * @param[in] i_pImage points to the start of HOMER image of P9 chip.
+ * @param[out] o_inCompVector list of incompatibilities found.
+ * @return STOP_SAVE_SUCCESS if if API succeeds, error code otherwise.
+ */
+StopReturnCode_t proc_stop_api_discover_capability( void* const i_pImage, uint64_t* o_inCompVector );
+
#ifdef __cplusplus
} // extern "C"
}; // namespace stopImageSection ends
diff --git a/libpore/p9_stop_util.H b/libpore/p9_stop_util.H
index 79b4e959..1328a54b 100644
--- a/libpore/p9_stop_util.H
+++ b/libpore/p9_stop_util.H
@@ -72,18 +72,18 @@ namespace stopImageSection
( (((WORD) >> 8) & 0x00FF) | (((WORD) << 8) & 0xFF00) )
#define SWIZZLE_4_BYTE(WORD) \
- ( (((WORD) >> 24) & 0x000000FF) | (((WORD) >> 8) & 0x0000FF00) | \
- (((WORD) << 8) & 0x00FF0000) | (((WORD) << 24) & 0xFF000000) )
+ ( (((WORD) & 0x000000FF) << 24) | (((WORD) & 0x0000FF00) << 8) | \
+ (((WORD) & 0x00FF0000) >> 8) | (((WORD) & 0xFF000000) >> 24) )
#define SWIZZLE_8_BYTE(WORD) \
- ( (((WORD) >> 56) & 0x00000000000000FF) | \
- (((WORD) >> 40) & 0x000000000000FF00)| \
- (((WORD) >> 24) & 0x0000000000FF0000) | \
- (((WORD) >> 8) & 0x00000000FF000000) | \
- (((WORD) << 8) & 0x000000FF00000000) | \
- (((WORD) << 24) & 0x0000FF0000000000) | \
- (((WORD) << 40) & 0x00FF000000000000) | \
- (((WORD) << 56) & 0xFF00000000000000) )
+ ( (((WORD) & 0x00000000000000ffULL) << 56) | \
+ (((WORD) & 0x000000000000ff00ULL) << 40) | \
+ (((WORD) & 0x0000000000ff0000ULL) << 24) | \
+ (((WORD) & 0x00000000ff000000ULL) << 8) | \
+ (((WORD) & 0x000000ff00000000ULL) >> 8) | \
+ (((WORD) & 0x0000ff0000000000ULL) >> 24) | \
+ (((WORD) & 0x00ff000000000000ULL) >> 40) | \
+ (((WORD) & 0xff00000000000000ULL) >> 56) )
#endif
/**
--
2.25.1
^ permalink raw reply related
* [PATCH v8 0/3] Support for Self Save API in OPAL
From: Pratik Rajesh Sampat @ 2020-04-23 10:54 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
pratik.r.sampat, psampat
v7: https://lists.ozlabs.org/pipermail/skiboot/2020-April/016763.html
Changelog
v6 --> v7
1. Simplified approach. Instead of advertising support bitmask for
each SPR, advertising support only for the presence of the new
self-save API Complete design specification is detailed below
in the cover-leter.
2. Patch re-organization, move introducing self-save [2] after STOP
API versioning [3].
Background
==========
The power management framework on POWER systems include core idle
states that lose context. Deep idle states namely "winkle" on POWER8
and "stop4" and "stop5" on POWER9 can be entered by a CPU to save
different levels of power, as a consequence of which all the
hypervisor resources such as SPRs and SCOMs are lost.
For most SPRs, saving and restoration of content for SPRs and SCOMs
is handled by the hypervisor kernel prior to entering an post exit
from an idle state respectively. However, there is a small set of
critical SPRs and XSCOMs that are expected to contain sane values even
before the control is transferred to the hypervisor kernel at system
reset vector.
For this purpose, microcode firmware provides a mechanism to restore
values on certain SPRs. The communication mechanism between the
hypervisor kernel and the microcode is a standard interface called
sleep-winkle-engine (SLW) on Power8 and Stop-API on Power9 which is
abstracted by OPAL calls from the hypervisor kernel. The Stop-API
provides an interface known as the self-restore API, to which the SPR
number and a predefined value to be restored on wake-up from a deep
stop state is supplied.
Motivation to introduce a new Stop-API
======================================
The self-restore API expects not just the SPR number but also the
value with which the SPR is restored. This is good for those SPRs such
as HSPRG0 whose values do not change at runtime, since for them, the
kernel can invoke the self-restore API at boot time once the values of
these SPRs are determined.
However, there are use-cases where-in the value to be saved cannot be
known or cannot be updated in the layer it currently is.
The shortcomings and the new use-cases which cannot be served by the
existing self-restore API, serves as motivation for a new API:
Shortcoming1:
------------
In a special wakeup scenario when a CPU is woken up in stop4/5 and
after the task is done, the HCODE puts it back to stop. The value of
PSSCR is passed to the HCODE via the self-restore API. The kernel
currently provides the value of the deepest stop state due to being
conservative. Thus if a core that was in stop4 was woken up due to
special wakeup, the HCODE will now put it back to stop5 thus increasing
the subsequent wakeup latency to ~200us.
Shortcoming2:
------------
The value of LPCR is dynamic based on if the CPU is entered a stop
state during cpu idle versus cpu hotplug.
Today, an additional self-restore call is made before entering
CPU-Hotplug to clear the PECE1 bit in stop-API so that if we are
woken up by a special wakeup on an offlined CPU, we go back to stop
with the the bit cleared.
There is a overhead of an extra call
New Use-case:
-------------
In the case where the hypervisor is running on an
ultravisor environment, the boot time is too late in the cycle to make
the self-restore API calls, as these cannot be invoked from an
non-secure context anymore
To address these shortcomings, the firmware provides another API known
as the self-save API. The self-save API only takes the SPR number as a
parameter and will ensure that on wakeup from a deep-stop state the
SPR is restored with the value that it contained prior to entering the
deep-stop.
Contrast between self-save and self-restore APIs
================================================
Before entering
deep idle |---------------|
------------> | HCODE A |
| |---------------|
|---------| |
| CPU |----|
|---------| |
| |---------------|
|------------>| HCODE B |
On waking up |---------------|
from deep idle
When a self-restore API is invoked, the HCODE inserts instructions
into "HCODE B" region of the above figure to restore the content of
the SPR to the said value. The "HCODE B" region gets executed soon
after the CPU wakes up from a deep idle state, thus executing the
inserted instructions, thereby restoring the contents of the SPRs to
the required values.
When a self-save API is invoked, the HCODE inserts instructions into
the "HCODE A" region of the above figure to save the content of the
SPR into some location in memory. It also inserts instructions into
the "HCODE B" region to restore the content of the SPR to the
corresponding value saved in the memory by the instructions in "HCODE
A" region.
Thus, in contrast with self-restore, the self-save API *does not* need
a value to be passed to it, since it ensures that the value of SPR
before entering deep stop is saved, and subsequently the same value is
restored.
Self-save and self-restore are complementary features since,
self-restore can help in restoring a different value in the SPR on
wakeup from a deep-idle state than what it had before entering the
deep idle state. This was used in POWER8 for HSPRG0 to distinguish a
wakeup from Winkle vs Fastsleep.
Limitations of self-save
========================
Ideally all SPRs should be available for self-save, but HID0 is very
tricky to implement in microcode due to various endianess quirks.
Couple of implementation schemes were buggy and hence HID0 was left
out to be self-restore only.
The fallout of this limitation is as follows:
* In Non PEF environment, no issue. Linux will use self-restore for
HID0 as it does today and no functional impact.
* In PEF environment, the HID0 restore value is decided by OPAL during
boot and it is setup for LE hypervisor with radix MMU. This is the
default and current working configuration of a PEF environment.
However if there is a change, then HV Linux will try to change the
HID0 value to something different than what OPAL decided, at which
time deep-stop states will be disabled under this new PEF
environment.
A simple and workable design is achieved by scoping the power
management deep-stop state support only to a known default PEF
environment. Any deviation will affect *only* deep stop-state support
(stop4,5) in that environment and not have any functional impediment
to the environment itself.
In future, if there is a need to support changing of HID0 to various
values under PEF environment and support deep-stop states, it can be
worked out via an ultravisor call or improve the microcode design to
include HID0 in self-save. These future scheme would be an extension
and does not break or make the current implementation scheme
redundant.
Design Choices
==============
Presenting the design choices in front of us:
Design-Choice 1:
----------------
Only expose one of self-save or self-restore for all the SPRs. Prefer
Self-save
Pros:
- Simplifies the design heavily, since the Kernel can unambiguously
make one API call for all the SPRs on discovering the presence of
the API type.
Cons:
- Breaks backward compatibility if OPAL always chooses to expose
only the self-save API as the older kernels assume the existence
of self-restore.
- The set of SPRs supported by self-save and self-restore are not
identical. Eg: HID0 is not supported by self-save API. PSSCR
support via self-restore is not robust during special-wakeup.
- As discussed above, self-save and self-restore are
complementary. Thus OPAL apriory choosing one over the other for
all SPRs takes away the flexibility from the kernel.
Design-Choice 2:
----------------
Expose two arrays of SPRs: One set of SPRs that are supported by
self-save. Another set of SPRs supported by self-restore. These two
sets do not intersect. Further, if an SPR is supported by both
self-save and self-restore APIs, expose it only via self-save.
Pros:
- For an SPR the choice for the kernel is unambiguous.
Cons:
- Breaks backward compatibility if OPAL always chooses to expose
the legacy SPRs only via the self-save API as the older kernels
assume the existence of self-restore.
- By making the decision early on, we take away the flexibility
from the kernel to use an API of its choice for an SPR.
Design-Choice 3
---------------
Expose two arrays of SPRs. One set of SPRs that are supported by
self-save API. Another set of SPRs supported by self-restore API. Let
the kernel choose which API to invoke. Even if it wants to always
prefer self-save over self-restore, let that be kernel's choice.
Pros:
- Keeps the design flexible to allow the kernel to take a
decision based on its functional and performance requirements.
Thus, the kernel for instance can make a choice to invoke
self-restore API (when available) for SPRs whose values do not
evolve at runtime, and invoke the self-save API (when
available)
for SPRs whose values will change during runtime.
- Design is backward compatible with older kernels.
Cons:
- The Kernel code will have additional complexity for parsing two
lists of SPRs and making a choice w.r.t invocation of a specific
stop-api.
Design-Choice 4
===============
Expose only the availability of the self-save feature not the subsequent
SPR list. The kernel will have to make a hard choice of preference,
although can still have fallback available.
This makes the design although less flexible for the kernel however
keeps it simple while still adhering to correctness and backward
compatibility.
Patches Organization
====================
Design choice 4 has been chosen as an implementation to demonstrate in
this patch series.
Patch 1:
Commit adds support calling into the self save firmware API.
Also adds abstraction for making platform agnostic calls.
Patch 2:
Commit adds API to determine the version of the STOP API. This helps
to identify support for self save in the firmware
Patch 3:
commit adds wrappers for the self save api for which an opal call can
be made. The commit also advertises the self-save feature if the STOP
API versions are consistent with the firmware.
Pratik Rajesh Sampat (1):
Self save API integration
Prem Shanker Jha (2):
Self Save: Introducing Support for SPR Self Save
API to verify the STOP API and image compatibility
doc/opal-api/opal-slw-self-save-reg-181.rst | 51 ++
doc/opal-api/opal-slw-set-reg-100.rst | 5 +
doc/power-management.rst | 48 +
hw/slw.c | 106 +++
include/opal-api.h | 3 +-
include/p9_stop_api.H | 122 ++-
include/skiboot.h | 3 +
libpore/p9_cpu_reg_restore_instruction.H | 11 +-
libpore/p9_hcd_memmap_base.H | 7 +
libpore/p9_stop_api.C | 964 +++++++++++---------
libpore/p9_stop_api.H | 141 ++-
libpore/p9_stop_data_struct.H | 4 +-
libpore/p9_stop_util.H | 27 +-
13 files changed, 1060 insertions(+), 432 deletions(-)
create mode 100644 doc/opal-api/opal-slw-self-save-reg-181.rst
--
2.25.1
^ permalink raw reply
* Re: [PATCH v2 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Cornelia Huck @ 2020-04-23 10:39 UTC (permalink / raw)
To: Tianjia Zhang
Cc: wanpengli, kvm, david, heiko.carstens, peterx, linux-mips, hpa,
kvmarm, linux-s390, frankja, maz, joro, x86,
Christian Borntraeger, mingo, julien.thierry.kdev, thuth, gor,
suzuki.poulose, kvm-ppc, bp, tglx, linux-arm-kernel, jmattson,
tsbogend, christoffer.dall, sean.j.christopherson, linux-kernel,
james.morse, pbonzini, vkuznets, linuxppc-dev
In-Reply-To: <5e1e126d-f1b0-196c-594b-4289d0afb9a8@linux.alibaba.com>
On Thu, 23 Apr 2020 11:01:43 +0800
Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
> On 2020/4/23 0:04, Cornelia Huck wrote:
> > On Wed, 22 Apr 2020 17:58:04 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >
> >> On 22.04.20 15:45, Cornelia Huck wrote:
> >>> On Wed, 22 Apr 2020 20:58:04 +0800
> >>> Tianjia Zhang <tianjia.zhang@linux.alibaba.com> wrote:
> >>>
> >>>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
> >>>> structure. Earlier than historical reasons, many kvm-related function
> >>>
> >>> s/Earlier than/For/ ?
> >>>
> >>>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
> >>>> This patch does a unified cleanup of these remaining redundant parameters.
> >>>>
> >>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> >>>> ---
> >>>> arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++---------------
> >>>> 1 file changed, 22 insertions(+), 15 deletions(-)
> >>>>
> >>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> >>>> index e335a7e5ead7..d7bb2e7a07ff 100644
> >>>> --- a/arch/s390/kvm/kvm-s390.c
> >>>> +++ b/arch/s390/kvm/kvm-s390.c
> >>>> @@ -4176,8 +4176,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
> >>>> return rc;
> >>>> }
> >>>>
> >>>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> >>>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
> >>>> {
> >>>> + struct kvm_run *kvm_run = vcpu->run;
> >>>> struct runtime_instr_cb *riccb;
> >>>> struct gs_cb *gscb;
> >>>>
> >>>> @@ -4235,7 +4236,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> >>>> }
> >>>> if (vcpu->arch.gs_enabled) {
> >>>> current->thread.gs_cb = (struct gs_cb *)
> >>>> - &vcpu->run->s.regs.gscb;
> >>>> + &kvm_run->s.regs.gscb;
> >>>
> >>> Not sure if these changes (vcpu->run-> => kvm_run->) are really worth
> >>> it. (It seems they amount to at least as much as the changes advertised
> >>> in the patch description.)
> >>>
> >>> Other opinions?
> >>
> >> Agreed. It feels kind of random. Maybe just do the first line (move kvm_run from the
> >> function parameter list into the variable declaration)? Not sure if this is better.
> >>
> >
> > There's more in this patch that I cut... but I think just moving
> > kvm_run from the parameter list would be much less disruptive.
> >
>
> I think there are two kinds of code(`vcpu->run->` and `kvm_run->`), but
> there will be more disruptive, not less.
I just fail to see the benefit; sure, kvm_run-> is convenient, but the
current code is just fine, and any rework should be balanced against
the cost (e.g. cluttering git annotate).
^ permalink raw reply
* Re: [PATCH v5 0/5] Track and expose idle PURR and SPURR ticks
From: Gautham R Shenoy @ 2020-04-23 10:02 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: Nathan Lynch, Gautham R. Shenoy, linux-kernel, Kamalesh Babulal,
Naveen N. Rao, Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <04b5e2fa-089f-93c9-cde9-33a930455bb2@linux.ibm.com>
On Mon, Apr 20, 2020 at 03:46:35PM -0700, Tyrel Datwyler wrote:
> On 4/7/20 1:47 AM, Gautham R. Shenoy wrote:
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> >
> > Hi,
> >
> > This is the fifth version of the patches to track and expose idle PURR
> > and SPURR ticks. These patches are required by tools such as lparstat
> > to compute system utilization for capacity planning purposes.
> >
> > The previous versions can be found here:
> > v4: https://lkml.org/lkml/2020/3/27/323
> > v3: https://lkml.org/lkml/2020/3/11/331
> > v2: https://lkml.org/lkml/2020/2/21/21
> > v1: https://lore.kernel.org/patchwork/cover/1159341/
> >
> > They changes from v4 are:
> >
> > - As suggested by Naveen, moved the functions read_this_idle_purr()
> > and read_this_idle_spurr() from Patch 2 and Patch 3 respectively
> > to Patch 4 where it is invoked.
> >
> > - Dropped Patch 6 which cached the values of purr, spurr,
> > idle_purr, idle_spurr in order to minimize the number of IPIs
> > sent.
> >
> > - Updated the dates for the idle_purr, idle_spurr in the
> > Documentation Patch 5.
> >
> > Motivation:
> > ===========
> > On PSeries LPARs, the data centers planners desire a more accurate
> > view of system utilization per resource such as CPU to plan the system
> > capacity requirements better. Such accuracy can be obtained by reading
> > PURR/SPURR registers for CPU resource utilization.
> >
> > Tools such as lparstat which are used to compute the utilization need
> > to know [S]PURR ticks when the cpu was busy or idle. The [S]PURR
> > counters are already exposed through sysfs. We already account for
> > PURR ticks when we go to idle so that we can update the VPA area. This
> > patchset extends support to account for SPURR ticks when idle, and
> > expose both via per-cpu sysfs files.
> >
> > These patches are required for enhancement to the lparstat utility
> > that compute the CPU utilization based on PURR and SPURR which can be
> > found here :
> > https://groups.google.com/forum/#!topic/powerpc-utils-devel/fYRo69xO9r4
> >
> >
> > With the patches, when lparstat is run on a LPAR running CPU-Hogs,
> > =========================================================================
> > sudo ./src/lparstat -E 1 3
> >
> > System Configuration
> > type=Dedicated mode=Capped smt=8 lcpu=2 mem=4834112 kB cpus=0 ent=2.00
> >
> > ---Actual--- -Normalized-
> > %busy %idle Frequency %busy %idle
> > ------ ------ ------------- ------ ------
> > 1 99.99 0.00 3.35GHz[111%] 110.99 0.00
> > 2 100.00 0.00 3.35GHz[111%] 111.01 0.00
> > 3 100.00 0.00 3.35GHz[111%] 111.00 0.00
> >
> > With patches, when lparstat is run on and idle LPAR
> > =========================================================================
> > System Configuration
> > type=Dedicated mode=Capped smt=8 lcpu=2 mem=4834112 kB cpus=0 ent=2.00
> > ---Actual--- -Normalized-
> > %busy %idle Frequency %busy %idle
> > ------ ------ ------------- ------ ------
> > 1 0.15 99.84 2.17GHz[ 72%] 0.11 71.89
> > 2 0.24 99.76 2.11GHz[ 70%] 0.18 69.82
> > 3 0.24 99.75 2.11GHz[ 70%] 0.18 69.81
> >
> > Gautham R. Shenoy (5):
> > powerpc: Move idle_loop_prolog()/epilog() functions to header file
> > powerpc/idle: Store PURR snapshot in a per-cpu global variable
> > powerpc/pseries: Account for SPURR ticks on idle CPUs
> > powerpc/sysfs: Show idle_purr and idle_spurr for every CPU
> > Documentation: Document sysfs interfaces purr, spurr, idle_purr,
> > idle_spurr
> >
> > Documentation/ABI/testing/sysfs-devices-system-cpu | 39 +++++++++
> > arch/powerpc/include/asm/idle.h | 93 ++++++++++++++++++++++
> > arch/powerpc/kernel/sysfs.c | 82 ++++++++++++++++++-
> > arch/powerpc/platforms/pseries/setup.c | 8 +-
> > drivers/cpuidle/cpuidle-pseries.c | 39 ++-------
> > 5 files changed, 224 insertions(+), 37 deletions(-)
> > create mode 100644 arch/powerpc/include/asm/idle.h
> >
>
> Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Thanks for reviewing the patches.
>
> Any chance this is going to be merged in the near future? There is a patchset to
> update lparstat in the powerpc-utils package to calculate PURR/SPURR cpu
> utilization that I would like to merge, but have been holding off to make sure
> we are synced with this proposed patchset.
Michael, could you please consider this for 5.8 ?
--
Thanks and Regards
gautham.
^ permalink raw reply
* [PATCH][next] ASoC: fsl_easrc: fix spelling mistake "prefitler" -> "prefilter"
From: Colin King @ 2020-04-23 8:39 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel,
linuxppc-dev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a deb_dbg message, fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
sound/soc/fsl/fsl_easrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 233f26ff885c..97658e1f4989 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -1769,7 +1769,7 @@ static void fsl_easrc_dump_firmware(struct fsl_asrc *easrc)
}
dev_dbg(dev, "Firmware v%u dump:\n", firm->firmware_version);
- dev_dbg(dev, "Num prefitler scenarios: %u\n", firm->prefil_scen);
+ dev_dbg(dev, "Num prefilter scenarios: %u\n", firm->prefil_scen);
dev_dbg(dev, "Num interpolation scenarios: %u\n", firm->interp_scen);
dev_dbg(dev, "\nInterpolation scenarios:\n");
--
2.25.1
^ permalink raw reply related
* [Bug 206695] kmemleak reports leaks in drivers/macintosh/windfarm
From: bugzilla-daemon @ 2020-04-23 6:54 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206695-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206695
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |CODE_FIX
--- Comment #7 from Michael Ellerman (michael@ellerman.id.au) ---
Patch posted:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200423060038.3308530-1-mpe@ellerman.id.au/
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206695] kmemleak reports leaks in drivers/macintosh/windfarm
From: bugzilla-daemon @ 2020-04-23 6:53 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206695-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206695
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
CC| |michael@ellerman.id.au
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH 18/21] mm: rename free_area_init_node() to free_area_init_memoryless_node()
From: Mike Rapoport @ 2020-04-23 6:18 UTC (permalink / raw)
To: Baoquan He
Cc: Rich Felker, linux-ia64, linux-doc, Catalin Marinas,
Heiko Carstens, Michal Hocko, James E.J. Bottomley, Max Filippov,
Guo Ren, linux-csky, linux-parisc, sparclinux, linux-hexagon,
linux-riscv, Greg Ungerer, linux-arch, linux-s390, linux-snps-arc,
linux-c6x-dev, Brian Cain, Jonathan Corbet, linux-sh,
Helge Deller, x86, Russell King, Ley Foon Tan, Mike Rapoport,
Geert Uytterhoeven, linux-arm-kernel, Mark Salter, Matt Turner,
linux-mips, uclinux-h8-devel, linux-xtensa, linux-alpha, linux-um,
linux-m68k, Tony Luck, Greentime Hu, Paul Walmsley,
Stafford Horne, Guan Xuetao, Hoan Tran, Michal Simek,
Thomas Bogendoerfer, Yoshinori Sato, Nick Hu, linux-mm,
Vineet Gupta, linux-kernel, openrisc, Richard Weinberger,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200423031454.GB4247@MiWiFi-R3L-srv>
On Thu, Apr 23, 2020 at 11:14:54AM +0800, Baoquan He wrote:
> On 04/12/20 at 10:48pm, Mike Rapoport wrote:
> > From: Mike Rapoport <rppt@linux.ibm.com>
> >
> > The free_area_init_node() is only used by x86 to initialize a memory-less
> > nodes.
> > Make its name reflect this and drop all the function parameters except node
> > ID as they are anyway zero.
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > ---
> > arch/x86/mm/numa.c | 5 +----
> > include/linux/mm.h | 9 +++------
> > mm/page_alloc.c | 7 ++-----
> > 3 files changed, 6 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> > index fe024b2ac796..8ee952038c80 100644
> > --- a/arch/x86/mm/numa.c
> > +++ b/arch/x86/mm/numa.c
> > @@ -737,12 +737,9 @@ void __init x86_numa_init(void)
> >
> > static void __init init_memory_less_node(int nid)
> > {
> > - unsigned long zones_size[MAX_NR_ZONES] = {0};
> > - unsigned long zholes_size[MAX_NR_ZONES] = {0};
> > -
> > /* Allocate and initialize node data. Memory-less node is now online.*/
> > alloc_node_data(nid);
> > - free_area_init_node(nid, zones_size, 0, zholes_size);
> > + free_area_init_memoryless_node(nid);
> >
> > /*
> > * All zonelists will be built later in start_kernel() after per cpu
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 1c2ecb42e043..27660f6cf26e 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2272,8 +2272,7 @@ static inline spinlock_t *pud_lock(struct mm_struct *mm, pud_t *pud)
> > }
> >
> > extern void __init pagecache_init(void);
> > -extern void __init free_area_init_node(int nid, unsigned long * zones_size,
> > - unsigned long zone_start_pfn, unsigned long *zholes_size);
> > +extern void __init free_area_init_memoryless_node(int nid);
> > extern void free_initmem(void);
> >
> > /*
> > @@ -2345,10 +2344,8 @@ static inline unsigned long get_num_physpages(void)
> >
> > /*
> > * Using memblock node mappings, an architecture may initialise its
> > - * zones, allocate the backing mem_map and account for memory holes in a more
> > - * architecture independent manner. This is a substitute for creating the
> > - * zone_sizes[] and zholes_size[] arrays and passing them to
> > - * free_area_init_node()
> > + * zones, allocate the backing mem_map and account for memory holes in an
> > + * architecture independent manner.
> > *
> > * An architecture is expected to register range of page frames backed by
> > * physical memory with memblock_add[_node]() before calling
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 376434c7a78b..e46232ec4849 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -6979,12 +6979,9 @@ static void __init __free_area_init_node(int nid, unsigned long *zones_size,
> > free_area_init_core(pgdat);
> > }
> >
> > -void __init free_area_init_node(int nid, unsigned long *zones_size,
> > - unsigned long node_start_pfn,
> > - unsigned long *zholes_size)
> > +void __init free_area_init_memoryless_node(int nid)
> > {
> > - __free_area_init_node(nid, zones_size, node_start_pfn, zholes_size,
> > - true);
> > + __free_area_init_node(nid, NULL, 0, NULL, false);
>
> Can we move free_area_init_memoryless_node() definition into
> arch/x86/mm/numa.c since there's only one caller there?
>
> And I am also wondering if adding a wrapper
> free_area_init_memoryless_node() is necessary if it's only called the
> function free_area_init_node().
Yeah, I think this patch can be entirely dropped and the next one could
be slightly updated :)
Thanks!
> > }
> >
> > #if !defined(CONFIG_FLAT_NODE_MEM_MAP) --
> > 2.25.1
> >
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* [Bug 199471] [Bisected][Regression] windfarm_pm* no longer gets automatically loaded when CONFIG_I2C_POWERMAC=y is set
From: bugzilla-daemon @ 2020-04-23 6:11 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-199471-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=199471
Wolfram Sang (wsa@the-dreams.de) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |CODE_FIX
--- Comment #22 from Wolfram Sang (wsa@the-dreams.de) ---
Fixed upstream with commit bcf3588d8ed3517e6ffaf083f034812aee9dc8e2.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH] drivers/macintosh: Fix memleak in windfarm_pm112 driver
From: Michael Ellerman @ 2020-04-23 6:00 UTC (permalink / raw)
To: linuxppc-dev; +Cc: erhard_f
create_cpu_loop() calls smu_sat_get_sdb_partition() which does
kmalloc() and returns the allocated buffer. In fact it's called twice,
and neither buffer is freed.
This results in a memory leak as reported by Erhard:
unreferenced object 0xc00000047081f840 (size 32):
comm "kwindfarm", pid 203, jiffies 4294880630 (age 5552.877s)
hex dump (first 32 bytes):
c8 06 02 7f ff 02 ff 01 fb bf 00 41 00 20 00 00 ...........A. ..
00 07 89 37 00 a0 00 00 00 00 00 00 00 00 00 00 ...7............
backtrace:
[<0000000083f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0 [windfarm_smu_sat]
[<000000003010fcb7>] .pm112_wf_notify+0x104c/0x13bc [windfarm_pm112]
[<00000000b958b2dd>] .notifier_call_chain+0xa8/0x180
[<0000000070490868>] .blocking_notifier_call_chain+0x64/0x90
[<00000000131d8149>] .wf_thread_func+0x114/0x1a0
[<000000000d54838d>] .kthread+0x13c/0x190
[<00000000669b72bc>] .ret_from_kernel_thread+0x58/0x64
unreferenced object 0xc0000004737089f0 (size 16):
comm "kwindfarm", pid 203, jiffies 4294880879 (age 5552.050s)
hex dump (first 16 bytes):
c4 04 01 7f 22 11 e0 e6 ff 55 7b 12 ec 11 00 00 ...."....U{.....
backtrace:
[<0000000083f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0 [windfarm_smu_sat]
[<00000000b94ef7e1>] .pm112_wf_notify+0x1294/0x13bc [windfarm_pm112]
[<00000000b958b2dd>] .notifier_call_chain+0xa8/0x180
[<0000000070490868>] .blocking_notifier_call_chain+0x64/0x90
[<00000000131d8149>] .wf_thread_func+0x114/0x1a0
[<000000000d54838d>] .kthread+0x13c/0x190
[<00000000669b72bc>] .ret_from_kernel_thread+0x58/0x64
Fix it by rearranging the logic so we deal with each buffer
separately, which then makes it easy to free the buffer once we're
done with it.
Fixes: ac171c46667c ("[PATCH] powerpc: Thermal control for dual core G5s")
Cc: stable@vger.kernel.org # v2.6.16+
Reported-by: Erhard F. <erhard_f@mailbox.org>
Tested-by: Erhard F. <erhard_f@mailbox.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
drivers/macintosh/windfarm_pm112.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/macintosh/windfarm_pm112.c b/drivers/macintosh/windfarm_pm112.c
index 4150301a89a5..e8377ce0a95a 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c
@@ -132,14 +132,6 @@ static int create_cpu_loop(int cpu)
s32 tmax;
int fmin;
- /* Get PID params from the appropriate SAT */
- hdr = smu_sat_get_sdb_partition(chip, 0xC8 + core, NULL);
- if (hdr == NULL) {
- printk(KERN_WARNING"windfarm: can't get CPU PID fan config\n");
- return -EINVAL;
- }
- piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
-
/* Get FVT params to get Tmax; if not found, assume default */
hdr = smu_sat_get_sdb_partition(chip, 0xC4 + core, NULL);
if (hdr) {
@@ -152,6 +144,16 @@ static int create_cpu_loop(int cpu)
if (tmax < cpu_all_tmax)
cpu_all_tmax = tmax;
+ kfree(hdr);
+
+ /* Get PID params from the appropriate SAT */
+ hdr = smu_sat_get_sdb_partition(chip, 0xC8 + core, NULL);
+ if (hdr == NULL) {
+ printk(KERN_WARNING"windfarm: can't get CPU PID fan config\n");
+ return -EINVAL;
+ }
+ piddata = (struct smu_sdbp_cpupiddata *)&hdr[1];
+
/*
* Darwin has a minimum fan speed of 1000 rpm for the 4-way and
* 515 for the 2-way. That appears to be overkill, so for now,
@@ -174,6 +176,9 @@ static int create_cpu_loop(int cpu)
pid.min = fmin;
wf_cpu_pid_init(&cpu_pid[cpu], &pid);
+
+ kfree(hdr);
+
return 0;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 17/21] mm: free_area_init: allow defining max_zone_pfn in descending order
From: Mike Rapoport @ 2020-04-23 5:55 UTC (permalink / raw)
To: Baoquan He
Cc: Rich Felker, linux-ia64, linux-doc, Catalin Marinas,
Heiko Carstens, Michal Hocko, James E.J. Bottomley, Max Filippov,
Guo Ren, linux-csky, linux-parisc, sparclinux, linux-hexagon,
linux-riscv, Greg Ungerer, linux-arch, linux-s390, linux-snps-arc,
linux-c6x-dev, Brian Cain, Jonathan Corbet, linux-sh,
Helge Deller, x86, Russell King, Ley Foon Tan, Mike Rapoport,
Geert Uytterhoeven, linux-arm-kernel, Mark Salter, Matt Turner,
linux-mips, uclinux-h8-devel, linux-xtensa, linux-alpha, linux-um,
linux-m68k, Tony Luck, Greentime Hu, Paul Walmsley,
Stafford Horne, Guan Xuetao, Hoan Tran, Michal Simek,
Thomas Bogendoerfer, Yoshinori Sato, Nick Hu, linux-mm,
Vineet Gupta, linux-kernel, openrisc, Richard Weinberger,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200423025720.GA4247@MiWiFi-R3L-srv>
On Thu, Apr 23, 2020 at 10:57:20AM +0800, Baoquan He wrote:
> On 04/23/20 at 10:53am, Baoquan He wrote:
> > On 04/12/20 at 10:48pm, Mike Rapoport wrote:
> > > From: Mike Rapoport <rppt@linux.ibm.com>
> > >
> > > Some architectures (e.g. ARC) have the ZONE_HIGHMEM zone below the
> > > ZONE_NORMAL. Allowing free_area_init() parse max_zone_pfn array even it is
> > > sorted in descending order allows using free_area_init() on such
> > > architectures.
> > >
> > > Add top -> down traversal of max_zone_pfn array in free_area_init() and use
> > > the latter in ARC node/zone initialization.
> >
> > Or maybe leave ARC as is. The change in this patchset doesn't impact
> > ARC's handling about zone initialization, leaving it as is can reduce
> > the complication in implementation of free_area_init(), which is a
> > common function. So I personally don't see a strong motivation to have
> > this patch.
>
> OK, seems this patch is prepared to simplify free_area_init_node(), so
> take back what I said at above.
>
> Then this looks necessary, even though it introduces special case into
> common function free_area_init().
The idea is to have a single free_area_init() for all architectures
without keeping two completely different ways of calculating the zone
extents.
Another thing, is that with this we could eventually switch ARC from
DISCONTIGMEM.
> Reviewed-by: Baoquan He <bhe@redhat.com>
>
> >
> > >
> > > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > > ---
> > > arch/arc/mm/init.c | 36 +++++++-----------------------------
> > > mm/page_alloc.c | 24 +++++++++++++++++++-----
> > > 2 files changed, 26 insertions(+), 34 deletions(-)
> > >
> > > diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> > > index 0920c969c466..41eb9be1653c 100644
> > > --- a/arch/arc/mm/init.c
> > > +++ b/arch/arc/mm/init.c
> > > @@ -63,11 +63,13 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
> > >
> > > low_mem_sz = size;
> > > in_use = 1;
> > > + memblock_add_node(base, size, 0);
> > > } else {
> > > #ifdef CONFIG_HIGHMEM
> > > high_mem_start = base;
> > > high_mem_sz = size;
> > > in_use = 1;
> > > + memblock_add_node(base, size, 1);
> > > #endif
> > > }
> > >
> > > @@ -83,8 +85,7 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
> > > */
> > > void __init setup_arch_memory(void)
> > > {
> > > - unsigned long zones_size[MAX_NR_ZONES];
> > > - unsigned long zones_holes[MAX_NR_ZONES];
> > > + unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
> > >
> > > init_mm.start_code = (unsigned long)_text;
> > > init_mm.end_code = (unsigned long)_etext;
> > > @@ -115,7 +116,6 @@ void __init setup_arch_memory(void)
> > > * the crash
> > > */
> > >
> > > - memblock_add_node(low_mem_start, low_mem_sz, 0);
> > > memblock_reserve(CONFIG_LINUX_LINK_BASE,
> > > __pa(_end) - CONFIG_LINUX_LINK_BASE);
> > >
> > > @@ -133,22 +133,7 @@ void __init setup_arch_memory(void)
> > > memblock_dump_all();
> > >
> > > /*----------------- node/zones setup --------------------------*/
> > > - memset(zones_size, 0, sizeof(zones_size));
> > > - memset(zones_holes, 0, sizeof(zones_holes));
> > > -
> > > - zones_size[ZONE_NORMAL] = max_low_pfn - min_low_pfn;
> > > - zones_holes[ZONE_NORMAL] = 0;
> > > -
> > > - /*
> > > - * We can't use the helper free_area_init(zones[]) because it uses
> > > - * PAGE_OFFSET to compute the @min_low_pfn which would be wrong
> > > - * when our kernel doesn't start at PAGE_OFFSET, i.e.
> > > - * PAGE_OFFSET != CONFIG_LINUX_RAM_BASE
> > > - */
> > > - free_area_init_node(0, /* node-id */
> > > - zones_size, /* num pages per zone */
> > > - min_low_pfn, /* first pfn of node */
> > > - zones_holes); /* holes */
> > > + max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
> > >
> > > #ifdef CONFIG_HIGHMEM
> > > /*
> > > @@ -168,20 +153,13 @@ void __init setup_arch_memory(void)
> > > min_high_pfn = PFN_DOWN(high_mem_start);
> > > max_high_pfn = PFN_DOWN(high_mem_start + high_mem_sz);
> > >
> > > - zones_size[ZONE_NORMAL] = 0;
> > > - zones_holes[ZONE_NORMAL] = 0;
> > > -
> > > - zones_size[ZONE_HIGHMEM] = max_high_pfn - min_high_pfn;
> > > - zones_holes[ZONE_HIGHMEM] = 0;
> > > -
> > > - free_area_init_node(1, /* node-id */
> > > - zones_size, /* num pages per zone */
> > > - min_high_pfn, /* first pfn of node */
> > > - zones_holes); /* holes */
> > > + max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
> > >
> > > high_memory = (void *)(min_high_pfn << PAGE_SHIFT);
> > > kmap_init();
> > > #endif
> > > +
> > > + free_area_init(max_zone_pfn);
> > > }
> > >
> > > /*
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 343d87b8697d..376434c7a78b 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -7429,7 +7429,8 @@ static void check_for_memory(pg_data_t *pgdat, int nid)
> > > void __init free_area_init(unsigned long *max_zone_pfn)
> > > {
> > > unsigned long start_pfn, end_pfn;
> > > - int i, nid;
> > > + int i, nid, zone;
> > > + bool descending = false;
> > >
> > > /* Record where the zone boundaries are */
> > > memset(arch_zone_lowest_possible_pfn, 0,
> > > @@ -7439,13 +7440,26 @@ void __init free_area_init(unsigned long *max_zone_pfn)
> > >
> > > start_pfn = find_min_pfn_with_active_regions();
> > >
> > > + /*
> > > + * Some architecturs, e.g. ARC may have ZONE_HIGHMEM below
> > > + * ZONE_NORMAL. For such cases we allow max_zone_pfn sorted in the
> > > + * descending order
> > > + */
> > > + if (MAX_NR_ZONES > 1 && max_zone_pfn[0] > max_zone_pfn[1])
> > > + descending = true;
> > > +
> > > for (i = 0; i < MAX_NR_ZONES; i++) {
> > > - if (i == ZONE_MOVABLE)
> > > + if (descending)
> > > + zone = MAX_NR_ZONES - i - 1;
> > > + else
> > > + zone = i;
> > > +
> > > + if (zone == ZONE_MOVABLE)
> > > continue;
> > >
> > > - end_pfn = max(max_zone_pfn[i], start_pfn);
> > > - arch_zone_lowest_possible_pfn[i] = start_pfn;
> > > - arch_zone_highest_possible_pfn[i] = end_pfn;
> > > + end_pfn = max(max_zone_pfn[zone], start_pfn);
> > > + arch_zone_lowest_possible_pfn[zone] = start_pfn;
> > > + arch_zone_highest_possible_pfn[zone] = end_pfn;
> > >
> > > start_pfn = end_pfn;
> > > }
> > > --
> > > 2.25.1
> > >
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH 3/3] selftests/powerpc: ensure PMC reads are set and ordered on count_pmc
From: Rashmica Gupta @ 2020-04-23 5:54 UTC (permalink / raw)
To: Desnes A. Nunes do Rosario, linuxppc-dev; +Cc: shuah, gromero
In-Reply-To: <20200408223543.21168-4-desnesn@linux.ibm.com>
On Wed, 2020-04-08 at 19:35 -0300, Desnes A. Nunes do Rosario wrote:
> Function count_pmc() needs a memory barrier to ensure that PMC reads
> are
> fully consistent. The lack of it can occasionally fail pmc56_overflow
> test,
> since depending on the workload on the system, PMC5 & 6 can have past
> val-
> ues from the time the counters are frozen and turned back on. These
> past
> values will be accounted as overflows and make the test fail.
>
> =========
> test: pmc56_overflow
> ...
> ebb_state:
> ...
> > > pmc[5] count = 0xfd4cbc8c
> > > pmc[6] count = 0xddd8b3b6
> HW state:
> MMCR0 0x0000000084000000 FC PMAE
> MMCR2 0x0000000000000000
> EBBHR 0x0000000010003f68
> BESCR 0x8000000000000000 GE
> ...
> PMC5 0x0000000000000000
> PMC6 0x0000000000000000
> SIAR 0x0000000010003398
> ...
> [3]: register SPRN_PMC2 = 0x0000000080000003
> [4]: register SPRN_PMC5 = 0x0000000000000000
> [5]: register SPRN_PMC6 = 0x0000000000000000
> [6]: register SPRN_PMC2 = 0x0000000080000003
> > > [7]: register SPRN_PMC5 = 0x000000008f21266d
> > > [8]: register SPRN_PMC6 = 0x000000000da80f8d
> [9]: register SPRN_PMC2 = 0x0000000080000003
> > > [10]: register SPRN_PMC5 = 0x000000006e2b961f
> > > [11]: register SPRN_PMC6 = 0x00000000d030a429
> [12]: register SPRN_PMC2 = 0x0000000080000003
> [13]: register SPRN_PMC5 = 0x0000000000000000
> [14]: register SPRN_PMC6 = 0x0000000000000000
> ...
> PMC5/6 overflow 2
> [FAIL] Test FAILED on line 87
> failure: pmc56_overflow
> =========
>
> Signed-off-by: Desnes A. Nunes do Rosario <desnesn@linux.ibm.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
> tools/testing/selftests/powerpc/pmu/ebb/ebb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
> b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
> index bf6f25dfcf7b..6199f3cea0f9 100644
> --- a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
> +++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
> @@ -258,6 +258,10 @@ int count_pmc(int pmc, uint32_t sample_period)
> start_value = pmc_sample_period(sample_period);
>
> val = read_pmc(pmc);
> +
> + /* Ensure pmc value is consistent between freezes */
> + mb();
> +
> if (val < start_value)
> ebb_state.stats.negative++;
> else
^ permalink raw reply
* Re: [PATCH 16/21] mm: remove early_pfn_in_nid() and CONFIG_NODES_SPAN_OTHER_NODES
From: Mike Rapoport @ 2020-04-23 5:50 UTC (permalink / raw)
To: Baoquan He
Cc: Rich Felker, linux-ia64, linux-doc, Catalin Marinas,
Heiko Carstens, Michal Hocko, James E.J. Bottomley, Max Filippov,
Guo Ren, linux-csky, linux-parisc, sparclinux, linux-hexagon,
linux-riscv, Greg Ungerer, linux-arch, linux-s390, linux-snps-arc,
linux-c6x-dev, Brian Cain, Jonathan Corbet, linux-sh,
Helge Deller, x86, Russell King, Ley Foon Tan, Mike Rapoport,
Geert Uytterhoeven, linux-arm-kernel, Mark Salter, Matt Turner,
linux-mips, uclinux-h8-devel, linux-xtensa, linux-alpha, linux-um,
linux-m68k, Tony Luck, Greentime Hu, Paul Walmsley,
Stafford Horne, Guan Xuetao, Hoan Tran, Michal Simek,
Thomas Bogendoerfer, Yoshinori Sato, Nick Hu, linux-mm,
Vineet Gupta, linux-kernel, openrisc, Richard Weinberger,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200423011312.GY4247@MiWiFi-R3L-srv>
On Thu, Apr 23, 2020 at 09:13:12AM +0800, Baoquan He wrote:
> On 04/12/20 at 10:48pm, Mike Rapoport wrote:
> > From: Mike Rapoport <rppt@linux.ibm.com>
> >
> > The commit f47ac088c406 ("mm: memmap_init: iterate over memblock regions
>
> This commit id should be a temporary one, will be changed when merged
> into maintainer's tree and linus's tree. Only saying last patch plus the
> patch subject is OK?
Right, the commit id here is not stable. I'll update the changelog.
> > rather that check each PFN") made early_pfn_in_nid() obsolete and since
> > CONFIG_NODES_SPAN_OTHER_NODES is only used to pick a stub or a real
> > implementation of early_pfn_in_nid() it is also not needed anymore.
> >
> > Remove both early_pfn_in_nid() and the CONFIG_NODES_SPAN_OTHER_NODES.
> >
> > Co-developed-by: Hoan Tran <Hoan@os.amperecomputing.com>
> > Signed-off-by: Hoan Tran <Hoan@os.amperecomputing.com>
> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > ---
> > arch/powerpc/Kconfig | 9 ---------
> > arch/sparc/Kconfig | 9 ---------
> > arch/x86/Kconfig | 9 ---------
> > mm/page_alloc.c | 20 --------------------
> > 4 files changed, 47 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 5f86b22b7d2c..74f316deeae1 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -685,15 +685,6 @@ config ARCH_MEMORY_PROBE
> > def_bool y
> > depends on MEMORY_HOTPLUG
> >
> > -# Some NUMA nodes have memory ranges that span
> > -# other nodes. Even though a pfn is valid and
> > -# between a node's start and end pfns, it may not
> > -# reside on that node. See memmap_init_zone()
> > -# for details.
> > -config NODES_SPAN_OTHER_NODES
> > - def_bool y
> > - depends on NEED_MULTIPLE_NODES
> > -
> > config STDBINUTILS
> > bool "Using standard binutils settings"
> > depends on 44x
> > diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> > index 795206b7b552..0e4f3891b904 100644
> > --- a/arch/sparc/Kconfig
> > +++ b/arch/sparc/Kconfig
> > @@ -286,15 +286,6 @@ config NODES_SHIFT
> > Specify the maximum number of NUMA Nodes available on the target
> > system. Increases memory reserved to accommodate various tables.
> >
> > -# Some NUMA nodes have memory ranges that span
> > -# other nodes. Even though a pfn is valid and
> > -# between a node's start and end pfns, it may not
> > -# reside on that node. See memmap_init_zone()
> > -# for details.
> > -config NODES_SPAN_OTHER_NODES
> > - def_bool y
> > - depends on NEED_MULTIPLE_NODES
> > -
> > config ARCH_SPARSEMEM_ENABLE
> > def_bool y if SPARC64
> > select SPARSEMEM_VMEMMAP_ENABLE
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 9d3e95b4fb85..37dac095659e 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1581,15 +1581,6 @@ config X86_64_ACPI_NUMA
> > ---help---
> > Enable ACPI SRAT based node topology detection.
> >
> > -# Some NUMA nodes have memory ranges that span
> > -# other nodes. Even though a pfn is valid and
> > -# between a node's start and end pfns, it may not
> > -# reside on that node. See memmap_init_zone()
> > -# for details.
> > -config NODES_SPAN_OTHER_NODES
> > - def_bool y
> > - depends on X86_64_ACPI_NUMA
> > -
> > config NUMA_EMU
> > bool "NUMA emulation"
> > depends on NUMA
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index c43ce8709457..343d87b8697d 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1541,26 +1541,6 @@ int __meminit early_pfn_to_nid(unsigned long pfn)
> > }
> > #endif /* CONFIG_NEED_MULTIPLE_NODES */
> >
> > -#ifdef CONFIG_NODES_SPAN_OTHER_NODES
> > -/* Only safe to use early in boot when initialisation is single-threaded */
> > -static inline bool __meminit early_pfn_in_nid(unsigned long pfn, int node)
> > -{
> > - int nid;
> > -
> > - nid = __early_pfn_to_nid(pfn, &early_pfnnid_cache);
> > - if (nid >= 0 && nid != node)
> > - return false;
> > - return true;
> > -}
> > -
> > -#else
> > -static inline bool __meminit early_pfn_in_nid(unsigned long pfn, int node)
> > -{
> > - return true;
> > -}
> > -#endif
>
> And macro early_pfn_valid() is not needed either, we may need remove it
> too.
Ok.
> Otherwise, removing NODES_SPAN_OTHER_NODES in this patch looks good.
>
> Reviewed-by: Baoquan He <bhe@redhat.com>
>
> > -
> > -
> > void __init memblock_free_pages(struct page *page, unsigned long pfn,
> > unsigned int order)
> > {
> > --
> > 2.25.1
> >
>
--
Sincerely yours,
Mike.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox