LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] tty: 8250: Add 64byte UART support for FSL platforms
From: Vijay Rai @ 2015-01-05 15:44 UTC (permalink / raw)
  To: gregkh, linux-serial, linux-kernel, linuxppc-dev
  Cc: Vijay Rai, Poonam Aggrwal, Priyanka Jain

Some of FSL SoCs like T1040 has new version of UART controller which
can support 64byte FiFo.
To enable 64 byte support, following needs to be done:
-FCR[EN64] needs to be programmed to 1 to enable it.
-Also, when FCR[EN64]==1, RTL bits to be used as below
to define various Receive Trigger Levels:
        -FCR[RTL] = 00  1 byte
        -FCR[RTL] = 01  16 bytes
        -FCR[RTL] = 10  32 bytes
        -FCR[RTL] = 11  56 bytes
-tx_loadsz is set to 63-bytes instead of 64-bytes to implement
 workaround of errata A-008006 which states that tx_loadsz should
 be configured less than Maximum supported fifo bytes

Signed-off-by: Vijay Rai <vijay.rai@freescale.com>
Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
---
Changes from V1:
- tx_loadsz is set to 63-bytes now earlier it was set to 36
- Added comment mentioning the erratum A-008006
- Updated PORT_MAX_8250 to 30

 drivers/tty/serial/8250/8250_core.c |   23 ++++++++++++++++++++++-
 include/uapi/linux/serial_core.h    |    3 ++-
 include/uapi/linux/serial_reg.h     |    3 ++-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 11c6685..221e047 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -329,6 +329,17 @@ static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
 	},
+/* tx_loadsz is set to 63-bytes instead of 64-bytes to implement
+workaround of errata A-008006 which states that tx_loadsz should  be
+configured less than Maximum supported fifo bytes */
+	[PORT_16550A_FSL64] = {
+		.name		= "16550A_FSL64",
+		.fifo_size	= 64,
+		.tx_loadsz	= 63,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10 |
+				  UART_FCR7_64BYTE,
+		.flags		= UART_CAP_FIFO,
+	},
 };
 
 /* Uart divisor latch read */
@@ -956,7 +967,17 @@ static void autoconfig_16550a(struct uart_8250_port *up)
 			up->port.type = PORT_16650;
 			up->capabilities |= UART_CAP_EFR | UART_CAP_SLEEP;
 		} else {
-			DEBUG_AUTOCONF("Motorola 8xxx DUART ");
+			serial_out(up, UART_LCR, 0);
+			serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO |
+				   UART_FCR7_64BYTE);
+			status1 = serial_in(up, UART_IIR) >> 5;
+			serial_out(up, UART_FCR, 0);
+			serial_out(up, UART_LCR, 0);
+
+			if (status1 == 7)
+				up->port.type = PORT_16550A_FSL64;
+			else
+				DEBUG_AUTOCONF("Motorola 8xxx DUART ");
 		}
 		serial_out(up, UART_EFR, 0);
 		return;
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index c172180..263918b 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -55,7 +55,8 @@
 #define PORT_ALTR_16550_F64 27	/* Altera 16550 UART with 64 FIFOs */
 #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
 #define PORT_RT2880	29	/* Ralink RT2880 internal UART */
-#define PORT_MAX_8250	29	/* max port ID */
+#define PORT_16550A_FSL64 30	/* Freescale 16550 UART with 64 FIFOs */
+#define PORT_MAX_8250	30	/* max port ID */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 53af3b7..00adb01 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -86,7 +86,8 @@
 #define UART_FCR6_T_TRIGGER_8	0x10 /* Mask for transmit trigger set at 8 */
 #define UART_FCR6_T_TRIGGER_24  0x20 /* Mask for transmit trigger set at 24 */
 #define UART_FCR6_T_TRIGGER_30	0x30 /* Mask for transmit trigger set at 30 */
-#define UART_FCR7_64BYTE	0x20 /* Go into 64 byte mode (TI16C750) */
+#define UART_FCR7_64BYTE	0x20 /* Go into 64 byte mode (TI16C750 and
+					some Freescale UARTs) */
 
 #define UART_FCR_R_TRIG_SHIFT		6
 #define UART_FCR_R_TRIG_BITS(x)		\
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"
From: Dan Carpenter @ 2015-01-05 13:58 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: alsa-devel, Russell King, linux-parisc, Takashi Iwai,
	kernel-janitors, Clemens Ladisch, LKML, Jaroslav Kysela,
	Julia Lawall, Thibaut Varene, Johannes Berg, linuxppc-dev
In-Reply-To: <54A941C1.4020906@users.sourceforge.net>

On Sun, Jan 04, 2015 at 02:36:01PM +0100, SF Markus Elfring wrote:
>  	/* unmap PCI memory space, mapped during device init. */
> -	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) {
> -		if (pci.ap_mem_base[idx])
> -			iounmap(pci.ap_mem_base[idx]);
> -	}
> +	for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx)
> +		iounmap(pci.ap_mem_base[idx]);
>  

Don't do the gratuitous idx++ to ++idx changes.  You do it a couple
other places as well.  It belongs in a separate patch if you really feel
it is worth doing.  (It is not a clean up and it is not worth doing).

regards,
dan carpenter

^ permalink raw reply

* [PATCH] powerpc/8xx: reduce pressure on TLB due to context switches
From: Christophe Leroy @ 2015-01-05 14:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	scottwood
  Cc: linuxppc-dev, linux-kernel

For nohash powerpc, when we run out of contexts, contexts are freed by stealing
used contexts in-turn. When a victim has been selected, the associated TLB
entries are freed using _tlbil_pid(). Unfortunatly, on the PPC 8xx, _tlbil_pid()
does a tlbia, hence flushes ALL TLB entries and not only the one linked to the
stolen context. Therefore, as implented today, at each task switch requiring a
new context, all entries are flushed.

This patch modifies the implementation so that when running out of contexts, all
contexts get freed at once, hence dividing the number of calls to tlbia by 16.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
 arch/powerpc/mm/mmu_context_nohash.c | 44 +++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index 928ebe7..c648677 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -52,12 +52,15 @@
 #include <asm/mmu_context.h>
 #include <asm/tlbflush.h>
 
+#include "mmu_decl.h"
+
 static unsigned int first_context, last_context;
 static unsigned int next_context, nr_free_contexts;
 static unsigned long *context_map;
 static unsigned long *stale_map[NR_CPUS];
 static struct mm_struct **context_mm;
 static DEFINE_RAW_SPINLOCK(context_lock);
+static bool no_selective_tlbil;
 
 #define CTX_MAP_SIZE	\
 	(sizeof(unsigned long) * (last_context / BITS_PER_LONG + 1))
@@ -133,6 +136,39 @@ static unsigned int steal_context_smp(unsigned int id)
 }
 #endif  /* CONFIG_SMP */
 
+static unsigned int steal_all_contexts(void)
+{
+	struct mm_struct *mm;
+	int cpu = smp_processor_id();
+	unsigned int id;
+
+	for (id = first_context; id <= last_context; id++) {
+		/* Pick up the victim mm */
+		mm = context_mm[id];
+
+		pr_hardcont(" | steal %d from 0x%p", id, mm);
+
+		/* Mark this mm as having no context anymore */
+		mm->context.id = MMU_NO_CONTEXT;
+		if (id != first_context) {
+			context_mm[id] = NULL;
+			__clear_bit(id, context_map);
+#ifdef DEBUG_MAP_CONSISTENCY
+			mm->context.active = 0;
+#endif
+		}
+		__clear_bit(id, stale_map[cpu]);
+
+	}
+
+	/* Flush the TLB for all contexts */
+	_tlbil_all();
+
+	nr_free_contexts = last_context - first_context;
+
+	return first_context;
+}
+
 /* Note that this will also be called on SMP if all other CPUs are
  * offlined, which means that it may be called for cpu != 0. For
  * this to work, we somewhat assume that CPUs that are onlined
@@ -241,7 +277,10 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
 			goto stolen;
 		}
 #endif /* CONFIG_SMP */
-		id = steal_context_up(id);
+		if (no_selective_tlbil)
+			id = steal_all_contexts();
+		else
+			id = steal_context_up(id);
 		goto stolen;
 	}
 	nr_free_contexts--;
@@ -407,12 +446,15 @@ void __init mmu_context_init(void)
 	if (mmu_has_feature(MMU_FTR_TYPE_8xx)) {
 		first_context = 0;
 		last_context = 15;
+		no_selective_tlbil = true;
 	} else if (mmu_has_feature(MMU_FTR_TYPE_47x)) {
 		first_context = 1;
 		last_context = 65535;
+		no_selective_tlbil = false;
 	} else {
 		first_context = 1;
 		last_context = 255;
+		no_selective_tlbil = false;
 	}
 
 #ifdef DEBUG_CLAMP_LAST_CONTEXT
-- 
2.1.0

^ permalink raw reply related

* RE: PROBLEM: USB isochronous urb leak on EHCI driver
From: Michael Tessier @ 2015-01-05 15:12 UTC (permalink / raw)
  To: Alan Stern; +Cc: linuxppc-dev@lists.ozlabs.org, linux-usb@vger.kernel.org
