* Re: [PATCH 3/3] powerpc/kprobes: Check return value of patch_instruction()
From: Christophe Leroy @ 2020-04-23 15:41 UTC (permalink / raw)
To: Naveen N. Rao, linuxppc-dev; +Cc: Steven Rostedt
In-Reply-To: <3a132ac385340244b8d74179ac7bbbda7bf1f503.1587654213.git.naveen.n.rao@linux.vnet.ibm.com>
Le 23/04/2020 à 17:09, Naveen N. Rao a écrit :
> 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)
> +
I hate this kind of macro which hides the "return".
What about keeping the return action in the caller ?
Otherwise, what about implementing something based on the use of goto,
on the same model as unsafe_put_user() for instance ?
Christophe
^ permalink raw reply
* [PATCH 3/3] powerpc/hw_bkpt: Update printk format specifiers for kernel addresses
From: Naveen N. Rao @ 2020-04-23 15:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Ravi Bangoria, Balamuruhan S
In-Reply-To: <cover.1587652966.git.naveen.n.rao@linux.vnet.ibm.com>
Change prinkt format specifier from %lx to %pK to indicate kernel
pointer, and to hide the addresses from unprivileged users.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 72f461bd70fb..93a303cf0c67 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -257,7 +257,8 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
if (!ret && (type == LARX || type == STCX)) {
printk_ratelimited("Breakpoint hit on instruction that can't be emulated."
- " Breakpoint at 0x%lx will be disabled.\n", addr);
+ " Breakpoint at 0x%pK will be disabled.\n",
+ (void *)addr);
goto disable;
}
@@ -286,7 +287,7 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
* it and throw a warning message to let the user know about it.
*/
WARN(1, "Unable to handle hardware breakpoint. Breakpoint at "
- "0x%lx will be disabled.", addr);
+ "0x%pK will be disabled.", (void *)addr);
disable:
perf_event_disable_inatomic(bp);
--
2.25.1
^ permalink raw reply related
* [PATCH 1/3] powerpc/kprobes: Use appropriate format specifier for printing kernel address
From: Naveen N. Rao @ 2020-04-23 15:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Ravi Bangoria, Balamuruhan S
In-Reply-To: <cover.1587652966.git.naveen.n.rao@linux.vnet.ibm.com>
From: Balamuruhan S <bala24@linux.ibm.com>
Change use of %p to %pK when printing address of the instruction slot so
that the actual kernel address is visible for privileged users.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/optprobes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index ef0924b0809d..d5f8c25b7cac 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -247,7 +247,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
/* Setup template */
/* We can optimize this via patch_instruction_window later */
size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
- pr_devel("Copying template to %p, size %lu\n", buff, size);
+ pr_devel("Copying template to %pK, size %lu\n", (void *)buff, size);
for (i = 0; i < size; i++) {
rc = patch_instruction(buff + i, *(optprobe_template_entry + i));
if (rc) {
--
2.25.1
^ permalink raw reply related
* [PATCH 2/3] powerpc/ftrace: Use appropriate format specifier for printing kernel addresses
From: Naveen N. Rao @ 2020-04-23 15:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Ravi Bangoria, Balamuruhan S
In-Reply-To: <cover.1587652966.git.naveen.n.rao@linux.vnet.ibm.com>
Update use of printk format specifiers in ftrace code, so that addresses
are made visible for privileged users, or always for pr_devel() code:
- change %lx to use %px or %pK
- change %p to %px or %pK
- add %pS in certain places to show the symbol as well
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/trace/ftrace.c | 74 +++++++++++++++++-------------
1 file changed, 41 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 679d5249b002..29b77204f46d 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -83,8 +83,8 @@ ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new)
/* Make sure it is what we expect it to be */
if (replaced != old) {
- pr_err("%p: replaced (%#x) != old (%#x)",
- (void *)ip, replaced, old);
+ pr_err("%pK (%pS): replaced (%#x) != old (%#x)",
+ (void *)ip, (void *)ip, replaced, old);
return -EINVAL;
}
@@ -152,19 +152,20 @@ __ftrace_make_nop(struct module *mod,
/* lets find where the pointer goes */
tramp = find_bl_target(ip, op);
- pr_devel("ip:%lx jumps to %lx", ip, tramp);
+ pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
if (module_trampoline_target(mod, tramp, &ptr)) {
pr_err("Failed to get trampoline target\n");
return -EFAULT;
}
- pr_devel("trampoline target %lx", ptr);
+ pr_devel("trampoline target 0x%px", (void *)ptr);
entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
if (ptr != entry) {
- pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+ pr_err("addr 0x%pK does not match expected 0x%pK\n",
+ (void *)ptr, (void *)entry);
return -EINVAL;
}
@@ -173,7 +174,8 @@ __ftrace_make_nop(struct module *mod,
pop = PPC_INST_NOP;
if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
- pr_err("Fetching instruction at %lx failed.\n", ip - 4);
+ pr_err("Fetching instruction at 0x%pK (%pS) failed.\n",
+ (void *)(ip - 4), (void *)(ip - 4));
return -EFAULT;
}
@@ -249,11 +251,11 @@ __ftrace_make_nop(struct module *mod,
* 0x4e, 0x80, 0x04, 0x20 bctr
*/
- pr_devel("ip:%lx jumps to %lx", ip, tramp);
+ pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
/* Find where the trampoline jumps to */
if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
- pr_err("Failed to read %lx\n", tramp);
+ pr_err("Failed to read 0x%pK\n", (void *)tramp);
return -EFAULT;
}
@@ -273,11 +275,11 @@ __ftrace_make_nop(struct module *mod,
if (tramp & 0x8000)
tramp -= 0x10000;
- pr_devel(" %lx ", tramp);
+ pr_devel(" 0x%px ", (void *)tramp);
if (tramp != addr) {
- pr_err("Trampoline location %08lx does not match addr\n",
- tramp);
+ pr_err("Trampoline location 0x%pK does not match addr\n",
+ (void *)tramp);
return -EINVAL;
}
@@ -362,7 +364,8 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
ptr = find_bl_target(tramp, op);
if (ptr != ppc_global_function_entry((void *)_mcount)) {
- pr_debug("Trampoline target %p is not _mcount\n", (void *)ptr);
+ pr_debug("Trampoline target 0x%px (%pS) is not _mcount\n",
+ (void *)ptr, (void *)ptr);
return -1;
}
@@ -374,8 +377,8 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
#endif
op = create_branch((void *)tramp, ptr, 0);
if (!op) {
- pr_debug("%ps is not reachable from existing mcount tramp\n",
- (void *)ptr);
+ pr_debug("0x%px (%ps) is not reachable from existing mcount tramp\n",
+ (void *)ptr, (void *)ptr);
return -1;
}
@@ -409,13 +412,13 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
/* Let's find where the pointer goes */
tramp = find_bl_target(ip, op);
- pr_devel("ip:%lx jumps to %lx", ip, tramp);
+ pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
if (setup_mcount_compiler_tramp(tramp)) {
/* Are other trampolines reachable? */
if (!find_ftrace_tramp(ip)) {
- pr_err("No ftrace trampolines reachable from %ps\n",
- (void *)ip);
+ pr_err("No ftrace trampolines reachable from 0x%pK (%pS)\n",
+ (void *)ip, (void *)ip);
return -EINVAL;
}
}
@@ -452,13 +455,13 @@ int ftrace_make_nop(struct module *mod,
*/
if (!rec->arch.mod) {
if (!mod) {
- pr_err("No module loaded addr=%lx\n", addr);
+ pr_err("No module loaded addr=0x%pK\n", (void *)addr);
return -EFAULT;
}
rec->arch.mod = mod;
} else if (mod) {
if (mod != rec->arch.mod) {
- pr_err("Record mod %p not equal to passed in mod %p\n",
+ pr_err("Record mod %pK not equal to passed in mod %pK\n",
rec->arch.mod, mod);
return -EINVAL;
}
@@ -521,8 +524,8 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return -EFAULT;
if (!expected_nop_sequence(ip, op[0], op[1])) {
- pr_err("Unexpected call sequence at %p: %x %x\n",
- ip, op[0], op[1]);
+ pr_err("Unexpected call sequence at %pK: %x %x\n",
+ ip, op[0], op[1]);
return -EINVAL;
}
@@ -548,12 +551,13 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return -EFAULT;
}
- pr_devel("trampoline target %lx", ptr);
+ pr_devel("trampoline target 0x%px", (void *)ptr);
entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
if (ptr != entry) {
- pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+ pr_err("addr 0x%pK does not match expected 0x%pK\n",
+ (void *)ptr, (void *)entry);
return -EINVAL;
}
@@ -600,7 +604,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return -EINVAL;
}
- pr_devel("write to %lx\n", rec->ip);
+ pr_devel("write to 0x%px\n", (void *)rec->ip);
PATCH_INSN(ip, op);
@@ -624,7 +628,8 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
entry = ppc_global_function_entry((void *)ftrace_regs_caller);
if (ptr != entry) {
#endif
- pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
+ pr_err("Unknown ftrace addr to patch: 0x%pK (%pS)\n",
+ (void *)ptr, (void *)ptr);
return -EINVAL;
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
}
@@ -633,18 +638,19 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
/* Make sure we have a nop */
if (probe_kernel_read(&op, ip, sizeof(op))) {
- pr_err("Unable to read ftrace location %p\n", ip);
+ pr_err("Unable to read ftrace location %pK\n", ip);
return -EFAULT;
}
if (op != PPC_INST_NOP) {
- pr_err("Unexpected call sequence at %p: %x\n", ip, op);
+ pr_err("Unexpected call sequence at %pK: %x\n", ip, op);
return -EINVAL;
}
tramp = find_ftrace_tramp((unsigned long)ip);
if (!tramp) {
- pr_err("No ftrace trampolines reachable from %ps\n", ip);
+ pr_err("No ftrace trampolines reachable from 0x%pK (%pS)\n",
+ ip, ip);
return -EINVAL;
}
@@ -728,7 +734,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
tramp = find_bl_target(ip, op);
entry = ppc_global_function_entry((void *)old_addr);
- pr_devel("ip:%lx jumps to %lx", ip, tramp);
+ pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
if (tramp != entry) {
/* old_addr is not within range, so we must have used a trampoline */
@@ -737,11 +743,12 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
return -EFAULT;
}
- pr_devel("trampoline target %lx", ptr);
+ pr_devel("trampoline target 0x%px", (void *)ptr);
/* This should match what was called */
if (ptr != entry) {
- pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+ pr_err("addr 0x%pK does not match expected 0x%pK\n",
+ (void *)ptr, (void *)entry);
return -EINVAL;
}
}
@@ -765,12 +772,13 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
return -EFAULT;
}
- pr_devel("trampoline target %lx", ptr);
+ pr_devel("trampoline target 0x%px", (void *)ptr);
entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
if (ptr != entry) {
- pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+ pr_err("addr 0x%pK does not match expected 0x%pK\n",
+ (void *)ptr, (void *)entry);
return -EINVAL;
}
--
2.25.1
^ permalink raw reply related
* [PATCH 0/3] powerpc: Use proper printk format specifiers
From: Naveen N. Rao @ 2020-04-23 15:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Ravi Bangoria, Balamuruhan S
This series changes printk format specifiers from bare %p to %px/%pK in
ftrace, kprobes and hw bkpts code. In addition, use of %lx is also
changed over to conform to the recommended practice.
This series applies on top of the below patch series:
https://lore.kernel.org/r/cover.1587654213.git.naveen.n.rao@linux.vnet.ibm.com
- Naveen
Balamuruhan S (1):
powerpc/kprobes: Use appropriate format specifier for printing kernel
address
Naveen N. Rao (2):
powerpc/ftrace: Use appropriate format specifier for printing kernel
addresses
powerpc/hw_bkpt: Update printk format specifiers for kernel addresses
arch/powerpc/kernel/hw_breakpoint.c | 5 +-
arch/powerpc/kernel/optprobes.c | 2 +-
arch/powerpc/kernel/trace/ftrace.c | 74 ++++++++++++++++-------------
3 files changed, 45 insertions(+), 36 deletions(-)
base-commit: 8299da600ad05b8aa0f15ec0f5f03bd40e37d6f0
-- 2.25.1
^ permalink raw reply
* [PATCH 1/3] powerpc: Properly return error code from do_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>
With STRICT_KERNEL_RWX, we are currently ignoring return value from
__patch_instruction() in do_patch_instruction(), resulting in the error
not being propagated back. Fix the same.
Fixes: 37bc3e5fd764f ("powerpc/lib/code-patching: Use alternate map for patch_instruction()")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/lib/code-patching.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 3345f039a876..5c713a6c0bd8 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -138,7 +138,7 @@ static inline int unmap_patch_area(unsigned long addr)
static int do_patch_instruction(unsigned int *addr, unsigned int instr)
{
- int err;
+ int err, rc = 0;
unsigned int *patch_addr = NULL;
unsigned long flags;
unsigned long text_poke_addr;
@@ -163,7 +163,7 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
patch_addr = (unsigned int *)(text_poke_addr) +
((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
- __patch_instruction(addr, instr, patch_addr);
+ rc = __patch_instruction(addr, instr, patch_addr);
err = unmap_patch_area(text_poke_addr);
if (err)
@@ -172,7 +172,7 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
out:
local_irq_restore(flags);
- return err;
+ return rc ? rc : err;
}
#else /* !CONFIG_STRICT_KERNEL_RWX */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] PCI/DPC: Allow Native DPC Host Bridges to use DPC
From: Derrick, Jonathan @ 2020-04-23 15:11 UTC (permalink / raw)
To: sathyanarayanan.kuppuswamy@linux.intel.com, helgaas@kernel.org
Cc: bhelgaas@google.com, Patel, Mayurkumar, fred@fredlawl.com,
sbobroff@linux.ibm.com, linuxppc-dev@lists.ozlabs.org,
Wysocki, Rafael J, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, andriy.shevchenko@linux.intel.com,
olof@lixom.net, alex.williamson@redhat.com, oohall@gmail.com,
kbusch@kernel.org, rajatja@google.com,
mika.westerberg@linux.intel.com
In-Reply-To: <0058b993-0663-7fed-ed31-cb0adf845a39@linux.intel.com>
Hi Sathyanarayanan,
On Wed, 2020-04-22 at 15:50 -0700, Kuppuswamy, Sathyanarayanan wrote:
>
> On 4/20/20 2:37 PM, Jon Derrick wrote:
> > The existing portdrv model prevents DPC services without either OS
> > control (_OSC) granted to AER services, a Host Bridge requesting Native
> > AER, or using one of the 'pcie_ports=' parameters of 'native' or
> > 'dpc-native'.
> >
> > The DPC port service driver itself will also fail to probe if the kernel
> > assumes the port is using Firmware-First AER. It's a reasonable
> > expectation that a port using Firmware-First AER will also be using
> > Firmware-First DPC, however if a Host Bridge requests Native DPC, the
> > DPC driver should allow it and not fail to bind due to AER capability
> > settings.
> >
> > Host Bridges which request Native DPC port services will also likely
> > request Native AER, however it shouldn't be a requirement. This patch
> > allows ports on those Host Bridges to have DPC port services.
> >
> > This will avoid the unlikely situation where the port is Firmware-First
> > AER and Native DPC, and a BIOS or switch firmware preconfiguration of
> > the DPC trigger could result in unhandled DPC events.
> >
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> > drivers/pci/pcie/dpc.c | 3 ++-
> > drivers/pci/pcie/portdrv_core.c | 3 ++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> > index 7621704..3f3106f 100644
> > --- a/drivers/pci/pcie/dpc.c
> > +++ b/drivers/pci/pcie/dpc.c
> > @@ -284,7 +284,8 @@ static int dpc_probe(struct pcie_device *dev)
> > int status;
> > u16 ctl, cap;
> >
> > - if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native)
> > + if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native &&
> > + !pci_find_host_bridge(pdev->bus)->native_dpc)
> Why do it in probe as well ? if host->native_dpc is not set then the
> device DPC probe it self won't happen right ?
Portdrv only enables the interrupt and allows the probe to occur.
The probe itself will still fail if there's a mixed-mode _OSC
negotiated AER & DPC, due to pcie_aer_get_firmware_first returning 1
for AER and no check for DPC.
I don't know if such a platform will exist, but the kernel is already
wired for 'dpc-native' so it makes sense to extend it for this..
This transform might be more readable:
if (pcie_aer_get_firmware_first(pdev) &&
!(pcie_ports_dpc_native || hb->native_dpc))
> > return -ENOTSUPP;
> >
> > status = devm_request_threaded_irq(device, dev->irq, dpc_irq,
> > diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> > index 50a9522..f2139a1 100644
> > --- a/drivers/pci/pcie/portdrv_core.c
> > +++ b/drivers/pci/pcie/portdrv_core.c
> > @@ -256,7 +256,8 @@ static int get_port_device_capability(struct pci_dev *dev)
> > */
> > if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > pci_aer_available() &&
> > - (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> > + (pcie_ports_dpc_native || host->native_dpc ||
> > + (services & PCIE_PORT_SERVICE_AER)))
> > services |= PCIE_PORT_SERVICE_DPC;
> >
> > if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM ||
> >
^ permalink raw reply
* Re: [PATCH v2 1/2] PCI/AER: Allow Native AER Host Bridges to use AER
From: Derrick, Jonathan @ 2020-04-23 15:11 UTC (permalink / raw)
To: sathyanarayanan.kuppuswamy@linux.intel.com, helgaas@kernel.org
Cc: bhelgaas@google.com, Patel, Mayurkumar, fred@fredlawl.com,
sbobroff@linux.ibm.com, linuxppc-dev@lists.ozlabs.org,
Wysocki, Rafael J, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, andriy.shevchenko@linux.intel.com,
olof@lixom.net, alex.williamson@redhat.com, oohall@gmail.com,
kbusch@kernel.org, rajatja@google.com,
mika.westerberg@linux.intel.com
In-Reply-To: <9f8c2a62-e67d-2869-db11-4644b69815f4@linux.intel.com>
Hi Sathyanarayanan,
On Wed, 2020-04-22 at 15:48 -0700, Kuppuswamy, Sathyanarayanan wrote:
>
> On 4/20/20 2:37 PM, Jon Derrick wrote:
> > Some platforms have a mix of ports whose capabilities can be negotiated
> > by _OSC, and some ports which are not described by ACPI and instead
> > managed by Native drivers. The existing Firmware-First HEST model can
> > incorrectly tag these Native, Non-ACPI ports as Firmware-First managed
> > ports by advertising the HEST Global Flag and matching the type and
> > class of the port (aer_hest_parse).
> Is there a real use case for mixed mode (one host bridge in FF mode and
> another in native)?
Intel's VMD exposes PCIe segments containing Root Ports and Bridges and
other DPC consumers. These extra PCIe domains aren't described by ACPI.
There have been a few versions where DPC won't bind due to platform's
HEST configuration.
> > If the port requests Native AER through the Host Bridge's capability
> > settings, the AER driver should honor those settings and allow the port
> > to bind. This patch changes the definition of Firmware-First to exclude
> > ports whose Host Bridges request Native AER.
> >
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> > drivers/pci/pcie/aer.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index f4274d3..30fbd1f 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -314,6 +314,9 @@ int pcie_aer_get_firmware_first(struct pci_dev *dev)
> > if (pcie_ports_native)
> > return 0;
> >
> > + if (pci_find_host_bridge(dev->bus)->native_aer)
> > + return 0;
> > +
> > if (!dev->__aer_firmware_first_valid)
> > aer_set_firmware_first(dev);
> > return dev->__aer_firmware_first;
> >
^ permalink raw reply
* [PATCH 2/3] powerpc/ftrace: Simplify error checking when patching instructions
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>
Introduce a macro PATCH_INSN() to simplify instruction patching, and to
make the error messages more uniform and useful:
- print an error message that includes the original return value
- print the function name and line numbers, so that the offending
location is clear
- always return -EPERM, which ftrace_bug() expects for proper error
handling
Also eliminate use of patch_branch() since most such uses already call
create_branch() for error checking before patching. Instead, use the
return value from create_branch() with PATCH_INSN().
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/trace/ftrace.c | 69 ++++++++++++++----------------
1 file changed, 33 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 7ea0ca044b65..5cf84c0c64cb 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -31,6 +31,17 @@
#ifdef CONFIG_DYNAMIC_FTRACE
+#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 -EPERM; \
+ } \
+} while (0)
+
/*
* We generally only have a single long_branch tramp and at most 2 or 3 plt
* tramps generated. But, we don't use the plt tramps currently. We also allot
@@ -78,8 +89,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new)
}
/* replace the text with the new text */
- if (patch_instruction((unsigned int *)ip, new))
- return -EPERM;
+ PATCH_INSN(ip, new);
return 0;
}
@@ -204,10 +214,7 @@ __ftrace_make_nop(struct module *mod,
}
#endif /* CONFIG_MPROFILE_KERNEL */
- if (patch_instruction((unsigned int *)ip, pop)) {
- pr_err("Patching NOP failed.\n");
- return -EPERM;
- }
+ PATCH_INSN(ip, pop);
return 0;
}
@@ -276,8 +283,7 @@ __ftrace_make_nop(struct module *mod,
op = PPC_INST_NOP;
- if (patch_instruction((unsigned int *)ip, op))
- return -EPERM;
+ PATCH_INSN(ip, op);
return 0;
}
@@ -322,7 +328,7 @@ static int add_ftrace_tramp(unsigned long tramp)
*/
static int setup_mcount_compiler_tramp(unsigned long tramp)
{
- int i, op;
+ unsigned int i, op;
unsigned long ptr;
static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
@@ -366,16 +372,14 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
#else
ptr = ppc_global_function_entry((void *)ftrace_caller);
#endif
- if (!create_branch((void *)tramp, ptr, 0)) {
+ op = create_branch((void *)tramp, ptr, 0);
+ if (!op) {
pr_debug("%ps is not reachable from existing mcount tramp\n",
(void *)ptr);
return -1;
}
- if (patch_branch((unsigned int *)tramp, ptr, 0)) {
- pr_debug("REL24 out of range!\n");
- return -1;
- }
+ PATCH_INSN(tramp, op);
if (add_ftrace_tramp(tramp)) {
pr_debug("No tramp locations left\n");
@@ -416,10 +420,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
}
}
- if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
- pr_err("Patching NOP failed.\n");
- return -EPERM;
- }
+ PATCH_INSN(ip, PPC_INST_NOP);
return 0;
}
@@ -557,15 +558,13 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
}
/* Ensure branch is within 24 bits */
- if (!create_branch(ip, tramp, BRANCH_SET_LINK)) {
+ op[0] = create_branch(ip, tramp, BRANCH_SET_LINK);
+ if (!op[0]) {
pr_err("Branch out of range\n");
return -EINVAL;
}
- if (patch_branch(ip, tramp, BRANCH_SET_LINK)) {
- pr_err("REL24 out of range!\n");
- return -EINVAL;
- }
+ PATCH_INSN(ip, op[0]);
return 0;
}
@@ -603,8 +602,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
pr_devel("write to %lx\n", rec->ip);
- if (patch_instruction((unsigned int *)ip, op))
- return -EPERM;
+ PATCH_INSN(ip, op);
return 0;
}
@@ -650,11 +648,14 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
return -EINVAL;
}
- if (patch_branch(ip, tramp, BRANCH_SET_LINK)) {
- pr_err("Error patching branch to ftrace tramp!\n");
+ op = create_branch(ip, tramp, BRANCH_SET_LINK);
+ if (!op) {
+ pr_err("Branch out of range\n");
return -EINVAL;
}
+ PATCH_INSN(ip, op);
+
return 0;
}
@@ -748,10 +749,8 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
/* The new target may be within range */
if (test_24bit_addr(ip, addr)) {
/* within range */
- if (patch_branch((unsigned int *)ip, addr, BRANCH_SET_LINK)) {
- pr_err("REL24 out of range!\n");
- return -EINVAL;
- }
+ op = create_branch((unsigned int *)ip, addr, BRANCH_SET_LINK);
+ PATCH_INSN(ip, op);
return 0;
}
@@ -776,15 +775,13 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
}
/* Ensure branch is within 24 bits */
- if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
+ op = create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK);
+ if (!op) {
pr_err("Branch out of range\n");
return -EINVAL;
}
- if (patch_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
- pr_err("REL24 out of range!\n");
- return -EINVAL;
- }
+ PATCH_INSN(ip, op);
return 0;
}
--
2.25.1
^ permalink raw reply related
* [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
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