From: ynorov@caviumnetworks.com (Yury Norov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 2/4] asm-generic: Provide a fncpy() implementation
Date: Tue, 20 Jun 2017 17:27:02 +0300 [thread overview]
Message-ID: <20170620142702.mmks36ydoujvvhan@yury-thinkpad> (raw)
In-Reply-To: <20170619174348.GA23750@n2100.armlinux.org.uk>
On Mon, Jun 19, 2017 at 06:43:48PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 19, 2017 at 06:18:18PM +0300, Yury Norov wrote:
> > One else thing I forgot to ask - now you have the generic
> > implementation for fncpy(), so do you really need to save arm
> > version of it?
>
> This was covered in the review of v1, which took the ARM version
> and incorrectly used it as an asm-generic implementation.
>
> I explicitly asked Florian _not_ to copy the ARM fncpy() version
> to asm-generic because it has (surprise surprise) ARM specific
> behaviours that do not belong in a cross-architecture generic
> version.
>
> Namely, the ARM specific behaviour that bit 0 of a code address is
> used to signal whether the code should be executed as ARM code or
> as Thumb code.
>
> This behaviour has no meaning on other architectures (eg, x86)
> where code addresses are not 32-bit aligned.
>
> So, suggesting that the ARM fncpy() should be used as an asm-generic
> version is completely absurd, and just because we have an asm-generic
> version also does not mean ARM should use it.
>
> Florian's approach to providing an asm-generic version, leaving the
> ARM specific version is entirely correct and appropriate.
>
> So, in answer to your question, yes, we need _both_ an ARM specific
> version and an asm-generic version, where the ARM specific version is
> different from the asm-generic version. Purely because it needs
> architecture specific details.
Hi Russell, Florian,
Thanks for clarifications. Thumb bit is a good reason to save arm
version, and I completely agree with you in this. Sorry that missed it
in the v1 discussion.
> I explicitly asked Florian _not_ to copy the ARM fncpy() version
> to asm-generic because it has (surprise surprise) ARM specific
> behaviours that do not belong in a cross-architecture generic
> version.
But it seems that v3 does exactly that - copies arm with very small
changes. :) Maybe there are good reasons to have arm version exactly
how it looks now, but in general case, for me, some things that
it does are not needed. I mean checking the alignment of the source and
the type of destination. And after some headscratching I became even
more convinced that for the general case it would be much preferable
to write the fncpy() as regular function in .c file, not a macro, at
least to have the corresponding symbol in binary and let the assembler
code to call it, which is very probable.
Yury
next prev parent reply other threads:[~2017-06-20 14:27 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-17 0:07 [PATCH v3 0/4] Generalize fncpy availability Florian Fainelli
2017-06-17 0:07 ` [PATCH v3 1/4] ARM: fncpy: Rename include guards Florian Fainelli
2017-06-17 0:07 ` [PATCH v3 2/4] asm-generic: Provide a fncpy() implementation Florian Fainelli
2017-06-18 23:51 ` Yury Norov
2017-06-19 1:11 ` Yury Norov
2017-06-19 15:18 ` Yury Norov
2017-06-19 17:27 ` Florian Fainelli
2017-06-19 17:43 ` Russell King - ARM Linux
2017-06-20 14:27 ` Yury Norov [this message]
2017-06-19 20:58 ` Florian Fainelli
2017-06-20 14:24 ` Yury Norov
2017-06-17 0:07 ` [PATCH v3 3/4] arm64: Provide a fncpy implementation Florian Fainelli
2017-06-17 0:07 ` [PATCH v3 4/4] misc: sram: Allow ARM64 to select SRAM_EXEC Florian Fainelli
2017-06-28 14:55 ` Mark Rutland
2017-06-19 12:24 ` [PATCH v3 0/4] Generalize fncpy availability Mark Rutland
2017-06-19 13:53 ` Tony Lindgren
2017-06-19 17:32 ` Florian Fainelli
2017-06-20 9:10 ` Lorenzo Pieralisi
2017-06-20 16:20 ` Florian Fainelli
2017-06-20 16:46 ` Lorenzo Pieralisi
2017-06-20 16:54 ` Sudeep Holla
2017-06-20 17:03 ` Florian Fainelli
2017-06-19 13:34 ` David Howells
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=20170620142702.mmks36ydoujvvhan@yury-thinkpad \
--to=ynorov@caviumnetworks.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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).