In-Reply-To: <Pine.LNX.4.44L0.1412171119590.2947-100000@netrider.rowland.org>

>
> On Mon, 15 Dec 2014, Michael Tessier wrote:
>
> > Hi,
> >=20
> > I am dealing with a USB EHCI driver bug. Here is the info:
> >=20
> > My configuration:
> > -----------------
> >=20
> > Host: Freescale i.MX512 with ARM Cortex A8 (USB 2.0 host controller)=20
> > Linux kernel: 2.6.31, using EHCI USB driver
>
> As mentioned by other people, the age of that kernel makes any bug report=
 completely irrelevant.  It's hard to count the number of non-trivial chang=
es that have  > been made to the isochronous code in ehci-hcd since 2.6.31,=
 but there have been quite a few.
>
> > Hub: 4-PORT USB 1.1 HUB (Texas Instruments PN: tusb2046b)
> > Devices: 4 USB 1.1 audio codecs (Texas Instruments PN: pcm2901)
> >=20
> > Note: each codec is being used in R/W access, so with 4 codecs, I have
> > 4 playback and 4 capture streams.
> >=20
> > My problem:
> > -----------
> >=20
> > I have usb urb leaks when connecting more than 1 codec to the USB 1.1=20
> > Hub.
>
> What do you mean by "urb leak"?  Normally, people use the word "leak" =20
> to refer to memory that is dynamically allocated and never deallocated, b=
ut you seem to mean something else.

You are right. What I mean by leak is the following: At application level,
all my calls to "Read" or "Write" operation to the codec driver will return
with the correct amount of bytes read/written, with a "choppy" sound. Then
when looking at lower levels:

snd_pcm_oss_write (pcm_oss.c) 	-> OK
snd_pcm_lib_write (pcm_lib.c) 	-> OK
usb_submit_urb 	(urb.c)		-> FAIL with 3 codecs

The "FAIL" here indicates that the total amount of bytes transferred does
not correspond to what was expected. And indeed the sound is "choppy" when
using more than a certain amount of bandwidth. However this amount of
bandwidth is higher when connecting only 1 codec with different settings
(48khz-stereo 16-bits instead of 32 khz-mono 16-bits).So at some point it
looks like the bug is in the scheduler, only with several isochronous links=
.

>
> > (the result is that some of the audio data is not transferred, part of=
=20
> > the sound is simply missing) No problem when using only 1 of the 4=20
> > codecs connected to the hub; When I connect a second codec, the sound=20
> > quality starts to degrade. With 3 codecs, we just cannot recognize a=20
> > speach.
> >=20
> > Tests and observations:
> > -----------------------
> >=20
> > Since I have 3 usb ports available on the i.MX512, I tried to connect
> > 3 codecs directly on USB ports: the sound is perfect on each of the=20
> > three ports.
> >=20
> > I bought a consumer USB 2.0 Hub: no problem when using 3 codecs=20
> > connected to that Hub, however, the audio will completly stop on all=20
> > channels when connecting the 4th codec.
>
> Above you said the sound started to degrade when the second codec was con=
nected; here you say there is no problem when using 3 of them. =20
> Which is it?  Do you mean that the high-speed hub works better than the f=
ull-speed hub?
>
Yes, that's it. Using the high-speed hub will allow for more data throughpu=
t
before starting to "miss" some usb packets (and result in a choppy sound).

> > I checked the communication between the Hub (USB 1.1) and the Host=20
> > controller (USB 2.0) with a scope and concluded that the communication=
=20
> > speed is 1.5 MBytes/s has expected (so the communication is downgraded=
=20
> > to USB 1.1, since codecs and hub are USB
> > 1.1 devices).
> >=20
> > Also, I know that there is physically enough bandwidth to transfer the=
=20
> > data for two reasons:
> > 1) I have an older CPU with a USB 1.1 host controller (using the OHCI=20
> > driver), using the same hub and the same codecs: works like a champ,=20
> > using less than 50% of the available bandwidth (observed with a
> > scope)
> > 2) 1 audio stream is 32khz-mono, 16 bits =3D 64 kB/s,
> > 4 codecs =3D 8 streams(R/W) x 64 kB/s =3D 512 kB/s (out of 1.5MB/s)
>
> The amount of bandwidth available is usually not as much of an issue as t=
he ability of the scheduling alogorithm to divide the bandwidth among the s=
treams.  The
> algorithm is not very smart and it often runs into a wall even when lots =
of physical bandwidth is still available.

That is interresting, however, I have an older kernel running an OHCI
driver which is able to handle 4 codecs. Same usb hardware (codecs and
hub), but older kernel on a different CPU, with much less power. This makes
me believe that there's a solution to make it work...

> > I noticed that my sound problem starts happening with only 2 codecs
> > (4 streams, 256 kB/s). I first thought that it was a bandwidth=20
> > limitation, so I decided to connect only 1 codec using more bandwidth.
> > I configured it to 48khz-stereo (16-bits), using 384 kB/s for both=20
> > read and write streams: no problem. With that configuration, the scope=
=20
> > shows about 30% of total bandwidth usage (300us used out of 1ms=20
> > periods). Then, I added a second codec (48khz-stereo-16bits): very=20
> > strange, now the total bandwidth usage felt down to about 200us, which=
=20
> > seems to keep the same, whatever the number of codec I add (I also=20
> > tried 3 and 4...). So it looks like the scheduler is not able to=20
> > properly allocate Isochronous time slots when more than one device is=20
> > connected to the hub. However, without the hub, it works perfectly.
>
> How does your hardware connect the host controller to a full-speed device=
?  Is there an internal hub (Intel motherboards have used this approach)?  =
Is there a=20
> companion USB-1.1 controller (older motherboards from Intel and other com=
panys have used this approach)?  Does the EHCI controller have a built-in T=
ransaction=20
> Translator (some SOC systems use this approach)?

The CPU is a Freescale i.MX512, with 3 USB 2.0 Host controllers. My hub
is connected to the main CPU board with a standard USB cable, so it's easy
to swap my 4-port hub from a USB 1.1 to a USB 2.0. My codecs are always
the same: USB 1.1 Texas Instruments PN# pcm2901. I don't believe there's
a built-in Transaction Translator. How can I check that?

> > Another interresting fact is that at application level, the Read and=20
> > Write operations are returning the good amount of bytes read/written.
> > This is not the case at kernel level: I noticed that function=20
> > "usb_submit_urb" (from /drivers/usb/core/urb.c) will only tranfer part=
=20
> > of the "urbs" when the sound is degraded. I tried to figure out where=20
> > the leak comes from without success. Also, there are no error messages=
=20
> > from kernel so everything appears to work well, excepted that part of=20
> > the sound is missing!
> >=20
> > I can't change my hardware (this is in the hand of customers), so the=20
> > only possible solution for me is to correct the software.
> >=20
> > I tried to change my ehci driver with the one from kernel 2.6.39.4 but=
=20
> > did not work, same problem.
> >=20
> > Question:
> > ---------
> >=20
> > Before attempting to upgrade to an earlier kernel driver (this is
>
> "upgrade to an earlier kernel driver" is a contradiction in terms. =20
> Moving to an earlier driver would be a _downgrade_.

Sorry, I meant to say "newer"...

> > a fairly big amount of work), I would really like to know if this=20
> > problem would still be in the 3.x kernels. Has anyone seen that issue=20
> > in 3.x kernels?
>
> It depends a lot on the system hardware.  Many people are using USB audio=
 in 3.x kernels with no problem.  On the other hand, some people have repor=
ted a bug=20
> (quite different from yours) so recently that the patch to fix it has not=
 yet been merged.

I understand. However, if one could test the following with a 3.x kernel:
- CPU with USB 2.0 Host controller (using EHCI-hcd driver)
- 4-port USB 1.1 Hub
- 4x USB codecs (configured at 32khz-mono, 16-bits audio)

