linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Christophe LEROY <christophe.leroy@c-s.fr>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Michal Sojka <sojkam1@fel.cvut.cz>
Cc: Scott Wood <scottwood@freescale.com>,
	linuxppc-dev@lists.ozlabs.org, Paul Mackerras <paulus@samba.org>
Subject: Re: memcpy regression
Date: Mon, 7 Sep 2015 09:08:01 +0200	[thread overview]
Message-ID: <55ED37D1.3080503@c-s.fr> (raw)
In-Reply-To: <1441588450.12945.1.camel@ellerman.id.au>

Hi Michael

Le 07/09/2015 03:14, Michael Ellerman a écrit :
> Hi Michal,
>
> Thanks for finding the problem.
>
> On Sun, 2015-09-06 at 23:01 +0200, Michal Sojka wrote:
>> I found the problem. The compiler replaces an assignment with a call to
>> memcpy. The following patch fixes the problem for me. However, I'm not
>> sure whether this is the real solution. I guess the compiler is free to
>> generate a call to memcpy wherever it wants so other compilers or other
>> optimization levels may need fixes at other places. What do others
>> think?
> I think you're right that it's not a good solution, the compiler could generate
> other calls to memcpy depending on various factors, and people will add new
> code that causes memcpy to get called and it will break your platform.
>
> Christophe, am I right that the problem here is that your new memcpy() doesn't
> work until later in boot when caches are enabled?
>
>

That's right, memset() and memcpy() are for setting/copying data into 
cacheable RAM.
They are using dczb instruction in order to avoid wasting time loading 
the cacheline with data that will be overwritten.

memset_io() and memcpy_toio() are the functions to use when using not 
cacheable memory.

The issue identified by Michal is in function setup_cpu_spec() which is 
called by identify_cpu(). identify_cpu() is called from early_init().
In the begining of early_init(), there is (code from Paul in 2005)

	/* First zero the BSS -- use memset_io, some platforms don't have
	 * caches on yet */
	memset_io((void __iomem *)PTRRELOC(&__bss_start), 0,
			__bss_stop - __bss_start);

It shows that it is already expected that the cache is not active yet 
and standard memset() shall not be used yet. That's the same with memcpy().

I think GCC uses memcpy() in well known situations like initialising 
structures or copying structures.
Shouldn't we just avoid this kind of actions in the very few early init 
functions ?

Christophe

  reply	other threads:[~2015-09-07  7:08 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-04 13:33 memcpy regression Michal Sojka
2015-09-04 13:57 ` Christophe LEROY
2015-09-04 14:35   ` Michal Sojka
2015-09-04 18:10     ` christophe leroy
2015-09-04 19:49       ` Michal Sojka
2015-09-05  0:08         ` Michal Sojka
2015-09-06  8:18           ` christophe leroy
2015-09-06 19:05             ` Michal Sojka
2015-09-06 21:01               ` Michal Sojka
2015-09-07  1:14                 ` Michael Ellerman
2015-09-07  7:08                   ` Christophe LEROY [this message]
2015-09-07  8:40                     ` Michael Ellerman
2015-09-07  9:45                       ` Michal Sojka
2015-09-07 10:59                         ` David Laight
2015-09-08  3:54                           ` Michael Ellerman
2015-09-08  8:59                             ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55ED37D1.3080503@c-s.fr \
    --to=christophe.leroy@c-s.fr \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=scottwood@freescale.com \
    --cc=sojkam1@fel.cvut.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).