* Re: [RFC PATCH 1/4] powerpc: Optimize register usage for esr register
From: Xiongwei Song @ 2021-08-06 13:14 UTC (permalink / raw)
To: Michael Ellerman
Cc: ravi.bangoria, Xiongwei Song, oleg, Linux Kernel Mailing List,
efremov, Paul Mackerras, npiggin, aneesh.kumar, peterx, PowerPC,
akpm, sandipan
In-Reply-To: <874kc3njxh.fsf@mpe.ellerman.id.au>
On Fri, Aug 6, 2021 at 2:53 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> sxwjean@me.com writes:
> > From: Xiongwei Song <sxwjean@gmail.com>
> >
> > Create an anonymous union for dsisr and esr regsiters, we can reference
> > esr to get the exception detail when CONFIG_4xx=y or CONFIG_BOOKE=y.
> > Otherwise, reference dsisr. This makes code more clear.
> >
> > Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> > ---
> > arch/powerpc/include/asm/ptrace.h | 5 ++++-
> > arch/powerpc/include/uapi/asm/ptrace.h | 5 ++++-
> > arch/powerpc/kernel/process.c | 2 +-
> > arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> > arch/powerpc/kernel/traps.c | 2 +-
> > arch/powerpc/mm/fault.c | 16 ++++++++++++++--
> > arch/powerpc/platforms/44x/machine_check.c | 4 ++--
> > arch/powerpc/platforms/4xx/machine_check.c | 2 +-
> > 8 files changed, 29 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> > index 3e5d470a6155..c252d04b1206 100644
> > --- a/arch/powerpc/include/asm/ptrace.h
> > +++ b/arch/powerpc/include/asm/ptrace.h
> > @@ -44,7 +44,10 @@ struct pt_regs
> > #endif
> > unsigned long trap;
> > unsigned long dar;
> > - unsigned long dsisr;
> > + union {
> > + unsigned long dsisr;
> > + unsigned long esr;
> > + };
>
> I don't mind doing that.
>
> > unsigned long result;
> > };
> > };
> > diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> > index 7004cfea3f5f..e357288b5f34 100644
> > --- a/arch/powerpc/include/uapi/asm/ptrace.h
> > +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> > @@ -53,7 +53,10 @@ struct pt_regs
> > /* N.B. for critical exceptions on 4xx, the dar and dsisr
> > fields are overloaded to hold srr0 and srr1. */
> > unsigned long dar; /* Fault registers */
> > - unsigned long dsisr; /* on 4xx/Book-E used for ESR */
> > + union {
> > + unsigned long dsisr; /* on Book-S used for DSISR */
> > + unsigned long esr; /* on 4xx/Book-E used for ESR */
> > + };
> > unsigned long result; /* Result of a system call */
> > };
>
> But I'm not sure about the use of anonymous unions in UAPI headers. Old
> compilers don't support them, so there's a risk of breakage.
>
> I'd rather we didn't touch the uapi version.
Ok. Will discard the change.
>
>
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index 185beb290580..f74af8f9133c 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1499,7 +1499,7 @@ static void __show_regs(struct pt_regs *regs)
> > trap == INTERRUPT_DATA_STORAGE ||
> > trap == INTERRUPT_ALIGNMENT) {
> > if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > - pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr);
> > + pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->esr);
> > else
> > pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
> > }
> > diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> > index 0a0a33eb0d28..00789ad2c4a3 100644
> > --- a/arch/powerpc/kernel/ptrace/ptrace.c
> > +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> > @@ -375,6 +375,8 @@ void __init pt_regs_check(void)
> > offsetof(struct user_pt_regs, dar));
> > BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
> > offsetof(struct user_pt_regs, dsisr));
> > + BUILD_BUG_ON(offsetof(struct pt_regs, esr) !=
> > + offsetof(struct user_pt_regs, esr));
> > BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> > offsetof(struct user_pt_regs, result));
> >
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> > index dfbce527c98e..2164f5705a0b 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -562,7 +562,7 @@ static inline int check_io_access(struct pt_regs *regs)
> > #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> > /* On 4xx, the reason for the machine check or program exception
> > is in the ESR. */
> > -#define get_reason(regs) ((regs)->dsisr)
> > +#define get_reason(regs) ((regs)->esr)
> > #define REASON_FP ESR_FP
> > #define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
> > #define REASON_PRIVILEGED ESR_PPR
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index a8d0ce85d39a..62953d4e7c93 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -541,7 +541,11 @@ static __always_inline void __do_page_fault(struct pt_regs *regs)
> > {
> > long err;
> >
> > - err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> > + if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > + err = ___do_page_fault(regs, regs->dar, regs->esr);
> > + else
> > + err = ___do_page_fault(regs, regs->dar, regs->dsisr);
>
> As Christophe said, I don't thinks this is an improvement.
>
> It makes the code less readable. If anyone is confused about what is
> passed to ___do_page_fault() they can either read the comment above it,
> or look at the definition of pt_regs to see that esr and dsisr share
> storage.
Ok, thanks a lot. Will send v2.
Regards,
Xiongwei
>
> cheers
^ permalink raw reply
* Re: [RFC PATCH 1/4] powerpc: Optimize register usage for esr register
From: Xiongwei Song @ 2021-08-06 13:22 UTC (permalink / raw)
To: Christophe Leroy
Cc: ravi.bangoria, Xiongwei Song, oleg, npiggin,
Linux Kernel Mailing List, efremov, Paul Mackerras, aneesh.kumar,
peterx, PowerPC, akpm, sandipan
In-Reply-To: <26814448-c30a-1de1-bad4-79e2bffc3054@csgroup.eu>
On Fri, Aug 6, 2021 at 3:32 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 06/08/2021 à 05:16, Xiongwei Song a écrit :
> > On Thu, Aug 5, 2021 at 6:06 PM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >>
> >>
> >> Le 26/07/2021 à 16:30, sxwjean@me.com a écrit :
> >>> From: Xiongwei Song <sxwjean@gmail.com>
> >>>
> >>> Create an anonymous union for dsisr and esr regsiters, we can reference
> >>> esr to get the exception detail when CONFIG_4xx=y or CONFIG_BOOKE=y.
> >>> Otherwise, reference dsisr. This makes code more clear.
> >>
> >> I'm not sure it is worth doing that.
> > Why don't we use "esr" as reference manauls mentioned?
> >
> >>
> >> What is the point in doing the following when you know that regs->esr and regs->dsisr are exactly
> >> the same:
> >>
> >> > - err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> >> > + if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> >> > + err = ___do_page_fault(regs, regs->dar, regs->esr);
> >> > + else
> >> > + err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> >> > +
> > Yes, we can drop this. But it's a bit vague.
> >
> >> Or even
> >>
> >> > - int is_write = page_fault_is_write(regs->dsisr);
> >> > + unsigned long err_reg;
> >> > + int is_write;
> >> > +
> >> > + if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> >> > + err_reg = regs->esr;
> >> > + else
> >> > + err_reg = regs->dsisr;
> >> > +
> >> > + is_write = page_fault_is_write(err_reg);
> >>
> >>
> >> Artificially growing the code for that makes no sense to me.
> >
> > We can drop this too.
> >>
> >>
> >> To avoid anbiguity, maybe the best would be to rename regs->dsisr to something like regs->sr , so
> >> that we know it represents the status register, which is DSISR or ESR depending on the platform.
> >
> > If so, this would make other people more confused. My consideration is
> > to follow what the reference
> > manuals represent.
>
> Maybe then we could rename the fields as regs->dsisr_esr and regs->dar_dear
I still prefer my method.
>
> That would be more explicit for everyone.
>
> The UAPI header however should remain as is because anonymous unions are not supported by old
> compilers as mentioned by Michael.
Sure. Will update in v2.
>
> But nevertheless, there are also situations where was is stored in regs->dsisr is not what we have
> in DSISR register. For instance on an ISI exception, we store a subset of the content of SRR1
> register into regs->dsisr.
Can I think my method has better expansibility here;-)?
Let me finish esr and dear first. Thank you for the reminder.
Regards,
Xiongwei
>
> Christophe
^ permalink raw reply
* [PATCH] powerpc/mce: check if event info is valid
From: Ganesh Goudar @ 2021-08-06 13:23 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Ganesh Goudar, mahesh, npiggin
Check if the event info is valid before printing the
event information. When a fwnmi enabled nested kvm guest
hits a machine check exception L0 and L2 would generate
machine check event info, But L1 would not generate any
machine check event info as it won't go through 0x200
vector and prints some unwanted message.
To fix this, 'in_use' variable in machine check event info is
no more in use, rename it to 'valid' and check if the event
information is valid before logging the event information.
without this patch L1 would print following message for
exceptions encountered in L2, as event structure will be
empty in L1.
"Machine Check Exception, Unknown event version 0".
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
arch/powerpc/include/asm/mce.h | 2 +-
arch/powerpc/kernel/mce.c | 7 +++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 331d944280b8..3646f53f228f 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -113,7 +113,7 @@ enum MCE_LinkErrorType {
struct machine_check_event {
enum MCE_Version version:8;
- u8 in_use;
+ u8 valid;
enum MCE_Severity severity:8;
enum MCE_Initiator initiator:8;
enum MCE_ErrorType error_type:8;
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 47a683cd00d2..b778394a06b5 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -114,7 +114,7 @@ void save_mce_event(struct pt_regs *regs, long handled,
mce->srr0 = nip;
mce->srr1 = regs->msr;
mce->gpr3 = regs->gpr[3];
- mce->in_use = 1;
+ mce->valid = 1;
mce->cpu = get_paca()->paca_index;
/* Mark it recovered if we have handled it and MSR(RI=1). */
@@ -202,7 +202,7 @@ int get_mce_event(struct machine_check_event *mce, bool release)
if (mce)
*mce = *mc_evt;
if (release)
- mc_evt->in_use = 0;
+ mc_evt->valid = 0;
ret = 1;
}
/* Decrement the count to free the slot. */
@@ -413,6 +413,9 @@ void machine_check_print_event_info(struct machine_check_event *evt,
"Probable Software error (some chance of hardware cause)",
};
+ if (!evt->valid)
+ return;
+
/* Print things out */
if (evt->version != MCE_V1) {
pr_err("Machine Check Exception, Unknown event version %d !\n",
--
2.31.1
^ permalink raw reply related
* [PATCH v6 0/2] KVM: PPC: Book3S HV: Nested guest state sanitising changes
From: Fabiano Rosas @ 2021-08-06 13:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, npiggin
This series aims to stop contaminating the l2_hv structure with bits
that might have come from L1 state.
Patch 1 makes l2_hv read-only (mostly). It is now only changed when we
explicitly want to pass information to L1.
Patch 2 makes sure that L1 is not forwarded HFU interrupts when the
host has decided to disable any facilities (theoretical for now, since
HFSCR bits are always the same between L1/Ln).
Changes since v5:
- patch 2 now reads the instruction earlier at the nested exit handler
to allow the guest to retry if the load fails.
v5:
- moved setting of the Cause bits under BOOK3S_INTERRUPT_H_FAC_UNAVAIL.
https://lkml.kernel.org/r/20210726201710.2432874-1-farosas@linux.ibm.com
v4:
- now passing lpcr separately into load_l2_hv_regs to solve the
conflict with commit a19b70abc69a ("KVM: PPC: Book3S HV: Nested move
LPCR sanitising to sanitise_hv_regs");
- patch 2 now forwards a HEAI instead of injecting a Program.
https://lkml.kernel.org/r/20210722221240.2384655-1-farosas@linux.ibm.com
v3:
- removed the sanitise functions;
- moved the entry code into a new load_l2_hv_regs and the exit code
into the existing save_hv_return_state;
- new patch: removes the cause bits when L0 has disabled the
corresponding facility.
https://lkml.kernel.org/r/20210415230948.3563415-1-farosas@linux.ibm.com
v2:
- made the change more generic, not only applies to hfscr anymore;
- sanitisation is now done directly on the vcpu struct, l2_hv is left
unchanged.
https://lkml.kernel.org/r/20210406214645.3315819-1-farosas@linux.ibm.com
v1:
https://lkml.kernel.org/r/20210305231055.2913892-1-farosas@linux.ibm.com
Fabiano Rosas (2):
KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
KVM: PPC: Book3S HV: Stop forwarding all HFUs to L1
arch/powerpc/kvm/book3s_hv.c | 13 ++++
arch/powerpc/kvm/book3s_hv_nested.c | 117 ++++++++++++++++------------
2 files changed, 79 insertions(+), 51 deletions(-)
--
2.29.2
^ permalink raw reply
* [PATCH v6 2/2] KVM: PPC: Book3S HV: Stop forwarding all HFUs to L1
From: Fabiano Rosas @ 2021-08-06 13:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, npiggin
In-Reply-To: <20210806134506.2649735-1-farosas@linux.ibm.com>
If the nested hypervisor has no access to a facility because it has
been disabled by the host, it should also not be able to see the
Hypervisor Facility Unavailable that arises from one of its guests
trying to access the facility.
This patch turns a HFU that happened in L2 into a Hypervisor Emulation
Assistance interrupt and forwards it to L1 for handling. The ones that
happened because L1 explicitly disabled the facility for L2 are still
let through, along with the corresponding Cause bits in the HFSCR.
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++
arch/powerpc/kvm/book3s_hv_nested.c | 29 +++++++++++++++++++++++------
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 085fb8ecbf68..9123b493c79e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1837,6 +1837,19 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu)
r = RESUME_HOST;
break;
}
+ case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
+ /*
+ * We might decide later to turn this interrupt into a
+ * HEAI. Load the last instruction now that we can go
+ * back into the guest to retry if needed.
+ */
+ r = kvmppc_get_last_inst(vcpu, INST_GENERIC,
+ &vcpu->arch.emul_inst);
+ if (r != EMULATE_DONE)
+ r = RESUME_GUEST;
+ else
+ r = RESUME_HOST;
+ break;
default:
r = RESUME_HOST;
break;
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 1823674d46ef..1904697a3132 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -99,7 +99,7 @@ static void byteswap_hv_regs(struct hv_guest_state *hr)
hr->dawrx1 = swab64(hr->dawrx1);
}
-static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
+static void save_hv_return_state(struct kvm_vcpu *vcpu,
struct hv_guest_state *hr)
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
@@ -118,7 +118,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
hr->pidr = vcpu->arch.pid;
hr->cfar = vcpu->arch.cfar;
hr->ppr = vcpu->arch.ppr;
- switch (trap) {
+ switch (vcpu->arch.trap) {
case BOOK3S_INTERRUPT_H_DATA_STORAGE:
hr->hdar = vcpu->arch.fault_dar;
hr->hdsisr = vcpu->arch.fault_dsisr;
@@ -128,9 +128,26 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
hr->asdr = vcpu->arch.fault_gpa;
break;
case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
- hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
- (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
- break;
+ {
+ u64 cause = vcpu->arch.hfscr >> 56;
+
+ WARN_ON_ONCE(cause >= BITS_PER_LONG);
+
+ if (!(hr->hfscr & (1UL << cause))) {
+ hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
+ (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
+ break;
+ }
+
+ /*
+ * We have disabled this facility, so it does not
+ * exist from L1's perspective. Turn it into a
+ * HEAI. The instruction was already loaded at
+ * kvmppc_handle_nested_exit().
+ */
+ vcpu->arch.trap = BOOK3S_INTERRUPT_H_EMUL_ASSIST;
+ fallthrough;
+ }
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
hr->heir = vcpu->arch.emul_inst;
break;
@@ -388,7 +405,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
delta_spurr = vcpu->arch.spurr - l2_hv.spurr;
delta_ic = vcpu->arch.ic - l2_hv.ic;
delta_vtb = vc->vtb - l2_hv.vtb;
- save_hv_return_state(vcpu, vcpu->arch.trap, &l2_hv);
+ save_hv_return_state(vcpu, &l2_hv);
/* restore L1 state */
vcpu->arch.nested = NULL;
--
2.29.2
^ permalink raw reply related
* [PATCH v6 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Fabiano Rosas @ 2021-08-06 13:45 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, npiggin
In-Reply-To: <20210806134506.2649735-1-farosas@linux.ibm.com>
As one of the arguments of the H_ENTER_NESTED hypercall, the nested
hypervisor (L1) prepares a structure containing the values of various
hypervisor-privileged registers with which it wants the nested guest
(L2) to run. Since the nested HV runs in supervisor mode it needs the
host to write to these registers.
To stop a nested HV manipulating this mechanism and using a nested
guest as a proxy to access a facility that has been made unavailable
to it, we have a routine that sanitises the values of the HV registers
before copying them into the nested guest's vcpu struct.
However, when coming out of the guest the values are copied as they
were back into L1 memory, which means that any sanitisation we did
during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
This patch alters this sanitisation to have effect on the vcpu->arch
registers directly before entering and after exiting the guest,
leaving the structure that is copied back into L1 unchanged (except
when we really want L1 to access the value, e.g the Cause bits of
HFSCR).
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 94 ++++++++++++++---------------
1 file changed, 46 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 898f942eb198..1823674d46ef 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -105,7 +105,6 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
struct kvmppc_vcore *vc = vcpu->arch.vcore;
hr->dpdes = vc->dpdes;
- hr->hfscr = vcpu->arch.hfscr;
hr->purr = vcpu->arch.purr;
hr->spurr = vcpu->arch.spurr;
hr->ic = vcpu->arch.ic;
@@ -128,55 +127,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
case BOOK3S_INTERRUPT_H_INST_STORAGE:
hr->asdr = vcpu->arch.fault_gpa;
break;
+ case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
+ hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
+ (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
+ break;
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
hr->heir = vcpu->arch.emul_inst;
break;
}
}
-/*
- * This can result in some L0 HV register state being leaked to an L1
- * hypervisor when the hv_guest_state is copied back to the guest after
- * being modified here.
- *
- * There is no known problem with such a leak, and in many cases these
- * register settings could be derived by the guest by observing behaviour
- * and timing, interrupts, etc., but it is an issue to consider.
- */
-static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
-{
- struct kvmppc_vcore *vc = vcpu->arch.vcore;
- u64 mask;
-
- /*
- * Don't let L1 change LPCR bits for the L2 except these:
- */
- mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
- LPCR_LPES | LPCR_MER;
-
- /*
- * Additional filtering is required depending on hardware
- * and configuration.
- */
- hr->lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
- (vc->lpcr & ~mask) | (hr->lpcr & mask));
-
- /*
- * Don't let L1 enable features for L2 which we've disabled for L1,
- * but preserve the interrupt cause field.
- */
- hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
-
- /* Don't let data address watchpoint match in hypervisor state */
- hr->dawrx0 &= ~DAWRX_HYP;
- hr->dawrx1 &= ~DAWRX_HYP;
-
- /* Don't let completed instruction address breakpt match in HV state */
- if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
- hr->ciabr &= ~CIABR_PRIV;
-}
-
-static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
+static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state *hr)
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
@@ -288,6 +249,43 @@ static int kvmhv_write_guest_state_and_regs(struct kvm_vcpu *vcpu,
sizeof(struct pt_regs));
}
+static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
+ const struct hv_guest_state *l2_hv,
+ const struct hv_guest_state *l1_hv, u64 *lpcr)
+{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ u64 mask;
+
+ restore_hv_regs(vcpu, l2_hv);
+
+ /*
+ * Don't let L1 change LPCR bits for the L2 except these:
+ */
+ mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
+ LPCR_LPES | LPCR_MER;
+
+ /*
+ * Additional filtering is required depending on hardware
+ * and configuration.
+ */
+ *lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
+ (vc->lpcr & ~mask) | (*lpcr & mask));
+
+ /*
+ * Don't let L1 enable features for L2 which we've disabled for L1,
+ * but preserve the interrupt cause field.
+ */
+ vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
+
+ /* Don't let data address watchpoint match in hypervisor state */
+ vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
+ vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
+
+ /* Don't let completed instruction address breakpt match in HV state */
+ if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
+ vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
+}
+
long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
{
long int err, r;
@@ -296,7 +294,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
struct hv_guest_state l2_hv = {0}, saved_l1_hv;
struct kvmppc_vcore *vc = vcpu->arch.vcore;
u64 hv_ptr, regs_ptr;
- u64 hdec_exp;
+ u64 hdec_exp, lpcr;
s64 delta_purr, delta_spurr, delta_ic, delta_vtb;
if (vcpu->kvm->arch.l1_ptcr == 0)
@@ -369,8 +367,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
/* Guest must always run with ME enabled, HV disabled. */
vcpu->arch.shregs.msr = (vcpu->arch.regs.msr | MSR_ME) & ~MSR_HV;
- sanitise_hv_regs(vcpu, &l2_hv);
- restore_hv_regs(vcpu, &l2_hv);
+ lpcr = l2_hv.lpcr;
+ load_l2_hv_regs(vcpu, &l2_hv, &saved_l1_hv, &lpcr);
vcpu->arch.ret = RESUME_GUEST;
vcpu->arch.trap = 0;
@@ -380,7 +378,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
r = RESUME_HOST;
break;
}
- r = kvmhv_run_single_vcpu(vcpu, hdec_exp, l2_hv.lpcr);
+ r = kvmhv_run_single_vcpu(vcpu, hdec_exp, lpcr);
} while (is_kvmppc_resume_guest(r));
/* save L2 state for return */
--
2.29.2
^ permalink raw reply related
* Re: [RFC PATCH 1/4] powerpc: Optimize register usage for esr register
From: Segher Boessenkool @ 2021-08-06 14:26 UTC (permalink / raw)
To: Michael Ellerman
Cc: ravi.bangoria, sxwjean, Xiongwei Song, aneesh.kumar, oleg,
npiggin, linux-kernel, peterx, paulus, efremov, akpm,
linuxppc-dev, sandipan
In-Reply-To: <874kc3njxh.fsf@mpe.ellerman.id.au>
On Fri, Aug 06, 2021 at 04:53:14PM +1000, Michael Ellerman wrote:
> But I'm not sure about the use of anonymous unions in UAPI headers. Old
> compilers don't support them, so there's a risk of breakage.
More precisely, it exists only since C11, so even with all not-so-ancient
compilers it will not work if the user uses (say) -std=c99, which still
is popular.
> I'd rather we didn't touch the uapi version.
Yeah.
> > - err = ___do_page_fault(regs, regs->dar, regs->dsisr);
> > + if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > + err = ___do_page_fault(regs, regs->dar, regs->esr);
> > + else
> > + err = ___do_page_fault(regs, regs->dar, regs->dsisr);
>
> As Christophe said, I don't thinks this is an improvement.
>
> It makes the code less readable. If anyone is confused about what is
> passed to ___do_page_fault() they can either read the comment above it,
> or look at the definition of pt_regs to see that esr and dsisr share
> storage.
Esp. since the affected platforms are legacy, yup.
Segher
^ permalink raw reply
* Re: [PATCH v4 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Arnd Bergmann @ 2021-08-06 14:51 UTC (permalink / raw)
To: Xianting Tian
Cc: Arnd Bergmann, Jiri Slaby, Amit Shah, gregkh,
Linux Kernel Mailing List,
open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE, Guo Ren,
linuxppc-dev, Omar Sandoval
In-Reply-To: <20210806030138.123479-2-xianting.tian@linux.alibaba.com>
On Fri, Aug 6, 2021 at 5:01 AM Xianting Tian
<xianting.tian@linux.alibaba.com> wrote:
> @@ -163,6 +155,13 @@ static void hvc_console_print(struct console *co, const char *b,
> if (vtermnos[index] == -1)
> return;
>
> + list_for_each_entry(hp, &hvc_structs, next)
> + if (hp->vtermno == vtermnos[index])
> + break;
> +
> + c = hp->c;
> +
> + spin_lock_irqsave(&hp->c_lock, flags);
The loop looks like it might race against changes to the list. It seems strange
that the print function has to actually search for the structure here.
It may be better to have yet another array for the buffer pointers next to
the cons_ops[] and vtermnos[] arrays.
> +/*
> + * These sizes are most efficient for vio, because they are the
> + * native transfer size. We could make them selectable in the
> + * future to better deal with backends that want other buffer sizes.
> + */
> +#define N_OUTBUF 16
> +#define N_INBUF 16
> +
> +#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
I think you need a higher alignment for DMA buffers, instead of sizeof(long),
I would suggest ARCH_DMA_MINALIGN.
Arnd
^ permalink raw reply
* Re: [PATCH kernel v2] KVM: PPC: Use arch_get_random_seed_long instead of powernv variant
From: Fabiano Rosas @ 2021-08-06 14:53 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy, kvm, kvm-ppc
In-Reply-To: <20210805075649.2086567-1-aik@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> The powernv_get_random_long() does not work in nested KVM (which is
> pseries) and produces a crash when accessing in_be64(rng->regs) in
> powernv_get_random_long().
>
> This replaces powernv_get_random_long with the ppc_md machine hook
> wrapper.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>
> Changes:
> v2:
> * replaces [PATCH kernel] powerpc/powernv: Check if powernv_rng is initialized
>
> ---
> arch/powerpc/kvm/book3s_hv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index be0cde26f156..ecfd133e0ca8 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1165,7 +1165,7 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> break;
> #endif
> case H_RANDOM:
> - if (!powernv_get_random_long(&vcpu->arch.regs.gpr[4]))
> + if (!arch_get_random_seed_long(&vcpu->arch.regs.gpr[4]))
> ret = H_HARDWARE;
> break;
> case H_RPT_INVALIDATE:
^ permalink raw reply
* Re: [PATCH v6 6/6] powerpc/pseries: Consolidate form1 distance initialization into a helper
From: Aneesh Kumar K.V @ 2021-08-06 16:23 UTC (permalink / raw)
To: David Gibson; +Cc: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
In-Reply-To: <YQzbCxwfEdE3CQZw@yekko>
On 8/6/21 12:17 PM, David Gibson wrote:
> On Tue, Jul 27, 2021 at 03:33:11PM +0530, Aneesh Kumar K.V wrote:
>> Currently, we duplicate parsing code for ibm,associativity and
>> ibm,associativity-lookup-arrays in the kernel. The associativity array provided
>> by these device tree properties are very similar and hence can use
>> a helper to parse the node id and numa distance details.
>
> Oh... sorry.. comments on the earlier patch were from before I read
> and saw you adjusted things here.
>
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/mm/numa.c | 83 ++++++++++++++++++++++++++----------------
>> 1 file changed, 51 insertions(+), 32 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index fffb3c40f595..7506251e17f2 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -171,19 +171,19 @@ static void unmap_cpu_from_node(unsigned long cpu)
>> }
>> #endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
>>
>> -/*
>> - * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
>> - * info is found.
>> - */
>> -static int associativity_to_nid(const __be32 *associativity)
>> +static int __associativity_to_nid(const __be32 *associativity,
>> + int max_array_sz)
>> {
>> int nid = NUMA_NO_NODE;
>> + /*
>> + * primary_domain_index is 1 based array index.
>> + */
>> + int index = primary_domain_index - 1;
>>
>> - if (!numa_enabled)
>> + if (!numa_enabled || index >= max_array_sz)
>> goto out;
>
> You don't need a goto, you can just return NUMA_NO_NODE.
updated
>
>>
>> - if (of_read_number(associativity, 1) >= primary_domain_index)
>> - nid = of_read_number(&associativity[primary_domain_index], 1);
>> + nid = of_read_number(&associativity[index], 1);
>>
>> /* POWER4 LPAR uses 0xffff as invalid node */
>> if (nid == 0xffff || nid >= nr_node_ids)
>> @@ -191,6 +191,17 @@ static int associativity_to_nid(const __be32 *associativity)
>> out:
>> return nid;
>> }
>> +/*
>> + * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
>> + * info is found.
>> + */
>> +static int associativity_to_nid(const __be32 *associativity)
>> +{
>> + int array_sz = of_read_number(associativity, 1);
>> +
>> + /* Skip the first element in the associativity array */
>> + return __associativity_to_nid((associativity + 1), array_sz);
>> +}
>>
>> static int __cpu_form2_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>> {
>> @@ -295,24 +306,41 @@ int of_node_to_nid(struct device_node *device)
>> }
>> EXPORT_SYMBOL(of_node_to_nid);
>>
>> -static void __initialize_form1_numa_distance(const __be32 *associativity)
>> +static void ___initialize_form1_numa_distance(const __be32 *associativity,
>> + int max_array_sz)
>> {
>> int i, nid;
>>
>> if (affinity_form != FORM1_AFFINITY)
>> return;
>>
>> - nid = associativity_to_nid(associativity);
>> + nid = __associativity_to_nid(associativity, max_array_sz);
>> if (nid != NUMA_NO_NODE) {
>> for (i = 0; i < distance_ref_points_depth; i++) {
>> const __be32 *entry;
>> + int index = be32_to_cpu(distance_ref_points[i]) - 1;
>> +
>> + /*
>> + * broken hierarchy, return with broken distance table
>
> WARN_ON, maybe?
updated
>
>> + */
>> + if (index >= max_array_sz)
>> + return;
>>
>> - entry = &associativity[be32_to_cpu(distance_ref_points[i])];
>> + entry = &associativity[index];
>> distance_lookup_table[nid][i] = of_read_number(entry, 1);
>> }
>> }
>> }
>>
>> +static void __initialize_form1_numa_distance(const __be32 *associativity)
>
> Do you actually use this in-between wrapper?
yes used in
static void initialize_form1_numa_distance(struct device_node *node)
{
const __be32 *associativity;
associativity = of_get_associativity(node);
if (!associativity)
return;
__initialize_form1_numa_distance(associativity);
}
>
>> +{
>> + int array_sz;
>> +
>> + array_sz = of_read_number(associativity, 1);
>> + /* Skip the first element in the associativity array */
>> + ___initialize_form1_numa_distance(associativity + 1, array_sz);
>> +}
>> +
>> static void initialize_form1_numa_distance(struct device_node *node)
>> {
>> const __be32 *associativity;
>> @@ -586,27 +614,18 @@ static int get_nid_and_numa_distance(struct drmem_lmb *lmb)
>>
>> if (primary_domain_index <= aa.array_sz &&
>> !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
>> - index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
>> - nid = of_read_number(&aa.arrays[index], 1);
>> + const __be32 *associativity;
>>
>> - if (nid == 0xffff || nid >= nr_node_ids)
>> - nid = default_nid;
>> + index = lmb->aa_index * aa.array_sz;
>> + associativity = &aa.arrays[index];
>> + nid = __associativity_to_nid(associativity, aa.array_sz);
>> if (nid > 0 && affinity_form == FORM1_AFFINITY) {
>> - int i;
>> - const __be32 *associativity;
>> -
>> - index = lmb->aa_index * aa.array_sz;
>> - associativity = &aa.arrays[index];
>> /*
>> - * lookup array associativity entries have different format
>> - * There is no length of the array as the first element.
>> + * lookup array associativity entries have
>> + * no length of the array as the first element.
>> */
>> - for (i = 0; i < distance_ref_points_depth; i++) {
>> - const __be32 *entry;
>> -
>> - entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
>> - distance_lookup_table[nid][i] = of_read_number(entry, 1);
>> - }
>> + ___initialize_form1_numa_distance(associativity,
>> + aa.array_sz);
>
> Better, thanks.
>
>> }
>> }
>> return nid;
>> @@ -632,11 +651,11 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
>>
>> if (primary_domain_index <= aa.array_sz &&
>> !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
>> - index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
>> - nid = of_read_number(&aa.arrays[index], 1);
>> + const __be32 *associativity;
>>
>> - if (nid == 0xffff || nid >= nr_node_ids)
>> - nid = default_nid;
>> + index = lmb->aa_index * aa.array_sz;
>> + associativity = &aa.arrays[index];
>> + nid = __associativity_to_nid(associativity, aa.array_sz);
>> }
>> return nid;
>> }
>
-aneesh
^ permalink raw reply
* Re: [PATCH v2] scripts/Makefile.clang: default to LLVM_IAS=1
From: Nathan Chancellor @ 2021-08-06 19:52 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-s390, Michal Marek, Vasily Gorbik, Jonathan Corbet,
Masahiro Yamada, linux-kbuild, linux-doc, Khem Raj,
Matthew Wilcox, linux-kernel, clang-built-linux,
Christian Borntraeger, Albert Ou, Palmer Dabbelt, Paul Walmsley,
linux-riscv, linuxppc-dev, Heiko Carstens
In-Reply-To: <20210806172701.3993843-1-ndesaulniers@google.com>
On Fri, Aug 06, 2021 at 10:27:01AM -0700, Nick Desaulniers wrote:
> LLVM_IAS=1 controls enabling clang's integrated assembler via
> -integrated-as. This was an explicit opt in until we could enable
> assembler support in Clang for more architecures. Now we have support
> and CI coverage of LLVM_IAS=1 for all architecures except a few more
> bugs affecting s390 and powerpc.
The powerpc and s390 folks have been testing with clang, I think they
should have been on CC for this change (done now).
> This commit flips the default from opt in via LLVM_IAS=1 to opt out via
> LLVM_IAS=0. CI systems or developers that were previously doing builds
> with CC=clang or LLVM=1 without explicitly setting LLVM_IAS must now
> explicitly opt out via LLVM_IAS=0, otherwise they will be implicitly
> opted-in.
>
> This finally shortens the command line invocation when cross compiling
> with LLVM to simply:
>
> $ make ARCH=arm64 LLVM=1
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
I am still not really sure how I feel about this. I would prefer not to
break people's builds but I suppose this is inevitabile eventually.
A little support matrix that I drafted up where based on ARCH and clang
version for LLVM_IAS=1 support:
| 10.x | 11.x | 12.x | 13.x | 14.x |
ARCH=arm | NO | NO | NO | YES | YES |
ARCH=arm64 | NO | YES | YES | YES | YES |
ARCH=i386 | YES | YES | YES | YES | YES |
ARCH=mips* | YES | YES | YES | YES | YES |
ARCH=powerpc | NO | NO | NO | NO | NO |
ARCH=s390 | NO | NO | NO | NO | NO |
ARCH=x86_64 | NO | YES | YES | YES | YES |
The main issue that I have with this change is that all of these
architectures work fine with CC=clang and their build commands that used
to work fine will not with this change, as they will have to specify
LLVM_IAS=0. I think that making this change for LLVM=1 makes sense but
changing the default for just CC=clang feels like a bit much at this
point in time. I would love to hear from others on this though, I am not
going to object much further than this.
Regardless of that concern, this patch does what it says so:
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Changes v1 -> v2:
> * Drop "Currently" from Documentation/, as per Matthew.
> * Drop Makefile and riscv Makefile, rebase on
> https://lore.kernel.org/lkml/20210805150102.131008-1-masahiroy@kernel.org/
> as per Masahiro.
> * Base is kbuild/for-next, plus
> https://lore.kernel.org/lkml/20210802183910.1802120-1-ndesaulniers@google.com/
> https://lore.kernel.org/lkml/20210805150102.131008-1-masahiroy@kernel.org/.
>
> Documentation/kbuild/llvm.rst | 14 ++++++++------
> scripts/Makefile.clang | 6 +++---
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/kbuild/llvm.rst b/Documentation/kbuild/llvm.rst
> index f8a360958f4c..e87ed5479963 100644
> --- a/Documentation/kbuild/llvm.rst
> +++ b/Documentation/kbuild/llvm.rst
> @@ -60,17 +60,14 @@ They can be enabled individually. The full list of the parameters: ::
> OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
> HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld
>
> -Currently, the integrated assembler is disabled by default. You can pass
> -``LLVM_IAS=1`` to enable it.
> +The integrated assembler is enabled by default. You can pass ``LLVM_IAS=0`` to
> +disable it.
>
> Omitting CROSS_COMPILE
> ----------------------
>
> As explained above, ``CROSS_COMPILE`` is used to set ``--target=<triple>``.
>
> -Unless ``LLVM_IAS=1`` is specified, ``CROSS_COMPILE`` is also used to derive
> -``--prefix=<path>`` to search for the GNU assembler and linker.
> -
> If ``CROSS_COMPILE`` is not specified, the ``--target=<triple>`` is inferred
> from ``ARCH``.
>
> @@ -78,7 +75,12 @@ That means if you use only LLVM tools, ``CROSS_COMPILE`` becomes unnecessary.
>
> For example, to cross-compile the arm64 kernel::
>
> - make ARCH=arm64 LLVM=1 LLVM_IAS=1
> + make ARCH=arm64 LLVM=1
> +
> +If ``LLVM_IAS=0`` is specified, ``CROSS_COMPILE`` is also used to derive
> +``--prefix=<path>`` to search for the GNU assembler and linker. ::
> +
> + make ARCH=arm64 LLVM=1 LLVM_IAS=0 CROSS_COMPILE=aarch64-linux-gnu-
>
> Supported Architectures
> -----------------------
> diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang
> index 1f4e3eb70f88..3ae63bd35582 100644
> --- a/scripts/Makefile.clang
> +++ b/scripts/Makefile.clang
> @@ -22,12 +22,12 @@ else
> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> endif # CROSS_COMPILE
>
> -ifeq ($(LLVM_IAS),1)
> -CLANG_FLAGS += -integrated-as
> -else
> +ifeq ($(LLVM_IAS),0)
> CLANG_FLAGS += -no-integrated-as
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> +else
> +CLANG_FLAGS += -integrated-as
> endif
> CLANG_FLAGS += -Werror=unknown-warning-option
> KBUILD_CFLAGS += $(CLANG_FLAGS)
>
> base-commit: d7a86429dbc691bf540688fcc8542cc20246a85b
> prerequisite-patch-id: 0d3072ecb5fd06ff6fd6ea81fe601f6c54c23910
> prerequisite-patch-id: 2654829756eb8a094a0ffad1679caa75a4d86619
> prerequisite-patch-id: a51e7885ca2376d008bbf146a5589da247806f7b
> prerequisite-patch-id: 6a0342755115ec459610657edac1075f069faa3d
> --
> 2.32.0.605.g8dce9f2422-goog
>
^ permalink raw reply
* Re: [PATCH v1 30/55] KVM: PPC: Book3S HV P9: Only execute mtSPR if the value changed
From: Fabiano Rosas @ 2021-08-06 20:45 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-31-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Keep better track of the current SPR value in places where
> they are to be loaded with a new context, to reduce expensive
> mtSPR operations.
>
> -73 cycles (7354) POWER9 virt-mode NULL hcall
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 64 ++++++++++++++++++++++--------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 0d97138e6fa4..56429b53f4dc 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4009,19 +4009,28 @@ static void switch_pmu_to_host(struct kvm_vcpu *vcpu,
> }
> }
>
> -static void load_spr_state(struct kvm_vcpu *vcpu)
> +static void load_spr_state(struct kvm_vcpu *vcpu,
> + struct p9_host_os_sprs *host_os_sprs)
> {
> - mtspr(SPRN_DSCR, vcpu->arch.dscr);
> - mtspr(SPRN_IAMR, vcpu->arch.iamr);
> - mtspr(SPRN_PSPB, vcpu->arch.pspb);
> - mtspr(SPRN_FSCR, vcpu->arch.fscr);
> mtspr(SPRN_TAR, vcpu->arch.tar);
> mtspr(SPRN_EBBHR, vcpu->arch.ebbhr);
> mtspr(SPRN_EBBRR, vcpu->arch.ebbrr);
> mtspr(SPRN_BESCR, vcpu->arch.bescr);
> - mtspr(SPRN_TIDR, vcpu->arch.tid);
> - mtspr(SPRN_AMR, vcpu->arch.amr);
> - mtspr(SPRN_UAMOR, vcpu->arch.uamor);
> +
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + mtspr(SPRN_TIDR, vcpu->arch.tid);
> + if (host_os_sprs->iamr != vcpu->arch.iamr)
> + mtspr(SPRN_IAMR, vcpu->arch.iamr);
> + if (host_os_sprs->amr != vcpu->arch.amr)
> + mtspr(SPRN_AMR, vcpu->arch.amr);
> + if (vcpu->arch.uamor != 0)
> + mtspr(SPRN_UAMOR, vcpu->arch.uamor);
> + if (host_os_sprs->fscr != vcpu->arch.fscr)
> + mtspr(SPRN_FSCR, vcpu->arch.fscr);
> + if (host_os_sprs->dscr != vcpu->arch.dscr)
> + mtspr(SPRN_DSCR, vcpu->arch.dscr);
> + if (vcpu->arch.pspb != 0)
> + mtspr(SPRN_PSPB, vcpu->arch.pspb);
>
> /*
> * DAR, DSISR, and for nested HV, SPRGs must be set with MSR[RI]
> @@ -4036,28 +4045,31 @@ static void load_spr_state(struct kvm_vcpu *vcpu)
>
> static void store_spr_state(struct kvm_vcpu *vcpu)
> {
> - vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
> -
> - vcpu->arch.iamr = mfspr(SPRN_IAMR);
> - vcpu->arch.pspb = mfspr(SPRN_PSPB);
> - vcpu->arch.fscr = mfspr(SPRN_FSCR);
> vcpu->arch.tar = mfspr(SPRN_TAR);
> vcpu->arch.ebbhr = mfspr(SPRN_EBBHR);
> vcpu->arch.ebbrr = mfspr(SPRN_EBBRR);
> vcpu->arch.bescr = mfspr(SPRN_BESCR);
> - vcpu->arch.tid = mfspr(SPRN_TIDR);
> +
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + vcpu->arch.tid = mfspr(SPRN_TIDR);
> + vcpu->arch.iamr = mfspr(SPRN_IAMR);
> vcpu->arch.amr = mfspr(SPRN_AMR);
> vcpu->arch.uamor = mfspr(SPRN_UAMOR);
> + vcpu->arch.fscr = mfspr(SPRN_FSCR);
> vcpu->arch.dscr = mfspr(SPRN_DSCR);
> + vcpu->arch.pspb = mfspr(SPRN_PSPB);
> +
> + vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
> }
>
> static void save_p9_host_os_sprs(struct p9_host_os_sprs *host_os_sprs)
> {
> - host_os_sprs->dscr = mfspr(SPRN_DSCR);
> - host_os_sprs->tidr = mfspr(SPRN_TIDR);
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + host_os_sprs->tidr = mfspr(SPRN_TIDR);
> host_os_sprs->iamr = mfspr(SPRN_IAMR);
> host_os_sprs->amr = mfspr(SPRN_AMR);
> host_os_sprs->fscr = mfspr(SPRN_FSCR);
> + host_os_sprs->dscr = mfspr(SPRN_DSCR);
> }
>
> /* vcpu guest regs must already be saved */
> @@ -4066,18 +4078,20 @@ static void restore_p9_host_os_sprs(struct kvm_vcpu *vcpu,
> {
> mtspr(SPRN_SPRG_VDSO_WRITE, local_paca->sprg_vdso);
>
> - mtspr(SPRN_PSPB, 0);
> - mtspr(SPRN_UAMOR, 0);
> -
> - mtspr(SPRN_DSCR, host_os_sprs->dscr);
> - mtspr(SPRN_TIDR, host_os_sprs->tidr);
> - mtspr(SPRN_IAMR, host_os_sprs->iamr);
> -
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + mtspr(SPRN_TIDR, host_os_sprs->tidr);
> + if (host_os_sprs->iamr != vcpu->arch.iamr)
> + mtspr(SPRN_IAMR, host_os_sprs->iamr);
> + if (vcpu->arch.uamor != 0)
> + mtspr(SPRN_UAMOR, 0);
> if (host_os_sprs->amr != vcpu->arch.amr)
> mtspr(SPRN_AMR, host_os_sprs->amr);
> -
> if (host_os_sprs->fscr != vcpu->arch.fscr)
> mtspr(SPRN_FSCR, host_os_sprs->fscr);
> + if (host_os_sprs->dscr != vcpu->arch.dscr)
> + mtspr(SPRN_DSCR, host_os_sprs->dscr);
> + if (vcpu->arch.pspb != 0)
> + mtspr(SPRN_PSPB, 0);
>
> /* Save guest CTRL register, set runlatch to 1 */
> if (!(vcpu->arch.ctrl & 1))
> @@ -4169,7 +4183,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> #endif
> mtspr(SPRN_VRSAVE, vcpu->arch.vrsave);
>
> - load_spr_state(vcpu);
> + load_spr_state(vcpu, &host_os_sprs);
>
> if (kvmhv_on_pseries()) {
> /*
^ permalink raw reply
* Re: [PATCH v1 31/55] KVM: PPC: Book3S HV P9: Juggle SPR switching around
From: Fabiano Rosas @ 2021-08-06 20:46 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-32-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This juggles SPR switching on the entry and exit sides to be more
> symmetric, which makes the next refactoring patch possible with no
> functional change.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 56429b53f4dc..c2c72875fca9 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4175,7 +4175,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> msr = mfmsr(); /* TM restore can update msr */
> }
>
> - switch_pmu_to_guest(vcpu, &host_os_sprs);
> + load_spr_state(vcpu, &host_os_sprs);
>
> load_fp_state(&vcpu->arch.fp);
> #ifdef CONFIG_ALTIVEC
> @@ -4183,7 +4183,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> #endif
> mtspr(SPRN_VRSAVE, vcpu->arch.vrsave);
>
> - load_spr_state(vcpu, &host_os_sprs);
> + switch_pmu_to_guest(vcpu, &host_os_sprs);
>
> if (kvmhv_on_pseries()) {
> /*
> @@ -4283,6 +4283,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> vcpu->arch.slb_max = 0;
> }
>
> + switch_pmu_to_host(vcpu, &host_os_sprs);
> +
> store_spr_state(vcpu);
>
> store_fp_state(&vcpu->arch.fp);
> @@ -4297,8 +4299,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>
> vcpu_vpa_increment_dispatch(vcpu);
>
> - switch_pmu_to_host(vcpu, &host_os_sprs);
> -
> timer_rearm_host_dec(*tb);
>
> restore_p9_host_os_sprs(vcpu, &host_os_sprs);
^ permalink raw reply
* Re: [PATCH v1 32/55] KVM: PPC: Book3S HV P9: Move vcpu register save/restore into functions
From: Fabiano Rosas @ 2021-08-06 20:49 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-33-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This should be no functional difference but makes the caller easier
> to read.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 65 +++++++++++++++++++++++-------------
> 1 file changed, 41 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index c2c72875fca9..45211458ac05 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4062,6 +4062,44 @@ static void store_spr_state(struct kvm_vcpu *vcpu)
> vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
> }
>
> +/* Returns true if current MSR and/or guest MSR may have changed */
> +static bool load_vcpu_state(struct kvm_vcpu *vcpu,
> + struct p9_host_os_sprs *host_os_sprs)
> +{
> + bool ret = false;
> +
> + if (cpu_has_feature(CPU_FTR_TM) ||
> + cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)) {
> + kvmppc_restore_tm_hv(vcpu, vcpu->arch.shregs.msr, true);
> + ret = true;
> + }
> +
> + load_spr_state(vcpu, host_os_sprs);
> +
> + load_fp_state(&vcpu->arch.fp);
> +#ifdef CONFIG_ALTIVEC
> + load_vr_state(&vcpu->arch.vr);
> +#endif
> + mtspr(SPRN_VRSAVE, vcpu->arch.vrsave);
> +
> + return ret;
> +}
> +
> +static void store_vcpu_state(struct kvm_vcpu *vcpu)
> +{
> + store_spr_state(vcpu);
> +
> + store_fp_state(&vcpu->arch.fp);
> +#ifdef CONFIG_ALTIVEC
> + store_vr_state(&vcpu->arch.vr);
> +#endif
> + vcpu->arch.vrsave = mfspr(SPRN_VRSAVE);
> +
> + if (cpu_has_feature(CPU_FTR_TM) ||
> + cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST))
> + kvmppc_save_tm_hv(vcpu, vcpu->arch.shregs.msr, true);
> +}
> +
> static void save_p9_host_os_sprs(struct p9_host_os_sprs *host_os_sprs)
> {
> if (!cpu_has_feature(CPU_FTR_ARCH_31))
> @@ -4169,19 +4207,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>
> vcpu_vpa_increment_dispatch(vcpu);
>
> - if (cpu_has_feature(CPU_FTR_TM) ||
> - cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST)) {
> - kvmppc_restore_tm_hv(vcpu, vcpu->arch.shregs.msr, true);
> - msr = mfmsr(); /* TM restore can update msr */
> - }
> -
> - load_spr_state(vcpu, &host_os_sprs);
> -
> - load_fp_state(&vcpu->arch.fp);
> -#ifdef CONFIG_ALTIVEC
> - load_vr_state(&vcpu->arch.vr);
> -#endif
> - mtspr(SPRN_VRSAVE, vcpu->arch.vrsave);
> + if (unlikely(load_vcpu_state(vcpu, &host_os_sprs)))
> + msr = mfmsr(); /* MSR may have been updated */
>
> switch_pmu_to_guest(vcpu, &host_os_sprs);
>
> @@ -4285,17 +4312,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>
> switch_pmu_to_host(vcpu, &host_os_sprs);
>
> - store_spr_state(vcpu);
> -
> - store_fp_state(&vcpu->arch.fp);
> -#ifdef CONFIG_ALTIVEC
> - store_vr_state(&vcpu->arch.vr);
> -#endif
> - vcpu->arch.vrsave = mfspr(SPRN_VRSAVE);
> -
> - if (cpu_has_feature(CPU_FTR_TM) ||
> - cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST))
> - kvmppc_save_tm_hv(vcpu, vcpu->arch.shregs.msr, true);
> + store_vcpu_state(vcpu);
>
> vcpu_vpa_increment_dispatch(vcpu);
^ permalink raw reply
* Re: [PATCH v4 2/5] dt-bindings: nintendo-otp: Document the Wii and Wii U OTP support
From: Rob Herring @ 2021-08-06 21:07 UTC (permalink / raw)
To: Emmanuel Gil Peyrot
Cc: devicetree, linux-kernel, Rob Herring, Paul Mackerras, Ash Logan,
Srinivas Kandagatla, linuxppc-dev, Jonathan Neuschäfer
In-Reply-To: <20210801073822.12452-3-linkmauve@linkmauve.fr>
On Sun, 01 Aug 2021 09:38:19 +0200, Emmanuel Gil Peyrot wrote:
> Both of these consoles use the exact same two registers, even at the
> same address, but the Wii U has eight banks of 128 bytes memory while
> the Wii only has one, hence the two compatible strings.
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
> .../bindings/nvmem/nintendo-otp.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* RE: [PATCH v4] soc: fsl: qe: convert QE interrupt controller to platform_device
From: Leo Li @ 2021-08-06 23:42 UTC (permalink / raw)
To: Maxim Kochetkov, linuxppc-dev@lists.ozlabs.org
Cc: kernel test robot, saravanak@google.com,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
Dan Carpenter, linux-arm-kernel@lists.infradead.org, Qiang Zhao
In-Reply-To: <20210803113538.560277-1-fido_max@inbox.ru>
> -----Original Message-----
> From: Maxim Kochetkov <fido_max@inbox.ru>
> Sent: Tuesday, August 3, 2021 6:36 AM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com>;
> gregkh@linuxfoundation.org; saravanak@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Maxim Kochetkov
> <fido_max@inbox.ru>; kernel test robot <lkp@intel.com>; Dan Carpenter
> <dan.carpenter@oracle.com>
> Subject: [PATCH v4] soc: fsl: qe: convert QE interrupt controller to
> platform_device
>
> Since 5.13 QE's ucc nodes can't get interrupts from devicetree:
>
> ucc@2000 {
> cell-index = <1>;
> reg = <0x2000 0x200>;
> interrupts = <32>;
> interrupt-parent = <&qeic>;
> };
>
> Now fw_devlink expects driver to create and probe a struct device for
> interrupt controller.
>
> So lets convert this driver to simple platform_device with probe().
> Also use platform_get_ and devm_ family function to get/allocate resources
> and drop unused .compatible = "qeic".
>
> [1] -
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k
> ernel.org%2Flkml%2FCAGETcx9PiX%3D%3DmLxB9PO8Myyk6u2vhPVwTMsA
> 5NkD-
> ywH5xhusw%40mail.gmail.com&data=04%7C01%7Cleoyang.li%40nxp.co
> m%7C1833b32e26de4ed7ef7908d956728eae%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C637635872281355718%7CUnknown%7CTWFpbGZsb3d
> 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> 3D%7C1000&sdata=HrivK73GYFAwygPz24JtO%2BTdkicCVYXOl3uywjOqS
> %2BA%3D&reserved=0
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Fixes: ea718c699055 ("Revert "Revert "driver core: Set fw_devlink=on by
> default""")
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to fix. Thanks.
> ---
> drivers/soc/fsl/qe/qe_ic.c | 75 ++++++++++++++++++++++----------------
> 1 file changed, 44 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c index
> 3f711c1a0996..e710d554425d 100644
> --- a/drivers/soc/fsl/qe/qe_ic.c
> +++ b/drivers/soc/fsl/qe/qe_ic.c
> @@ -23,6 +23,7 @@
> #include <linux/signal.h>
> #include <linux/device.h>
> #include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> #include <asm/irq.h>
> #include <asm/io.h>
> #include <soc/fsl/qe/qe.h>
> @@ -404,41 +405,40 @@ static void qe_ic_cascade_muxed_mpic(struct
> irq_desc *desc)
> chip->irq_eoi(&desc->irq_data);
> }
>
> -static void __init qe_ic_init(struct device_node *node)
> +static int qe_ic_init(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
> void (*low_handler)(struct irq_desc *desc);
> void (*high_handler)(struct irq_desc *desc);
> struct qe_ic *qe_ic;
> - struct resource res;
> - u32 ret;
> + struct resource *res;
> + struct device_node *node = pdev->dev.of_node;
>
> - ret = of_address_to_resource(node, 0, &res);
> - if (ret)
> - return;
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (res == NULL) {
> + dev_err(dev, "no memory resource defined\n");
> + return -ENODEV;
> + }
>
> - qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL);
> + qe_ic = devm_kzalloc(dev, sizeof(*qe_ic), GFP_KERNEL);
> if (qe_ic == NULL)
> - return;
> + return -ENOMEM;
>
> - qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
> - &qe_ic_host_ops, qe_ic);
> - if (qe_ic->irqhost == NULL) {
> - kfree(qe_ic);
> - return;
> + qe_ic->regs = devm_ioremap(dev, res->start, resource_size(res));
> + if (qe_ic->regs == NULL) {
> + dev_err(dev, "failed to ioremap() registers\n");
> + return -ENODEV;
> }
>
> - qe_ic->regs = ioremap(res.start, resource_size(&res));
> -
> qe_ic->hc_irq = qe_ic_irq_chip;
>
> - qe_ic->virq_high = irq_of_parse_and_map(node, 0);
> - qe_ic->virq_low = irq_of_parse_and_map(node, 1);
> + qe_ic->virq_high = platform_get_irq(pdev, 0);
> + qe_ic->virq_low = platform_get_irq(pdev, 1);
>
> - if (!qe_ic->virq_low) {
> - printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
> - kfree(qe_ic);
> - return;
> + if (qe_ic->virq_low < 0) {
> + return -ENODEV;
> }
> +
> if (qe_ic->virq_high != qe_ic->virq_low) {
> low_handler = qe_ic_cascade_low;
> high_handler = qe_ic_cascade_high;
> @@ -447,6 +447,13 @@ static void __init qe_ic_init(struct device_node
> *node)
> high_handler = NULL;
> }
>
> + qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
> + &qe_ic_host_ops, qe_ic);
> + if (qe_ic->irqhost == NULL) {
> + dev_err(dev, "failed to add irq domain\n");
> + return -ENODEV;
> + }
> +
> qe_ic_write(qe_ic->regs, QEIC_CICR, 0);
>
> irq_set_handler_data(qe_ic->virq_low, qe_ic); @@ -456,20 +463,26
> @@ static void __init qe_ic_init(struct device_node *node)
> irq_set_handler_data(qe_ic->virq_high, qe_ic);
> irq_set_chained_handler(qe_ic->virq_high, high_handler);
> }
> + return 0;
> }
> +static const struct of_device_id qe_ic_ids[] = {
> + { .compatible = "fsl,qe-ic"},
> + { .type = "qeic"},
> + {},
> +};
>
> -static int __init qe_ic_of_init(void)
> +static struct platform_driver qe_ic_driver =
> {
> - struct device_node *np;
> + .driver = {
> + .name = "qe-ic",
> + .of_match_table = qe_ic_ids,
> + },
> + .probe = qe_ic_init,
> +};
>
> - np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> - if (!np) {
> - np = of_find_node_by_type(NULL, "qeic");
> - if (!np)
> - return -ENODEV;
> - }
> - qe_ic_init(np);
> - of_node_put(np);
> +static int __init qe_ic_of_init(void)
> +{
> + platform_driver_register(&qe_ic_driver);
> return 0;
> }
> subsys_initcall(qe_ic_of_init);
> --
> 2.31.1
^ permalink raw reply
* Re: [PATCH v2 0/6] PCI: Drop duplicated tracking of a pci_dev's bound driver
From: Bjorn Helgaas @ 2021-08-06 21:24 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Mark Rutland, Giovanni Cabiddu, Rafał Miłecki,
Peter Zijlstra, linux-pci, Alexander Duyck, x86, oss-drivers,
netdev, Paul Mackerras, H. Peter Anvin, Jiri Olsa,
Thomas Gleixner, Taras Chornyi, Stefano Stabellini, Herbert Xu,
linux-scsi, Sathya Prakash, qat-linux, Alexander Shishkin,
Ingo Molnar, Jakub Kicinski, Yisen Zhuang,
Suganath Prabu Subramani, Fiona Trahe, Oliver O'Halloran,
Andrew Donnellan, Mathias Nyman, Konrad Rzeszutek Wilk,
Ido Schimmel, Arnaldo Carvalho de Melo, Frederic Barrat,
Borislav Petkov, Michael Buesch, Jiri Pirko, Bjorn Helgaas,
Namhyung Kim, Boris Ostrovsky, Andy Shevchenko, Juergen Gross,
Salil Mehta, Sreekanth Reddy, xen-devel, Vadym Kochan,
MPT-FusionLinux.pdl, linux-usb, linux-wireless, linux-kernel,
linux-perf-users, Zhou Wang, Arnd Bergmann, linux-crypto, kernel,
Greg Kroah-Hartman, Simon Horman, Wojciech Ziemba, linuxppc-dev,
David S. Miller
In-Reply-To: <20210806064623.3lxl4clzbjpmchef@pengutronix.de>
On Fri, Aug 06, 2021 at 08:46:23AM +0200, Uwe Kleine-König wrote:
> On Thu, Aug 05, 2021 at 06:42:34PM -0500, Bjorn Helgaas wrote:
> > I looked at all the bus_type.probe() methods, it looks like pci_dev is
> > not the only offender here. At least the following also have a driver
> > pointer in the device struct:
> >
> > parisc_device.driver
> > acpi_device.driver
> > dio_dev.driver
> > hid_device.driver
> > pci_dev.driver
> > pnp_dev.driver
> > rio_dev.driver
> > zorro_dev.driver
>
> Right, when I converted zorro_dev it was pointed out that the code was
> copied from pci and the latter has the same construct. :-)
> See
> https://lore.kernel.org/r/20210730191035.1455248-5-u.kleine-koenig@pengutronix.de
> for the patch, I don't find where pci was pointed out, maybe it was on
> irc only.
Oh, thanks! I looked to see if you'd done something similar
elsewhere, but I missed this one.
> > Looking through the places that care about pci_dev.driver (the ones
> > updated by patch 5/6), many of them are ... a little dubious to begin
> > with. A few need the "struct pci_error_handlers *err_handler"
> > pointer, so that's probably legitimate. But many just need a name,
> > and should probably be using dev_driver_string() instead.
>
> Yeah, I considered adding a function to get the driver name from a
> pci_dev and a function to get the error handlers. Maybe it's an idea to
> introduce these two and then use to_pci_driver(pdev->dev.driver) for the
> few remaining users? Maybe doing that on top of my current series makes
> sense to have a clean switch from pdev->driver to pdev->dev.driver?!
I'd propose using dev_driver_string() for these places:
eeh_driver_name() (could change callers to use dev_driver_string())
bcma_host_pci_probe()
qm_alloc_uacce()
hns3_get_drvinfo()
prestera_pci_probe()
mlxsw_pci_probe()
nfp_get_drvinfo()
ssb_pcihost_probe()
The use in mpt_device_driver_register() looks unnecessary: it's only
to get a struct pci_device_id *, which is passed to ->probe()
functions that don't need it.
The use in adf_enable_aer() looks wrong: it sets the err_handler
pointer in one of the adf_driver structs. I think those structs
should be basically immutable, and the drivers that call
adf_enable_aer() from their .probe() methods should set
".err_handler = &adf_err_handler" in their static adf_driver
definitions instead.
I think that basically leaves these:
uncore_pci_probe() # .id_table, custom driver "registration"
match_id() # .id_table, arch/x86/kernel/probe_roms.c
xhci_pci_quirks() # .id_table
pci_error_handlers() # roll-your-own AER handling, drivers/misc/cxl/guest.c
I think it would be fine to use to_pci_driver(pdev->dev.driver) for
these few.
Bjorn
^ permalink raw reply
* [PATCH v2 0/4] Some improvements on regs usage
From: sxwjean @ 2021-08-07 1:02 UTC (permalink / raw)
To: linuxppc-dev
Cc: ravi.bangoria, Xiongwei Song, aneesh.kumar, oleg, npiggin,
linux-kernel, efremov, paulus
From: Xiongwei Song <sxwjean@gmail.com>
When CONFIG_4xx=y or CONFIG_BOOKE=y, currently in code we reference dsisr
to get interrupt reasons and reference dar to get excepiton address.
However, in reference manuals, esr is used for interrupt reasons and dear
is used for excepiton address, so the patchset changes dsisr -> esr,
dar -> dear for CONFIG_4xx=y or CONFIG_BOOKE=y.
Meanwhile, we use _ESR and _DEAR to get offsets of esr and dear on stack.
v2:
- Discard changes in arch/powerpc/mm/fault.c as Christophe and Michael
suggested.
- Discard changes in UAPI headers to avoid possible compile issue.
v1:
- https://lkml.org/lkml/2021/8/6/57
- https://lkml.org/lkml/2021/7/26/533
- https://lkml.org/lkml/2021/7/26/534
- https://lkml.org/lkml/2021/7/26/535
Xiongwei Song (4):
powerpc: Optimize register usage for esr register
powerpc/64e: Get esr offset with _ESR macro
powerpc: Optimize register usage for dear register
powerpc/64e: Get dear offset with _DEAR macro
arch/powerpc/include/asm/ptrace.h | 10 ++++++++--
arch/powerpc/kernel/asm-offsets.c | 15 ++++-----------
arch/powerpc/kernel/exceptions-64e.S | 18 +++++++++---------
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/kernel/ptrace/ptrace.c | 4 ++++
arch/powerpc/kernel/traps.c | 2 +-
arch/powerpc/platforms/44x/machine_check.c | 4 ++--
arch/powerpc/platforms/4xx/machine_check.c | 2 +-
8 files changed, 30 insertions(+), 27 deletions(-)
--
2.30.2
^ permalink raw reply
* [PATCH v2 1/4] powerpc: Optimize register usage for esr register
From: sxwjean @ 2021-08-07 1:02 UTC (permalink / raw)
To: linuxppc-dev
Cc: ravi.bangoria, Xiongwei Song, aneesh.kumar, oleg, npiggin,
linux-kernel, efremov, paulus
In-Reply-To: <20210807010239.416055-1-sxwjean@me.com>
From: Xiongwei Song <sxwjean@gmail.com>
Create an anonymous union for dsisr and esr regsiters, we can reference
esr to get the exception detail when CONFIG_4xx=y or CONFIG_BOOKE=y.
Otherwise, reference dsisr. This makes code more clear.
Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
---
arch/powerpc/include/asm/ptrace.h | 5 ++++-
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
arch/powerpc/kernel/traps.c | 2 +-
arch/powerpc/platforms/44x/machine_check.c | 4 ++--
arch/powerpc/platforms/4xx/machine_check.c | 2 +-
6 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 3e5d470a6155..c252d04b1206 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -44,7 +44,10 @@ struct pt_regs
#endif
unsigned long trap;
unsigned long dar;
- unsigned long dsisr;
+ union {
+ unsigned long dsisr;
+ unsigned long esr;
+ };
unsigned long result;
};
};
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..f74af8f9133c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1499,7 +1499,7 @@ static void __show_regs(struct pt_regs *regs)
trap == INTERRUPT_DATA_STORAGE ||
trap == INTERRUPT_ALIGNMENT) {
if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
- pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr);
+ pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->esr);
else
pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 0a0a33eb0d28..a222fd4d6334 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -375,6 +375,8 @@ void __init pt_regs_check(void)
offsetof(struct user_pt_regs, dar));
BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
offsetof(struct user_pt_regs, dsisr));
+ BUILD_BUG_ON(offsetof(struct pt_regs, esr) !=
+ offsetof(struct user_pt_regs, dsisr));
BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
offsetof(struct user_pt_regs, result));
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index dfbce527c98e..2164f5705a0b 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -562,7 +562,7 @@ static inline int check_io_access(struct pt_regs *regs)
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/* On 4xx, the reason for the machine check or program exception
is in the ESR. */
-#define get_reason(regs) ((regs)->dsisr)
+#define get_reason(regs) ((regs)->esr)
#define REASON_FP ESR_FP
#define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
#define REASON_PRIVILEGED ESR_PPR
diff --git a/arch/powerpc/platforms/44x/machine_check.c b/arch/powerpc/platforms/44x/machine_check.c
index a5c898bb9bab..5d19daacd78a 100644
--- a/arch/powerpc/platforms/44x/machine_check.c
+++ b/arch/powerpc/platforms/44x/machine_check.c
@@ -11,7 +11,7 @@
int machine_check_440A(struct pt_regs *regs)
{
- unsigned long reason = regs->dsisr;
+ unsigned long reason = regs->esr;
printk("Machine check in kernel mode.\n");
if (reason & ESR_IMCP){
@@ -48,7 +48,7 @@ int machine_check_440A(struct pt_regs *regs)
#ifdef CONFIG_PPC_47x
int machine_check_47x(struct pt_regs *regs)
{
- unsigned long reason = regs->dsisr;
+ unsigned long reason = regs->esr;
u32 mcsr;
printk(KERN_ERR "Machine check in kernel mode.\n");
diff --git a/arch/powerpc/platforms/4xx/machine_check.c b/arch/powerpc/platforms/4xx/machine_check.c
index a71c29892a91..a905da1d6f41 100644
--- a/arch/powerpc/platforms/4xx/machine_check.c
+++ b/arch/powerpc/platforms/4xx/machine_check.c
@@ -10,7 +10,7 @@
int machine_check_4xx(struct pt_regs *regs)
{
- unsigned long reason = regs->dsisr;
+ unsigned long reason = regs->esr;
if (reason & ESR_IMCP) {
printk("Instruction");
--
2.30.2
^ permalink raw reply related
* [PATCH v2 2/4] powerpc/64e: Get esr offset with _ESR macro
From: sxwjean @ 2021-08-07 1:02 UTC (permalink / raw)
To: linuxppc-dev
Cc: ravi.bangoria, Xiongwei Song, aneesh.kumar, oleg, npiggin,
linux-kernel, efremov, paulus
In-Reply-To: <20210807010239.416055-1-sxwjean@me.com>
From: Xiongwei Song <sxwjean@gmail.com>
Use _ESR to get the offset of esr register in pr_regs for 64e cpus.
Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
---
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/exceptions-64e.S | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index a47eefa09bcb..f4ebc435fd78 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -287,6 +287,7 @@ int main(void)
STACK_PT_REGS_OFFSET(_XER, xer);
STACK_PT_REGS_OFFSET(_DAR, dar);
STACK_PT_REGS_OFFSET(_DSISR, dsisr);
+ STACK_PT_REGS_OFFSET(_ESR, esr);
STACK_PT_REGS_OFFSET(ORIG_GPR3, orig_gpr3);
STACK_PT_REGS_OFFSET(RESULT, result);
STACK_PT_REGS_OFFSET(_TRAP, trap);
@@ -298,7 +299,6 @@ int main(void)
* we use them to hold SRR0 and SRR1.
*/
STACK_PT_REGS_OFFSET(_DEAR, dar);
- STACK_PT_REGS_OFFSET(_ESR, dsisr);
#else /* CONFIG_PPC64 */
STACK_PT_REGS_OFFSET(SOFTE, softe);
STACK_PT_REGS_OFFSET(_PPR, ppr);
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 1401787b0b93..bf8c4c2f98ea 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -546,7 +546,7 @@ __end_interrupts:
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
std r14,_DAR(r1)
- std r15,_DSISR(r1)
+ std r15,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x300)
@@ -559,7 +559,7 @@ __end_interrupts:
li r15,0
mr r14,r10
std r14,_DAR(r1)
- std r15,_DSISR(r1)
+ std r15,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x400)
@@ -576,7 +576,7 @@ __end_interrupts:
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
std r14,_DAR(r1)
- std r15,_DSISR(r1)
+ std r15,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x600)
@@ -587,7 +587,7 @@ __end_interrupts:
NORMAL_EXCEPTION_PROLOG(0x700, BOOKE_INTERRUPT_PROGRAM,
PROLOG_ADDITION_1REG)
mfspr r14,SPRN_ESR
- std r14,_DSISR(r1)
+ std r14,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
EXCEPTION_COMMON(0x700)
addi r3,r1,STACK_FRAME_OVERHEAD
@@ -1058,7 +1058,7 @@ bad_stack_book3e:
mfspr r10,SPRN_DEAR
mfspr r11,SPRN_ESR
std r10,_DAR(r1)
- std r11,_DSISR(r1)
+ std r11,_ESR(r1)
std r0,GPR0(r1); /* save r0 in stackframe */ \
std r2,GPR2(r1); /* save r2 in stackframe */ \
SAVE_4GPRS(3, r1); /* save r3 - r6 in stackframe */ \
--
2.30.2
^ permalink raw reply related
* [PATCH v2 3/4] powerpc: Optimize register usage for dear register
From: sxwjean @ 2021-08-07 1:02 UTC (permalink / raw)
To: linuxppc-dev
Cc: ravi.bangoria, Xiongwei Song, aneesh.kumar, oleg, npiggin,
linux-kernel, efremov, paulus
In-Reply-To: <20210807010239.416055-1-sxwjean@me.com>
From: Xiongwei Song <sxwjean@gmail.com>
Create an anonymous union for dar and dear regsiters, we can reference
dear to get the effective address when CONFIG_4xx=y or CONFIG_BOOKE=y.
Otherwise, reference dar. This makes code more clear.
Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
---
arch/powerpc/include/asm/ptrace.h | 5 ++++-
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index c252d04b1206..fa725e3238c2 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -43,7 +43,10 @@ struct pt_regs
unsigned long mq;
#endif
unsigned long trap;
- unsigned long dar;
+ union {
+ unsigned long dar;
+ unsigned long dear;
+ };
union {
unsigned long dsisr;
unsigned long esr;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index f74af8f9133c..50436b52c213 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1499,7 +1499,7 @@ static void __show_regs(struct pt_regs *regs)
trap == INTERRUPT_DATA_STORAGE ||
trap == INTERRUPT_ALIGNMENT) {
if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
- pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->esr);
+ pr_cont("DEAR: "REG" ESR: "REG" ", regs->dear, regs->esr);
else
pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index a222fd4d6334..7c7093c17c45 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -373,6 +373,8 @@ void __init pt_regs_check(void)
offsetof(struct user_pt_regs, trap));
BUILD_BUG_ON(offsetof(struct pt_regs, dar) !=
offsetof(struct user_pt_regs, dar));
+ BUILD_BUG_ON(offsetof(struct pt_regs, dear) !=
+ offsetof(struct user_pt_regs, dar));
BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
offsetof(struct user_pt_regs, dsisr));
BUILD_BUG_ON(offsetof(struct pt_regs, esr) !=
--
2.30.2
^ permalink raw reply related
* [PATCH v2 4/4] powerpc/64e: Get dear offset with _DEAR macro
From: sxwjean @ 2021-08-07 1:02 UTC (permalink / raw)
To: linuxppc-dev
Cc: ravi.bangoria, Xiongwei Song, aneesh.kumar, oleg, npiggin,
linux-kernel, efremov, paulus
In-Reply-To: <20210807010239.416055-1-sxwjean@me.com>
From: Xiongwei Song <sxwjean@gmail.com>
Use _DEAR to get the offset of dear register in pr_regs for 64e cpus.
Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
---
arch/powerpc/kernel/asm-offsets.c | 13 +++----------
arch/powerpc/kernel/exceptions-64e.S | 8 ++++----
2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index f4ebc435fd78..8357d5fcd09e 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -286,23 +286,16 @@ int main(void)
STACK_PT_REGS_OFFSET(_CCR, ccr);
STACK_PT_REGS_OFFSET(_XER, xer);
STACK_PT_REGS_OFFSET(_DAR, dar);
+ STACK_PT_REGS_OFFSET(_DEAR, dear);
STACK_PT_REGS_OFFSET(_DSISR, dsisr);
STACK_PT_REGS_OFFSET(_ESR, esr);
STACK_PT_REGS_OFFSET(ORIG_GPR3, orig_gpr3);
STACK_PT_REGS_OFFSET(RESULT, result);
STACK_PT_REGS_OFFSET(_TRAP, trap);
-#ifndef CONFIG_PPC64
- /*
- * The PowerPC 400-class & Book-E processors have neither the DAR
- * nor the DSISR SPRs. Hence, we overload them to hold the similar
- * DEAR and ESR SPRs for such processors. For critical interrupts
- * we use them to hold SRR0 and SRR1.
- */
- STACK_PT_REGS_OFFSET(_DEAR, dar);
-#else /* CONFIG_PPC64 */
+#ifdef CONFIG_PPC64
STACK_PT_REGS_OFFSET(SOFTE, softe);
STACK_PT_REGS_OFFSET(_PPR, ppr);
-#endif /* CONFIG_PPC64 */
+#endif
#ifdef CONFIG_PPC_PKEY
STACK_PT_REGS_OFFSET(STACK_REGS_AMR, amr);
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index bf8c4c2f98ea..221e085e8c8c 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -545,7 +545,7 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
- std r14,_DAR(r1)
+ std r14,_DEAR(r1)
std r15,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
ld r15,PACA_EXGEN+EX_R15(r13)
@@ -558,7 +558,7 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
li r15,0
mr r14,r10
- std r14,_DAR(r1)
+ std r14,_DEAR(r1)
std r15,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
ld r15,PACA_EXGEN+EX_R15(r13)
@@ -575,7 +575,7 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
- std r14,_DAR(r1)
+ std r14,_DEAR(r1)
std r15,_ESR(r1)
ld r14,PACA_EXGEN+EX_R14(r13)
ld r15,PACA_EXGEN+EX_R15(r13)
@@ -1057,7 +1057,7 @@ bad_stack_book3e:
std r11,_CCR(r1)
mfspr r10,SPRN_DEAR
mfspr r11,SPRN_ESR
- std r10,_DAR(r1)
+ std r10,_DEAR(r1)
std r11,_ESR(r1)
std r0,GPR0(r1); /* save r0 in stackframe */ \
std r2,GPR2(r1); /* save r2 in stackframe */ \
--
2.30.2
^ permalink raw reply related
* Re: Debian SID kernel doesn't boot on PowerBook 3400c
From: Finn Thain @ 2021-08-07 2:05 UTC (permalink / raw)
To: Stan Johnson; +Cc: debian-powerpc, linuxppc-dev, Nicholas Piggin
In-Reply-To: <ee724da4-4a5b-65c3-9c1c-d78954fdc7b4@csgroup.eu>
On Fri, 6 Aug 2021, Christophe Leroy wrote:
>
> I have cooked a tentative fix for that KUAP stuff.
> Could you try the branch 'bugtest' at https://github.com/chleroy/linux.git
>
Thanks, Christophe.
Stan, please test the following build.
$ git remote add chleroy-linux https://github.com/chleroy/linux.git -f -t bugtest
...
$ git checkout chleroy-linux/bugtest
HEAD is now at 63e3756d1bdf powerpc/interrupts: Also perform KUAP/KUEP lock and usertime accounting on NMI
$ cp ../dot-config-powermac-5.13 .config
$ scripts/config -e CONFIG_PPC_KUAP -e CONFIG_PPC_KUAP_DEBUG -e CONFIG_VMAP_STACK
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig vmlinux
$ egrep "CONFIG_PPC_KUAP|CONFIG_VMAP_STACK" .config
$ strings vmlinux |grep "Linux version"
If that kernel produces errors, I'd try a second build as well:
$ scripts/config -e CONFIG_PPC_KUAP -e CONFIG_PPC_KUAP_DEBUG -d CONFIG_VMAP_STACK
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -j4 clean olddefconfig vmlinux
$ egrep "CONFIG_PPC_KUAP|CONFIG_VMAP_STACK" .config
$ strings vmlinux |grep "Linux version"
Please boot using the same kernel parameters as last time and capture the
serial console logs. In case we're still dealing with intermittent bugs it
might be necessary to repeat these tests so I suggest you retain the
vmlinux files.
^ permalink raw reply
* Re: Debian SID kernel doesn't boot on PowerBook 3400c
From: Finn Thain @ 2021-08-07 4:08 UTC (permalink / raw)
To: Stan Johnson; +Cc: debian-powerpc, linuxppc-dev, Nicholas Piggin
In-Reply-To: <06ddf5ab-b0c9-1c64-92ea-a9cfbfb9f3b0@yahoo.com>
On Fri, 6 Aug 2021, Stan Johnson wrote:
> $ egrep '(CONFIG_PPC_KUAP|CONFIG_VMAP_STACK)' .config
> CONFIG_PPC_KUAP=y
> CONFIG_PPC_KUAP_DEBUG=y
> CONFIG_VMAP_STACK=y
> $ strings vmlinux | fgrep "Linux version"
> Linux version 5.13.0-pmac-00004-g63e3756d1bd ...
> $ cp vmlinux ../vmlinux-5.13.0-pmac-00004-g63e3756d1bd-1
>
> 1) PB 3400c
> vmlinux-5.13.0-pmac-00004-g63e3756d1bd-1
> Boots, no errors logging in at (text) fb console. Logging in via ssh and
> running "ls -Rail /usr/include" generated errors (and a hung ssh
> session). Once errors started, they repeated for almost every command.
> See pb3400c-63e3756d1bdf-1.txt.
>
> 2) Wallstreet
> vmlinux-5.13.0-pmac-00004-g63e3756d1bd-1
> X login failed, there were errors ("Oops: Kernel access of bad area",
> "Oops: Exception in kernel mode"). Logging in via SSH, there were no
> additional errors after running "ls -Rail /usr/include" -- the errors
> did not escalate as they did on the PB 3400.
> See Wallstreet-63e3756d1bdf-1.txt.
>
...
> $ egrep '(CONFIG_PPC_KUAP|CONFIG_VMAP_STACK)' .config
> CONFIG_PPC_KUAP=y
> CONFIG_PPC_KUAP_DEBUG=y
> # CONFIG_VMAP_STACK is not set
> $ strings vmlinux | fgrep "Linux version"
> Linux version 5.13.0-pmac-00004-g63e3756d1bd ...
> $ cp vmlinux ../vmlinux-5.13.0-pmac-00004-g63e3756d1bd-2
>
> 3) PB 3400c
> vmlinux-5.13.0-pmac-00004-g63e3756d1bd-2
> Filesystem was corrupt from the previous test (probably from all the
> errors during shutdown). After fixing the filesystem:
> Boots, no errors logging in at (text) fb console. Logging in via ssh and
> running "ls -Rail /usr/include" generated a few errors. There didn't
> seem to be as many errors as in the previous test, there were a few
> errors during shutdown but the shutdown was otherwise normal.
> See pb3400c-63e3756d1bdf-2.txt.
>
> 4) Wallstreet
> vmlinux-5.13.0-pmac-00004-g63e3756d1bd-2
> X login worked, and there were no errors. There were no errors during
> ssh access.
> See Wallstreet-63e3756d1bdf-2.txt.
>
Thanks for collecting these results, Stan. Do you think that the
successful result from test 4) could have been just chance?
It appears that the bug affecting the Powerbook 3400 is unaffected by
CONFIG_VMAP_STACK.
Whereas the bug affecting the Powerbook G3 disappears when
CONFIG_VMAP_STACK is disabled (assuming the result from 4 is reliable).
Either way, these results reiterate that "Oops: Kernel access of bad area,
sig: 11" was not entirely resolved by "powerpc/32s: Fix napping restore in
data storage interrupt (DSI)".
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc: Optimize register usage for esr register
From: Christophe Leroy @ 2021-08-07 6:56 UTC (permalink / raw)
To: sxwjean, linuxppc-dev
Cc: ravi.bangoria, Xiongwei Song, oleg, npiggin, linux-kernel,
efremov, paulus, aneesh.kumar
In-Reply-To: <20210807010239.416055-2-sxwjean@me.com>
Le 07/08/2021 à 03:02, sxwjean@me.com a écrit :
> From: Xiongwei Song <sxwjean@gmail.com>
>
> Create an anonymous union for dsisr and esr regsiters, we can reference
> esr to get the exception detail when CONFIG_4xx=y or CONFIG_BOOKE=y.
> Otherwise, reference dsisr. This makes code more clear.
>
> Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> ---
> arch/powerpc/include/asm/ptrace.h | 5 ++++-
> arch/powerpc/kernel/process.c | 2 +-
> arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> arch/powerpc/kernel/traps.c | 2 +-
> arch/powerpc/platforms/44x/machine_check.c | 4 ++--
> arch/powerpc/platforms/4xx/machine_check.c | 2 +-
> 6 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index 3e5d470a6155..c252d04b1206 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -44,7 +44,10 @@ struct pt_regs
> #endif
> unsigned long trap;
> unsigned long dar;
> - unsigned long dsisr;
> + union {
> + unsigned long dsisr;
> + unsigned long esr;
> + };
> unsigned long result;
> };
> };
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 185beb290580..f74af8f9133c 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1499,7 +1499,7 @@ static void __show_regs(struct pt_regs *regs)
> trap == INTERRUPT_DATA_STORAGE ||
> trap == INTERRUPT_ALIGNMENT) {
> if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> - pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr);
> + pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->esr);
> else
> pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
> }
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> index 0a0a33eb0d28..a222fd4d6334 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -375,6 +375,8 @@ void __init pt_regs_check(void)
> offsetof(struct user_pt_regs, dar));
> BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
> offsetof(struct user_pt_regs, dsisr));
> + BUILD_BUG_ON(offsetof(struct pt_regs, esr) !=
> + offsetof(struct user_pt_regs, dsisr));
esr and dsisr are the same, so checking the same thing a second time is pointless.
> BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> offsetof(struct user_pt_regs, result));
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index dfbce527c98e..2164f5705a0b 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -562,7 +562,7 @@ static inline int check_io_access(struct pt_regs *regs)
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> /* On 4xx, the reason for the machine check or program exception
> is in the ESR. */
> -#define get_reason(regs) ((regs)->dsisr)
> +#define get_reason(regs) ((regs)->esr)
> #define REASON_FP ESR_FP
> #define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
> #define REASON_PRIVILEGED ESR_PPR
> diff --git a/arch/powerpc/platforms/44x/machine_check.c b/arch/powerpc/platforms/44x/machine_check.c
> index a5c898bb9bab..5d19daacd78a 100644
> --- a/arch/powerpc/platforms/44x/machine_check.c
> +++ b/arch/powerpc/platforms/44x/machine_check.c
> @@ -11,7 +11,7 @@
>
> int machine_check_440A(struct pt_regs *regs)
> {
> - unsigned long reason = regs->dsisr;
> + unsigned long reason = regs->esr;
>
> printk("Machine check in kernel mode.\n");
> if (reason & ESR_IMCP){
> @@ -48,7 +48,7 @@ int machine_check_440A(struct pt_regs *regs)
> #ifdef CONFIG_PPC_47x
> int machine_check_47x(struct pt_regs *regs)
> {
> - unsigned long reason = regs->dsisr;
> + unsigned long reason = regs->esr;
> u32 mcsr;
>
> printk(KERN_ERR "Machine check in kernel mode.\n");
> diff --git a/arch/powerpc/platforms/4xx/machine_check.c b/arch/powerpc/platforms/4xx/machine_check.c
> index a71c29892a91..a905da1d6f41 100644
> --- a/arch/powerpc/platforms/4xx/machine_check.c
> +++ b/arch/powerpc/platforms/4xx/machine_check.c
> @@ -10,7 +10,7 @@
>
> int machine_check_4xx(struct pt_regs *regs)
> {
> - unsigned long reason = regs->dsisr;
> + unsigned long reason = regs->esr;
>
> if (reason & ESR_IMCP) {
> printk("Instruction");
>
^ 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