Then try to stream audio on each of the 4 codecs at the same time (this
includes one Read and one Write stream on each codec, so total of 4 "Read"
and 4 "Write" streams. Then listen to the output...

If sound is ok when using only 1 codec and becomes choppy when adding a
second codec, then it means that this issue is still in the 3.x kernel. Thi=
s
answer will tell me if it is worth working on using a newer kernel or not.
I have to say that I'm not a linux expert, so I see the migration to a newe=
r
kernel as a quite big amount of work...

> > I am pretty new to USB driver debugging, so any ideas of where/how to=20
> > find solutions will be appreciated. Thank you very much in advance for=
=20
> > the support. Also don't hesitate to redirect me if I'm not at the=20
> > right place to ask these questions. I can also provide some code if=20
> > someone need it to help.
>
> Your first step should be to use an up-to-date kernel, as recommended by =
other people.
>
> Alan Stern

Thank you for your prompt response.

Michael Tessier

^ permalink raw reply

* [PATCH] powerpc/8xx: reduce pressure on TLB due to context switches
From: Christophe Leroy @ 2015-01-05 15:14 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	scottwood
  Cc: linuxppc-dev, linux-kernel

For nohash powerpc, when we run out of contexts, contexts are freed by stealing
used contexts in-turn. When a victim has been selected, the associated TLB
entries are freed using _tlbil_pid(). Unfortunatly, on the PPC 8xx, _tlbil_pid()
does a tlbia, hence flushes ALL TLB entries and not only the one linked to the
stolen context. Therefore, as implented today, at each task switch requiring a
new context, all entries are flushed.

This patch modifies the implementation so that when running out of contexts, all
contexts get freed at once, hence dividing the number of calls to tlbia by 16.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
 arch/powerpc/mm/mmu_context_nohash.c | 44 +++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index 928ebe7..c648677 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -52,12 +52,15 @@
 #include <asm/mmu_context.h>
 #include <asm/tlbflush.h>
 
+#include "mmu_decl.h"
+
 static unsigned int first_context, last_context;
 static unsigned int next_context, nr_free_contexts;
 static unsigned long *context_map;
 static unsigned long *stale_map[NR_CPUS];
 static struct mm_struct **context_mm;
 static DEFINE_RAW_SPINLOCK(context_lock);
+static bool no_selective_tlbil;
 
 #define CTX_MAP_SIZE	\
 	(sizeof(unsigned long) * (last_context / BITS_PER_LONG + 1))
@@ -133,6 +136,39 @@ static unsigned int steal_context_smp(unsigned int id)
 }
 #endif  /* CONFIG_SMP */
 
+static unsigned int steal_all_contexts(void)
+{
+	struct mm_struct *mm;
+	int cpu = smp_processor_id();
+	unsigned int id;
+
+	for (id = first_context; id <= last_context; id++) {
+		/* Pick up the victim mm */
+		mm = context_mm[id];
+
+		pr_hardcont(" | steal %d from 0x%p", id, mm);
+
+		/* Mark this mm as having no context anymore */
+		mm->context.id = MMU_NO_CONTEXT;
+		if (id != first_context) {
+			context_mm[id] = NULL;
+			__clear_bit(id, context_map);
+#ifdef DEBUG_MAP_CONSISTENCY
+			mm->context.active = 0;
+#endif
+		}
+		__clear_bit(id, stale_map[cpu]);
+
+	}
+
+	/* Flush the TLB for all contexts */
+	_tlbil_all();
+
+	nr_free_contexts = last_context - first_context;
+
+	return first_context;
+}
+
 /* Note that this will also be called on SMP if all other CPUs are
  * offlined, which means that it may be called for cpu != 0. For
  * this to work, we somewhat assume that CPUs that are onlined
@@ -241,7 +277,10 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
 			goto stolen;
 		}
 #endif /* CONFIG_SMP */
-		id = steal_context_up(id);
+		if (no_selective_tlbil)
+			id = steal_all_contexts();
+		else
+			id = steal_context_up(id);
 		goto stolen;
 	}
 	nr_free_contexts--;
@@ -407,12 +446,15 @@ void __init mmu_context_init(void)
 	if (mmu_has_feature(MMU_FTR_TYPE_8xx)) {
 		first_context = 0;
 		last_context = 15;
+		no_selective_tlbil = true;
 	} else if (mmu_has_feature(MMU_FTR_TYPE_47x)) {
 		first_context = 1;
 		last_context = 65535;
+		no_selective_tlbil = false;
 	} else {
 		first_context = 1;
 		last_context = 255;
+		no_selective_tlbil = false;
 	}
 
 #ifdef DEBUG_CLAMP_LAST_CONTEXT
-- 
2.1.0

^ permalink raw reply related

* RE: PROBLEM: USB isochronous urb leak on EHCI driver
From: Alan Stern @ 2015-01-05 16:00 UTC (permalink / raw)
  To: Michael Tessier; +Cc: linuxppc-dev@lists.ozlabs.org, linux-usb@vger.kernel.org
In-Reply-To: <66A26A9AA227D947AF088537F041526E2008FE@VSVR-EX10-MB1.pocatec.com>

On Mon, 5 Jan 2015, Michael Tessier wrote:

> > > Hi,
> > > 
> > > I am dealing with a USB EHCI driver bug. Here is the info:
> > > 
> > > My configuration:
> > > -----------------
> > > 
> > > Host: Freescale i.MX512 with ARM Cortex A8 (USB 2.0 host controller) 
> > > Linux kernel: 2.6.31, using EHCI USB driver
> >
> > As mentioned by other people, the age of that kernel makes any bug report completely irrelevant.  It's hard to count the number of non-trivial changes that have  > been made to the isochronous code in ehci-hcd since 2.6.31, but there have been quite a few.
> >
> > > Hub: 4-PORT USB 1.1 HUB (Texas Instruments PN: tusb2046b)
> > > Devices: 4 USB 1.1 audio codecs (Texas Instruments PN: pcm2901)
> > > 
> > > Note: each codec is being used in R/W access, so with 4 codecs, I have
> > > 4 playback and 4 capture streams.
> > > 
> > > My problem:
> > > -----------
> > > 
> > > I have usb urb leaks when connecting more than 1 codec to the USB 1.1 
> > > Hub.
> >
> > What do you mean by "urb leak"?  Normally, people use the word "leak"  
> > to refer to memory that is dynamically allocated and never deallocated, but you seem to mean something else.
> 
> You are right. What I mean by leak is the following: At application level,
> all my calls to "Read" or "Write" operation to the codec driver will return
> with the correct amount of bytes read/written, with a "choppy" sound. Then
> when looking at lower levels:
> 
> snd_pcm_oss_write (pcm_oss.c) 	-> OK
> snd_pcm_lib_write (pcm_lib.c) 	-> OK
> usb_submit_urb 	(urb.c)		-> FAIL with 3 codecs
> 
> The "FAIL" here indicates that the total amount of bytes transferred does
> not correspond to what was expected. And indeed the sound is "choppy" when
> using more than a certain amount of bandwidth. However this amount of
> bandwidth is higher when connecting only 1 codec with different settings
> (48khz-stereo 16-bits instead of 32 khz-mono 16-bits).So at some point it
> looks like the bug is in the scheduler, only with several isochronous links.

Agreed.

> > The amount of bandwidth available is usually not as much of an issue as the ability of the scheduling alogorithm to divide the bandwidth among the streams.  The
> > algorithm is not very smart and it often runs into a wall even when lots of physical bandwidth is still available.
> 
> That is interresting, however, I have an older kernel running an OHCI
> driver which is able to handle 4 codecs. Same usb hardware (codecs and
> hub), but older kernel on a different CPU, with much less power. This makes
> me believe that there's a solution to make it work...

Of course there is: Install an OHCI host controller and use it to drive
your codecs.  It should work fine.

The periodic scheduling algorithm for OHCI is very different from the
algorithm for EHCI.

> > How does your hardware connect the host controller to a full-speed device?  Is there an internal hub (Intel motherboards have used this approach)?  Is there a 
> > companion USB-1.1 controller (older motherboards from Intel and other companys have used this approach)?  Does the EHCI controller have a built-in Transaction 
> > Translator (some SOC systems use this approach)?
> 
> The CPU is a Freescale i.MX512, with 3 USB 2.0 Host controllers. My hub
> is connected to the main CPU board with a standard USB cable, so it's easy
> to swap my 4-port hub from a USB 1.1 to a USB 2.0. My codecs are always
> the same: USB 1.1 Texas Instruments PN# pcm2901. I don't believe there's
> a built-in Transaction Translator. How can I check that?

You can tell by seeing what shows up in the "lsusb -t" output when you
plug in the USB-1.1 hub.  If the hub's parent is the EHCI controller 
then there must be a built-in TT.

Also, if you enable CONFIG_USB_DEBUG in your kernel then the dmesg log
for boot-up should say whether or not the controller has a built-in TT.

