* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: pacman @ 2010-10-19 18:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Mel Gorman, linux-mm, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <1287483410.2341.66.camel@pasglop>
Benjamin Herrenschmidt writes:
> >
> > I thought of that, but as far as I can tell, this CPU doesn't have DABR.
>
> AFAIK, the 7447 is just a derivative of the 7450 design which -does-
> have a DABR ... Unless it's broken :-)
Hmm. gdb resorts to single-stepping when I set a watchpoint while debugging
some userspace program, which I assumed was caused by lack of hardware
watchpoint support. But that's not important right now.
I made a new discovery. During a test boot while looking at the usual symptom
of a corrupted page cache, I run md5sum /sbin/e2fsck twice and got 2
different results, neither one of them correct. The third time, yet another
different result. A few dozen more times, a few dozen more unique results. I
had somehow managed to get a usable interactive shell while corruption was
ongoing.
So then I ran
dd if=/dev/mem bs=4 count=1 skip=$((0xfc5c080/4)) | od -t x4
a few times very fast, plucking the first affected word directly out of
memory by its physical address. The result:
The low 16 bits are always zero as before. The high 16 bits are a counter,
being incremented at about 1000Hz (as close as I could measure with a crude
shell script. 1024Hz would also be within the margin of error). And it's
little-endian.
While I was watching this happen, there were only 5 or 6 userspace processes
running, and 3 of them were shells. So I doubt that anything in userspace was
doing it. It went on for a few minutes before I exited the interactive shell
and allowed the boot to continue, while keeping an extra shell running on
tty2 to continue making observations. It stopped incrementing almost
immediately.
So what type of driver, firmware, or hardware bug puts a 16-bit 1000Hz timer
in memory, and does it in little-endian instead of the CPU's native byte
order? And why does it stop doing it some time during the early init scripts,
shortly after the root filesystem fsck?
I have not yet attempted to repeat the experiment. If it is repeatable, I'll
probe more deeply into those init scripts later. I'm looking hard at
/etc/rcS.d/S11hwclock.sh
--
Alan Curry
^ permalink raw reply
* Re: Freescale P2020 CPU Freeze over PCIe abort signal
From: Eran Liberty @ 2010-10-19 16:53 UTC (permalink / raw)
To: Eran Liberty; +Cc: linuxppc-dev, linux-pci
In-Reply-To: <4CBC8B40.4060706@extricom.com>
Eran Liberty wrote:
> Eran Liberty wrote:
>> This should probably go to the Freescale support, as it feels like a
>> hardware issue yet the end result is a very frozen Linux kernel so I
>> post here first...
>>
>> I have a programmable FPGA PCIe device connected to a Freescale's
>> P2020 PCIe port. As part of the bring-up tests, we are testing two
>> faulty scenarios:
>> 1. The FPGA totally ignores the PCIe transaction.
>> 2. The FPGA return a transaction abort.
>>
>> Both are plausible PCIe behavior and their should be outcome is
>> documented in the PCIe spec. The first should be terminated by the
>> transaction requestor timeout mechanism and raise an error, the
>> second should abort the transaction and raise and error.
>>
>> In P2020 if I do any of those the CPU is left hung over the transaction.
>>
>> something like:
>> in_le32(addr)
>>
>> is turned into:
>> 7c 00 04 ac sync 7c 00 4c 2c lwbrx r0,0,r9
>> 0c 00 00 00 twi 0,r0,0
>> 4c 00 01 2c isync
>>
>> assembly code, where in r9 (in this example) hold an address which is
>> physically mapped into the PCIe resource space.
>>
>> The CPU will hang over the load instruction.
>>
>> Just for the fun of it, I have wrote my own assembly function
>> omitting everything but the load instruction; still freeze.
>> Replace "lwbrx" with a simple "lwz"; still freeze.
>>
>> It looks like the CPU snoozes till the PCIe transaction is done with
>> no timeouts, ignoring any abort signal.
>>
>> I am going to:
>> A. Try to reach the Freescale support.
>> B. Asked the FPGA designed to give me a new behavior that will stall
>> the PCIe transaction replay for 10 sec, but after those return ok.
>> C. report back here with either A or B.
>>
>> If you have any ideas I would love to hear them.
>>
>> -- Liberty
>>
> Some more info:
>
> As said the the FPGA designer provided me a PCIe device that will
> stall its response to a variable amount of time. The CPU became
> un-frozen after this amount of time. More over, we have found that in
> that period till it un-froze the PCIe core did a retry to that
> transaction over and over every 40 ms. This gave me the bright idea to
> look for the word "retry" in the Freescale documentation which
> rewarded me with these registers:
>
> ------------------------------------------------------- snip
> -------------------------------------------------------
> 16.3.2.3 PCI Express Outbound Completion Timeout Register
> (PEX_OTB_CPL_TOR)
> The PCI Express outbound completion timeout register, shown in Figure
> 16-4, contains the maximum wait
> time for a response to come back as a result of an outbound non-posted
> request before a timeout condition
> occurs.
> Offset
> 0x00C
> Access: Read/Write
> 0 1 5 7
> 8
> 31
> R
> TD
> — TC
> W
> Reset 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0
> 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1
> Figure 16-4. PCI Express Outbound Completion Timeout
> Register (PEX_OTB_CPL_TOR)
> Table 16-6 describes the PCI Express outbound completion timeout
> register fields.
> Table 16-6. PEX_OTB_CPL_TOR Field
> Descriptions
> Bits Name
> Description
> 0 TD Timeout disable. This bit controls the
> enabling/disabling of the timeout function.
> 0 Enable completion timeout
> 1 Disable completion timeout
> 1–7 — Reserved
> 8–31 TC Timeout counter. This is the value that is used to
> load the response counter of the completion timeout.
> One TC unit is 8× the PCI Express controller clock
> period; that is, one TC unit is 20 ns at 400 MHz, and 30
> ns at 266.66 MHz.
> The following are examples of timeout periods based
> on different TC settings:
> 0x00_0000 Reserved
> 0x10_FFFF 22.28 ms at 400 MHz controller clock;
> 33.34 ms at 266.66 MHz controller clock
> 0xFF_FFFF 335.54 ms at 400 MHz controller clock;
> 503.31 ms at 266.66 MHz controller clock
>
>
> 16.3.2.4 PCI Express Configuration Retry Timeout Register
> (PEX_CONF_RTY_TOR)
> The PCI Express configuration retry timeout register, shown in Figure
> 16-5, contains the maximum time
> period during which retries of configuration transactions which
> resulted in a CRS response occur.
> Offset
> 0x010
> Access: Read/Write
> 0 1 3
> 4
> 31
> R
> RD — TC
> W
> Reset 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 1 1 1
> 1 1 1 1 1 1 1 1 1 1 1 1 1
> Figure 16-5. PCI Express Configuration Retry Timeout
> Register (PEX_CONF_RTY_TOR)
> QorIQ P2020 Integrated Processor Reference
> Manual, Rev. 0
> 16-12
> Freescale Semiconductor
>
> PCI Express Interface Controller
> Table 16-7 describes the PCI Express configuration retry timeout
> register fields.
> Table 16-7. PEX_CONF_RTY_TOR Field
> Descriptions
> Bits Name
> Description
> 0 RD Retry disable. This bit disables the retry of a
> configuration transaction that receives a CRS status response
> packet.
> 0 Enable retry of a configuration transaction in
> response to receiving a CRS status response until the timeout
> counter (defined by the PEX_CONF_RTY_TOR[TC] field)
> has expired.
> 1 Disable retry of a configuration transaction
> regardless of receiving a CRS status response.
> 1–3 — Reserved
> 4–31 TC Timeout counter. This is the value that is used to load
> the CRS response counter.
> One TC unit is 8× the PCI Express controller clock
> period; that is, one TC unit is 20 ns at 400 MHz and 30 ns
> at 266.66 MHz.
> Timeout period based on different TC settings:
> 0x000_0000 Reserved
> 0x400_FFFF 1.34 s at 400 MHz controller clock,
> 2.02 s at 266.66 MHz controller clock
> 0xFFF_FFFF 5.37 s at 400 MHz controller clock,
> 8.05 s at 266.66 MHz controller clock
> ------------------------------------------------------- snap
> -------------------------------------------------------
>
> Now this is all nice on the paper, but what the P2020 seems to be
> doing in reality is
> 1. never expire
> 2. do re-tries even in the non configuration access
>
> I am going to try to disable completion timeout and see if I get
> better behavior.
>
> -- Liberty
>
>
Disabling PEX_OTB_CPL_TOR, PEX_CONF_RTY_TOR, or both yields the same
behavior. The kernel freezes over the load command while the underlying
hardware does PCIe transaction retries to infinity and beyond.
-- Liberty
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Helmut Grohne @ 2010-10-19 16:24 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Mel Gorman, LKML, linux-mm, pacman, Andrew Morton, linuxppc-dev
In-Reply-To: <alpine.LFD.2.00.1010182342490.6815@localhost6.localdomain6>
On Mon, Oct 18, 2010 at 11:55:44PM +0200, Thomas Gleixner wrote:
> I might be completely one off as usual, but this thing reminds me of a
> bug I stared at yesterday night:
This problem is completely unrelated. My problem was caused by using
binutils-gold.
Helmut
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Thomas Gleixner @ 2010-10-19 16:42 UTC (permalink / raw)
To: Helmut Grohne
Cc: Mel Gorman, LKML, linux-mm, pacman, Andrew Morton, linuxppc-dev
In-Reply-To: <20101019162407.GB10148@alf.mars>
On Tue, 19 Oct 2010, Helmut Grohne wrote:
> On Mon, Oct 18, 2010 at 11:55:44PM +0200, Thomas Gleixner wrote:
> > I might be completely one off as usual, but this thing reminds me of a
> > bug I stared at yesterday night:
>
> This problem is completely unrelated. My problem was caused by using
> binutils-gold.
Ok, thanks for the update. One thing less to worry about :)
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH] mxc_udc: add workaround for ENGcm09152 for i.MX35
From: Greg KH @ 2010-10-19 16:10 UTC (permalink / raw)
To: Eric Bénard
Cc: dbrownell, Dinh.Nguyen, linux-usb, linux-kernel, linuxppc-dev,
gregkh, linux-arm-kernel
In-Reply-To: <4CB8AA76.2020309@eukrea.com>
On Fri, Oct 15, 2010 at 09:24:38PM +0200, Eric Bénard wrote:
> Hi Greg,
>
> Le 15/10/2010 21:10, Greg KH a écrit :
> >On Fri, Oct 15, 2010 at 02:30:58PM +0200, Eric Bénard wrote:
> >>this patch gives the possibility to workaround bug ENGcm09152
> >>on i.MX35 when the hardware workaround is also implemented on
> >>the board.
> >>It covers the workaround described on page 25 of the following Errata :
> >>http://cache.freescale.com/files/dsp/doc/errata/IMX35CE.pdf
> >>
> >>Signed-off-by: Eric Bénard<eric@eukrea.com>
> >>---
> >> arch/arm/mach-mx3/mach-cpuimx35.c | 1 +
> >> drivers/usb/gadget/fsl_mxc_udc.c | 15 +++++++++++++++
> >> include/linux/fsl_devices.h | 3 +++
> >> 3 files changed, 19 insertions(+), 0 deletions(-)
> >
> >Do you want me to take this through my usb tree, or will it go through
> >some other developer's tree?
> >
> as most of the changes are in drivers/usb that would be great if you can take it.
No problem, now queued up.
greg k-h
^ permalink raw reply
* Re: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
From: Kumar Gala @ 2010-10-19 13:18 UTC (permalink / raw)
To: Roy Zang
Cc: B07421, dedekind1, B25806, linuxppc-dev, linux-mtd, akpm, dwmw2,
B11780
In-Reply-To: <1287386552-10647-1-git-send-email-tie-fei.zang@freescale.com>
On Oct 18, 2010, at 2:22 AM, Roy Zang wrote:
> Move Freescale elbc interrupt from nand dirver to elbc driver.
> Then all elbc devices can use the interrupt instead of ONLY nand.
>=20
> For former nand driver, it had the two functions:
>=20
> 1. detecting nand flash partitions;
> 2. registering elbc interrupt.
>=20
> Now, second function is removed to fsl_lbc.c.
>=20
> Signed-off-by: Lan Chunhe-B25806 <b25806@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>
> Cc: Wood Scott-B07421 <B07421@freescale.com>
> ---
Roy, this is a nit, but are these really p4080 specific? just wondering =
why the subject is P4080/eLBC:...
- k=
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Benjamin Herrenschmidt @ 2010-10-19 10:16 UTC (permalink / raw)
To: pacman; +Cc: Mel Gorman, linux-mm, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <20101018213348.10281.qmail@kosh.dhis.org>
> > >From there, you might be able to close onto the culprit a bit more, for
> > example, try using the DABR register to set data access breakpoints
> > shortly before the corruption spot. AFAIK, On those old 32-bit CPUs, you
> > can set whether you want it to break on a real or a virtual address.
>
> I thought of that, but as far as I can tell, this CPU doesn't have DABR.
> /proc/cpuinfo
> processor : 0
> cpu : 7447/7457
> clock : 999.999990MHz
> revision : 1.1 (pvr 8002 0101)
> bogomips : 66.66
> timebase : 33333333
> platform : CHRP
> model : Pegasos2
> machine : CHRP Pegasos2
> Memory : 512 MB
AFAIK, the 7447 is just a derivative of the 7450 design which -does-
have a DABR ... Unless it's broken :-)
> My next thought was: right after the correct value appears in memory, unmap
> the page from the kernel and let it Oops when it tries to write there. Then I
> found out that the kernel is using BATs instead of page tables for its own
> view of memory. Booting with "nobats" completely changes the memory usage
> pattern (probably because it's allocating a lot of pages to hold PTEs that it
> didn't need before)
Right. And that hides the problem I suppose ?
> > You can also sprinkle tests for the page content through the code if
> > that doesn't work to try to "close in" on the culprit (for example if
> > it's a case of stray DMA, like a network driver bug or such).
>
> No network drivers are loaded when this happens.
Ok.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RFC] PPC: /dev/mem: All RAM is cacheable, not just the kernel's.
From: Li Yang @ 2010-10-19 9:37 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20101018223256.GA30946@udp111988uds.am.freescale.net>
On Tue, Oct 19, 2010 at 6:32 AM, Scott Wood <scottwood@freescale.com> wrote=
:
> If mem=3D is used on the kernel command line to create reserved regions
> for userspace to map using /dev/mem, let it be mapped cacheable as long
> as it is within the memory region described in the device tree.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> This isn't just a performance issue, but it could also be a correctness
> issue, if the reserved portion of RAM is mapped cacheable by e.g. a KVM
> guest. =C2=A0This patch does not address cases where such regions could s=
how up
> as something other than a standard memory node -- such as shared regions
> in an AMP configuration. =C2=A0Ideally there would be some means for a pl=
atform
> to register cacheable regions, without having to completely replace the
> entire phys_mem_access_prot function.
Agreed. Such requirement might be special case in server/desktop
world, but much more common in embedded market when migrating to
multi-core in which shared memory is commonly used for inter-core
communication. It will be best to have a common way to achieve this.
Not only cache-ability but also coherent property might need to be tunable.
- Leo
^ permalink raw reply
* unsubscribe
From: Roberto Mantovani @ 2010-10-19 8:51 UTC (permalink / raw)
To: Linuxppc-dev
--
Roberto Mantovani <rmantovani@automazionelogistica.it>
A&L srl - Automazione e Logistica www.automazionelogistica.it
Via Lidice, 20
40139 Bologna
Cap Sociale € 10.000 I.V.
C.F., P.IVA e registro imprese di Bologna N.02296311208
^ permalink raw reply
* RE: eSDHC controller driver on MPC8308rdb
From: Maria Johansen @ 2010-10-19 8:49 UTC (permalink / raw)
To: Bo.Liu; +Cc: linuxppc-dev
In-Reply-To: <4CBD4EFB.6070603@windriver.com>
Pi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+RnJvbTogVG9ueWxpdSBbbWFpbHRvOkJvLkxp
dUB3aW5kcml2ZXIuY29tXSANCj48c25pcD4NCj4+IFRoYW5rcywgdGhlIG1pc3NpbmcgY2xvY2st
ZnJlcXVlbmN5IHdhcyBwYXJ0IG9mIG15IHByb2JsZW0uIFRoZSByZXN0IHdhcyBzb2x2ZWQgYnkg
YWRkaW5nIHNvbWUgbW9yZSBxdWlya3MgdG8gdGhlIGVzZGhjLWRyaXZlciAoU0RIQ0lfUVVJUktf
Rk9SQ0VfMV9CSVRfREFUQSBhbmQgPlNESENJX1FVSVJLX1JFU0VUX0FGVEVSX1JFUVVFU1QpLiBX
aHkgaXQgaXMgbmVjZXNzYXJ5IHRvIGZvcmNlIDEtYml0IGRhdGEgdHJhbnNmZXJzIEkgYW0gYSBi
aXQgdW5zdXJlIGFib3V0LCBzaW5jZSB0aGUgbXBjODMwOCByZWZlcmVuY2UgbWFudWFsIGNsZWFy
bHkgc3RhdGVzIHRoYXQgNC1iaXQgPnRyYW5zZmVycyBhcmUgc3VwcG9ydGVkLg0KPj4gICANCj5B
Y3R1YWxseSwgSSBkaWQgdXNlIDQtYml0IG1vZGUuIFlvdSBjYW4gdHJ5IGFub3RoZXIgMiBxdWlj
a3M6DQo+DQo+KyAgIGlmIChvZl9nZXRfcHJvcGVydHkobnAsICJzZGhjaSxhdXRvLWNtZDEyIiwg
TlVMTCkpDQo+KyAgICAgICBob3N0LT5xdWlya3MgfD0gU0RIQ0lfUVVJUktfTVVMVElCTE9DS19S
RUFEX0FDTUQxMjsNCj4rDQo+KyAgIGlmIChvZl9nZXRfcHJvcGVydHkobnAsICJzZGhjaSxicm9r
ZW4tdGltZW91dCIsIE5VTEwpKQ0KPisgICAgICAgaG9zdC0+cXVpcmtzIHw9IFNESENJX1FVSVJL
X0JST0tFTl9USU1FT1VUX1ZBTDsNCj4rDQo+DQoNClNESENJX1FVSVJLX01VTFRJQkxPQ0tfUkVB
RF9BQ01EMTIgd29ya2VkICh3aXRoIG15IG90aGVyIHF1aXJrcyBkaXNhYmxlZCksIHRoYW5rcyEN
Cg0KLS0NCk1hcmlhDQo=
^ permalink raw reply
* [QUESTION] MPC8343 'internal only' DMA support
From: KRONSTORFER Horst @ 2010-10-19 8:15 UTC (permalink / raw)
To: linuxppc-dev
hi!
i assume the mpc8343 dma controllers ability to do internally controlled
operations (csb/csb)
is _not_ affected by deactivating externally controlled operations via
pinmultiplexing in sicrl.
am I correct?=20
thanks
horst kronstorfer
^ permalink raw reply
* RE: eSDHC controller driver on MPC8308rdb
From: Zang Roy-R61911 @ 2010-10-19 8:09 UTC (permalink / raw)
To: Bo.Liu, Maria Johansen; +Cc: linuxppc-dev
In-Reply-To: <4CBD4EFB.6070603@windriver.com>
> -----Original Message-----
> From: =
linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.org
>
[mailto:linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.or=
g
] On
> Behalf Of Tonyliu
> Sent: Tuesday, October 19, 2010 15:56 PM
> To: Maria Johansen
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: eSDHC controller driver on MPC8308rdb
>=20
> Maria Johansen wrote:
> >
> >> -----Original Message-----
> >> From: Tonyliu [mailto:Bo.Liu@windriver.com]
> >> <snip>
> >>
> >>> Could this be a problem related the eSDHC controller (or the
driver),
> >>> or is it the memory card? (a 4GB SanDisk Extreme SDHC card, which
> unfortunately is the only card I have available at the moment.) I will
keep
> digging into drivers/mmc/host/sdhci.c in search of a solution, and any
tips to
> how I should proceed would be greatly appreciated!
> >>>
> >>>
> >> Hi,
> >> Try the patch in attatchment. By default, on some e300 platforms
such as
> mpc8308_rdb, the entry "clock-frequency" of section sdchi in DTB is
not fixed
> by u-boot, so has to >caculate the input clock for sdhci controller
explicitly
> in driver.
> >>
> >> Tony
> >>
> >
> > Thanks, the missing clock-frequency was part of my problem. The rest
was
> solved by adding some more quirks to the esdhc-driver
> (SDHCI_QUIRK_FORCE_1_BIT_DATA and SDHCI_QUIRK_RESET_AFTER_REQUEST).
Why it is
> necessary to force 1-bit data transfers I am a bit unsure about, since
the
> mpc8308 reference manual clearly states that 4-bit transfers are
supported.
> >
> Actually, I did use 4-bit mode. You can try another 2 quicks:
>=20
> + if (of_get_property(np, "sdhci,auto-cmd12", NULL))
> + host->quirks |=3D SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
> +
> + if (of_get_property(np, "sdhci,broken-timeout", NULL))
> + host->quirks |=3D SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> +
>=20
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 9d4fdfa..b7a1ce4 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -807,8 +807,12 @@ static void sdhci_set_transfer_mode(struct
> sdhci_host *host,
> WARN_ON(!host->data);
>=20
> mode =3D SDHCI_TRNS_BLK_CNT_EN;
> - if (data->blocks > 1)
> + if (data->blocks > 1) {
> + if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> + mode |=3D SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
> + else
> mode |=3D SDHCI_TRNS_MULTI;
> + }
> if (data->flags & MMC_DATA_READ)
> mode |=3D SDHCI_TRNS_READ;
> if (host->flags & SDHCI_REQ_USE_DMA)
> @@ -1099,6 +1103,13 @@ static void sdhci_request(struct mmc_host *mmc,
> struct mmc_request *mrq)
> sdhci_activate_led(host);
> #endif
>=20
> + if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
> + if (mrq->stop) {
> + mrq->data->stop =3D NULL;
> + mrq->stop =3D NULL;
> + }
> + }
> +
> host->mrq =3D mrq;
>=20
>=20
> I don't have patch for upstream, you can add them manually.
https://patchwork.kernel.org/patch/176752/
Roy
^ permalink raw reply
* Re: eSDHC controller driver on MPC8308rdb
From: Tonyliu @ 2010-10-19 7:55 UTC (permalink / raw)
To: Maria Johansen; +Cc: linuxppc-dev
In-Reply-To: <DF61B4AD1C20A54EADF5C05E1F1110DE08D44C@Exchange.parkairsystems.net>
Maria Johansen wrote:
>
>> -----Original Message-----
>> From: Tonyliu [mailto:Bo.Liu@windriver.com]
>> <snip>
>>
>>> Could this be a problem related the eSDHC controller (or the driver),
>>> or is it the memory card? (a 4GB SanDisk Extreme SDHC card, which unfortunately is the only card I have available at the moment.) I will keep digging into drivers/mmc/host/sdhci.c in search of a solution, and any tips to how I should proceed would be greatly appreciated!
>>>
>>>
>> Hi,
>> Try the patch in attatchment. By default, on some e300 platforms such as mpc8308_rdb, the entry "clock-frequency" of section sdchi in DTB is not fixed by u-boot, so has to >caculate the input clock for sdhci controller explicitly in driver.
>>
>> Tony
>>
>
> Thanks, the missing clock-frequency was part of my problem. The rest was solved by adding some more quirks to the esdhc-driver (SDHCI_QUIRK_FORCE_1_BIT_DATA and SDHCI_QUIRK_RESET_AFTER_REQUEST). Why it is necessary to force 1-bit data transfers I am a bit unsure about, since the mpc8308 reference manual clearly states that 4-bit transfers are supported.
>
Actually, I did use 4-bit mode. You can try another 2 quicks:
+ if (of_get_property(np, "sdhci,auto-cmd12", NULL))
+ host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
+
+ if (of_get_property(np, "sdhci,broken-timeout", NULL))
+ host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+
===================================================
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9d4fdfa..b7a1ce4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -807,8 +807,12 @@ static void sdhci_set_transfer_mode(struct
sdhci_host *host,
WARN_ON(!host->data);
mode = SDHCI_TRNS_BLK_CNT_EN;
- if (data->blocks > 1)
+ if (data->blocks > 1) {
+ if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+ mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
+ else
mode |= SDHCI_TRNS_MULTI;
+ }
if (data->flags & MMC_DATA_READ)
mode |= SDHCI_TRNS_READ;
if (host->flags & SDHCI_REQ_USE_DMA)
@@ -1099,6 +1103,13 @@ static void sdhci_request(struct mmc_host *mmc,
struct mmc_request *mrq)
sdhci_activate_led(host);
#endif
+ if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
+ if (mrq->stop) {
+ mrq->data->stop = NULL;
+ mrq->stop = NULL;
+ }
+ }
+
host->mrq = mrq;
I don't have patch for upstream, you can add them manually.
Tony
> Thanks for your help :)
>
> --
> Maria
>
>
>
>
>
^ permalink raw reply related
* RE: eSDHC controller driver on MPC8308rdb
From: Maria Johansen @ 2010-10-19 7:26 UTC (permalink / raw)
To: Bo.Liu; +Cc: linuxppc-dev
In-Reply-To: <4CBC04E8.5030706@windriver.com>
DQoNCj4tLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPkZyb206IFRvbnlsaXUgW21haWx0bzpC
by5MaXVAd2luZHJpdmVyLmNvbV0gDQo+PHNuaXA+DQo+PiBDb3VsZCB0aGlzIGJlIGEgcHJvYmxl
bSByZWxhdGVkIHRoZSBlU0RIQyBjb250cm9sbGVyIChvciB0aGUgZHJpdmVyKSwgDQo+PiBvciBp
cyBpdCB0aGUgbWVtb3J5IGNhcmQ/IChhIDRHQiBTYW5EaXNrIEV4dHJlbWUgU0RIQyBjYXJkLCB3
aGljaCB1bmZvcnR1bmF0ZWx5IGlzIHRoZSBvbmx5IGNhcmQgSSBoYXZlIGF2YWlsYWJsZSBhdCB0
aGUgbW9tZW50LikgSSB3aWxsIGtlZXAgZGlnZ2luZyBpbnRvIGRyaXZlcnMvbW1jL2hvc3Qvc2Ro
Y2kuYyBpbiBzZWFyY2ggb2YgYSBzb2x1dGlvbiwgYW5kIGFueSB0aXBzIHRvIGhvdyBJIHNob3Vs
ZCBwcm9jZWVkIHdvdWxkIGJlIGdyZWF0bHkgYXBwcmVjaWF0ZWQhDQo+PiAgIA0KPkhpLA0KPlRy
eSB0aGUgcGF0Y2ggaW4gYXR0YXRjaG1lbnQuIEJ5IGRlZmF1bHQsIG9uIHNvbWUgZTMwMCBwbGF0
Zm9ybXMgc3VjaCBhcyBtcGM4MzA4X3JkYiwgdGhlIGVudHJ5ICJjbG9jay1mcmVxdWVuY3kiIG9m
IHNlY3Rpb24gc2RjaGkgaW4gRFRCIGlzIG5vdCBmaXhlZCBieSB1LWJvb3QsIHNvIGhhcyB0byA+
Y2FjdWxhdGUgdGhlIGlucHV0IGNsb2NrIGZvciBzZGhjaSBjb250cm9sbGVyIGV4cGxpY2l0bHkg
aW4gZHJpdmVyLg0KPg0KPlRvbnkNCg0KVGhhbmtzLCB0aGUgbWlzc2luZyBjbG9jay1mcmVxdWVu
Y3kgd2FzIHBhcnQgb2YgbXkgcHJvYmxlbS4gVGhlIHJlc3Qgd2FzIHNvbHZlZCBieSBhZGRpbmcg
c29tZSBtb3JlIHF1aXJrcyB0byB0aGUgZXNkaGMtZHJpdmVyIChTREhDSV9RVUlSS19GT1JDRV8x
X0JJVF9EQVRBIGFuZCBTREhDSV9RVUlSS19SRVNFVF9BRlRFUl9SRVFVRVNUKS4gV2h5IGl0IGlz
IG5lY2Vzc2FyeSB0byBmb3JjZSAxLWJpdCBkYXRhIHRyYW5zZmVycyBJIGFtIGEgYml0IHVuc3Vy
ZSBhYm91dCwgc2luY2UgdGhlIG1wYzgzMDggcmVmZXJlbmNlIG1hbnVhbCBjbGVhcmx5IHN0YXRl
cyB0aGF0IDQtYml0IHRyYW5zZmVycyBhcmUgc3VwcG9ydGVkLg0KDQpUaGFua3MgZm9yIHlvdXIg
aGVscCA6KQ0KDQotLQ0KTWFyaWENCg0KDQoNCg0K
^ permalink raw reply
* RE: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
From: Zang Roy-R61911 @ 2010-10-19 6:37 UTC (permalink / raw)
To: Wood Scott-B07421, tiejun.chen
Cc: dedekind1, Lan Chunhe-B25806, linuxppc-dev, linux-mtd, akpm,
dwmw2, Gala Kumar-B11780
In-Reply-To: <20101018110631.22d6f081@udp111988uds.am.freescale.net>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, October 19, 2010 0:07 AM
> To: tiejun.chen
> Cc: Zang Roy-R61911; linux-mtd@lists.infradead.org; Wood Scott-B07421;
> dedekind1@gmail.com; Lan Chunhe-B25806; linuxppc-dev@ozlabs.org;
akpm@linux-
> foundation.org; dwmw2@infradead.org; Gala Kumar-B11780
> Subject: Re: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt
common to
> elbc devices
>=20
> On Mon, 18 Oct 2010 16:55:49 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>=20
> > Looks you always iounmap(fsl_lbc_ctrl_dev->regs) on position 'err'
but here
> > of_iomap() is already failed you should skip iounmap()
fsl_lbc_ctrl_dev-
> >regs
> > again. So you should improve that as the following on 'err', or
layout 'err'
> in
> > gain.
> > ------
> > if(fsl_lbc_ctrl_dev->regs)
> > iounmap(fsl_lbc_ctrl_dev->regs);
> >
>=20
> It looks like iounmap(NULL) is a no-op, just like kfree(NULL).
Thanks for the reminder. I will keep original post, if there is no more
comment.
Roy
^ permalink raw reply
* Re: linux-next: manual merge of the tip tree with the powerpc tree
From: Stephen Rothwell @ 2010-10-19 4:56 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra
Cc: linux-next, Paul Mackerras, linux-kernel, linuxppc-dev
In-Reply-To: <20101019154849.126a736a.sfr@canb.auug.org.au>
Hi again,
On Tue, 19 Oct 2010 15:48:49 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the tip tree got a conflict in
> arch/powerpc/kernel/time.c between commit
> cf9efce0ce3136fa076f53e53154e98455229514 ("powerpc: Account time using
> timebase rather than PURR") from the powerpc tree and commit
> e360adbe29241a0194e10e20595360dd7b98a2b3 ("irq_work: Add generic hardirq
> context callbacks") from the tip tree.
>
> Just context changes. I fixed it up and can carry the fix as necessary.
I forgot the merge diff, sorry:
diff --cc arch/powerpc/kernel/time.c
index 644f918,54888eb..0000000
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@@ -578,9 -585,11 +578,9 @@@ void timer_interrupt(struct pt_regs * r
old_regs = set_irq_regs(regs);
irq_enter();
- if (test_perf_event_pending()) {
- clear_perf_event_pending();
- perf_event_do_pending();
- calculate_steal_time();
-
+ if (test_irq_work_pending()) {
+ clear_irq_work_pending();
+ irq_work_run();
}
#ifdef CONFIG_PPC_ISERIES
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
^ permalink raw reply
* linux-next: manual merge of the tip tree with the powerpc tree
From: Stephen Rothwell @ 2010-10-19 4:48 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Peter Zijlstra
Cc: linux-next, Paul Mackerras, linux-kernel, linuxppc-dev
Hi all,
Today's linux-next merge of the tip tree got a conflict in
arch/powerpc/kernel/time.c between commit
cf9efce0ce3136fa076f53e53154e98455229514 ("powerpc: Account time using
timebase rather than PURR") from the powerpc tree and commit
e360adbe29241a0194e10e20595360dd7b98a2b3 ("irq_work: Add generic hardirq
context callbacks") from the tip tree.
Just context changes. I fixed it up and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
^ permalink raw reply
* Re: CONFIG_FEC is not good for mpc8xx ethernet?
From: tiejun.chen @ 2010-10-19 2:03 UTC (permalink / raw)
To: Scott Wood; +Cc: ppcdev
In-Reply-To: <20101018114214.2e72a616@udp111988uds.am.freescale.net>
Scott Wood wrote:
> On Mon, 18 Oct 2010 16:40:42 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>
>> Shawn Jin wrote:
>>> Hi,
>>>
>>> My target is a mpc875 based board and has FEC ethernet. The phy is
>>> AM79C874. I have the following configuration for the network support.
>>>
>>> CONFIG_PHYLIB=y
>>> CONFIG_NET_ETHERNET=y
>>> CONFIG_MII=y
>>> CONFIG_FS_ENET=y
>>> CONFIG_FS_ENET_HAS_FEC=y
>>> CONFIG_FS_ENET_MDIO_FEC=y
>>>
>>> However I found that the phy support (AM79C874) is actually in
>>> drivers/net/fec.c which is compiled only when CONFIG_FEC=y. However
>> The phy driver should not be embedded into the NIC driver in theory.
>
> Right, those are handled by drivers/net/phy/.
>
>> I think you should include the phy driver, mdio-bitbang.c, which should be
>> support AMD79C874.
>
> On MPC8xx you want drivers/net/fs_enet/mii-fec.c. This is just the
> MDIO driver; it doesn't handle any particular PHY. I don't know if
> there is a driver specifically for AM79C874, though the generic PHY
> support may be good enough.
Maybe.
I can found one related patch for supporting PHY AM79C874 on 2.6.15,
------
http://lists.ozlabs.org/pipermail/linuxppc-embedded/2005-November/021043.html
But I don't see that on the latest kernel, and also I don't know the history
completely for that. Maybe its already merged into one generic PHY driver but
I'm not sure.
Tiejun
>
> -Scott
>
>
^ permalink raw reply
* Re: [PATCH 1/2] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices
From: tiejun.chen @ 2010-10-19 1:35 UTC (permalink / raw)
To: Scott Wood
Cc: B07421, dedekind1, B25806, linuxppc-dev, linux-mtd, akpm, dwmw2,
B11780
In-Reply-To: <20101018110631.22d6f081@udp111988uds.am.freescale.net>
Scott Wood wrote:
> On Mon, 18 Oct 2010 16:55:49 +0800
> "tiejun.chen" <tiejun.chen@windriver.com> wrote:
>
>> Looks you always iounmap(fsl_lbc_ctrl_dev->regs) on position 'err' but here
>> of_iomap() is already failed you should skip iounmap() fsl_lbc_ctrl_dev->regs
>> again. So you should improve that as the following on 'err', or layout 'err' in
>> gain.
>> ------
>> if(fsl_lbc_ctrl_dev->regs)
>> iounmap(fsl_lbc_ctrl_dev->regs);
>>
>
> It looks like iounmap(NULL) is a no-op, just like kfree(NULL).
>
Absolutely, I know what you mean :)
But I think we should take care of every line as one normal progress/rule. That
will make us understand easily, and maybe we also would benefit a little good
performance from those codes.
Tiejun
> -Scott
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: BUG: dead loop in PowerPC hcall tracepoint (Was: [LTP] [PATCH v2] Add ftrace-stress-test to LTP)
From: Li Zefan @ 2010-10-19 0:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: ltp-list, Peter Zijlstra, linuxppc-dev, LKML, Paul Mackerras,
Anton Blanchard, Ingo Molnar, subrata
In-Reply-To: <1287411903.16971.275.camel@gandalf.stny.rr.com>
Steven Rostedt wrote:
> On Mon, 2010-10-18 at 11:19 +0800, Li Zefan wrote:
>
>> This is a dead loop:
>>
>> trace_hcall_entry() -> trace_clock_global() -> trace_hcall_entry() ..
>>
>> And this is a PPC specific bug. Hope some ppc guys will fix it?
>> Or we kill trace_clock_global() if no one actually uses it..
>
> trace_clock_global() is used by many. I use it (and recommend using it)
> on boxes where the TSC is horribly out of sync, and the trace needs
> synchronization between CPUs.
>
> The trace_hcall_entry and exit has wrappers already. Just add recursion
> protection there.
>
Right, I thought of this. But as I have no machine to test, I'll leave
this to others.
> Perhaps something like this:
>
> (Not compiled nor ran)
>
> +static DEFINE_PER_CPU(hcall_trace_disable);
> +
> void hcall_tracepoint_regfunc(void)
> {
> hcall_tracepoint_refcount++;
> }
>
> void hcall_tracepoint_unregfunc(void)
> {
> hcall_tracepoint_refcount--;
> }
>
> +int __trace_disable_check(void)
> +{
> + if (!hcall_tracepoint_refcount)
> + return 1;
> +
> + if (get_cpu_var(hcall_trace_disable)) {
> + put_cpu_var(hcall_trace_disable);
> + return 1;
> + }
> +
> + __get_cpu_var(hcall_trace_disable)++;
> +
> + return 0;
> +}
> +
> +void __trace_disable_put(void)
> +{
> + __get_cpu_var(hcall_trace_disable)--;
> + put_cpu_var(hcall_trace_disable);
> +}
> +
> void __trace_hcall_entry(unsigned long opcode, unsigned long *args)
> {
> + int trace_disable;
> +
> + if (__trace_disable_check())
> + return;
> +
> trace_hcall_entry(opcode, args);
> + __trace_disable_put();
> }
>
> void __trace_hcall_exit(long opcode, unsigned long retval,
> unsigned long *retbuf)
> {
> + if (__trace_disable_check())
> + return;
> +
> trace_hcall_exit(opcode, retval, retbuf);
> + __trace_disable_put();
> }
>
> -- Steve
>
>
>
>
^ permalink raw reply
* Re: [PATCH 2/2] ppc: lazy flush_tlb_mm for nohash architectures
From: Benjamin Herrenschmidt @ 2010-10-18 23:34 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev list
In-Reply-To: <1287439021.3700.22.camel@shaggy-w500>
On Mon, 2010-10-18 at 16:57 -0500, Dave Kleikamp wrote:
>
> I didn't like the implementation of a per-core stale map. The existing
> implementation flushes the core's tlb, but only clears a specific entry
> from the stale map. It's dealing with the stale contexts one at a time,
> where the new function is accumulating many stale contexts, with the
> intent to do a single tlb flush per core.
Right, because I wrote it with A2 in mind which has a TLB invalidate by
PID instruction :-) So I don't flush the whole TLB there... but then
this instruction can take hundreds (or more) of cycles so it might not
necessarily be that great anyways...
> Since I originally intended the new function only to be enabled on the
> 47x, I left the context-stealing code as untouched as possible thinking
> it wouldn't be exercised in 47x-land. This was probably narrow-minded,
> and I should look at either 1) aligning the context-stealing code closer
> to the new lazy flush code, or 2) dropping this code on the floor and
> picking back up the new design that we worked on last year.
In any case, we can probably merge you current stuff upstream (with
appropriate bug fixes if necessary) for now and move from there.
> > At the very least, the "old style" stale map code and "new style" stale
> > TLB code should be more in sync, you may end up flushing the TLB
> > twice...
>
> yeah. if we enable this for 440, it is more likely to be an issue than
> on 476.
Right.
> > > void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > > {
> > > unsigned int i, id, cpu = smp_processor_id();
> > > @@ -197,6 +260,8 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > > /* No lockless fast path .. yet */
> > > raw_spin_lock(&context_lock);
> > >
> > > + flush_recycled_contexts(cpu);
> > > +
> >
> > Ok so here's the nasty one I think. You need to make sure that whenever
> > you pick something off the context_available_map, you've done the above
> > first within the same context_lock section right ? At least before you
> > actually -use- said context.
>
> right.
>
> > So far so good ... but steal_context can drop the lock iirc. So you may
> > need to re-flush there. Not sure that can happen in practice but better
> > safe than sorry. I would have preferred seeing that flush near the end
> > of the function to avoid such problem.
>
> I can fix this. For 476, I don't think that even if steal_context()
> could be called, it wouldn't drop the lock. But then again, if we
> enable this for other architectures, it may be a possibility.
Yeah, it's a minor issue but I'd rather get the code right to avoid
surprises later.
.../...
> > Now...
> >
> > > +/*
> > > + * This is called from flush_tlb_mm(). Mark the current context as stale
> > > + * and grab an available one. The tlb will be flushed when no more
> > > + * contexts are available
> > > + */
> > > +void lazy_flush_context(struct mm_struct *mm)
> > > +{
> > > + unsigned int id;
> > > + unsigned long flags;
> > > + unsigned long *map;
> > > +
> > > + raw_spin_lock_irqsave(&context_lock, flags);
> > > +
> > > + id = mm->context.id;
> > > + if (unlikely(id == MMU_NO_CONTEXT))
> > > + goto no_context;
> >
> > First thing is ... you reproduce quite a bit of logic from
> > switch_mmu_context() here. Shouldn't it be abstracted in a separate
> > function ?
>
> I'm sure there's something I can do there.
>
> > The other thing here is that another CPU might have done a
> > recycle_stale_contexts() before you get here. IE. Your TLB may be stale.
> > Shouln't you do a flush here ? Since you are picking up a new PID from
> > the context_available_map, it can potentially be stale if your tlb needs
> > flushing due to another CPU having just done a recycle.
>
> It looks like I missed that. It does seem that there should be a flush
> in here.
Ok, so It wasn't just shit in my eyes :-)
> I think I'm going to dust off the newer implementation based on your and
> Paul's design. I can probably get that in good working order without
> too much more work, and it's something we need to look at eventually
> anyway. If I find anything that really gets in my way, I might fix up
> this patch in the mean time.
As you like. As I said earlier, I'm happy to merge a fixed version of
this one first if you think it's going to take a while to get the other
one right. However, I believe this is too late for the next merge window
anyways so that gives you some time ahead to play and make a decision.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RFC] PPC: /dev/mem: All RAM is cacheable, not just the kernel's.
From: Kumar Gala @ 2010-10-18 22:41 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20101018223256.GA30946@udp111988uds.am.freescale.net>
On Oct 18, 2010, at 5:32 PM, Scott Wood wrote:
> If mem=3D is used on the kernel command line to create reserved =
regions
> for userspace to map using /dev/mem, let it be mapped cacheable as =
long
> as it is within the memory region described in the device tree.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
Just a nit, but subject should be powerpc:.. not PPC:
> This isn't just a performance issue, but it could also be a =
correctness
> issue, if the reserved portion of RAM is mapped cacheable by e.g. a =
KVM
> guest. This patch does not address cases where such regions could =
show up
> as something other than a standard memory node -- such as shared =
regions
> in an AMP configuration. Ideally there would be some means for a =
platform
> to register cacheable regions, without having to completely replace =
the
> entire phys_mem_access_prot function.
>=20
> This patch assumes that there is no region between memstart and memend =
that
> must be non-cacheable. This is already the case with the "for now"
> implementation of page_is_ram on 32-bit, but will this be a problem on
> 64-bit?
>=20
> arch/powerpc/kernel/pci-common.c | 5 ++++-
> arch/powerpc/kernel/prom.c | 3 +++
> arch/powerpc/mm/mem.c | 3 ++-
> arch/powerpc/mm/mmu_decl.h | 1 +
> 4 files changed, 10 insertions(+), 2 deletions(-)
For some time I've been meaning for us to look at the address range =
tracking that x86 does so one can make sure we aren't mapping regions =
with different WIMG settings. However, never enough time for this. So =
I think this is reasonable for now. Hopefully Ben will comment on =
64-bit side of the world.
- k=
^ permalink raw reply
* [PATCH RFC] PPC: /dev/mem: All RAM is cacheable, not just the kernel's.
From: Scott Wood @ 2010-10-18 22:32 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
If mem= is used on the kernel command line to create reserved regions
for userspace to map using /dev/mem, let it be mapped cacheable as long
as it is within the memory region described in the device tree.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
This isn't just a performance issue, but it could also be a correctness
issue, if the reserved portion of RAM is mapped cacheable by e.g. a KVM
guest. This patch does not address cases where such regions could show up
as something other than a standard memory node -- such as shared regions
in an AMP configuration. Ideally there would be some means for a platform
to register cacheable regions, without having to completely replace the
entire phys_mem_access_prot function.
This patch assumes that there is no region between memstart and memend that
must be non-cacheable. This is already the case with the "for now"
implementation of page_is_ram on 32-bit, but will this be a problem on
64-bit?
arch/powerpc/kernel/pci-common.c | 5 ++++-
arch/powerpc/kernel/prom.c | 3 +++
arch/powerpc/mm/mem.c | 3 ++-
arch/powerpc/mm/mmu_decl.h | 1 +
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 10a44e6..4298a56 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -39,6 +39,8 @@
#include <asm/firmware.h>
#include <asm/eeh.h>
+#include <mm/mmu_decl.h>
+
static DEFINE_SPINLOCK(hose_spinlock);
LIST_HEAD(hose_list);
@@ -398,7 +400,8 @@ pgprot_t pci_phys_mem_access_prot(struct file *file,
resource_size_t offset = ((resource_size_t)pfn) << PAGE_SHIFT;
int i;
- if (page_is_ram(pfn))
+ if (pfn >= (memstart_addr >> PAGE_SHIFT) &&
+ pfn <= (memend_addr >> PAGE_SHIFT))
return prot;
prot = pgprot_noncached(prot);
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index fed9bf6..f8f5966 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -62,6 +62,8 @@
#define DBG(fmt...)
#endif
+phys_addr_t memend_addr;
+
#ifdef CONFIG_PPC64
int __initdata iommu_is_off;
int __initdata iommu_force_on;
@@ -504,6 +506,7 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
memblock_add(base, size);
memstart_addr = min((u64)memstart_addr, base);
+ memend_addr = max((u64)memend_addr, base + size - 1);
}
u64 __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 1a84a8d..f947a39 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -104,7 +104,8 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
if (ppc_md.phys_mem_access_prot)
return ppc_md.phys_mem_access_prot(file, pfn, size, vma_prot);
- if (!page_is_ram(pfn))
+ if (pfn < (memstart_addr >> PAGE_SHIFT) ||
+ pfn > (memend_addr >> PAGE_SHIFT))
vma_prot = pgprot_noncached(vma_prot);
return vma_prot;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index dd0a258..05f1ac6 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -119,6 +119,7 @@ extern phys_addr_t __initial_memory_limit_addr;
extern phys_addr_t total_memory;
extern phys_addr_t total_lowmem;
extern phys_addr_t memstart_addr;
+extern phys_addr_t memend_addr;
extern phys_addr_t lowmem_end_addr;
#ifdef CONFIG_WII
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/2] ppc: lazy flush_tlb_mm for nohash architectures
From: Dave Kleikamp @ 2010-10-18 21:57 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list
In-Reply-To: <1287017539.2205.48.camel@pasglop>
On Thu, 2010-10-14 at 11:52 +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2010-09-24 at 13:01 -0500, Dave Kleikamp wrote:
> > On PPC_MMU_NOHASH processors that support a large number of contexts,
> > implement a lazy flush_tlb_mm() that switches to a free context, marking
> > the old one stale. The tlb is only flushed when no free contexts are
> > available.
> >
> > The lazy tlb flushing is controlled by the global variable tlb_lazy_flush
> > which is set during init, dependent upon MMU_FTR_TYPE_47x.
>
> Unless I'm mistaken, there are some issues with that patch... sorry for
> the late review, I've been away for a couple of weeks.
>
> > +int tlb_lazy_flush;
> > +static int tlb_needs_flush[NR_CPUS];
> > +static unsigned long *context_available_map;
> > +static unsigned int nr_stale_contexts;
>
> Now I understand what you're doing here, but wouldn't it have been
> possible to re-use the existing stale map concept or do you reckon it
> would have been too messy ?
I didn't like the implementation of a per-core stale map. The existing
implementation flushes the core's tlb, but only clears a specific entry
from the stale map. It's dealing with the stale contexts one at a time,
where the new function is accumulating many stale contexts, with the
intent to do a single tlb flush per core.
Since I originally intended the new function only to be enabled on the
47x, I left the context-stealing code as untouched as possible thinking
it wouldn't be exercised in 47x-land. This was probably narrow-minded,
and I should look at either 1) aligning the context-stealing code closer
to the new lazy flush code, or 2) dropping this code on the floor and
picking back up the new design that we worked on last year.
> At the very least, the "old style" stale map code and "new style" stale
> TLB code should be more in sync, you may end up flushing the TLB
> twice...
yeah. if we enable this for 440, it is more likely to be an issue than
on 476.
> > @@ -189,6 +220,38 @@ static void context_check_map(void)
> > static void context_check_map(void) { }
> > #endif
> >
> > +/*
> > + * On architectures that support a large number of contexts, the tlb
> > + * can be flushed lazily by picking a new context and making the stale
> > + * context unusable until a lazy tlb flush has been issued.
> > + *
> > + * context_available_map keeps track of both active and stale contexts,
> > + * while context_map continues to track only active contexts. When the
> > + * lazy tlb flush is triggered, context_map is copied to
> > + * context_available_map, making the once-stale contexts available again
> > + */
> > +static void recycle_stale_contexts(void)
> > +{
> > + if (nr_free_contexts == 0 && nr_stale_contexts > 0) {
>
> Do an early return and avoid the indentation instead ?
Yeah, that makes sense.
> > + unsigned int cpu = smp_processor_id();
> > + unsigned int i;
> > +
> > + pr_hard("[%d] recycling stale contexts\n", cpu);
> > + /* Time to flush the TLB's */
> > + memcpy(context_available_map, context_map, CTX_MAP_SIZE);
> > + nr_free_contexts = nr_stale_contexts;
> > + nr_stale_contexts = 0;
> > + for_each_online_cpu(i) {
> > + if ((i < cpu_first_thread_in_core(cpu)) ||
> > + (i > cpu_last_thread_in_core(cpu)))
> > + tlb_needs_flush[i] = 1;
> > + else
> > + tlb_needs_flush[i] = 0; /* This core */
> > + }
> > + _tlbil_all();
> > + }
> > +}
> > +
> > void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > {
> > unsigned int i, id, cpu = smp_processor_id();
> > @@ -197,6 +260,8 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > /* No lockless fast path .. yet */
> > raw_spin_lock(&context_lock);
> >
> > + flush_recycled_contexts(cpu);
> > +
>
> Ok so here's the nasty one I think. You need to make sure that whenever
> you pick something off the context_available_map, you've done the above
> first within the same context_lock section right ? At least before you
> actually -use- said context.
right.
> So far so good ... but steal_context can drop the lock iirc. So you may
> need to re-flush there. Not sure that can happen in practice but better
> safe than sorry. I would have preferred seeing that flush near the end
> of the function to avoid such problem.
I can fix this. For 476, I don't think that even if steal_context()
could be called, it wouldn't drop the lock. But then again, if we
enable this for other architectures, it may be a possibility.
> Then, you can end up in cases where you flush the TLB, but your context
> is marked stale due to stealing, and flush again. That's one of the
> reason I wonder if we can consolidate a bit the two orthogonal
> "staleness" concepts we have now.
>
> Granted, stealing on 47x is unlikely, but I have reasons to think that
> this lazy flushing will benefit 440 too.
>
> > pr_hard("[%d] activating context for mm @%p, active=%d, id=%d",
> > cpu, next, next->context.active, next->context.id);
> >
> > @@ -227,7 +292,12 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > id = next_context;
> > if (id > last_context)
> > id = first_context;
> > - map = context_map;
> > +
> > + if (tlb_lazy_flush) {
> > + recycle_stale_contexts();
> > + map = context_available_map;
> > + } else
> > + map = context_map;
> >
> > /* No more free contexts, let's try to steal one */
> > if (nr_free_contexts == 0) {
> > @@ -250,6 +320,13 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > if (id > last_context)
> > id = first_context;
> > }
> > + if (tlb_lazy_flush)
> > + /*
> > + * In the while loop above, we set the bit in
> > + * context_available_map, it also needs to be set in
> > + * context_map
> > + */
> > + __set_bit(id, context_map);
> > stolen:
> > next_context = id + 1;
> > context_mm[id] = next;
> > @@ -267,7 +344,7 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> > id, cpu_first_thread_in_core(cpu),
> > cpu_last_thread_in_core(cpu));
> >
> > - local_flush_tlb_mm(next);
> > + __local_flush_tlb_mm(next);
> >
> > /* XXX This clear should ultimately be part of local_flush_tlb_mm */
> > for (i = cpu_first_thread_in_core(cpu);
> > @@ -317,11 +394,61 @@ void destroy_context(struct mm_struct *mm)
> > mm->context.active = 0;
> > #endif
> > context_mm[id] = NULL;
> > - nr_free_contexts++;
> > +
> > + if (tlb_lazy_flush)
> > + nr_stale_contexts++;
> > + else
> > + nr_free_contexts++;
> > }
> > raw_spin_unlock_irqrestore(&context_lock, flags);
> > }
>
> Now...
>
> > +/*
> > + * This is called from flush_tlb_mm(). Mark the current context as stale
> > + * and grab an available one. The tlb will be flushed when no more
> > + * contexts are available
> > + */
> > +void lazy_flush_context(struct mm_struct *mm)
> > +{
> > + unsigned int id;
> > + unsigned long flags;
> > + unsigned long *map;
> > +
> > + raw_spin_lock_irqsave(&context_lock, flags);
> > +
> > + id = mm->context.id;
> > + if (unlikely(id == MMU_NO_CONTEXT))
> > + goto no_context;
>
> First thing is ... you reproduce quite a bit of logic from
> switch_mmu_context() here. Shouldn't it be abstracted in a separate
> function ?
I'm sure there's something I can do there.
> The other thing here is that another CPU might have done a
> recycle_stale_contexts() before you get here. IE. Your TLB may be stale.
> Shouln't you do a flush here ? Since you are picking up a new PID from
> the context_available_map, it can potentially be stale if your tlb needs
> flushing due to another CPU having just done a recycle.
It looks like I missed that. It does seem that there should be a flush
in here.
> > + /*
> > + * Make the existing context stale. It remains in
> > + * context_available_map as long as nr_free_contexts remains non-zero
> > + */
> > + __clear_bit(id, context_map);
> > + context_mm[id] = NULL;
> > + nr_stale_contexts++;
> > +
> > + recycle_stale_contexts();
> > + BUG_ON(nr_free_contexts == 0);
> > +
> > + nr_free_contexts--;
> > + id = last_context;
> > + map = context_available_map;
> > + while (__test_and_set_bit(id, map)) {
> > + id = find_next_zero_bit(map, last_context+1, id);
> > + if (id > last_context)
> > + id = first_context;
> > + }
> > + set_bit(id, context_map);
> > + next_context = id + 1;
> > + context_mm[id] = mm;
> > + mm->context.id = id;
> > + if (current->active_mm == mm)
> > + set_context(id, mm->pgd);
> > +no_context:
> > + raw_spin_unlock_irqrestore(&context_lock, flags);
> > +}
> > +
> > #ifdef CONFIG_SMP
> >
> > static int __cpuinit mmu_context_cpu_notify(struct notifier_block *self,
> > @@ -407,6 +534,7 @@ void __init mmu_context_init(void)
> > } else if (mmu_has_feature(MMU_FTR_TYPE_47x)) {
> > first_context = 1;
> > last_context = 65535;
> > + tlb_lazy_flush = 1;
> > } else {
> > first_context = 1;
> > last_context = 255;
>
> Somebody should measure on 440, might actually improve perfs. Something
> like a kernel compile sounds like a good test here.
I think I'm going to dust off the newer implementation based on your and
Paul's design. I can probably get that in good working order without
too much more work, and it's something we need to look at eventually
anyway. If I find anything that really gets in my way, I might fix up
this patch in the mean time.
Thanks,
Shaggy
--
Dave Kleikamp
IBM Linux Technology Center
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Thomas Gleixner @ 2010-10-18 21:55 UTC (permalink / raw)
To: Andrew Morton
Cc: Helmut Grohne, Mel Gorman, LKML, linux-mm, pacman, linuxppc-dev
In-Reply-To: <20101018123750.ef7d6d48.akpm@linux-foundation.org>
On Mon, 18 Oct 2010, Andrew Morton wrote:
> On Mon, 18 Oct 2010 12:33:31 +0100
> Mel Gorman <mel@csn.ul.ie> wrote:
>
> > A bit but I still don't know why it would cause corruption. Maybe this is still
> > a caching issue but the difference in timing between list_add and list_add_tail
> > is enough to hide the bug. It's also possible there are some registers
> > ioremapped after the memmap array and reading them is causing some
> > problem.
> >
> > Andrew, what is the right thing to do here? We could flail around looking
> > for explanations as to why the bug causes a user buffer corruption but never
> > get an answer or do we go with this patch, preferably before 2.6.36 releases?
>
> Well, you've spotted a bug so I'd say we fix it asap.
>
> It's a bit of a shame that we lose the only known way of reproducing a
> different bug, but presumably that will come back and bite someone else
> one day, and we'll fix it then :(
I might be completely one off as usual, but this thing reminds me of a
bug I stared at yesterday night:
http://permalink.gmane.org/gmane.linux.kernel/1049605
Reporter Cc'ed
Thanks,
tglx
^ 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