* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Segher Boessenkool @ 2015-01-08 0:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Purcareata Bogdan, linux-kernel
In-Reply-To: <1420641610.5830.29.camel@kernel.crashing.org>
On Wed, Jan 07, 2015 at 03:40:10PM +0100, Benjamin Herrenschmidt wrote:
> One way to work around the performance loss for you would be to add some
> DT property to indicate to the guest that the read isn't necessary.
Or KVM could use a virtual interrupt controller better suited to its
needs.
Segher
^ permalink raw reply
* Re: [PATCH v6 2/4] tools/perf: extend format_alias() to include event parameters
From: Sukadev Bhattiprolu @ 2015-01-07 23:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: peterz, linux-kernel, Arnaldo Carvalho de Melo, dev,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20150106093910.GA24841@krava.brq.redhat.com>
Jiri Olsa [jolsa@redhat.com] wrote:
| On Sun, Dec 21, 2014 at 11:49:25PM -0800, Sukadev Bhattiprolu wrote:
| > From: Cody P Schafer <cody@linux.vnet.ibm.com>
| >
| > This causes `perf list pmu` to show parameters for parameterized events
| > like:
| >
| > pmu/event_name,param1=?,param2=?/ [Kernel PMU event]
| >
| > An example:
| >
| > hv_24x7/HPM_TLBIE__PHYS_CORE,core=?/ [Kernel PMU event]
| >
| > Changelog[v6]
| > [Jir Olsa, Sukadev Bhattiprolu] Drop the '$' sign and go back to
| > just printing whatevever sysfs provides (which is '=?') to identify
| > required parameters. sysfs also now uses parameters like 'core'
| > and 'vcpu' rather than 'starting_index'.
| >
| > Changelog[v5]
| > [Jiri Olsa, Peter Zijlstra] Use '$' to prefix parameterized events.
| >
| > Changelog[v4]
| > [Jiri Olsa] If the parameter for an event in sysfs is 'param=val',
| > have perf-list show the event as 'param=?' rather than 'val=?'.
| >
| > CC: Haren Myneni <hbabu@us.ibm.com>
| > CC: Cody P Schafer <dev@codyps.com>
| > Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
| > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| > ---
| > tools/perf/util/pmu.c | 27 ++++++++++++++++++++++++++-
| > 1 file changed, 26 insertions(+), 1 deletion(-)
| >
| > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
| > index cb516dd..d208fef 100644
| > --- a/tools/perf/util/pmu.c
| > +++ b/tools/perf/util/pmu.c
| > @@ -810,10 +810,35 @@ void perf_pmu__set_format(unsigned long *bits, long from, long to)
| > set_bit(b, bits);
| > }
| >
| > +static int sub_non_neg(int a, int b)
| > +{
| > + if (b > a)
| > + return 0;
| > + return a - b;
| > +}
| > +
| > static char *format_alias(char *buf, int len, struct perf_pmu *pmu,
| > struct perf_pmu_alias *alias)
| > {
| > - snprintf(buf, len, "%s/%s/", pmu->name, alias->name);
| > + struct parse_events_term *term;
| > + int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
| > +
| > + list_for_each_entry(term, &alias->terms, list)
| > + if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
| > + used += snprintf(buf + used, sub_non_neg(len, used),
| > + ",%s=%s", term->config,
| > + term->val.str);
|
| why not display PARSE_EVENTS__TERM_TYPE_NUM as well?
Well, we are only trying to list the attributes that user needs to specify:
hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,core=?/
We could print the PARSE_EVENTS__TERM_TYPE_NUM terms as well,
hv_24x7/HPM_0THRD_NON_IDLE_CCYC__PHYS_CORE,domain=2,offset=e0,core=?,lpar=0/
but it would be needlessly verbose. The attributes with numeric values
like domain and offset, have fixed values for the event and user doesn't
need to know. If they do really need to know, they could just look up in
sysfs ?
Sukadev
^ permalink raw reply
* Re: [PATCH] [v3] power/fsl: add MDIO dt binding for FMan
From: Emil Medve @ 2015-01-07 19:44 UTC (permalink / raw)
To: Scott Wood, Xie Shaohui-B21989
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1420653934.4961.58.camel@freescale.com>
Hello Scott,
On 01/07/2015 12:05 PM, Scott Wood wrote:
> On Tue, 2015-01-06 at 23:29 -0600, Xie Shaohui-B21989 wrote:
>>>>> +- interrupts
>>>>> + Usage: optional
>>>>> + Value type: <prop-encoded-array>
>>>>> + Definition: Event interrupt of external MDIO controller.
>>>>> + 1 Gb/s MDIO and 10 Gb/s MDIO has one interrupt respectively.
>>>
>>> I'm confused by "respectively" here. Does fsl,fman-memac-mdio have two
>>> interrupts (one for 1 Gb/s and one for 10 Gb/s)?
>> [S.H] We use two MDIO controllers for external PHY management. One for 1 Gb/s,
>> One for 10 Gb/s, and two MDIO interrupts connected to MPIC.
>
> If there can be two interrupts you need to make that clear and specify
> the order.
>
> Is it possible for one MDIO controller to have an interrupt connected
> but not the other, on the same system? How would you represent that in
> the device tree? If there are two MDIO controllers why are they in the
> same node?
Historically (FMan v2 and even before/legacy) we've had each MAC include
an MDIO controller, but only one MDIO controller per MAC type/speed (1
Gb/s vs 10 Gb/s) is pinned out and all the same speed PHY(s) are
connected to the respective MDIO controllers. As such the first 1 Gb/s
MAC/MDIO controller is used to manage all the 1 Gb/s PHY(s) and the
first 10 Gb/s MAC/MDIO controller is used to manage all the 10 Gb/s
PHY(s). Each MDIO controller has the ability to generate interrupts but
only pinned out MDIO controllers are hooked up to the MPIC (as such the
talk about two interrupts)
(Each MAC has also integrated a SERDES/TBI/"internal" PHY that is
connected to the "local" MDIO controller)
As you can imagine this creates a number of problems in a partitioning
scenario (and not just, imagine RCWs where the first MAC is not
used/enabled). In order to help a bit (but not quite enough), in FMan
v3, two additional MDIO controllers (one for 1 the Gb/s PHY(s) and one
for 1 the 10 Gb/s PHY(s)) have been integrated that are not associated
with any MAC and these are the pinned out MDIO controllers on such
SoC(s) (chassis v2)
>> Does "optional" mean it's used if and
>>> only if external MDIO is used, or is it optional even with external MDIO? I see
>>> it's not present in the example -- do we not have a real example that has the
>>> interrupt?
>> [S.H] "optional" means it's available on hardware, but MDIO driver does not use interrupt.
>> So we don't have a real example.
>
> <record type="broken">The device tree describes the hardware, not the
> driver</record>
Anyway, only two MDIO nodes (out of 4 to 14) would have an interrupt
property describing exactly one interrupt. What language should we use
to convey this situation
Cheers,
^ permalink raw reply
* Re: [PATCH] [v3] power/fsl: add MDIO dt binding for FMan
From: Scott Wood @ 2015-01-07 18:05 UTC (permalink / raw)
To: Xie Shaohui-B21989
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Medve Emilian-EMMEDVE1
In-Reply-To: <DM2PR0301MB08645ABD93708FFE011EF362E2460@DM2PR0301MB0864.namprd03.prod.outlook.com>
On Tue, 2015-01-06 at 23:29 -0600, Xie Shaohui-B21989 wrote:
> > > > +- interrupts
> > > > + Usage: optional
> > > > + Value type: <prop-encoded-array>
> > > > + Definition: Event interrupt of external MDIO controller.
> > > > + 1 Gb/s MDIO and 10 Gb/s MDIO has one interrupt respectively.
> >
> > I'm confused by "respectively" here. Does fsl,fman-memac-mdio have two
> > interrupts (one for 1 Gb/s and one for 10 Gb/s)?
> [S.H] We use two MDIO controllers for external PHY management. One for 1 Gb/s,
> One for 10 Gb/s, and two MDIO interrupts connected to MPIC.
If there can be two interrupts you need to make that clear and specify
the order.
Is it possible for one MDIO controller to have an interrupt connected
but not the other, on the same system? How would you represent that in
the device tree? If there are two MDIO controllers why are they in the
same node?
> Does "optional" mean it's used if and
> > only if external MDIO is used, or is it optional even with external MDIO? I see
> > it's not present in the example -- do we not have a real example that has the
> > interrupt?
> [S.H] "optional" means it's available on hardware, but MDIO driver does not use interrupt.
> So we don't have a real example.
<record type="broken">The device tree describes the hardware, not the
driver</record>
-Scott
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info
From: Scott Wood @ 2015-01-07 17:59 UTC (permalink / raw)
To: Anton Blanchard
Cc: Alan Modra, Alexander Graf, ulrich.weigand, paulus, linuxppc-dev
In-Reply-To: <20150107161247.55591e93@kryten>
On Wed, 2015-01-07 at 16:12 +1100, Anton Blanchard wrote:
> Thanks for looking into this. Does that mean we were just getting lucky
> with the previous version:
>
> static inline struct thread_info *current_thread_info(void)
> {
> register unsigned long sp asm("r1");
>
> return (struct thread_info *)(sp & ~(THREAD_SIZE-1));
> }
>
> ie a static register asm instead of a global one. If so the safest fix
> for now might be to just eat the overead of a register move:
>
> static inline struct thread_info *current_thread_info(void)
> {
> unsigned long sp;
>
> asm("mr %0,1": "=r"(sp));
> return (struct thread_info *)(sp & ~(THREAD_SIZE-1));
You could avoid the register move by doing a rlwinm/rldicr in inline
asm, if it matters enough.
-Scott
^ permalink raw reply
* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Scott Wood @ 2015-01-07 17:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Purcareata Bogdan, linuxppc-dev, linux-kernel, Andreas Mohr
In-Reply-To: <1420641876.5830.32.camel@kernel.crashing.org>
On Wed, 2015-01-07 at 15:44 +0100, Benjamin Herrenschmidt wrote:
> On Mon, 2015-01-05 at 12:10 -0600, Scott Wood wrote:
> > It would have been nice if a code comment explained why it was doing the
> > readback... I don't see any particular need to wait for EOI completion
> > here (unlike when masking).
>
> The EOI is what causes the MPIC to drop it's EE output to the CPU, if the
> EOI is processed too slowly & asynchronously (posted write + 33Mhz MPIC)
> we observe cases of spurrious interrupts. We had some macs basically getting
> a spurrious irq for every MPIC interrupts...
Shouldn't reading INTACK be what causes the MPIC to drop its EE output?
-Scott
^ permalink raw reply
* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Benjamin Herrenschmidt @ 2015-01-07 14:44 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Purcareata Bogdan, linux-kernel, Andreas Mohr
In-Reply-To: <1420481454.4961.16.camel@freescale.com>
On Mon, 2015-01-05 at 12:10 -0600, Scott Wood wrote:
> It would have been nice if a code comment explained why it was doing the
> readback... I don't see any particular need to wait for EOI completion
> here (unlike when masking).
The EOI is what causes the MPIC to drop it's EE output to the CPU, if the
EOI is processed too slowly & asynchronously (posted write + 33Mhz MPIC)
we observe cases of spurrious interrupts. We had some macs basically getting
a spurrious irq for every MPIC interrupts...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 3/3] powerpc: Include linux/jump_label.h to get HAVE_JUMP_LABEL define
From: Jason Baron @ 2015-01-07 15:09 UTC (permalink / raw)
To: Anton Blanchard, benh@kernel.crashing.org, paulus@samba.org,
mpe@ellerman.id.au, mmarek@suse.cz, akpm@linux-foundation.org,
peterz@infradead.org, liuj97@gmail.com, mingo@kernel.org,
mgorman@suse.de, linux@arm.linux.org.uk, catalin.marinas@arm.com,
will.deacon@arm.com, ralf@linux-mips.org, schwidefsky@de.ibm.com,
heiko.carstens@de.ibm.com, davem@davemloft.net,
tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
rostedt@goodmis.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-kbuild@vger.kernel.org
In-Reply-To: <1420626958-31254-3-git-send-email-anton@samba.org>
On 01/07/2015 05:35 AM, Anton Blanchard wrote:
> Commit 1bc9e47aa8e4 ("powerpc/jump_label: Use HAVE_JUMP_LABEL")
> converted uses of CONFIG_JUMP_LABEL to HAVE_JUMP_LABEL in
> some assembly files.
>
> HAVE_JUMP_LABEL is defined in linux/jump_label.h, so we need to
> include this or we always get the non jump label fallback code.
>
> Fixes: 1bc9e47aa8e4 ("powerpc/jump_label: Use HAVE_JUMP_LABEL")
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> arch/powerpc/platforms/powernv/opal-wrappers.S | 2 +-
> arch/powerpc/platforms/pseries/hvCall.S | 2 +-
> arch/powerpc/platforms/pseries/lpar.c | 1 +
> 3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
> index 0509bca..fcbe899 100644
> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
> @@ -9,11 +9,11 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> +#include <linux/jump_label.h>
> #include <asm/ppc_asm.h>
> #include <asm/hvcall.h>
> #include <asm/asm-offsets.h>
> #include <asm/opal.h>
> -#include <asm/jump_label.h>
>
> .section ".text"
>
> diff --git a/arch/powerpc/platforms/pseries/hvCall.S b/arch/powerpc/platforms/pseries/hvCall.S
> index ccd53f9..74b5b8e 100644
> --- a/arch/powerpc/platforms/pseries/hvCall.S
> +++ b/arch/powerpc/platforms/pseries/hvCall.S
> @@ -7,12 +7,12 @@
> * as published by the Free Software Foundation; either version
> * 2 of the License, or (at your option) any later version.
> */
> +#include <linux/jump_label.h>
> #include <asm/hvcall.h>
> #include <asm/processor.h>
> #include <asm/ppc_asm.h>
> #include <asm/asm-offsets.h>
> #include <asm/ptrace.h>
> -#include <asm/jump_label.h>
>
> .section ".text"
>
> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> index 469751d..14ac1ad 100644
> --- a/arch/powerpc/platforms/pseries/lpar.c
> +++ b/arch/powerpc/platforms/pseries/lpar.c
> @@ -27,6 +27,7 @@
> #include <linux/console.h>
> #include <linux/export.h>
> #include <linux/static_key.h>
> +#include <linux/jump_label.h>
> #include <asm/processor.h>
> #include <asm/mmu.h>
> #include <asm/page.h>
You can drop the 'static_key.h' include here since its redundant.
Thanks,
-Jason
^ permalink raw reply
* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Benjamin Herrenschmidt @ 2015-01-07 14:43 UTC (permalink / raw)
To: Andreas Mohr; +Cc: linuxppc-dev, Purcareata Bogdan, linux-kernel
In-Reply-To: <20150105174616.GA3159@rhlx01.hs-esslingen.de>
On Mon, 2015-01-05 at 18:46 +0100, Andreas Mohr wrote:
> Hi,
>
> > I was curious why the mpic_cpu_read(MPIC_INFO(CPU_WHOAMI)) was there in
> > the first place and if it's still needed. If it's still required, I
> > guess a better approach is to eliminate the call only if the kernel is
> > running on the KVM guest side, where the MPIC is emulated and no longer
> > requires a readback.
>
> "Why not?"
>
> A mechanism being "emulated"/"virtual" or not
> may not necessarily be much of a distinction (if at all!).
> The readback might be required
> to properly fulfill all requirements
> of a full state change protocol specification,
> which might easily be the case for both RS(*) and virtual hardware.
> And especially for virtual hardware
> such a "readback" event
> might be an extremely important "end of transaction" marker
> which may often be needed for freeing of temporary resources etc.
In that case it was purely something we added after trial and error to
correct a problem, it's not specified as necessary. Basically it's about
making the store synchronous to the MPIC logic. It's definitely not
necessary on an emulated implementation.
> I'm talking out of my *ss without any MPIC specifics here
> (and especially not why the readback there actually is needed -
> if that doesn't happen to be the case for PCI Posting reasons or some such),
> but it's just intended as food for thought :)
>
> *) Real Silicon (rather than RL - Real Life)
>
> HTH,
>
> Andreas Mohr
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Benjamin Herrenschmidt @ 2015-01-07 14:40 UTC (permalink / raw)
To: Purcareata Bogdan; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <54AA9C4D.3000001@freescale.com>
On Mon, 2015-01-05 at 16:14 +0200, Purcareata Bogdan wrote:
> Hello,
>
> While doing some performance testing of a KVM guest on a PPC platform, I
> noticed that there's a read of the CPU_WHOAMI register after each MPIC
> EOI [1]. This has been present since the initial implementation of the
> MPIC driver [2]. In a KVM virtualized environment, this results in an
> additional kvm_exit.
>
> Is the read back necessary? Is it used to provide some sort of
> synchronization mechanism, making sure that nothing else is executed
> until the EOI write is finished? I eliminated the mpic_cpu_read call and
> run the kernel on hardware and noticed no anomaly, however I am not sure
> of all the implications and race conditions it might lead to.
It was done to ensure that the store to the EOI has reached the MPIC and
been fully processed before re-enabling interrupts on the CPU. On some
implementations, the MPIC runs quite slowly (significantly slower than
the core) and the stores to it are asynchronous, so we had situation
where we would restore interrupts while the MPIC hasn't yet de-asserted
the output line.
One way to work around the performance loss for you would be to add some
DT property to indicate to the guest that the read isn't necessary.
> I was curious why the mpic_cpu_read(MPIC_INFO(CPU_WHOAMI)) was there in
> the first place and if it's still needed. If it's still required, I
> guess a better approach is to eliminate the call only if the kernel is
> running on the KVM guest side, where the MPIC is emulated and no longer
> requires a readback.
>
> Thank you,
> Bogdan P.
>
> [1] http://lxr.free-electrons.com/source/arch/powerpc/sysdev/mpic.c#L659
> [2] https://lkml.org/lkml/2004/10/22/483
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 1/3] jump_label: Allow asm/jump_label.h to be included in assembly
From: Will Deacon @ 2015-01-07 11:30 UTC (permalink / raw)
To: Anton Blanchard
Cc: peterz@infradead.org, heiko.carstens@de.ibm.com, paulus@samba.org,
hpa@zytor.com, mingo@kernel.org, linux@arm.linux.org.uk,
mingo@redhat.com, mgorman@suse.de, Catalin Marinas,
liuj97@gmail.com, linux-kbuild@vger.kernel.org,
rostedt@goodmis.org, jbaron@akamai.com, tglx@linutronix.de,
mmarek@suse.cz, linux-kernel@vger.kernel.org, ralf@linux-mips.org,
schwidefsky@de.ibm.com, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, davem@davemloft.net
In-Reply-To: <1420626958-31254-1-git-send-email-anton@samba.org>
On Wed, Jan 07, 2015 at 10:35:56AM +0000, Anton Blanchard wrote:
> Wrap asm/jump_label.h for all archs with #ifndef __ASSEMBLY__.
> Since these are kernel only headers, we don't need #ifdef __KERNEL__
> so can simplify things a bit.
>
> If an architecture wants to use jump labels in assembly, it
> will still need to define a macro to create the __jump_table
> entries (see ARCH_STATIC_BRANCH in the powerpc asm/jump_label.h
> for an example).
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> arch/arm/include/asm/jump_label.h | 5 ++---
> arch/arm64/include/asm/jump_label.h | 8 ++++----
> arch/mips/include/asm/jump_label.h | 7 +++----
> arch/s390/include/asm/jump_label.h | 3 +++
> arch/sparc/include/asm/jump_label.h | 5 ++---
> arch/x86/include/asm/jump_label.h | 5 ++---
> 6 files changed, 16 insertions(+), 17 deletions(-)
[...]
> diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
> index 076a1c7..c0e5165 100644
> --- a/arch/arm64/include/asm/jump_label.h
> +++ b/arch/arm64/include/asm/jump_label.h
> @@ -18,11 +18,12 @@
> */
> #ifndef __ASM_JUMP_LABEL_H
> #define __ASM_JUMP_LABEL_H
> +
> +#ifndef __ASSEMBLY__
> +
> #include <linux/types.h>
> #include <asm/insn.h>
>
> -#ifdef __KERNEL__
> -
> #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
>
> static __always_inline bool arch_static_branch(struct static_key *key)
> @@ -39,8 +40,6 @@ l_yes:
> return true;
> }
>
> -#endif /* __KERNEL__ */
> -
> typedef u64 jump_label_t;
>
> struct jump_entry {
> @@ -49,4 +48,5 @@ struct jump_entry {
> jump_label_t key;
> };
>
> +#endif /* __ASSEMBLY__ */
> #endif /* __ASM_JUMP_LABEL_H */
The arm64 bit looks fine to me:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply
* [PATCH 3/3] powerpc: Include linux/jump_label.h to get HAVE_JUMP_LABEL define
From: Anton Blanchard @ 2015-01-07 10:35 UTC (permalink / raw)
To: benh, paulus, mpe, mmarek, akpm, jbaron, peterz, liuj97, mingo,
mgorman, linux, catalin.marinas, will.deacon, ralf, schwidefsky,
heiko.carstens, davem, tglx, mingo, hpa, rostedt
Cc: linuxppc-dev, linux-kernel, linux-kbuild
In-Reply-To: <1420626958-31254-1-git-send-email-anton@samba.org>
Commit 1bc9e47aa8e4 ("powerpc/jump_label: Use HAVE_JUMP_LABEL")
converted uses of CONFIG_JUMP_LABEL to HAVE_JUMP_LABEL in
some assembly files.
HAVE_JUMP_LABEL is defined in linux/jump_label.h, so we need to
include this or we always get the non jump label fallback code.
Fixes: 1bc9e47aa8e4 ("powerpc/jump_label: Use HAVE_JUMP_LABEL")
Signed-off-by: Anton Blanchard <anton@samba.org>
---
arch/powerpc/platforms/powernv/opal-wrappers.S | 2 +-
arch/powerpc/platforms/pseries/hvCall.S | 2 +-
arch/powerpc/platforms/pseries/lpar.c | 1 +
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 0509bca..fcbe899 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -9,11 +9,11 @@
* 2 of the License, or (at your option) any later version.
*/
+#include <linux/jump_label.h>
#include <asm/ppc_asm.h>
#include <asm/hvcall.h>
#include <asm/asm-offsets.h>
#include <asm/opal.h>
-#include <asm/jump_label.h>
.section ".text"
diff --git a/arch/powerpc/platforms/pseries/hvCall.S b/arch/powerpc/platforms/pseries/hvCall.S
index ccd53f9..74b5b8e 100644
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -7,12 +7,12 @@
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*/
+#include <linux/jump_label.h>
#include <asm/hvcall.h>
#include <asm/processor.h>
#include <asm/ppc_asm.h>
#include <asm/asm-offsets.h>
#include <asm/ptrace.h>
-#include <asm/jump_label.h>
.section ".text"
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 469751d..14ac1ad 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -27,6 +27,7 @@
#include <linux/console.h>
#include <linux/export.h>
#include <linux/static_key.h>
+#include <linux/jump_label.h>
#include <asm/processor.h>
#include <asm/mmu.h>
#include <asm/page.h>
--
2.1.0
^ permalink raw reply related
* [PATCH 2/3] jump_label: Allow jump labels to be used in assembly
From: Anton Blanchard @ 2015-01-07 10:35 UTC (permalink / raw)
To: benh, paulus, mpe, mmarek, akpm, jbaron, peterz, liuj97, mingo,
mgorman, linux, catalin.marinas, will.deacon, ralf, schwidefsky,
heiko.carstens, davem, tglx, mingo, hpa, rostedt
Cc: linuxppc-dev, linux-kernel, linux-kbuild
In-Reply-To: <1420626958-31254-1-git-send-email-anton@samba.org>
To use jump labels in assembly we need the HAVE_JUMP_LABEL define,
so we select a fallback version if the toolchain does not support
them.
Modify linux/jump_label.h so it can be included by assembly files.
We also need to add -DCC_HAVE_ASM_GOTO to KBUILD_AFLAGS.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Makefile | 1 +
include/linux/jump_label.h | 21 +++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index ef748e1..7b83c3f 100644
--- a/Makefile
+++ b/Makefile
@@ -774,6 +774,7 @@ KBUILD_ARFLAGS := $(call ar-option,D)
# check for 'asm goto'
ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC)), y)
KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
+ KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
endif
include $(srctree)/scripts/Makefile.extrawarn
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 98f923b6..f4de473 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -45,6 +45,12 @@
* same as using STATIC_KEY_INIT_FALSE.
*/
+#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
+# define HAVE_JUMP_LABEL
+#endif
+
+#ifndef __ASSEMBLY__
+
#include <linux/types.h>
#include <linux/compiler.h>
#include <linux/bug.h>
@@ -55,7 +61,7 @@ extern bool static_key_initialized;
"%s used before call to jump_label_init", \
__func__)
-#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
+#ifdef HAVE_JUMP_LABEL
struct static_key {
atomic_t enabled;
@@ -66,13 +72,18 @@ struct static_key {
#endif
};
-# include <asm/jump_label.h>
-# define HAVE_JUMP_LABEL
#else
struct static_key {
atomic_t enabled;
};
-#endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
+#endif /* HAVE_JUMP_LABEL */
+#endif /* __ASSEMBLY__ */
+
+#ifdef HAVE_JUMP_LABEL
+#include <asm/jump_label.h>
+#endif
+
+#ifndef __ASSEMBLY__
enum jump_label_type {
JUMP_LABEL_DISABLE = 0,
@@ -203,3 +214,5 @@ static inline bool static_key_enabled(struct static_key *key)
}
#endif /* _LINUX_JUMP_LABEL_H */
+
+#endif /* __ASSEMBLY__ */
--
2.1.0
^ permalink raw reply related
* [PATCH 1/3] jump_label: Allow asm/jump_label.h to be included in assembly
From: Anton Blanchard @ 2015-01-07 10:35 UTC (permalink / raw)
To: benh, paulus, mpe, mmarek, akpm, jbaron, peterz, liuj97, mingo,
mgorman, linux, catalin.marinas, will.deacon, ralf, schwidefsky,
heiko.carstens, davem, tglx, mingo, hpa, rostedt
Cc: linuxppc-dev, linux-kernel, linux-kbuild
Wrap asm/jump_label.h for all archs with #ifndef __ASSEMBLY__.
Since these are kernel only headers, we don't need #ifdef __KERNEL__
so can simplify things a bit.
If an architecture wants to use jump labels in assembly, it
will still need to define a macro to create the __jump_table
entries (see ARCH_STATIC_BRANCH in the powerpc asm/jump_label.h
for an example).
Signed-off-by: Anton Blanchard <anton@samba.org>
---
arch/arm/include/asm/jump_label.h | 5 ++---
arch/arm64/include/asm/jump_label.h | 8 ++++----
arch/mips/include/asm/jump_label.h | 7 +++----
arch/s390/include/asm/jump_label.h | 3 +++
arch/sparc/include/asm/jump_label.h | 5 ++---
arch/x86/include/asm/jump_label.h | 5 ++---
6 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h
index 70f9b9b..5f337dc 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -1,7 +1,7 @@
#ifndef _ASM_ARM_JUMP_LABEL_H
#define _ASM_ARM_JUMP_LABEL_H
-#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
#include <linux/types.h>
@@ -27,8 +27,6 @@ l_yes:
return true;
}
-#endif /* __KERNEL__ */
-
typedef u32 jump_label_t;
struct jump_entry {
@@ -37,4 +35,5 @@ struct jump_entry {
jump_label_t key;
};
+#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index 076a1c7..c0e5165 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -18,11 +18,12 @@
*/
#ifndef __ASM_JUMP_LABEL_H
#define __ASM_JUMP_LABEL_H
+
+#ifndef __ASSEMBLY__
+
#include <linux/types.h>
#include <asm/insn.h>
-#ifdef __KERNEL__
-
#define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE
static __always_inline bool arch_static_branch(struct static_key *key)
@@ -39,8 +40,6 @@ l_yes:
return true;
}
-#endif /* __KERNEL__ */
-
typedef u64 jump_label_t;
struct jump_entry {
@@ -49,4 +48,5 @@ struct jump_entry {
jump_label_t key;
};
+#endif /* __ASSEMBLY__ */
#endif /* __ASM_JUMP_LABEL_H */
diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
index fdbff44..608aa57 100644
--- a/arch/mips/include/asm/jump_label.h
+++ b/arch/mips/include/asm/jump_label.h
@@ -8,9 +8,9 @@
#ifndef _ASM_MIPS_JUMP_LABEL_H
#define _ASM_MIPS_JUMP_LABEL_H
-#include <linux/types.h>
+#ifndef __ASSEMBLY__
-#ifdef __KERNEL__
+#include <linux/types.h>
#define JUMP_LABEL_NOP_SIZE 4
@@ -39,8 +39,6 @@ l_yes:
return true;
}
-#endif /* __KERNEL__ */
-
#ifdef CONFIG_64BIT
typedef u64 jump_label_t;
#else
@@ -53,4 +51,5 @@ struct jump_entry {
jump_label_t key;
};
+#endif /* __ASSEMBLY__ */
#endif /* _ASM_MIPS_JUMP_LABEL_H */
diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
index 346b1c8..1e78ffd 100644
--- a/arch/s390/include/asm/jump_label.h
+++ b/arch/s390/include/asm/jump_label.h
@@ -1,6 +1,8 @@
#ifndef _ASM_S390_JUMP_LABEL_H
#define _ASM_S390_JUMP_LABEL_H
+#ifndef __ASSEMBLY__
+
#include <linux/types.h>
#define JUMP_LABEL_NOP_SIZE 6
@@ -34,4 +36,5 @@ struct jump_entry {
jump_label_t key;
};
+#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
index ec2e2e2..cc9b04a 100644
--- a/arch/sparc/include/asm/jump_label.h
+++ b/arch/sparc/include/asm/jump_label.h
@@ -1,7 +1,7 @@
#ifndef _ASM_SPARC_JUMP_LABEL_H
#define _ASM_SPARC_JUMP_LABEL_H
-#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
#include <linux/types.h>
@@ -22,8 +22,6 @@ l_yes:
return true;
}
-#endif /* __KERNEL__ */
-
typedef u32 jump_label_t;
struct jump_entry {
@@ -32,4 +30,5 @@ struct jump_entry {
jump_label_t key;
};
+#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 6a2cefb..a4c1cf7 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -1,7 +1,7 @@
#ifndef _ASM_X86_JUMP_LABEL_H
#define _ASM_X86_JUMP_LABEL_H
-#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
#include <linux/stringify.h>
#include <linux/types.h>
@@ -30,8 +30,6 @@ l_yes:
return true;
}
-#endif /* __KERNEL__ */
-
#ifdef CONFIG_X86_64
typedef u64 jump_label_t;
#else
@@ -44,4 +42,5 @@ struct jump_entry {
jump_label_t key;
};
+#endif /* __ASSEMBLY__ */
#endif
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 1/2] jump_label: Allow jump labels to be used in assembly
From: Anton Blanchard @ 2015-01-07 10:34 UTC (permalink / raw)
To: Steven Rostedt
Cc: mmarek, linux-kbuild, peterz, linux-kernel, jbaron, paulus,
mgorman, akpm, linuxppc-dev, mingo, liuj97
In-Reply-To: <20150106174822.GA29629@home.goodmis.org>
Hi Steve,
> Have you tested this on other archs? Because just looking at x86, it
> doesn't seem that asm/jump_label.h can handle being called in
> assembly.
Since no one is including linux/jump_label.h in assembly yet, nothing
should break. We could however add __ASSEMBLY__ protection to all the
asm/jump_label.h files. Patches on the way.
Anton
^ permalink raw reply
* [3.16.y-ckt stable] Patch "powerpc/powernv: Switch off MMU before entering nap/sleep/rvwinkle mode" has been added to staging queue
From: Luis Henriques @ 2015-01-07 10:32 UTC (permalink / raw)
To: Paul Mackerras
Cc: Luis Henriques, Shreyas B. Prabhu, kernel-team, linuxppc-dev
This is a note to let you know that I have just added a patch titled
powerpc/powernv: Switch off MMU before entering nap/sleep/rvwinkle mode
to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree
which can be found at:
http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.16.y-queue
This patch is scheduled to be released in version 3.16.7-ckt4.
If you, or anyone else, feels it should not be added to this tree, please
reply to this email.
For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
Thanks.
-Luis
------
>From 8a4ec5805ab7292f0c05d9b052aa343af57e7015 Mon Sep 17 00:00:00 2001
From: Paul Mackerras <paulus@samba.org>
Date: Wed, 10 Dec 2014 00:26:50 +0530
Subject: powerpc/powernv: Switch off MMU before entering nap/sleep/rvwinkle
mode
commit 8117ac6a6c2fa0f847ff6a21a1f32c8d2c8501d0 upstream.
Currently, when going idle, we set the flag indicating that we are in
nap mode (paca->kvm_hstate.hwthread_state) and then execute the nap
(or sleep or rvwinkle) instruction, all with the MMU on. This is bad
for two reasons: (a) the architecture specifies that those instructions
must be executed with the MMU off, and in fact with only the SF, HV, ME
and possibly RI bits set, and (b) this introduces a race, because as
soon as we set the flag, another thread can switch the MMU to a guest
context. If the race is lost, this thread will typically start looping
on relocation-on ISIs at 0xc...4400.
This fixes it by setting the MSR as required by the architecture before
setting the flag or executing the nap/sleep/rvwinkle instruction.
[ shreyas@linux.vnet.ibm.com: Edited to handle LE ]
Signed-off-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
arch/powerpc/include/asm/reg.h | 2 ++
arch/powerpc/kernel/idle_power7.S | 18 +++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index bffd89d27301..e73cdadab785 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -118,8 +118,10 @@
#define __MSR (MSR_ME | MSR_RI | MSR_IR | MSR_DR | MSR_ISF |MSR_HV)
#ifdef __BIG_ENDIAN__
#define MSR_ __MSR
+#define MSR_IDLE (MSR_ME | MSR_SF | MSR_HV)
#else
#define MSR_ (__MSR | MSR_LE)
+#define MSR_IDLE (MSR_ME | MSR_SF | MSR_HV | MSR_LE)
#endif
#define MSR_KERNEL (MSR_ | MSR_64BIT)
#define MSR_USER32 (MSR_ | MSR_PR | MSR_EE)
diff --git a/arch/powerpc/kernel/idle_power7.S b/arch/powerpc/kernel/idle_power7.S
index 5cf3d367190d..a29f5b0f9d3e 100644
--- a/arch/powerpc/kernel/idle_power7.S
+++ b/arch/powerpc/kernel/idle_power7.S
@@ -101,7 +101,23 @@ _GLOBAL(power7_powersave_common)
std r9,_MSR(r1)
std r1,PACAR1(r13)
-_GLOBAL(power7_enter_nap_mode)
+ /*
+ * Go to real mode to do the nap, as required by the architecture.
+ * Also, we need to be in real mode before setting hwthread_state,
+ * because as soon as we do that, another thread can switch
+ * the MMU context to the guest.
+ */
+ LOAD_REG_IMMEDIATE(r5, MSR_IDLE)
+ li r6, MSR_RI
+ andc r6, r9, r6
+ LOAD_REG_ADDR(r7, power7_enter_nap_mode)
+ mtmsrd r6, 1 /* clear RI before setting SRR0/1 */
+ mtspr SPRN_SRR0, r7
+ mtspr SPRN_SRR1, r5
+ rfid
+
+ .globl power7_enter_nap_mode
+power7_enter_nap_mode:
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
/* Tell KVM we're napping */
li r4,KVM_HWTHREAD_IN_NAP
--
2.1.4
^ permalink raw reply related
* offlining cpus breakage
From: Alexey Kardashevskiy @ 2015-01-07 9:37 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org, Shreyas B. Prabhu; +Cc: Paul Mackerras
Hi!
"ppc64_cpu --smt=off" produces multiple error on the latest upstream kernel
(sha1 bdec419):
NMI watchdog: BUG: soft lockup - CPU#20 stuck for 23s! [swapper/20:0]
or
INFO: rcu_sched detected stalls on CPUs/tasks: { 2 7 8 9 10 11 12 13 14 15
16 17 18 19 20 21 22 23 2
4 25 26 27 28 29 30 31} (detected by 6, t=2102 jiffies, g=1617, c=1616,
q=1441)
and many others, all about lockups
I did bisecting and found out that reverting these helps:
77b54e9f213f76a23736940cf94bcd765fc00f40 powernv/powerpc: Add winkle
support for offline cpus
7cba160ad789a3ad7e68b92bf20eaad6ed171f80 powernv/cpuidle: Redesign idle
states management
8eb8ac89a364305d05ad16be983b7890eb462cc3 powerpc/powernv: Enable Offline
CPUs to enter deep idle states
btw reverting just two of them produces a compile error.
It is pseries_le_defconfig, POWER8 machine:
timebase : 512000000
platform : PowerNV
model : palmetto
machine : PowerNV palmetto
firmware : OPAL v3
Please help to fix it. Thanks.
--
Alexey
^ permalink raw reply
* Re: [PATCH] powerpc/book3s: Fix the MCE code to use CONFIG_KVM_BOOK3S_HV_POSSIBLE
From: Aneesh Kumar K.V @ 2015-01-07 8:16 UTC (permalink / raw)
To: Mahesh J Salgaonkar, linuxppc-dev, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
In-Reply-To: <20150107044237.5803.85825.stgit@mars.in.ibm.com>
Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> commit id 9975f5e added new config variable CONFIG_KVM_BOOK3S_HV_POSSIBLE
> that helps to select the relevant code in the kernel when HV and PR
> bits are built as separate modules. As part of that commit, all the
> instances of #ifdef CONFIG_KVM_BOOK3S_64_HV was replaced with
> CONFIG_KVM_BOOK3S_HV_POSSIBLE. But the MCE code still depends on
> CONFIG_KVM_BOOK3S_64_HV which is wrong. When HV bits are built as a
> separate module the relevent MCE code gets excluded. This patch fixes
> the MCE code to use CONFIG_KVM_BOOK3S_HV_POSSIBLE.
>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/exceptions-64s.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index c2df815..7335857 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -1408,7 +1408,7 @@ machine_check_handle_early:
> bne 9f /* continue in V mode if we are. */
>
> 5:
> -#ifdef CONFIG_KVM_BOOK3S_64_HV
> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> /*
> * We are coming from kernel context. Check if we are coming from
> * guest. if yes, then we can continue. We will fall through
Do we also need to update the HSTATE_IN_GUEST check to avoid checking
the PR guest mode ?ie,
lbz r11,HSTATE_IN_GUEST(r13)
cmpwi r11,0 /* Check if coming from guest */
checking for 0 is what you want there ?
-aneesh
^ permalink raw reply
* Re: powerpc: refresh mpc85xx_defconfig using "make savedefconfig"
From: Alessio Igor Bogani @ 2015-01-07 8:08 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20141205094248.D5C9914011E@ozlabs.org>
Hi Michael,
On 5 December 2014 at 10:42, Michael Ellerman <mpe@ellerman.id.au> wrote:
> On Fri, 2014-05-12 at 08:17:42 UTC, Alessio Igor Bogani wrote:
>> Signed-off-by: Alessio Igor Bogani <alessio.bogani@elettra.eu>
>> ---
>> arch/powerpc/configs/mpc85xx_defconfig | 57 +++++++---------------------------
>> 1 file changed, 12 insertions(+), 45 deletions(-)
>
> Actually I was planning on doing this in bulk for all the powerpc defconfigs.
>
> Someone remind me to do it late next week once most of next has gone into
> Linus' tree.
Is there any news about this?
Thanks!
Ciao,
Alessio
^ permalink raw reply
* [PATCH] CXL: Fix device_node reference counting
From: Ian Munsie @ 2015-01-07 5:41 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, mikey, Aneesh Kumar K.V, linux-kernel, Ryan Grimm,
linuxppc-dev, anton, imunsie, jk
From: Ryan Grimm <grimm@linux.vnet.ibm.com>
When unbinding and rebinding the driver on a system with a card in PHB0, this
error condition is reached after a few attempts:
ERROR: Bad of_node_put() on /pciex@3fffe40000000
CPU: 0 PID: 3040 Comm: bash Not tainted 3.18.0-rc3-12545-g3627ffe #152
Call Trace:
[c000000721acb5c0] [c00000000086ef94] .dump_stack+0x84/0xb0 (unreliable)
[c000000721acb640] [c00000000073a0a8] .of_node_release+0xd8/0xe0
[c000000721acb6d0] [c00000000044bc44] .kobject_release+0x74/0xe0
[c000000721acb760] [c0000000007394fc] .of_node_put+0x1c/0x30
[c000000721acb7d0] [c000000000545cd8] .cxl_probe+0x1a98/0x1d50
[c000000721acb900] [c0000000004845a0] .local_pci_probe+0x40/0xc0
[c000000721acb980] [c000000000484998] .pci_device_probe+0x128/0x170
[c000000721acba30] [c00000000052400c] .driver_probe_device+0xac/0x2a0
[c000000721acbad0] [c000000000522468] .bind_store+0x108/0x160
[c000000721acbb70] [c000000000521448] .drv_attr_store+0x38/0x60
[c000000721acbbe0] [c000000000293840] .sysfs_kf_write+0x60/0xa0
[c000000721acbc50] [c000000000292500] .kernfs_fop_write+0x140/0x1d0
[c000000721acbcf0] [c000000000208648] .vfs_write+0xd8/0x260
[c000000721acbd90] [c000000000208b18] .SyS_write+0x58/0x100
[c000000721acbe30] [c000000000009258] syscall_exit+0x0/0x98
of_get_next_parent decrements parent's refcount and we need to call of_node_put
after the iteration. But, if while loop is not entered, of_node_put get called
on np without an of_node_get. So, call it before the while loop.
Signed-off-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
drivers/misc/cxl/pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 2ccd0a9..f801c28 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -319,6 +319,7 @@ static int init_implementation_adapter_regs(struct cxl *adapter, struct pci_dev
if (!(np = pnv_pci_to_phb_node(dev)))
return -ENODEV;
+ of_node_get(np);
while (np && !(prop = of_get_property(np, "ibm,chip-id", NULL)))
np = of_get_next_parent(np);
if (!np)
--
2.1.4
^ permalink raw reply related
* [PATCH] cxl: Fix issues when unmapping contexts
From: Ian Munsie @ 2015-01-07 5:33 UTC (permalink / raw)
To: mpe
Cc: cbe-oss-dev, Philippe Bergheaud, mikey, Aneesh Kumar K.V,
linux-kernel, linuxppc-dev, anton, imunsie, jk
From: Ian Munsie <imunsie@au1.ibm.com>
An issue was introduced with "cxl: Unmap MMIO regions when detaching a
context" (b123429e6a9e8d03aacf888d23262835f0081448) where closing a
context normally could also unmap the problem state area of other
contexts currently using the AFU.
It was also discovered that after a context's MMIO space had been
unmapped it would read 0s when accessing it, whereas the expected
behaviour was for the access to fail altogether.
In order to address these issues, this patch does two things:
- Forced mmap unmapping is only done when we are forcefully detaching
all contexts, and not in the normal detach path. Since the normal
context close path is tied to the file release any mmaps must have
already been released so we don't need to worry in that case.
- The mmap path now uses a vm_operations_struct with a fault handler.
The fault handler ensures that the context is in started state,
otherwise it fails the access attempt with a SIGBUS.
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
drivers/misc/cxl/context.c | 82 +++++++++++++++++++++++++++++++++++-----------
drivers/misc/cxl/file.c | 14 ++++----
2 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 51fd6b5..d1b55fe 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -100,6 +100,46 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master,
return 0;
}
+static int cxl_mmap_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct cxl_context *ctx = vma->vm_file->private_data;
+ unsigned long address = (unsigned long)vmf->virtual_address;
+ u64 area, offset;
+
+ offset = vmf->pgoff << PAGE_SHIFT;
+
+ pr_devel("%s: pe: %i address: 0x%lx offset: 0x%llx\n",
+ __func__, ctx->pe, address, offset);
+
+ if (ctx->afu->current_mode == CXL_MODE_DEDICATED) {
+ area = ctx->afu->psn_phys;
+ if (offset > ctx->afu->adapter->ps_size)
+ return VM_FAULT_SIGBUS;
+ } else {
+ area = ctx->psn_phys;
+ if (offset > ctx->psn_size)
+ return VM_FAULT_SIGBUS;
+ }
+
+ mutex_lock(&ctx->status_mutex);
+
+ if (ctx->status != STARTED) {
+ mutex_unlock(&ctx->status_mutex);
+ pr_devel("%s: Context not started, failing problem state access\n", __func__);
+ return VM_FAULT_SIGBUS;
+ }
+
+ vm_insert_pfn(vma, address, (area + offset) >> PAGE_SHIFT);
+
+ mutex_unlock(&ctx->status_mutex);
+
+ return VM_FAULT_NOPAGE;
+}
+
+static const struct vm_operations_struct cxl_mmap_vmops = {
+ .fault = cxl_mmap_fault,
+};
+
/*
* Map a per-context mmio space into the given vma.
*/
@@ -108,26 +148,25 @@ int cxl_context_iomap(struct cxl_context *ctx, struct vm_area_struct *vma)
u64 len = vma->vm_end - vma->vm_start;
len = min(len, ctx->psn_size);
- if (ctx->afu->current_mode == CXL_MODE_DEDICATED) {
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- return vm_iomap_memory(vma, ctx->afu->psn_phys, ctx->afu->adapter->ps_size);
- }
+ if (ctx->afu->current_mode != CXL_MODE_DEDICATED) {
+ /* make sure there is a valid per process space for this AFU */
+ if ((ctx->master && !ctx->afu->psa) || (!ctx->afu->pp_psa)) {
+ pr_devel("AFU doesn't support mmio space\n");
+ return -EINVAL;
+ }
- /* make sure there is a valid per process space for this AFU */
- if ((ctx->master && !ctx->afu->psa) || (!ctx->afu->pp_psa)) {
- pr_devel("AFU doesn't support mmio space\n");
- return -EINVAL;
+ /* Can't mmap until the AFU is enabled */
+ if (!ctx->afu->enabled)
+ return -EBUSY;
}
- /* Can't mmap until the AFU is enabled */
- if (!ctx->afu->enabled)
- return -EBUSY;
-
pr_devel("%s: mmio physical: %llx pe: %i master:%i\n", __func__,
ctx->psn_phys, ctx->pe , ctx->master);
+ vma->vm_flags |= VM_IO | VM_PFNMAP;
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- return vm_iomap_memory(vma, ctx->psn_phys, len);
+ vma->vm_ops = &cxl_mmap_vmops;
+ return 0;
}
/*
@@ -150,12 +189,6 @@ static void __detach_context(struct cxl_context *ctx)
afu_release_irqs(ctx);
flush_work(&ctx->fault_work); /* Only needed for dedicated process */
wake_up_all(&ctx->wq);
-
- /* Release Problem State Area mapping */
- mutex_lock(&ctx->mapping_lock);
- if (ctx->mapping)
- unmap_mapping_range(ctx->mapping, 0, 0, 1);
- mutex_unlock(&ctx->mapping_lock);
}
/*
@@ -184,6 +217,17 @@ void cxl_context_detach_all(struct cxl_afu *afu)
* created and torn down after the IDR removed
*/
__detach_context(ctx);
+
+ /*
+ * We are force detaching - remove any active PSA mappings so
+ * userspace cannot interfere with the card if it comes back.
+ * Easiest way to exercise this is to unbind and rebind the
+ * driver via sysfs while it is in use.
+ */
+ mutex_lock(&ctx->mapping_lock);
+ if (ctx->mapping)
+ unmap_mapping_range(ctx->mapping, 0, 0, 1);
+ mutex_unlock(&ctx->mapping_lock);
}
mutex_unlock(&afu->contexts_lock);
}
diff --git a/drivers/misc/cxl/file.c b/drivers/misc/cxl/file.c
index b09be44..4e85028 100644
--- a/drivers/misc/cxl/file.c
+++ b/drivers/misc/cxl/file.c
@@ -140,18 +140,20 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
pr_devel("%s: pe: %i\n", __func__, ctx->pe);
- mutex_lock(&ctx->status_mutex);
- if (ctx->status != OPENED) {
- rc = -EIO;
- goto out;
- }
-
+ /* Do this outside the status_mutex to avoid a circular dependency with
+ * the locking in cxl_mmap_fault() */
if (copy_from_user(&work, uwork,
sizeof(struct cxl_ioctl_start_work))) {
rc = -EFAULT;
goto out;
}
+ mutex_lock(&ctx->status_mutex);
+ if (ctx->status != OPENED) {
+ rc = -EIO;
+ goto out;
+ }
+
/*
* if any of the reserved fields are set or any of the unused
* flags are set it's invalid
--
2.1.4
^ permalink raw reply related
* RE: [PATCH] [v3] power/fsl: add MDIO dt binding for FMan
From: Shaohui Xie @ 2015-01-07 5:29 UTC (permalink / raw)
To: Scott Wood
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Emilian Medve
In-Reply-To: <1420590520.4961.38.camel@freescale.com>
PiA+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IHNoaC54aWVAZ21h
aWwuY29tIFttYWlsdG86c2hoLnhpZUBnbWFpbC5jb21dDQo+ID4gPiBTZW50OiBUdWVzZGF5LCBE
ZWNlbWJlciAyMywgMjAxNCAzOjU4IFBNDQo+ID4gPiBUbzogbGludXhwcGMtZGV2QGxpc3RzLm96
bGFicy5vcmc7IGRldmljZXRyZWVAdmdlci5rZXJuZWwub3JnOyBXb29kDQo+ID4gPiBTY290dC1C
MDc0MjENCj4gPiA+IENjOiBNZWR2ZSBFbWlsaWFuLUVNTUVEVkUxOyBYaWUgU2hhb2h1aS1CMjE5
ODkNCj4gPiA+IFN1YmplY3Q6IFtQQVRDSF0gW3YzXSBwb3dlci9mc2w6IGFkZCBNRElPIGR0IGJp
bmRpbmcgZm9yIEZNYW4NCj4gPiA+DQo+ID4gPiBGcm9tOiBTaGFvaHVpIFhpZSA8U2hhb2h1aS5Y
aWVAZnJlZXNjYWxlLmNvbT4NCj4gPiA+DQo+ID4gPiBUaGlzIGJpbmRpbmcgaXMgZm9yIEZNYW4g
TURJTywgaXQgY292ZXJzIEZNYW4gdjIgJiBGTWFuIHYzLg0KPiA+ID4NCj4gPiA+IFNpZ25lZC1v
ZmYtYnk6IFNoYW9odWkgWGllIDxTaGFvaHVpLlhpZUBmcmVlc2NhbGUuY29tPg0KPiA+ID4gLS0t
DQo+ID4gPiBjaGFuZ2VzIGluIHYzOg0KPiA+ID4gcmVwaHJhc2UgdGhlICdEZWZpbml0aW9uJyBv
ZiBwcm9wZXJ0eSAnYnVzLWZyZXF1ZW5jeScsIGRvbid0IGluY2x1ZGUNCj4gPiA+IHRoZSBwcm9w
ZXJ0eSBpbiBleGFtcGxlLg0KPiA+ID4NCj4gPiA+IGNoYW5nZXMgaW4gdjI6DQo+ID4gPiBhZGRy
ZXNzZWQgY29tbWVudHMgZnJvbSBTY290dCBpbiBWMS4NCj4gPiA+DQo+ID4gPiAgLi4uL2Rldmlj
ZXRyZWUvYmluZGluZ3MvcG93ZXJwYy9mc2wvZm1hbi50eHQgICAgICAgfCA2OQ0KPiArKysrKysr
KysrKysrKysrKysrKysrDQo+ID4gPiAgMSBmaWxlIGNoYW5nZWQsIDY5IGluc2VydGlvbnMoKykN
Cj4gPiA+DQo+ID4gPiBkaWZmIC0tZ2l0IGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRp
bmdzL3Bvd2VycGMvZnNsL2ZtYW4udHh0DQo+ID4gPiBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJl
ZS9iaW5kaW5ncy9wb3dlcnBjL2ZzbC9mbWFuLnR4dA0KPiA+ID4gaW5kZXggZWRlZWExNi4uNzM3
YWE0YyAxMDA2NDQNCj4gPiA+IC0tLSBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5n
cy9wb3dlcnBjL2ZzbC9mbWFuLnR4dA0KPiA+ID4gKysrIGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0
cmVlL2JpbmRpbmdzL3Bvd2VycGMvZnNsL2ZtYW4udHh0DQo+ID4gPiBAQCAtNyw2ICs3LDcgQEAg
Q09OVEVOVFMNCj4gPiA+ICAgIC0gRk1hbiBNVVJBTSBOb2RlDQo+ID4gPiAgICAtIEZNYW4gZFRT
RUMvWEdFQy9tRU1BQyBOb2RlDQo+ID4gPiAgICAtIEZNYW4gSUVFRSAxNTg4IE5vZGUNCj4gPiA+
ICsgIC0gRk1hbiBNRElPIE5vZGUNCj4gPiA+ICAgIC0gRXhhbXBsZQ0KPiA+ID4NCj4gPiA+DQo+
ID4gPiA9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PQ0KPiA+ID4gPT09PT09PT09IEBAIC0zNTcsNiArMzU4LDY5IEBAIHB0
cC10aW1lckBmZTAwMCB7ICB9Ow0KPiA+ID4NCj4gPiA+DQo+ID4gPiA9PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KPiA+
ID4gPT09PT09PT09DQo+ID4gPiArRk1hbiBNRElPIE5vZGUNCj4gPiA+ICsNCj4gPiA+ICtERVND
UklQVElPTg0KPiA+ID4gKw0KPiA+ID4gK1RoZSBNRElPIGlzIGEgYnVzIHRvIHdoaWNoIHRoZSBQ
SFkgZGV2aWNlcyBhcmUgY29ubmVjdGVkLg0KPiA+ID4gKw0KPiA+ID4gK1BST1BFUlRJRVMNCj4g
PiA+ICsNCj4gPiA+ICstIGNvbXBhdGlibGUNCj4gPiA+ICsJCVVzYWdlOiByZXF1aXJlZA0KPiA+
ID4gKwkJVmFsdWUgdHlwZTogPHN0cmluZ2xpc3Q+DQo+ID4gPiArCQlEZWZpbml0aW9uOiBBIHN0
YW5kYXJkIHByb3BlcnR5Lg0KPiA+ID4gKwkJTXVzdCBpbmNsdWRlICJmc2wsZm1hbi1tZGlvIiBm
b3IgMSBHYi9zIE1ESU8gZnJvbSBGTWFuIHYyLg0KPiA+ID4gKwkJTXVzdCBpbmNsdWRlICJmc2ws
Zm1hbi14bWRpbyIgZm9yIDEwIEdiL3MgTURJTyBmcm9tIEZNYW4gdjIuDQo+ID4gPiArCQlNdXN0
IGluY2x1ZGUgImZzbCxmbWFuLW1lbWFjLW1kaW8iIGZvciAxLzEwIEdiL3MgTURJTyBmcm9tDQo+
ID4gPiArCQlGTWFuIHYzLg0KPiA+ID4gKw0KPiA+ID4gKy0gcmVnDQo+ID4gPiArCQlVc2FnZTog
cmVxdWlyZWQNCj4gPiA+ICsJCVZhbHVlIHR5cGU6IDxwcm9wLWVuY29kZWQtYXJyYXk+DQo+ID4g
PiArCQlEZWZpbml0aW9uOiBBIHN0YW5kYXJkIHByb3BlcnR5Lg0KPiA+ID4gKw0KPiA+ID4gKy0g
YnVzLWZyZXF1ZW5jeQ0KPiA+ID4gKwkJVXNhZ2U6IG9wdGlvbmFsDQo+ID4gPiArCQlWYWx1ZSB0
eXBlOiA8dTMyPg0KPiA+ID4gKwkJRGVmaW5pdGlvbjogU3BlY2lmaWVzIHRoZSBleHRlcm5hbCBN
RElPIGJ1cyBjbG9jayBzcGVlZCB0bw0KPiA+ID4gKwkJYmUgdXNlZCwgaWYgZGlmZmVyZW50IGZy
b20gdGhlIHN0YW5kYXJkIDIuNSBNSHouDQo+ID4gPiArCQlUaGlzIG1heSBiZSBkdWUgdG8gdGhl
IHN0YW5kYXJkIHNwZWVkIGJlaW5nIHVuc3VwcG9ydGVkIChlLmcuDQo+ID4gPiArCQlkdWUgdG8g
YSBoYXJkd2FyZSBwcm9ibGVtKSwgb3IgdG8gYWR2ZXJ0aXNlIHRoYXQgYWxsIHJlbGV2YW50DQo+
ID4gPiArCQljb21wb25lbnRzIGluIHRoZSBzeXN0ZW0gc3VwcG9ydCBhIGZhc3RlciBzcGVlZC4N
Cj4gPiA+ICsNCj4gPiA+ICstIGludGVycnVwdHMNCj4gPiA+ICsJCVVzYWdlOiBvcHRpb25hbA0K
PiA+ID4gKwkJVmFsdWUgdHlwZTogPHByb3AtZW5jb2RlZC1hcnJheT4NCj4gPiA+ICsJCURlZmlu
aXRpb246IEV2ZW50IGludGVycnVwdCBvZiBleHRlcm5hbCBNRElPIGNvbnRyb2xsZXIuDQo+ID4g
PiArCQkxIEdiL3MgTURJTyBhbmQgMTAgR2IvcyBNRElPIGhhcyBvbmUgaW50ZXJydXB0IHJlc3Bl
Y3RpdmVseS4NCj4gDQo+IEknbSBjb25mdXNlZCBieSAicmVzcGVjdGl2ZWx5IiBoZXJlLiAgRG9l
cyBmc2wsZm1hbi1tZW1hYy1tZGlvIGhhdmUgdHdvDQo+IGludGVycnVwdHMgKG9uZSBmb3IgMSBH
Yi9zIGFuZCBvbmUgZm9yIDEwIEdiL3MpPw0KW1MuSF0gV2UgdXNlIHR3byBNRElPIGNvbnRyb2xs
ZXJzIGZvciBleHRlcm5hbCBQSFkgbWFuYWdlbWVudC4gT25lIGZvciAxIEdiL3MsDQpPbmUgZm9y
IDEwIEdiL3MsIGFuZCB0d28gTURJTyBpbnRlcnJ1cHRzIGNvbm5lY3RlZCB0byBNUElDLg0KDQog
IElmIHRoZXJlJ3Mgb25seSAoYXQNCj4gbW9zdCkgb25lIGludGVycnVwdCBwZXIgTURJTyBjb250
cm9sbGVyIHRoZW4ganVzdCBkZWxldGUgdGhpcyBsaW5lLg0KPiANCj4gV2hhdCBpbnRlcnJ1cHQg
aXMgdXNlZCBmb3IgaW50ZXJuYWwgTURJTz8NCltTLkhdIGludGVybmFsIE1ESU8gaGFzIG5vIGlu
dGVycnVwdC4NCg0KICBEb2VzICJvcHRpb25hbCIgbWVhbiBpdCdzIHVzZWQgaWYgYW5kDQo+IG9u
bHkgaWYgZXh0ZXJuYWwgTURJTyBpcyB1c2VkLCBvciBpcyBpdCBvcHRpb25hbCBldmVuIHdpdGgg
ZXh0ZXJuYWwgTURJTz8gIEkgc2VlDQo+IGl0J3Mgbm90IHByZXNlbnQgaW4gdGhlIGV4YW1wbGUg
LS0gZG8gd2Ugbm90IGhhdmUgYSByZWFsIGV4YW1wbGUgdGhhdCBoYXMgdGhlDQo+IGludGVycnVw
dD8NCltTLkhdICJvcHRpb25hbCIgbWVhbnMgaXQncyBhdmFpbGFibGUgb24gaGFyZHdhcmUsIGJ1
dCBNRElPIGRyaXZlciBkb2VzIG5vdCB1c2UgaW50ZXJydXB0LiANClNvIHdlIGRvbid0IGhhdmUg
YSByZWFsIGV4YW1wbGUuDQoNClRoYW5rcyENClNoYW9odWkNCg==
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info
From: Anton Blanchard @ 2015-01-07 5:12 UTC (permalink / raw)
To: Alan Modra
Cc: Alexander Graf, ulrich.weigand, paulus, Scott Wood, linuxppc-dev
In-Reply-To: <20141231122453.GH5183@bubble.grove.modra.org>
Hi Alan,
> Right. This is really an rs6000 backend bug. We describe one of the
> indirect calls that go wrong here as
>
> (call_insn 108 107 109 13 (parallel [
> (set (reg:DI 3 3)
> (call (mem:SI (reg:DI 288) [0 *_67 S4 A8])
> (const_int 64 [0x40])))
> (use (mem:DI (plus:DI (reg/f:DI 287 [ ops_44(D)->update ])
> (const_int 8 [0x8])) [0 S8 A8]))
> (set (reg:DI 2 2)
> (mem/v/c:DI (plus:DI (reg/f:DI 1 1)
> (const_int 40 [0x28])) [0 S8 A8]))
> (clobber (reg:DI 65 lr))
> ]) net/core/skbuff.c:2085 680 {*call_value_indirect_aixdi}
> <notes and arg uses omitted for clarity>
> )
>
> Notice that the RTL contains a "parallel". As you might guess, gcc
> treats the vector of expressions inside the square brackets of the
> parallel as happening "in parallel". Meaning that as far as gcc is
> concerned the toc restore part (third element) happens at the same
> time as the call (first element). So if gcc replaces (reg:DI 1) in
> the toc restore with some other register known to have the same value
> *before* the call, gcc's RTL analysis will conclude that such a
> replacement is valid.
Thanks for looking into this. Does that mean we were just getting lucky
with the previous version:
static inline struct thread_info *current_thread_info(void)
{
register unsigned long sp asm("r1");
return (struct thread_info *)(sp & ~(THREAD_SIZE-1));
}
ie a static register asm instead of a global one. If so the safest fix
for now might be to just eat the overead of a register move:
static inline struct thread_info *current_thread_info(void)
{
unsigned long sp;
asm("mr %0,1": "=r"(sp));
return (struct thread_info *)(sp & ~(THREAD_SIZE-1));
}
Anton
^ permalink raw reply
* [PATCH] powerpc/book3s: Fix the MCE code to use CONFIG_KVM_BOOK3S_HV_POSSIBLE
From: Mahesh J Salgaonkar @ 2015-01-07 4:43 UTC (permalink / raw)
To: linuxppc-dev, Benjamin Herrenschmidt, Aneesh Kumar K.V,
Paul Mackerras, Michael Ellerman
From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
commit id 9975f5e added new config variable CONFIG_KVM_BOOK3S_HV_POSSIBLE
that helps to select the relevant code in the kernel when HV and PR
bits are built as separate modules. As part of that commit, all the
instances of #ifdef CONFIG_KVM_BOOK3S_64_HV was replaced with
CONFIG_KVM_BOOK3S_HV_POSSIBLE. But the MCE code still depends on
CONFIG_KVM_BOOK3S_64_HV which is wrong. When HV bits are built as a
separate module the relevent MCE code gets excluded. This patch fixes
the MCE code to use CONFIG_KVM_BOOK3S_HV_POSSIBLE.
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
arch/powerpc/kernel/exceptions-64s.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index c2df815..7335857 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1408,7 +1408,7 @@ machine_check_handle_early:
bne 9f /* continue in V mode if we are. */
5:
-#ifdef CONFIG_KVM_BOOK3S_64_HV
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
/*
* We are coming from kernel context. Check if we are coming from
* guest. if yes, then we can continue. We will fall through
^ permalink raw reply related
* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Scott Wood @ 2015-01-07 2:56 UTC (permalink / raw)
To: Andreas Mohr; +Cc: linuxppc-dev, Purcareata Bogdan, linux-kernel
In-Reply-To: <20150105184322.GA30255@rhlx01.hs-esslingen.de>
On Mon, 2015-01-05 at 19:43 +0100, Andreas Mohr wrote:
> [CC related ppl]
>
> On Mon, Jan 05, 2015 at 12:10:54PM -0600, Scott Wood wrote:
> > On Mon, 2015-01-05 at 18:46 +0100, Andreas Mohr wrote:
> > > Hi,
> > >
> > > > I was curious why the mpic_cpu_read(MPIC_INFO(CPU_WHOAMI)) was there in
> > > > the first place and if it's still needed. If it's still required, I
> > > > guess a better approach is to eliminate the call only if the kernel is
> > > > running on the KVM guest side, where the MPIC is emulated and no longer
> > > > requires a readback.
> > >
> > > "Why not?"
> > >
> > > A mechanism being "emulated"/"virtual" or not
> > > may not necessarily be much of a distinction (if at all!).
> > > The readback might be required
> > > to properly fulfill all requirements
> > > of a full state change protocol specification,
> > > which might easily be the case for both RS(*) and virtual hardware.
> > > And especially for virtual hardware
> > > such a "readback" event
> > > might be an extremely important "end of transaction" marker
> > > which may often be needed for freeing of temporary resources etc.
> >
> > I'm not convinced that it's required in real silicon (though there are
> > many MPIC implementations which have their own quirks...), and I'm 100%
> > sure that it's not required in the QEMU/KVM implementation of MPIC.
> >
> > It would have been nice if a code comment explained why it was doing the
> > readback... I don't see any particular need to wait for EOI completion
> > here (unlike when masking).
>
> Hmm, yeah.
>
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git
> git checkout v2.6.11
> git blame ./ppc64/kernel/mpic.c
> git show 378193eb
> [PATCH] ppc64: Rewrite the openpic driver
>
> +/* Send an EOI */
> +static inline void mpic_eoi(struct mpic *mpic)
> +{
> + mpic_cpu_write(MPIC_CPU_EOI, 0);
> + (void)mpic_cpu_read(MPIC_CPU_WHOAMI);
> +}
>
>
> -static void openpic_eoi(void)
> -{
> - DECL_THIS_CPU;
> -
> - CHECK_THIS_CPU;
> - openpic_write(&OpenPIC->THIS_CPU.EOI, 0);
> - /* Handle PCI write posting */
> - (void)openpic_read(&OpenPIC->THIS_CPU.EOI);
> -}
>
>
> So, this does seem to be about PCI posted writes after all.
> Which begs the question whether all PIC hardware is connected via PCI bus,
> which... is not the case for emulated hardware at least, I'd think.
>
> And it's somewhat unfortunate
> that the comment in fact was removed in that commit
> (perhaps reinstate this comment in all of the various mpic.c life forms?).
But even for PCI, why do we need to wait for this write to complete?
Back in the arch/ppc days ppc_md.get_irq() was called in a loop, so it
would make sense that we'd need the next IRQ to be ready by the time
it's called again, but now that there's no get_irq() loop we shouldn't
need to wait.
-Scott
^ 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