> > > Question:
> > > ---------
> > > 
> > > Before attempting to upgrade to an earlier kernel driver (this is
> >
> > "upgrade to an earlier kernel driver" is a contradiction in terms.  
> > Moving to an earlier driver would be a _downgrade_.
> 
> Sorry, I meant to say "newer"...
> 
> > > a fairly big amount of work), I would really like to know if this 
> > > problem would still be in the 3.x kernels. Has anyone seen that issue 
> > > in 3.x kernels?
> >
> > It depends a lot on the system hardware.  Many people are using USB audio in 3.x kernels with no problem.  On the other hand, some people have reported a bug 
> > (quite different from yours) so recently that the patch to fix it has not yet been merged.
> 
> I understand. However, if one could test the following with a 3.x kernel:
> - CPU with USB 2.0 Host controller (using EHCI-hcd driver)
> - 4-port USB 1.1 Hub
> - 4x USB codecs (configured at 32khz-mono, 16-bits audio)
> 
> Then try to stream audio on each of the 4 codecs at the same time (this
> includes one Read and one Write stream on each codec, so total of 4 "Read"
> and 4 "Write" streams. Then listen to the output...

The result is likely to depend on what other USB hardware is attached.

> If sound is ok when using only 1 codec and becomes choppy when adding a
> second codec, then it means that this issue is still in the 3.x kernel. This
> answer will tell me if it is worth working on using a newer kernel or not.
> I have to say that I'm not a linux expert, so I see the migration to a newer
> kernel as a quite big amount of work...

Why don't you try this yourself?  It's easy to do; borrow a regular PC 
with a USB-2 host controller, boot it from a Live-CD version of Linux, 
plug in your hub with the codecs, and see what happens.

Alan Stern

^ permalink raw reply

* [RFC] PPC: MPIC: necessary readback after EOI?
From: Purcareata Bogdan @ 2015-01-05 14:14 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, linux-kernel

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.

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

^ permalink raw reply

* Re: PROBLEM: USB isochronous urb leak on EHCI driver
From: Fabio Estevam @ 2015-01-05 17:28 UTC (permalink / raw)
  To: Michael Tessier
  Cc: linuxppc-dev@lists.ozlabs.org, Alan Stern,
	linux-usb@vger.kernel.org
In-Reply-To: <66A26A9AA227D947AF088537F041526E2008FE@VSVR-EX10-MB1.pocatec.com>

On Mon, Jan 5, 2015 at 1:12 PM, Michael Tessier
<michael.tessier@axiontech.ca> wrote:

> If sound is ok when using only 1 codec and becomes choppy when adding a
> second codec, then it means that this issue is still in the 3.x kernel. This
> answer will tell me if it is worth working on using a newer kernel or not.
> I have to say that I'm not a linux expert, so I see the migration to a newer
> kernel as a quite big amount of work...

We have support for mx51 on the latest kernel. All you need to do is
to describe your hardware on a device tree file. You can refer to
arch/arm/boot/dts/imx51-babbage.dts as an example.

Should be simple for you to make such test with the latest kernel.

^ permalink raw reply

* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Andreas Mohr @ 2015-01-05 17:46 UTC (permalink / raw)
  To: Purcareata Bogdan; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <54AA9C4D.3000001@freescale.com>

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 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

^ permalink raw reply

* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Scott Wood @ 2015-01-05 18:10 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.

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).

-Scott

^ permalink raw reply

* Re: [PATCH 04/11] powerpc/8xx: Take benefit of aligned PGDIR
From: Joakim Tjernlund @ 2015-01-05 18:19 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141216150338.978F71A5E04@localhost.localdomain>


On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote:
> L1 base address is now aligned so we can insert L1 index into r11 directl=
y and
> then preserve r10
>=20
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Acked-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>

>=20
> ---
>  arch/powerpc/kernel/head_8xx.S | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8x=
x.S
> index 2c329f1..ae05f28 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -326,16 +326,15 @@ InstructionTLBMiss:
>         ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
>  3:
>  #endif
> -       /* Extract level 1 index */
> -       rlwinm  r10, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> -       lwzx    r11, r10, r11/* Get the level 1 entry */
> -       rlwinm  r10, r11,0,0,19/* Extract page descriptor page address */
> +       /* Insert level 1 index */
> +       rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> +       lwz     r11, 0(r11)/* Get the level 1 entry */
> =20
>         /* Load the MI_TWC with the attributes for this "segment." */
>         MTSPR_CPU6(SPRN_MI_TWC, r11, r3)/* Set segment attributes */
> -       mfspr   r11, SPRN_SRR0/* Get effective address of fault */
> +       rlwinm  r11, r11,0,0,19/* Extract page descriptor page address */
>         /* Extract level 2 index */
> -       rlwinm  r11, r11, 32 - (PAGE_SHIFT - 2), 32 - PAGE_SHIFT, 29
> +       rlwinm  r10, r10, 32 - (PAGE_SHIFT - 2), 32 - PAGE_SHIFT, 29
>         lwzx    r10, r10, r11/* Get the pte */
> =20
>  #ifdef CONFIG_SWAP
> @@ -380,13 +379,12 @@ DataStoreTLBMiss:
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
>         ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
>  3:
> -       /* Extract level 1 index */
> -       rlwinm  r10, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> -       lwzx    r11, r10, r11/* Get the level 1 entry */
> +       /* Insert level 1 index */
> +       rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> +       lwz     r11, 0(r11)/* Get the level 1 entry */
> =20
>         /* We have a pte table, so load fetch the pte from the table.
>                 */
> -       mfspr   r10, SPRN_MD_EPN/* Get address of fault */
>         /* Extract level 2 index */
>         rlwinm  r10, r10, 32 - (PAGE_SHIFT - 2), 32 - PAGE_SHIFT, 29
>         rlwimi  r10, r11, 0, 0, 32 - PAGE_SHIFT - 1/* Add level 2 base */
> @@ -515,16 +513,14 @@ FixupDAR:/* Entry point for dcbx workaround. */
>         beq-    3f      /* Branch if user space */
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
>         ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
> -       /* Extract level 1 index */
> -3:     rlwinm  r10, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> -       lwzx    r11, r10, r11/* Get the level 1 entry */
> -       rlwinm  r10, r11,0,0,19/* Extract page descriptor page address */
> -       mfspr   r11, SPRN_SRR0/* Get effective address of fault */
> -       /* Extract level 2 index */
> -       rlwinm  r11, r11, 32 - (PAGE_SHIFT - 2), 32 - PAGE_SHIFT, 29
> -       lwzx    r11, r10, r11/* Get the pte */
> +       /* Insert level 1 index */
> +3:     rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> +       lwz     r11, 0(r11)/* Get the level 1 entry */
> +       rlwinm  r11, r11,0,0,19/* Extract page descriptor page address */
> +       /* Insert level 2 index */
> +       rlwimi  r11, r10, 32 - (PAGE_SHIFT - 2), 32 - PAGE_SHIFT, 29
> +       lwz     r11, 0(r11)/* Get the pte */
>         /* concat physical page address(r11) and page offset(r10) */
> -       mfspr   r10, SPRN_SRR0
>         rlwimi  r11, r10, 0, 32 - PAGE_SHIFT, 31
>         lwz     r11,0(r11)
>  /* Check if it really is a dcbx instruction. */=

^ permalink raw reply

* Re: [PATCH v3 2/2] powerpc/8xx: use _PAGE_RO instead of _PAGE_RW
From: Joakim Tjernlund @ 2015-01-05 18:12 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141222101450.D7DD11A5E15@localhost.localdomain>


On Mon, 2014-12-22 at 11:14 +0100, Christophe Leroy wrote:
> On powerpc 8xx, in TLB entries, 0x400 bit is set to 1 for read-only pages
> and is set to 0 for RW pages. So we should use _PAGE_RO instead of _PAGE_=
RW
>=20
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Hi Christophe, been meaning to look over all you recent 8xx MMU/TLB patches
but got so little time :(

This is very cool (not sure if there will be a performance gain)  but ..
>=20
>=20
>=20
> diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/incl=
ude/asm/pgtable-ppc32.h
> index caf094a..b4e0c3b 100644
> --- a/arch/powerpc/include/asm/pgtable-ppc32.h
> +++ b/arch/powerpc/include/asm/pgtable-ppc32.h
> @@ -178,9 +178,10 @@ static inline unsigned long pte_update(pte_t *p,
>         andc    %1,%0,%5\n\
>         or      %1,%1,%6\n\
>         /* 0x200 =3D=3D Extended encoding, bit 22 */ \
> -       /* Bit 22 has to be 1 if neither _PAGE_USER nor _PAGE_RW are set =
*/ \
> +       /* Bit 22 has to be 1 when _PAGE_USER is unset and _PAGE_RO is se=
t */ \
>         rlwimi  %1,%1,32-2,0x200\n /* get _PAGE_USER */ \
> -       rlwinm  %3,%1,32-1,0x200\n /* get _PAGE_RW */ \
> +       rlwinm  %3,%1,32-1,0x200\n /* get _PAGE_RO */ \
> +       xori    %3,%3,0x200\n \
>         or      %1,%3,%1\n\
>         xori    %1,%1,0x200\n"
>  "      stwcx.  %1,0,%4\n\

... here I expected to loose the existing xori insn instead of adding one?

 Jocke=

^ permalink raw reply

* Re: [PATCH 02/11] powerpc/8xx: remove tests on PGDIR entry validity
From: Joakim Tjernlund @ 2015-01-05 18:15 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141216150338.317E61A5E03@localhost.localdomain>


On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote:
> Kernel MMU handling code handles validity of entries via _PMD_PRESENT whi=
ch
> corresponds to V bit in MD_TWC and MI_TWC. When the V bit is not set, MPC=
8xx
> triggers TLBError exception. So we don't have to check that and branch ou=
rself
> to TLBError. We can set TLB entries with non present entries, remove all =
those
> tests and let the 8xx handle it. This reduce the number of cycle when the
> entries are valid which is the case most of the time, and doesn't signifi=
cantly
> increase the time for handling invalid entries.
>=20
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Nice :)
Acked-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>

>=20
> ---
>  arch/powerpc/kernel/head_8xx.S | 41 ++++++++----------------------------=
-----
>  1 file changed, 8 insertions(+), 33 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8x=
x.S
> index 79b8a23..2c329f1 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -329,12 +329,9 @@ InstructionTLBMiss:
>         /* Extract level 1 index */
>         rlwinm  r10, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
>         lwzx    r11, r10, r11/* Get the level 1 entry */
> -       rlwinm. r10, r11,0,0,19/* Extract page descriptor page address */
> -       beq     2f      /* If zero, don't try to find a pte */
> +       rlwinm  r10, r11,0,0,19/* Extract page descriptor page address */
> =20
> -       /* We have a pte table, so load the MI_TWC with the attributes
> -               * for this "segment."
> -               */
> +       /* Load the MI_TWC with the attributes for this "segment." */
>         MTSPR_CPU6(SPRN_MI_TWC, r11, r3)/* Set segment attributes */
>         mfspr   r11, SPRN_SRR0/* Get effective address of fault */
>         /* Extract level 2 index */
> @@ -342,13 +339,11 @@ InstructionTLBMiss:
>         lwzx    r10, r10, r11/* Get the pte */
> =20
>  #ifdef CONFIG_SWAP
> -       andi.   r11, r10, _PAGE_ACCESSED | _PAGE_PRESENT
> -       cmpwi   cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
> -       li      r11, RPN_PATTERN
> -       bne-    cr0, 2f
> -#else
> -       li      r11, RPN_PATTERN
> +       rlwinm  r11, r10, 32-5, _PAGE_PRESENT
> +       and     r11, r11, r10
> +       rlwimi  r10, r11, 0, _PAGE_PRESENT
>  #endif
> +       li      r11, RPN_PATTERN
>         /* The Linux PTE won't go exactly into the MMU TLB.
>                 * Software indicator bits 21 and 28 must be clear.
>                 * Software indicator bits 24, 25, 26, and 27 must be
> @@ -366,21 +361,6 @@ InstructionTLBMiss:
>         mfspr   r10, SPRN_SPRG_SCRATCH2
>         EXCEPTION_EPILOG_0
>         rfi
> -2:
> -       mfspr   r10, SPRN_SRR1
> -       /* clear all error bits as TLB Miss
> -               * sets a few unconditionally
> -       */
> -       rlwinm  r10, r10, 0, 0xffff
> -       mtspr   SPRN_SRR1, r10
> -
> -       /* Restore registers */
> -#ifdef CONFIG_8xx_CPU6
> -       mfspr   r3, SPRN_DAR
> -       mtspr   SPRN_DAR, r11/* Tag DAR */
> -#endif
> -       mfspr   r10, SPRN_SPRG_SCRATCH2
> -       b       InstructionTLBError1
> =20
>         . =3D 0x1200
>  DataStoreTLBMiss:
> @@ -403,8 +383,6 @@ DataStoreTLBMiss:
>         /* Extract level 1 index */
>         rlwinm  r10, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
>         lwzx    r11, r10, r11/* Get the level 1 entry */
> -       rlwinm. r10, r11,0,0,19/* Extract page descriptor page address */
> -       beq     2f      /* If zero, don't try to find a pte */
> =20
>         /* We have a pte table, so load fetch the pte from the table.
>                 */
> @@ -450,7 +428,7 @@ DataStoreTLBMiss:
>                 * set.  All other Linux PTE bits control the behavior
>                 * of the MMU.
>                 */
> -2:     li      r11, RPN_PATTERN
> +       li      r11, RPN_PATTERN
>         rlwimi  r10, r11, 0, 24, 28/* Set 24-27, clear 28 */
>         MTSPR_CPU6(SPRN_MD_RPN, r10, r3)/* Update TLB entry */
> =20
> @@ -469,10 +447,7 @@ DataStoreTLBMiss:
>   */
>         . =3D 0x1300
>  InstructionTLBError:
> -       EXCEPTION_PROLOG_0
> -InstructionTLBError1:
> -       EXCEPTION_PROLOG_1
> -       EXCEPTION_PROLOG_2
> +       EXCEPTION_PROLOG
>         mr      r4,r12
>         mr      r5,r9
>         andis.  r10,r5,0x4000=

^ permalink raw reply

* Re: [PATCH 11/11] powerpc/8xx: Add support for TASK_SIZE greater than 0x80000000
From: Joakim Tjernlund @ 2015-01-05 18:21 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141216150340.55A031A5E07@localhost.localdomain>


On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote:
> By default, TASK_SIZE is set to 0x80000000 for PPC_8xx, which is most lik=
ely
> sufficient for most cases. However, kernel configuration allows to set TA=
SK_SIZE
> to another value, so the 8xx shall handle it.

On 8xx I would be just as happy with not supporting different TASK_SIZEs, b=
ut why not ..
Acked-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>

>=20
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>=20
> ---
>  arch/powerpc/kernel/head_8xx.S | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8x=
x.S
> index dbe110e..d380658 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -48,6 +48,19 @@
>         mtspr   spr, reg
>  #endif
> =20
> +/* Macro to test if an address is a kernel address */
> +#if CONFIG_TASK_SIZE <=3D 0x80000000
> +#define IS_KERNEL(tmp, addr)           \
> +       andis.  tmp, addr, 0x8000/* Address >=3D 0x80000000 */
> +#define BRANCH_UNLESS_KERNEL(label)    beq     label
> +#else
> +#define IS_KERNEL(tmp, addr)           \
> +       rlwinm  tmp, addr, 16, 16, 31;\
> +       cmpli   cr0, tmp, PAGE_OFFSET >> 16
> +#define BRANCH_UNLESS_KERNEL(label)    blt     label
> +#endif
> +
> +
>  /*
>   * Value for the bits that have fixed value in RPN entries.
>   * Also used for tagging DAR for DTLBerror.
> @@ -323,15 +336,15 @@ InstructionTLBMiss:
>         mfspr   r10, SPRN_SRR0/* Get effective address of fault */
>         DO_8xx_CPU15(r11, r10)
>         mfcr    r3
> -       andis.  r11, r10, 0x8000/* Address >=3D 0x80000000 */
> +       IS_KERNEL(r11, r10)
>  #else
>         mfspr   r11, SPRN_SRR0/* Get effective address of fault */
>         DO_8xx_CPU15(r10, r11)
>         mfcr    r10
> -       andis.  r11, r11, 0x8000/* Address >=3D 0x80000000 */
> +       IS_KERNEL(r11, r11)
>  #endif
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
> -       beq     3f
> +       BRANCH_UNLESS_KERNEL(3f)
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>  3:
>  #ifdef CONFIG_8xx_CPU6
> @@ -390,14 +403,14 @@ DataStoreTLBMiss:
>         mtspr   SPRN_SPRG_SCRATCH2, r3
>         mfcr    r3
>         mfspr   r10, SPRN_MD_EPN
> -       andis.  r11, r10, 0x8000
> +       IS_KERNEL(r11, r10)
>  #else
>         mfcr    r10
>         mfspr   r11, SPRN_MD_EPN
> -       andis.  r11, r11, 0x8000
> +       IS_KERNEL(r11, r11)
>  #endif
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
> -       beq     3f
> +       BRANCH_UNLESS_KERNEL(3f)
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>  3:
>  #ifdef CONFIG_8xx_CPU6
> @@ -536,9 +549,9 @@ FixupDAR:/* Entry point for dcbx workaround. */
>         mtspr   SPRN_SPRG_SCRATCH2, r10
>         /* fetch instruction from memory. */
>         mfspr   r10, SPRN_SRR0
> -       andis.  r11, r10, 0x8000/* Address >=3D 0x80000000 */
> +       IS_KERNEL(r11, r10)
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
> -       beq     3f
> +       BRANCH_UNLESS_KERNEL(3f)
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>         /* Insert level 1 index */
>  3:     rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29=

^ permalink raw reply

* Re: [PATCH 09/11] powerpc/8xx: dont save CR in SCRATCH registers
From: Joakim Tjernlund @ 2015-01-05 18:30 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141216150340.108641A5E05@localhost.localdomain>


On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote:
> CR only needs to be preserved when checking if we are handling a kernel a=
ddress.
> So we can preserve CR in a register:
> - In ITLBMiss, check is done only when CONFIG_MODULES is defined. Otherwi=
se we
> don't need to do anything at all with CR.
> - If CONFIG_8xx_CPU6 is defined, we have r3 available for saving CR
> - Otherwise, we use r10, then we reload SRR0/MD_EPN into r10 when CR is r=
estored

I think the #ifdef CPU6 code makes for too much maintenance. Can you
loose the #ifdef CPU6 and adjust code so it works for both cases?

>=20
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>=20
> ---
>  arch/powerpc/kernel/head_8xx.S | 53 +++++++++++++++++++++++++++++-------=
------
>  1 file changed, 37 insertions(+), 16 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8x=
x.S
> index c89aed9..a073918 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -308,14 +308,10 @@ SystemCall:
>  #endif
> =20
>  InstructionTLBMiss:
> +       EXCEPTION_PROLOG_0
>  #ifdef CONFIG_8xx_CPU6
>         mtspr   SPRN_DAR, r3
>  #endif
> -       EXCEPTION_PROLOG_0
> -       mfcr    r10
> -       mtspr   SPRN_SPRG_SCRATCH2, r10
> -       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
> -       DO_8xx_CPU15(r11, r10)
> =20
>         /* If we are faulting a kernel address, we have to use the
>                 * kernel page tables.
> @@ -323,14 +319,33 @@ InstructionTLBMiss:
>  #ifdef CONFIG_MODULES
>         /* Only modules will cause ITLB Misses as we always
>                 * pin the first 8MB of kernel memory */
> +#ifdef CONFIG_8xx_CPU6
> +       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
> +       DO_8xx_CPU15(r11, r10)
> +       mfcr    r3
>         andis.  r11, r10, 0x8000/* Address >=3D 0x80000000 */
> +#else
> +       mfspr   r11, SPRN_SRR0/* Get effective address of fault */
> +       DO_8xx_CPU15(r10, r11)
> +       mfcr    r10
> +       andis.  r11, r11, 0x8000/* Address >=3D 0x80000000 */
>  #endif
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
> -#ifdef CONFIG_MODULES
>         beq     3f
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>  3:
> +#ifdef CONFIG_8xx_CPU6
> +       mtcr    r3
> +#else
> +       mtcr    r10
> +       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
>  #endif
> +#else /* CONFIG_MODULES */
> +       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
> +       DO_8xx_CPU15(r11, r10)
> +       mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
> +#endif /* CONFIG_MODULES */
> +
>         /* Insert level 1 index */
>         rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
>         lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level =
1 entry */
> @@ -362,29 +377,37 @@ InstructionTLBMiss:
>         mfspr   r3, SPRN_DAR
>         mtspr   SPRN_DAR, r11/* Tag DAR */
>  #endif
> -       mfspr   r10, SPRN_SPRG_SCRATCH2
> -       mtcr    r10
>         EXCEPTION_EPILOG_0
>         rfi
> =20
>         . =3D 0x1200
>  DataStoreTLBMiss:
> -#ifdef CONFIG_8xx_CPU6
> -       mtspr   SPRN_DAR, r3
> -#endif
>         EXCEPTION_PROLOG_0
> -       mfcr    r10
> -       mtspr   SPRN_SPRG_SCRATCH2, r10
> -       mfspr   r10, SPRN_MD_EPN
> =20
>         /* If we are faulting a kernel address, we have to use the
>                 * kernel page tables.
>                 */
> +#ifdef CONFIG_8xx_CPU6
> +       mtspr   SPRN_DAR, r3
> +       mfcr    r3
> +       mfspr   r10, SPRN_MD_EPN
>         andis.  r11, r10, 0x8000
> +#else
> +       mfcr    r10
> +       mfspr   r11, SPRN_MD_EPN
> +       andis.  r11, r11, 0x8000
> +#endif
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
>         beq     3f
>         lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>  3:
> +#ifdef CONFIG_8xx_CPU6
> +       mtcr    r3
> +#else
> +       mtcr    r10
> +       mfspr   r10, SPRN_MD_EPN
> +#endif
> +
>         /* Insert level 1 index */
>         rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
>         lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level =
1 entry */
> @@ -441,8 +464,6 @@ DataStoreTLBMiss:
>         mfspr   r3, SPRN_DAR
>  #endif
>         mtspr   SPRN_DAR, r11/* Tag DAR */
> -       mfspr   r10, SPRN_SPRG_SCRATCH2
> -       mtcr    r10
>         EXCEPTION_EPILOG_0
>         rfi
>  =

^ permalink raw reply

* Re: [PATCH 05/11] powerpc/8xx: Optimise access to swapper_pg_dir
From: Joakim Tjernlund @ 2015-01-05 18:33 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141216150338.D4F0A1A5E0A@localhost.localdomain>


On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote:
> All accessed to PGD entries are done via 0(r11).
> By using lower part of swapper_pg_dir as load index to r11, we can remove=
 the
> ori instruction.
>=20
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Nice :)
Acked-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>

>=20
> ---
>  arch/powerpc/kernel/head_8xx.S | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8x=
x.S
> index ae05f28..aa45225 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -322,13 +322,12 @@ InstructionTLBMiss:
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
>  #ifdef CONFIG_MODULES
>         beq     3f
> -       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
> -       ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
> +       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>  3:
>  #endif
>         /* Insert level 1 index */
>         rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> -       lwz     r11, 0(r11)/* Get the level 1 entry */
> +       lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level =
1 entry */
> =20
>         /* Load the MI_TWC with the attributes for this "segment." */
>         MTSPR_CPU6(SPRN_MI_TWC, r11, r3)/* Set segment attributes */
> @@ -376,12 +375,11 @@ DataStoreTLBMiss:
>         andis.  r11, r10, 0x8000
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
>         beq     3f
> -       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
> -       ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
> +       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>  3:
>         /* Insert level 1 index */
>         rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> -       lwz     r11, 0(r11)/* Get the level 1 entry */
> +       lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level =
1 entry */
> =20
>         /* We have a pte table, so load fetch the pte from the table.
>                 */
> @@ -510,12 +508,11 @@ FixupDAR:/* Entry point for dcbx workaround. */
>         mfspr   r10, SPRN_SRR0
>         andis.  r11, r10, 0x8000/* Address >=3D 0x80000000 */
>         mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
> -       beq-    3f      /* Branch if user space */
> -       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@h
> -       ori     r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
> +       beq     3f
> +       lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>         /* Insert level 1 index */
>  3:     rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) =
<< 1, 29
> -       lwz     r11, 0(r11)/* Get the level 1 entry */
> +       lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level =
1 entry */
>         rlwinm  r11, r11,0,0,19/* Extract page descriptor page address */
>         /* Insert level 2 index */
>         rlwimi  r11, r10, 32 - (PAGE_SHIFT - 2), 32 - PAGE_SHIFT, 29
> @@ -670,8 +667,7 @@ start_here:
>                 * init's THREAD like the context switch code does, but th=
is is
>                 * easier......until someone changes init's static structu=
res.
>                 */
> -       lis     r6, swapper_pg_dir@h
> -       ori     r6, r6, swapper_pg_dir@l
> +       lis     r6, swapper_pg_dir@ha
>         tophys(r6,r6)
>  #ifdef CONFIG_8xx_CPU6
>         lis     r4, cpu6_errata_word@h
> @@ -850,6 +846,8 @@ _GLOBAL(set_context)
>         stw     r4, 0x4(r5)
>  #endif
> =20
> +       li      r5, (swapper_pg_dir-PAGE_OFFSET)@l
> +       sub     r4, r4, r5
>  #ifdef CONFIG_8xx_CPU6
>         lis     r6, cpu6_errata_word@h
>         ori     r6, r6, cpu6_errata_word@l=

^ permalink raw reply

* Re: [RFC] PPC: MPIC: necessary readback after EOI?
From: Andreas Mohr @ 2015-01-05 18:43 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Purcareata Bogdan, linux-kernel, Andreas Mohr
In-Reply-To: <1420481454.4961.16.camel@freescale.com>

[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?).

Andreas Mohr

^ permalink raw reply

* Re: [PATCH v3 2/2] powerpc/8xx: use _PAGE_RO instead of _PAGE_RW
From: leroy christophe @ 2015-01-06  7:03 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1420481520.25047.15.camel@transmode.se>


Le 05/01/2015 19:12, Joakim Tjernlund a écrit :
> On Mon, 2014-12-22 at 11:14 +0100, Christophe Leroy wrote:
>> On powerpc 8xx, in TLB entries, 0x400 bit is set to 1 for read-only pages
>> and is set to 0 for RW pages. So we should use _PAGE_RO instead of _PAGE_RW
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Hi Christophe, been meaning to look over all you recent 8xx MMU/TLB patches
> but got so little time :(
>
> This is very cool (not sure if there will be a performance gain)  but ..
I think every saved cycle is worth it.
Before I did any modification:
* ITLBMiss was 28 instructions.
* DTLBMiss was 32 instructions.
Now, (No MODULES, no CPU6, no CPU15):
* ITLBMiss is 15 instructions
* DTLBMiss is 24 instructions
>>
>>
>> diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h
>> index caf094a..b4e0c3b 100644
>> --- a/arch/powerpc/include/asm/pgtable-ppc32.h
>> +++ b/arch/powerpc/include/asm/pgtable-ppc32.h
>> @@ -178,9 +178,10 @@ static inline unsigned long pte_update(pte_t *p,
>>          andc    %1,%0,%5\n\
>>          or      %1,%1,%6\n\
>>          /* 0x200 == Extended encoding, bit 22 */ \
>> -       /* Bit 22 has to be 1 if neither _PAGE_USER nor _PAGE_RW are set */ \
>> +       /* Bit 22 has to be 1 when _PAGE_USER is unset and _PAGE_RO is set */ \
>>          rlwimi  %1,%1,32-2,0x200\n /* get _PAGE_USER */ \
>> -       rlwinm  %3,%1,32-1,0x200\n /* get _PAGE_RW */ \
>> +       rlwinm  %3,%1,32-1,0x200\n /* get _PAGE_RO */ \
>> +       xori    %3,%3,0x200\n \
>>          or      %1,%3,%1\n\
>>          xori    %1,%1,0x200\n"
>>   "      stwcx.  %1,0,%4\n\
> ... here I expected to loose the existing xori insn instead of adding one?
>
>
Well, I could have xored the PAGE_USER bit instead, but in that case, it 
is not anymore an 'or' but an 'and' that has to be performed between the 
bits, and then all other bits must be set to 1, or the result of the 
'and' shall be inserted using 'rlwimi'. So it would be more 
modifications than just adding an xori, and not less instructions.

Christophe

^ permalink raw reply

* Re: [PATCH 09/11] powerpc/8xx: dont save CR in SCRATCH registers
From: leroy christophe @ 2015-01-06  7:07 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: linux-kernel@vger.kernel.org, paulus@samba.org,
	scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1420482624.25047.25.camel@transmode.se>


Le 05/01/2015 19:30, Joakim Tjernlund a écrit :
> On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote:
>> CR only needs to be preserved when checking if we are handling a kernel address.
>> So we can preserve CR in a register:
>> - In ITLBMiss, check is done only when CONFIG_MODULES is defined. Otherwise we
>> don't need to do anything at all with CR.
>> - If CONFIG_8xx_CPU6 is defined, we have r3 available for saving CR
>> - Otherwise, we use r10, then we reload SRR0/MD_EPN into r10 when CR is restored
> I think the #ifdef CPU6 code makes for too much maintenance. Can you
> loose the #ifdef CPU6 and adjust code so it works for both cases?
I wanted to take the opportunity to save one cycle, but we are wasting 
so many cycles with CPU6 that it is propably not worth it.
Ok, I will remove the special handling for CPU6.

>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> ---
>>   arch/powerpc/kernel/head_8xx.S | 53 +++++++++++++++++++++++++++++-------------
>>   1 file changed, 37 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
>> index c89aed9..a073918 100644
>> --- a/arch/powerpc/kernel/head_8xx.S
>> +++ b/arch/powerpc/kernel/head_8xx.S
>> @@ -308,14 +308,10 @@ SystemCall:
>>   #endif
>>   
>>   InstructionTLBMiss:
>> +       EXCEPTION_PROLOG_0
>>   #ifdef CONFIG_8xx_CPU6
>>          mtspr   SPRN_DAR, r3
>>   #endif
>> -       EXCEPTION_PROLOG_0
>> -       mfcr    r10
>> -       mtspr   SPRN_SPRG_SCRATCH2, r10
>> -       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
>> -       DO_8xx_CPU15(r11, r10)
>>   
>>          /* If we are faulting a kernel address, we have to use the
>>                  * kernel page tables.
>> @@ -323,14 +319,33 @@ InstructionTLBMiss:
>>   #ifdef CONFIG_MODULES
>>          /* Only modules will cause ITLB Misses as we always
>>                  * pin the first 8MB of kernel memory */
>> +#ifdef CONFIG_8xx_CPU6
>> +       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
>> +       DO_8xx_CPU15(r11, r10)
>> +       mfcr    r3
>>          andis.  r11, r10, 0x8000/* Address >= 0x80000000 */
>> +#else
>> +       mfspr   r11, SPRN_SRR0/* Get effective address of fault */
>> +       DO_8xx_CPU15(r10, r11)
>> +       mfcr    r10
>> +       andis.  r11, r11, 0x8000/* Address >= 0x80000000 */
>>   #endif
>>          mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
>> -#ifdef CONFIG_MODULES
>>          beq     3f
>>          lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>>   3:
>> +#ifdef CONFIG_8xx_CPU6
>> +       mtcr    r3
>> +#else
>> +       mtcr    r10
>> +       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
>>   #endif
>> +#else /* CONFIG_MODULES */
>> +       mfspr   r10, SPRN_SRR0/* Get effective address of fault */
>> +       DO_8xx_CPU15(r11, r10)
>> +       mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
>> +#endif /* CONFIG_MODULES */
>> +
>>          /* Insert level 1 index */
>>          rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) << 1, 29
>>          lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level 1 entry */
>> @@ -362,29 +377,37 @@ InstructionTLBMiss:
>>          mfspr   r3, SPRN_DAR
>>          mtspr   SPRN_DAR, r11/* Tag DAR */
>>   #endif
>> -       mfspr   r10, SPRN_SPRG_SCRATCH2
>> -       mtcr    r10
>>          EXCEPTION_EPILOG_0
>>          rfi
>>   
>>          . = 0x1200
>>   DataStoreTLBMiss:
>> -#ifdef CONFIG_8xx_CPU6
>> -       mtspr   SPRN_DAR, r3
>> -#endif
>>          EXCEPTION_PROLOG_0
>> -       mfcr    r10
>> -       mtspr   SPRN_SPRG_SCRATCH2, r10
>> -       mfspr   r10, SPRN_MD_EPN
>>   
>>          /* If we are faulting a kernel address, we have to use the
>>                  * kernel page tables.
>>                  */
>> +#ifdef CONFIG_8xx_CPU6
>> +       mtspr   SPRN_DAR, r3
>> +       mfcr    r3
>> +       mfspr   r10, SPRN_MD_EPN
>>          andis.  r11, r10, 0x8000
>> +#else
>> +       mfcr    r10
>> +       mfspr   r11, SPRN_MD_EPN
>> +       andis.  r11, r11, 0x8000
>> +#endif
>>          mfspr   r11, SPRN_M_TW/* Get level 1 table base address */
>>          beq     3f
>>          lis     r11, (swapper_pg_dir-PAGE_OFFSET)@ha
>>   3:
>> +#ifdef CONFIG_8xx_CPU6
>> +       mtcr    r3
>> +#else
>> +       mtcr    r10
>> +       mfspr   r10, SPRN_MD_EPN
>> +#endif
>> +
>>          /* Insert level 1 index */
>>          rlwimi  r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) << 1, 29
>>          lwz     r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level 1 entry */
>> @@ -441,8 +464,6 @@ DataStoreTLBMiss:
>>          mfspr   r3, SPRN_DAR
>>   #endif
>>          mtspr   SPRN_DAR, r11/* Tag DAR */
>> -       mfspr   r10, SPRN_SPRG_SCRATCH2
>> -       mtcr    r10
>>          EXCEPTION_EPILOG_0
>>          rfi
>>   

^ permalink raw reply

* Re: [PATCH v6 2/4] tools/perf: extend format_alias() to include event parameters
From: Jiri Olsa @ 2015-01-06  9:39 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: peterz, linux-kernel, Arnaldo Carvalho de Melo, dev,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <1419234567-22784-3-git-send-email-sukadev@linux.vnet.ibm.com>

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?

jirka

^ permalink raw reply

* Re: Build regressions/improvements in v3.19-rc3
From: Geert Uytterhoeven @ 2015-01-06  9:39 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org
  Cc: linuxppc-dev@lists.ozlabs.org, Pratik Patel,
	linux-arm-kernel@lists.infradead.org, Mathieu Poirier,
	Linux-sh list
In-Reply-To: <1420536979-31149-1-git-send-email-geert@linux-m68k.org>

On Tue, Jan 6, 2015 at 10:36 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> JFYI, when comparing v3.19-rc3[1] to v3.19-rc2[3], the summaries are:
>   - build errors: +64/-9

  + /home/kisskb/slave/src/arch/arm/mm/dma-mapping.c: error:
'VM_ARM_DMA_CONSISTENT' undeclared (first use in this function):  =>
1200:4, 1281:29, 1410:4
  + /home/kisskb/slave/src/arch/arm/mm/dma-mapping.c: error:
'atomic_pool' undeclared (first use in this function):  => 1264:31
  + /home/kisskb/slave/src/arch/arm/mm/dma-mapping.c: error: implicit
declaration of function '__in_atomic_pool'
[-Werror=implicit-function-declaration]:  => 1274:2
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_BUFFERABLE' undeclared here (not in a function):  => 81:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_DEV_CACHED' undeclared here (not in a function):  => 117:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_DEV_NONSHARED' undeclared here (not in a function):  =>
108:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_DEV_SHARED' undeclared here (not in a function):  => 103:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_DEV_WC' undeclared here (not in a function):  => 113:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'L_PTE_MT_MASK'
undeclared here (not in a function):  => 76:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_MINICACHE' undeclared here (not in a function):  => 94:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_UNCACHED' undeclared here (not in a function):  => 77:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_WRITEALLOC' undeclared here (not in a function):  => 99:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_WRITEBACK' undeclared here (not in a function):  => 89:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'L_PTE_MT_WRITETHROUGH' undeclared here (not in a function):  => 85:10
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'L_PTE_RDONLY'
undeclared here (not in a function):  => 61:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'L_PTE_SHARED'
undeclared here (not in a function):  => 71:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'L_PTE_USER'
undeclared here (not in a function):  => 56:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'L_PTE_XN'
undeclared here (not in a function):  => 66:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'PMD_SECT_AP_READ' undeclared here (not in a function):  => 153:13
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'PMD_SECT_AP_WRITE' undeclared here (not in a function):  => 153:32
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'PMD_SECT_S'
undeclared here (not in a function):  => 175:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'PMD_SECT_XN'
undeclared here (not in a function):  => 170:11
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'PMD_SIZE'
undeclared (first use in this function):  => 279:22
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'PTRS_PER_PGD'
undeclared (first use in this function):  => 319:23
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'PTRS_PER_PMD'
undeclared (first use in this function):  => 278:18
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'PTRS_PER_PTE'
undeclared (first use in this function):  => 266:18
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: 'SECTION_SIZE'
undeclared (first use in this function):  => 285:7
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error:
'USER_PGTABLES_CEILING' undeclared (first use in this function):  =>
223:13, 311:23
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[0].mask'):  => 153:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[1].mask'):  => 157:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[1].val'):  => 158:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[2].mask'):  => 161:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[2].val'):  => 162:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[3].mask'):  => 165:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[3].val'):  => 166:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[4].mask'):  => 170:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[4].val'):  => 171:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[5].mask'):  => 175:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: (near
initialization for 'section_bits[5].val'):  => 176:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: implicit
declaration of function 'pmd_large'
[-Werror=implicit-function-declaration]:  => 280:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: implicit
declaration of function 'pmd_none'
[-Werror=implicit-function-declaration]:  => 280:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: implicit
declaration of function 'pmd_present'
[-Werror=implicit-function-declaration]:  => 280:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: implicit
declaration of function 'pte_offset_kernel'
[-Werror=implicit-function-declaration]:  => 262:2
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: initializer
element is not constant:  => 158:3, 162:3, 176:3, 170:3, 157:3, 175:3,
161:3, 153:3, 171:3, 165:3, 166:3
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: invalid operands
to binary * (have 'unsigned int' and 'const struct prot_bits *'):  =>
279:20
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: invalid operands
to binary / (have 'const struct prot_bits *' and 'long unsigned int'):
 => 311:45
  + /home/kisskb/slave/src/arch/arm/mm/dump.c: error: invalid operands
to binary | (have 'const struct prot_bits *' and 'const struct
prot_bits *'):  => 165:30, 153:30, 166:30, 157:30, 161:30
  + /home/kisskb/slave/src/arch/arm/mm/init.c: error: 'SECTION_SIZE'
undeclared (first use in this function):  => 733:2
  + /home/kisskb/slave/src/arch/arm/mm/init.c: error: 'pmdval_t'
undeclared here (not in a function):  => 633:12
  + /home/kisskb/slave/src/arch/arm/mm/init.c: error: expected
declaration specifiers or '...' before 'pmdval_t':  => 678:7, 677:55
  + /home/kisskb/slave/src/arch/arm/mm/init.c: error: implicit
declaration of function 'section_update'
[-Werror=implicit-function-declaration]:  => 733:2
  + /home/kisskb/slave/src/arch/arm/mm/init.c: error: unknown type
name 'pmdval_t':  => 625:2, 623:2, 624:2

arm-randconfig (CONFIG_ARM_PTDUMP is broken?)

  + /home/kisskb/slave/src/drivers/coresight/coresight-etb10.c: error:
implicit declaration of function 'of_get_coresight_platform_data'
[-Werror=implicit-function-declaration]:  => 430:3
  + /home/kisskb/slave/src/drivers/coresight/coresight-funnel.c:
error: implicit declaration of function
'of_get_coresight_platform_data'
[-Werror=implicit-function-declaration]:  => 189:3
  + /home/kisskb/slave/src/drivers/coresight/coresight-replicator.c:
error: implicit declaration of function
'of_get_coresight_platform_data'
[-Werror=implicit-function-declaration]:  => 72:3
  + /home/kisskb/slave/src/drivers/coresight/coresight-tmc.c: error:
implicit declaration of function 'of_get_coresight_platform_data'
[-Werror=implicit-function-declaration]:  => 625:3
  + /home/kisskb/slave/src/drivers/coresight/coresight-tpiu.c: error:
implicit declaration of function 'of_get_coresight_platform_data'
[-Werror=implicit-function-declaration]:  => 131:3

arm-randconfig

  + /home/kisskb/slave/src/arch/sh/mm/cache-sh4.c: error:
'cached_to_uncached' undeclared (first use in this function):  =>
99:17
  + /home/kisskb/slave/src/arch/sh/mm/cache-sh4.c: error: implicit
declaration of function 'cpu_context'
[-Werror=implicit-function-declaration]:  => 192:2

sh-randconfig

  + /home/kisskb/slave/src/kernel/bounds.c: error: -mcall-aixdesc must
be big endian:  => 1:0
  + /home/kisskb/slave/src/scripts/mod/devicetable-offsets.c: error:
-mcall-aixdesc must be big endian:  => 1:0
  + /home/kisskb/slave/src/scripts/mod/empty.c: error: -mcall-aixdesc
must be big endian:  => 1:0
  + <stdin>: error: -mcall-aixdesc must be big endian:  => 1:0

powerpc-randconfig

> [1] http://kisskb.ellerman.id.au/kisskb/head/8277/ (255 out of 262 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/8257/ (all 262 configs)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v6 2/4] tools/perf: extend format_alias() to include event parameters
From: Jiri Olsa @ 2015-01-06  9:40 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: peterz, linux-kernel, Arnaldo Carvalho de Melo, dev,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <1419234567-22784-3-git-send-email-sukadev@linux.vnet.ibm.com>

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);

also please enclose above loop and if in {}

jirka

^ permalink raw reply

* Re: [PATCH v6 1/4] tools/perf: support parsing parameterized events
From: Jiri Olsa @ 2015-01-06  9:42 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: peterz, linux-kernel, Arnaldo Carvalho de Melo, dev,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <20141223195850.GA26952@us.ibm.com>

On Tue, Dec 23, 2014 at 11:58:50AM -0800, Sukadev Bhattiprolu wrote:
> From a24b480ff54381a7e092597864cf615162afdd60 Mon Sep 17 00:00:00 2001
> From: Cody P Schafer <cody@linux.vnet.ibm.com>
> Date: Wed, 24 Sep 2014 12:27:15 -0700
> Subject: [PATCH 1/4] tools/perf: support parsing parameterized events
> 
> Enable event specification like:
> 
> 	pmu/event_name,param1=0x1,param2=0x4/
> 
> Assuming that
> 
> 	/sys/bus/event_source/devices/pmu/events/event_name
> 
> Contains something like
> 
> 	param2=?,bar=1,param1=?
> 
> Changelog[v6]:
> 	[Jiri Olsa] Add a check to make sure that sysfs entries with
> 	parameters exactly match '=?'.
> 
> Changelog[v4]:
> 	[Jiri Olsa] Merge to recent perf-core and fix a small conflict.
> 
> Changelog[v3]:
> 	[Jiri Olsa] If the sysfs event file specifies 'param=val', make the
> 	usage 'hv_24x7/event,param=123/' rather than 'hv_24x7/event,val=123/'.

Acked-by: Jiri Olsa <jolsa@kernel.org>

I'm ok with whole patchset, with some nits for patch 2
that I've already sent out..

Would you mind sending v7 so we avoid confusion for Arnaldo
what to pick up?

thanks,
jirka

^ permalink raw reply

* [PATCH] powernv: Fix OPAL tracepoint code
From: Anton Blanchard @ 2015-01-06 10:12 UTC (permalink / raw)
  To: benh, paulus, mpe; +Cc: linuxppc-dev

Patch c49f63530bb6 ("powernv: Add OPAL tracepoints") has a spurious
store to the stack:

	ld      r12,opal_tracepoint_refcount@toc(r2);           \
	std     r12,32(r1);                                     \

The store was originally used to save the current tracepoint status
so the entry and the exit tracepoints were always balanced. In the
end I just created a separate path when tracepoints are enabled.

The offset on the stack used for this store is not valid for ABIv2
and it causes strange issues. I noticed it because OPAL console input
was broken.

Fixes: c49f63530bb6 ("powernv: Add OPAL tracepoints")
Cc: <stable@vger.kernel.org> # v3.17+
Signed-off-by: Anton Blanchard <anton@samba.org>
---
 arch/powerpc/platforms/powernv/opal-wrappers.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 54eca8b..0509bca 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -40,7 +40,6 @@ BEGIN_FTR_SECTION;						\
 	b	1f;						\
 END_FTR_SECTION(0, 1);						\
 	ld	r12,opal_tracepoint_refcount@toc(r2);		\
-	std	r12,32(r1);					\
 	cmpdi	r12,0;						\
 	bne-	LABEL;						\
 1:
-- 
2.1.0

^ permalink raw reply related

* [PATCH 1/2] jump_label: Allow jump labels to be used in assembly
From: Anton Blanchard @ 2015-01-06 10:20 UTC (permalink / raw)
  To: benh, paulus, mpe, mmarek, akpm, jbaron, peterz, liuj97, mingo,
	mgorman
  Cc: linuxppc-dev, linux-kernel, linux-kbuild

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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox