* Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low
From: Alexander Sverdlin @ 2017-04-06 15:10 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, linux-crypto, netdev
In-Reply-To: <20170406081509.GB30557@gondor.apana.org.au>
Hi!
On 06/04/17 10:15, Herbert Xu wrote:
> On Thu, Mar 16, 2017 at 03:16:29PM +0100, Alexander Sverdlin wrote:
>> This is a regression caused by 856e3f4092
>> ("crypto: seqiv - Add support for new AEAD interface")
>>
>> As I've said above, I can offer one of the two solutions, which patch should I send?
>> Or do you see any better alternatives?
> Here is a series of patches which should fix the problem.
>
> The first three patches prepare the user-space interfaces to deal
> with longer names. The final patch extends it.
>
> Note that with crypto_user I haven't actually extended it to
> configure longer names. It'll only be able to configure names
> less than 64 bytes. However, it should be able to dump/read
> algorithms with longer names, albeit the name will be truncated
> to 64 bytes length.
>
> Steffen, when convenient could you look into extending the crypto
> user interface to handle longer names (preferably arbitraty length
> since netlink should be able to deal with that)?
>
> Likewise xfrm is still fixed to 64 bytes long. But this should
> be OK as the problematic case only arises with IV generators for
> now and we do not allow IV generators to be specified through xfrm.
>
> af_alg on the other hand now allows arbitrarily long names.
I'm not sure about patch 2 (as I've replied separately), but I've applied
and tested the whole series and it at least solves the original problem
with long algorithm name.
> As the final patch depends on all three it would be easiest if
> we pushed the xfrm patch through the crypto tree. Steffen/David?
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply
* Re: dm-crypt IV generation (summary)
From: Mike Snitzer @ 2017-04-06 14:37 UTC (permalink / raw)
To: Herbert Xu
Cc: Binoy Jayan, Ondrej Mosnacek, Gilad Ben-Yossef, dm-devel,
Mikulas Patocka, linux-crypto, Milan Broz, Alasdair Kergon
In-Reply-To: <20170406092957.GA31341@gondor.apana.org.au>
On Thu, Apr 06 2017 at 5:29am -0400,
Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Mar 10, 2017 at 02:44:26PM +0100, Ondrej Mosnacek wrote:
> > Hi all,
> >
> > I was tasked to post a summary the whole dm-crypt IV generation
> > problem and all the suggested solutions along with their drawbacks, so
> > here it goes...
>
> Thanks for the summary. It looks good to me.
There were 4 different solutions presented. In reading each, option 2
looked the most promising. BUT it requires a tradeoff/decision to be
made, please see my earlier reply where I asked for your (and/or
Milan's) thoughts:
https://www.spinics.net/lists/linux-crypto/msg24586.html
Thanks,
Mike
^ permalink raw reply
* (unknown),
From: agiva @ 2017-04-06 13:43 UTC (permalink / raw)
To: linux-crypto
[-- Attachment #1: 84.zip --]
[-- Type: application/zip, Size: 25 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] crypto: af_alg - Allow arbitrarily long algorithm names
From: Alexander Sverdlin @ 2017-04-06 12:32 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, linux-crypto, netdev
In-Reply-To: <E1cw2aP-00085S-Q4@gondobar>
Hi!
On 06/04/17 10:16, Herbert Xu wrote:
> This patch removes the hard-coded 64-byte limit on the length
> of the algorithm name through bind(2). The address length can
> now exceed that. The user-space structure remains unchanged.
> In order to use a longer name simply extend the salg_name array
> beyond its defined 64 bytes length.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
>
> crypto/af_alg.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 690deca..3556d8e 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -160,11 +160,11 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> if (sock->state == SS_CONNECTED)
> return -EINVAL;
>
> - if (addr_len != sizeof(*sa))
> + if (addr_len < sizeof(*sa))
> return -EINVAL;
>
> sa->salg_type[sizeof(sa->salg_type) - 1] = 0;
> - sa->salg_name[sizeof(sa->salg_name) - 1] = 0;
> + sa->salg_name[sizeof(sa->salg_name) + addr_len - sizeof(*sa) - 1] = 0;
>
> type = alg_get_type(sa->salg_type);
> if (IS_ERR(type) && PTR_ERR(type) == -ENOENT) {
Why should userspace ever extend the structure if salg_name is hardcoded to 64 in if_alg.h?
This patch doesn't change the behavior at all, or am I missing something?
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply
* Re: XTS Crypto Not Found In /proc/crypto Even After Compiled for 4.10.1.
From: Herbert Xu @ 2017-04-06 9:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Nathan Royce, davem, linux-crypto, linux-kernel, Marek Szyprowski
In-Reply-To: <20170313170601.ozolfzgixqu6aa4g@kozik-lap>
On Mon, Mar 13, 2017 at 07:06:01PM +0200, Krzysztof Kozlowski wrote:
>
> I bisected this to commit f1c131b45410 ("crypto: xts - Convert to
> skcipher"). The s5p-sss driver stays the same... but the xts changes and
> as a result we have a NULL pointer dereference (actually of value
> 00000004):
> [ 18.930195] Unable to handle kernel NULL pointer dereference at virtual address 00000004
> ...
> [ 18.972325] [<c0313c98>] (post_crypt) from [<c031408c>] (decrypt_done+0x4c/0x54)
> [ 18.972343] [<c031408c>] (decrypt_done) from [<c056309c>] (s5p_aes_interrupt+0x1bc/0x208)
> [ 18.972360] [<c056309c>] (s5p_aes_interrupt) from [<c0164930>] (irq_thread_fn+0x1c/0x54)
>
> Any hints?
I haven't found any smoking guns, but the locking between the
tasklet and the IRQ routine looks suspect. First of all the
tasklet is modifying the dev structure without holding any locks.
More importantly, the IRQ routine does not seem to be robust in
the face of spurious interrupts. Should a spurious interrupt
arrive, it is entirely possible for the tasklet's modifying of
dev->req to race with the IRQ routine which reads dev->req.
However, this does depend on there being a spurious interrupt so
I don't know how likely it is.
Anyway, if we can't get to the bottom of this, we should disable
the broken functionality.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: dm-crypt IV generation (summary)
From: Herbert Xu @ 2017-04-06 9:29 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Binoy Jayan, Mike Snitzer, Gilad Ben-Yossef, dm-devel,
Mikulas Patocka, linux-crypto, Milan Broz, Alasdair Kergon
In-Reply-To: <CAFqZXNs0=6+PvpGq3weRwCJkUOfSn06i_fD6ZuvXo_0rms9zeA@mail.gmail.com>
On Fri, Mar 10, 2017 at 02:44:26PM +0100, Ondrej Mosnacek wrote:
> Hi all,
>
> I was tasked to post a summary the whole dm-crypt IV generation
> problem and all the suggested solutions along with their drawbacks, so
> here it goes...
Thanks for the summary. It looks good to me.
Something else to keep mind is the potential to reuse IV generators.
Recently a patch has been proposed for fscrypt that also makes
use of essiv (search for "fscrypt: Add support for AES-128-CBC").
It would be great if we could reuse the same code for both dm-crypt
and fscrypt.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 1/4] crypto: user - Prepare for CRYPTO_MAX_ALG_NAME expansion
From: Herbert Xu @ 2017-04-06 8:16 UTC (permalink / raw)
To: Alexander Sverdlin, David S. Miller, linux-crypto, netdev
In-Reply-To: <20170406081509.GB30557@gondor.apana.org.au>
This patch hard-codes CRYPTO_MAX_NAME in the user-space API to
64, which is the current value of CRYPTO_MAX_ALG_NAME. This patch
also replaces all remaining occurences of CRYPTO_MAX_ALG_NAME
in the user-space API with CRYPTO_MAX_NAME.
This way the user-space API will not be modified when we raise
the value of CRYPTO_MAX_ALG_NAME.
Furthermore, the code has been updated to handle names longer than
the user-space API. They will be truncated.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/crypto_user.c | 18 +++++++++---------
include/uapi/linux/cryptouser.h | 10 +++++-----
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index a90404a..89acaab 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -83,7 +83,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_report_cipher rcipher;
- strncpy(rcipher.type, "cipher", sizeof(rcipher.type));
+ strlcpy(rcipher.type, "cipher", sizeof(rcipher.type));
rcipher.blocksize = alg->cra_blocksize;
rcipher.min_keysize = alg->cra_cipher.cia_min_keysize;
@@ -102,7 +102,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_report_comp rcomp;
- strncpy(rcomp.type, "compression", sizeof(rcomp.type));
+ strlcpy(rcomp.type, "compression", sizeof(rcomp.type));
if (nla_put(skb, CRYPTOCFGA_REPORT_COMPRESS,
sizeof(struct crypto_report_comp), &rcomp))
goto nla_put_failure;
@@ -116,7 +116,7 @@ static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_report_acomp racomp;
- strncpy(racomp.type, "acomp", sizeof(racomp.type));
+ strlcpy(racomp.type, "acomp", sizeof(racomp.type));
if (nla_put(skb, CRYPTOCFGA_REPORT_ACOMP,
sizeof(struct crypto_report_acomp), &racomp))
@@ -131,7 +131,7 @@ static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_report_akcipher rakcipher;
- strncpy(rakcipher.type, "akcipher", sizeof(rakcipher.type));
+ strlcpy(rakcipher.type, "akcipher", sizeof(rakcipher.type));
if (nla_put(skb, CRYPTOCFGA_REPORT_AKCIPHER,
sizeof(struct crypto_report_akcipher), &rakcipher))
@@ -146,7 +146,7 @@ static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_report_kpp rkpp;
- strncpy(rkpp.type, "kpp", sizeof(rkpp.type));
+ strlcpy(rkpp.type, "kpp", sizeof(rkpp.type));
if (nla_put(skb, CRYPTOCFGA_REPORT_KPP,
sizeof(struct crypto_report_kpp), &rkpp))
@@ -160,10 +160,10 @@ static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_one(struct crypto_alg *alg,
struct crypto_user_alg *ualg, struct sk_buff *skb)
{
- strncpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name));
- strncpy(ualg->cru_driver_name, alg->cra_driver_name,
+ strlcpy(ualg->cru_name, alg->cra_name, sizeof(ualg->cru_name));
+ strlcpy(ualg->cru_driver_name, alg->cra_driver_name,
sizeof(ualg->cru_driver_name));
- strncpy(ualg->cru_module_name, module_name(alg->cra_module),
+ strlcpy(ualg->cru_module_name, module_name(alg->cra_module),
sizeof(ualg->cru_module_name));
ualg->cru_type = 0;
@@ -176,7 +176,7 @@ static int crypto_report_one(struct crypto_alg *alg,
if (alg->cra_flags & CRYPTO_ALG_LARVAL) {
struct crypto_report_larval rl;
- strncpy(rl.type, "larval", sizeof(rl.type));
+ strlcpy(rl.type, "larval", sizeof(rl.type));
if (nla_put(skb, CRYPTOCFGA_REPORT_LARVAL,
sizeof(struct crypto_report_larval), &rl))
goto nla_put_failure;
diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
index 11d21fc..b4def5c 100644
--- a/include/uapi/linux/cryptouser.h
+++ b/include/uapi/linux/cryptouser.h
@@ -31,7 +31,7 @@ enum {
#define CRYPTO_MSG_MAX (__CRYPTO_MSG_MAX - 1)
#define CRYPTO_NR_MSGTYPES (CRYPTO_MSG_MAX + 1 - CRYPTO_MSG_BASE)
-#define CRYPTO_MAX_NAME CRYPTO_MAX_ALG_NAME
+#define CRYPTO_MAX_NAME 64
/* Netlink message attributes. */
enum crypto_attr_type_t {
@@ -53,9 +53,9 @@ enum crypto_attr_type_t {
};
struct crypto_user_alg {
- char cru_name[CRYPTO_MAX_ALG_NAME];
- char cru_driver_name[CRYPTO_MAX_ALG_NAME];
- char cru_module_name[CRYPTO_MAX_ALG_NAME];
+ char cru_name[CRYPTO_MAX_NAME];
+ char cru_driver_name[CRYPTO_MAX_NAME];
+ char cru_module_name[CRYPTO_MAX_NAME];
__u32 cru_type;
__u32 cru_mask;
__u32 cru_refcnt;
@@ -73,7 +73,7 @@ struct crypto_report_hash {
};
struct crypto_report_cipher {
- char type[CRYPTO_MAX_ALG_NAME];
+ char type[CRYPTO_MAX_NAME];
unsigned int blocksize;
unsigned int min_keysize;
unsigned int max_keysize;
^ permalink raw reply related
* [PATCH 2/4] crypto: af_alg - Allow arbitrarily long algorithm names
From: Herbert Xu @ 2017-04-06 8:16 UTC (permalink / raw)
To: Alexander Sverdlin, David S. Miller, linux-crypto, netdev
In-Reply-To: <20170406081509.GB30557@gondor.apana.org.au>
This patch removes the hard-coded 64-byte limit on the length
of the algorithm name through bind(2). The address length can
now exceed that. The user-space structure remains unchanged.
In order to use a longer name simply extend the salg_name array
beyond its defined 64 bytes length.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
crypto/af_alg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 690deca..3556d8e 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -160,11 +160,11 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
if (sock->state == SS_CONNECTED)
return -EINVAL;
- if (addr_len != sizeof(*sa))
+ if (addr_len < sizeof(*sa))
return -EINVAL;
sa->salg_type[sizeof(sa->salg_type) - 1] = 0;
- sa->salg_name[sizeof(sa->salg_name) - 1] = 0;
+ sa->salg_name[sizeof(sa->salg_name) + addr_len - sizeof(*sa) - 1] = 0;
type = alg_get_type(sa->salg_type);
if (IS_ERR(type) && PTR_ERR(type) == -ENOENT) {
^ permalink raw reply related
* [PATCH 4/4] crypto: api - Extend algorithm name limit to 128 bytes
From: Herbert Xu @ 2017-04-06 8:16 UTC (permalink / raw)
To: Alexander Sverdlin, David S. Miller, linux-crypto, netdev
In-Reply-To: <20170406081509.GB30557@gondor.apana.org.au>
With the new explicit IV generators, we may now exceed the 64-byte
length limit on the algorithm name, e.g., with
echainiv(authencesn(hmac(sha256-generic),cbc(des3_ede-generic)))
This patch extends the length limit to 128 bytes.
Reported-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/crypto.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index c0b0cf3..84da997 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -123,7 +123,7 @@
/*
* Miscellaneous stuff.
*/
-#define CRYPTO_MAX_ALG_NAME 64
+#define CRYPTO_MAX_ALG_NAME 128
/*
* The macro CRYPTO_MINALIGN_ATTR (along with the void * type in the actual
^ permalink raw reply related
* [PATCH 3/4] xfrm: Prepare for CRYPTO_MAX_ALG_NAME expansion
From: Herbert Xu @ 2017-04-06 8:16 UTC (permalink / raw)
To: Alexander Sverdlin, David S. Miller, linux-crypto, netdev
In-Reply-To: <20170406081509.GB30557@gondor.apana.org.au>
This patch fixes the xfrm_user code to use the actual array size
rather than the hard-coded CRYPTO_MAX_ALG_NAME length. This is
because the array size is fixed at 64 bytes while we want to increase
the in-kernel CRYPTO_MAX_ALG_NAME value.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/xfrm/xfrm_user.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 9705c27..96557cf 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -55,7 +55,7 @@ static int verify_one_alg(struct nlattr **attrs, enum xfrm_attr_type_t type)
return -EINVAL;
}
- algp->alg_name[CRYPTO_MAX_ALG_NAME - 1] = '\0';
+ algp->alg_name[sizeof(algp->alg_name) - 1] = '\0';
return 0;
}
@@ -71,7 +71,7 @@ static int verify_auth_trunc(struct nlattr **attrs)
if (nla_len(rt) < xfrm_alg_auth_len(algp))
return -EINVAL;
- algp->alg_name[CRYPTO_MAX_ALG_NAME - 1] = '\0';
+ algp->alg_name[sizeof(algp->alg_name) - 1] = '\0';
return 0;
}
@@ -87,7 +87,7 @@ static int verify_aead(struct nlattr **attrs)
if (nla_len(rt) < aead_len(algp))
return -EINVAL;
- algp->alg_name[CRYPTO_MAX_ALG_NAME - 1] = '\0';
+ algp->alg_name[sizeof(algp->alg_name) - 1] = '\0';
return 0;
}
^ permalink raw reply related
* [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low
From: Herbert Xu @ 2017-04-06 8:15 UTC (permalink / raw)
To: Alexander Sverdlin; +Cc: David S. Miller, linux-crypto, netdev
In-Reply-To: <367298c5-aa5d-3708-72d0-8a44702d0caa@nokia.com>
On Thu, Mar 16, 2017 at 03:16:29PM +0100, Alexander Sverdlin wrote:
>
> This is a regression caused by 856e3f4092
> ("crypto: seqiv - Add support for new AEAD interface")
>
> As I've said above, I can offer one of the two solutions, which patch should I send?
> Or do you see any better alternatives?
Here is a series of patches which should fix the problem.
The first three patches prepare the user-space interfaces to deal
with longer names. The final patch extends it.
Note that with crypto_user I haven't actually extended it to
configure longer names. It'll only be able to configure names
less than 64 bytes. However, it should be able to dump/read
algorithms with longer names, albeit the name will be truncated
to 64 bytes length.
Steffen, when convenient could you look into extending the crypto
user interface to handle longer names (preferably arbitraty length
since netlink should be able to deal with that)?
Likewise xfrm is still fixed to 64 bytes long. But this should
be OK as the problematic case only arises with IV generators for
now and we do not allow IV generators to be specified through xfrm.
af_alg on the other hand now allows arbitrarily long names.
As the final patch depends on all three it would be easiest if
we pushed the xfrm patch through the crypto tree. Steffen/David?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: What should be the algo priority
From: Harsh Jain @ 2017-04-06 5:26 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <42612664.V5g4013kpk@tauon.chronox.de>
On Tue, Apr 4, 2017 at 6:07 PM, Stephan Müller <smueller@chronox.de> wrote:
> Am Dienstag, 4. April 2017, 09:53:17 CEST schrieb Harsh Jain:
>
> Hi Harsh,
>
>> Hi,
>>
>> Do we have any guidelines documented to decide what should be the
>> algorithm priority. Specially for authenc implementation.Most of the
>> drivers have fixed priority for all algos. Problem comes in when we
>> have cbc(aes), hmac(sha1) and authenc(cbc(aes),hmac(sha1))
>> implementation in driver. Base authenc driver gets more precedence
>> because of higher priority(enc->base.cra_priority * 10 +
>> auth_base->cra_priority;)
>>
>> What should be the priority of
>> cbc(aes),
>> hmac(sha1)
>> authenc(cbc(aes),hmac(sha1))
>
> There is no general rule about the actual numbers. But commonly, the prios are
> set such that the prios of C implementations < ASM implementations < hardware
> accelerators. The idea is to give users the fastest implementation there is
> for his particular system.
It means cbc, hmac should have smaller(nearly 10 times less) priority
than their authenc implementation otherwise request will not offload
to driver because sw authenc priority is (aes * 10 + hmac).
>
> Ciao
> Stephan
^ permalink raw reply
* [PATCH v2 3/3] hw_random: timeriomem_rng: Improve performance for sub-jiffie update periods
From: Rick Altherr @ 2017-04-05 23:21 UTC (permalink / raw)
To: alex; +Cc: openbmc, Herbert Xu, Matt Mackall, linux-crypto, linux-kernel
In-Reply-To: <20170405232100.2023-1-raltherr@google.com>
Some hardware RNGs provide a single register for obtaining random data.
Instead of signaling when new data is available, the reader must wait a
fixed amount of time between reads for new data to be generated.
timeriomem_rng implements this scheme with the period specified in
platform data or device tree. While the period is specified in
microseconds, the implementation used a standard timer which has a
minimum delay of 1 jiffie and caused a significant bottleneck for
devices that can update at 1us. By switching to an hrtimer, 1us periods
now only delay at most 2us per read.
Signed-off-by: Rick Altherr <raltherr@google.com>
---
Changes in v2:
- Split performance improvements into separate patch
drivers/char/hw_random/timeriomem-rng.c | 86 +++++++++++++++++----------------
1 file changed, 45 insertions(+), 41 deletions(-)
diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index 024bdff7999f..a0faa5f05deb 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -21,23 +21,24 @@
*/
#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/hrtimer.h>
#include <linux/hw_random.h>
#include <linux/io.h>
-#include <linux/jiffies.h>
+#include <linux/ktime.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/time.h>
#include <linux/timeriomem-rng.h>
-#include <linux/timer.h>
struct timeriomem_rng_private {
void __iomem *io_base;
- unsigned int expires;
- unsigned int period;
+ ktime_t period;
unsigned int present:1;
- struct timer_list timer;
+ struct hrtimer timer;
struct completion completion;
struct hwrng rng_ops;
@@ -48,10 +49,13 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
{
struct timeriomem_rng_private *priv =
container_of(hwrng, struct timeriomem_rng_private, rng_ops);
- unsigned long cur;
- s32 delay;
+ int retval = 0;
+ int period_us = ktime_to_us(priv->period);
- /* The RNG provides 32-bit per read. Ensure there is enough space. */
+ /*
+ * The RNG provides 32-bits per read. Ensure there is enough space for
+ * at minimum one read.
+ */
if (max < sizeof(u32))
return 0;
@@ -66,33 +70,44 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
wait_for_completion(&priv->completion);
- *(u32 *)data = readl(priv->io_base);
+ do {
+ /*
+ * After the first read, all additional reads will need to wait
+ * for the RNG to generate new data. Since the period can have
+ * a wide range of values (1us to 1s have been observed), allow
+ * for 1% tolerance in the sleep time rather than a fixed value.
+ */
+ if (retval > 0)
+ usleep_range(period_us,
+ period_us + min(1, period_us / 100));
+
+ *(u32 *)data = readl(priv->io_base);
+ retval += sizeof(u32);
+ data += sizeof(u32);
+ max -= sizeof(u32);
+ } while (wait && max > sizeof(u32));
/*
* Block any new callers until the RNG has had time to generate new
* data.
*/
- cur = jiffies;
-
- delay = cur - priv->expires;
- delay = priv->period - (delay % priv->period);
-
- priv->expires = cur + delay;
priv->present = 0;
-
reinit_completion(&priv->completion);
- mod_timer(&priv->timer, priv->expires);
+ hrtimer_forward_now(&priv->timer, priv->period);
+ hrtimer_restart(&priv->timer);
- return 4;
+ return retval;
}
-static void timeriomem_rng_trigger(unsigned long data)
+static enum hrtimer_restart timeriomem_rng_trigger(struct hrtimer *timer)
{
struct timeriomem_rng_private *priv
- = (struct timeriomem_rng_private *)data;
+ = container_of(timer, struct timeriomem_rng_private, timer);
priv->present = 1;
complete(&priv->completion);
+
+ return HRTIMER_NORESTART;
}
static int timeriomem_rng_probe(struct platform_device *pdev)
@@ -140,43 +155,33 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
period = pdata->period;
}
- priv->period = usecs_to_jiffies(period);
- if (priv->period < 1) {
- dev_err(&pdev->dev, "period is less than one jiffy\n");
- return -EINVAL;
- }
-
- priv->expires = jiffies;
- priv->present = 1;
-
+ priv->period = ns_to_ktime(period * NSEC_PER_USEC);
init_completion(&priv->completion);
- complete(&priv->completion);
-
- setup_timer(&priv->timer, timeriomem_rng_trigger, (unsigned long)priv);
+ hrtimer_init(&priv->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+ priv->timer.function = timeriomem_rng_trigger;
priv->rng_ops.name = dev_name(&pdev->dev);
priv->rng_ops.read = timeriomem_rng_read;
priv->io_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(priv->io_base)) {
- err = PTR_ERR(priv->io_base);
- goto out_timer;
+ return PTR_ERR(priv->io_base);
}
+ /* Assume random data is already available. */
+ priv->present = 1;
+ complete(&priv->completion);
+
err = hwrng_register(&priv->rng_ops);
if (err) {
dev_err(&pdev->dev, "problem registering\n");
- goto out_timer;
+ return err;
}
dev_info(&pdev->dev, "32bits from 0x%p @ %dus\n",
priv->io_base, period);
return 0;
-
-out_timer:
- del_timer_sync(&priv->timer);
- return err;
}
static int timeriomem_rng_remove(struct platform_device *pdev)
@@ -184,8 +189,7 @@ static int timeriomem_rng_remove(struct platform_device *pdev)
struct timeriomem_rng_private *priv = platform_get_drvdata(pdev);
hwrng_unregister(&priv->rng_ops);
-
- del_timer_sync(&priv->timer);
+ hrtimer_cancel(&priv->timer);
return 0;
}
--
2.12.2.715.g7642488e1d-goog
^ permalink raw reply related
* [PATCH v2 2/3] hw_random: timeriomem_rng: Shorten verbose type and variable names
From: Rick Altherr @ 2017-04-05 23:20 UTC (permalink / raw)
To: alex; +Cc: openbmc, Herbert Xu, Matt Mackall, linux-crypto, linux-kernel
In-Reply-To: <20170405232100.2023-1-raltherr@google.com>
No functional changes.
Signed-off-by: Rick Altherr <raltherr@google.com>
---
Changes in v2:
- Split type and variable renames into separate patch
drivers/char/hw_random/timeriomem-rng.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index 17574452fd35..024bdff7999f 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -31,7 +31,7 @@
#include <linux/timeriomem-rng.h>
#include <linux/timer.h>
-struct timeriomem_rng_private_data {
+struct timeriomem_rng_private {
void __iomem *io_base;
unsigned int expires;
unsigned int period;
@@ -40,15 +40,14 @@ struct timeriomem_rng_private_data {
struct timer_list timer;
struct completion completion;
- struct hwrng timeriomem_rng_ops;
+ struct hwrng rng_ops;
};
static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
size_t max, bool wait)
{
- struct timeriomem_rng_private_data *priv =
- container_of(hwrng, struct timeriomem_rng_private_data,
- timeriomem_rng_ops);
+ struct timeriomem_rng_private *priv =
+ container_of(hwrng, struct timeriomem_rng_private, rng_ops);
unsigned long cur;
s32 delay;
@@ -89,8 +88,8 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
static void timeriomem_rng_trigger(unsigned long data)
{
- struct timeriomem_rng_private_data *priv
- = (struct timeriomem_rng_private_data *)data;
+ struct timeriomem_rng_private *priv
+ = (struct timeriomem_rng_private *)data;
priv->present = 1;
complete(&priv->completion);
@@ -99,7 +98,7 @@ static void timeriomem_rng_trigger(unsigned long data)
static int timeriomem_rng_probe(struct platform_device *pdev)
{
struct timeriomem_rng_data *pdata = pdev->dev.platform_data;
- struct timeriomem_rng_private_data *priv;
+ struct timeriomem_rng_private *priv;
struct resource *res;
int err = 0;
int period;
@@ -121,7 +120,7 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
/* Allocate memory for the device structure (and zero it) */
priv = devm_kzalloc(&pdev->dev,
- sizeof(struct timeriomem_rng_private_data), GFP_KERNEL);
+ sizeof(struct timeriomem_rng_private), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -155,8 +154,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
setup_timer(&priv->timer, timeriomem_rng_trigger, (unsigned long)priv);
- priv->timeriomem_rng_ops.name = dev_name(&pdev->dev);
- priv->timeriomem_rng_ops.read = timeriomem_rng_read;
+ priv->rng_ops.name = dev_name(&pdev->dev);
+ priv->rng_ops.read = timeriomem_rng_read;
priv->io_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(priv->io_base)) {
@@ -164,7 +163,7 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
goto out_timer;
}
- err = hwrng_register(&priv->timeriomem_rng_ops);
+ err = hwrng_register(&priv->rng_ops);
if (err) {
dev_err(&pdev->dev, "problem registering\n");
goto out_timer;
@@ -182,9 +181,9 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
static int timeriomem_rng_remove(struct platform_device *pdev)
{
- struct timeriomem_rng_private_data *priv = platform_get_drvdata(pdev);
+ struct timeriomem_rng_private *priv = platform_get_drvdata(pdev);
- hwrng_unregister(&priv->timeriomem_rng_ops);
+ hwrng_unregister(&priv->rng_ops);
del_timer_sync(&priv->timer);
--
2.12.2.715.g7642488e1d-goog
^ permalink raw reply related
* [PATCH v2 0/3] hw_random: timeriomem_rng: Migrate to new API and improve performance
From: Rick Altherr @ 2017-04-05 23:20 UTC (permalink / raw)
To: alex; +Cc: openbmc, Herbert Xu, Matt Mackall, linux-crypto, linux-kernel
AST2400 can generate 32-bits of random data every 1us. Original driver
was limited to one 32-bit read every jiffie due to deprecated API and use
of timers. Migrating to new hwrng API and switching to hrtimers
improves read performance of /dev/hwrng to 13Mb/s.
Changes in v2:
- Split API migration into separate patch
- Split type and variable renames into separate patch
- Split performance improvements into separate patch
Rick Altherr (3):
hw_random: Migrate timeriomem_rng to new API
hw_random: timeriomem_rng: Shorten verbose type and variable names
hw_random: timeriomem_rng: Improve performance for sub-jiffie update
periods
drivers/char/hw_random/timeriomem-rng.c | 157 ++++++++++++++++----------------
1 file changed, 80 insertions(+), 77 deletions(-)
--
2.12.2.715.g7642488e1d-goog
^ permalink raw reply
* [PATCH v2 1/3] hw_random: Migrate timeriomem_rng to new API
From: Rick Altherr @ 2017-04-05 23:20 UTC (permalink / raw)
To: alex; +Cc: openbmc, Herbert Xu, Matt Mackall, linux-crypto, linux-kernel
In-Reply-To: <20170405232100.2023-1-raltherr@google.com>
Preserves the existing behavior of only returning 32-bits per call.
Signed-off-by: Rick Altherr <raltherr@google.com>
---
Changes in v2:
- Split API migration into separate patch
drivers/char/hw_random/timeriomem-rng.c | 60 ++++++++++++++++-----------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index cf37db263ecd..17574452fd35 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -20,18 +20,16 @@
* TODO: add support for reading sizes other than 32bits and masking
*/
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/of.h>
+#include <linux/completion.h>
#include <linux/hw_random.h>
#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/timeriomem-rng.h>
-#include <linux/jiffies.h>
-#include <linux/sched.h>
#include <linux/timer.h>
-#include <linux/completion.h>
struct timeriomem_rng_private_data {
void __iomem *io_base;
@@ -45,32 +43,36 @@ struct timeriomem_rng_private_data {
struct hwrng timeriomem_rng_ops;
};
-#define to_rng_priv(rng) \
- ((struct timeriomem_rng_private_data *)rng->priv)
-
-/*
- * have data return 1, however return 0 if we have nothing
- */
-static int timeriomem_rng_data_present(struct hwrng *rng, int wait)
+static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
+ size_t max, bool wait)
{
- struct timeriomem_rng_private_data *priv = to_rng_priv(rng);
+ struct timeriomem_rng_private_data *priv =
+ container_of(hwrng, struct timeriomem_rng_private_data,
+ timeriomem_rng_ops);
+ unsigned long cur;
+ s32 delay;
- if (!wait || priv->present)
- return priv->present;
+ /* The RNG provides 32-bit per read. Ensure there is enough space. */
+ if (max < sizeof(u32))
+ return 0;
- wait_for_completion(&priv->completion);
+ /*
+ * There may not have been enough time for new data to be generated
+ * since the last request. If the caller doesn't want to wait, let them
+ * bail out. Otherwise, wait for the completion. If the new data has
+ * already been generated, the completion should already be available.
+ */
+ if (!wait && !priv->present)
+ return 0;
- return 1;
-}
-
-static int timeriomem_rng_data_read(struct hwrng *rng, u32 *data)
-{
- struct timeriomem_rng_private_data *priv = to_rng_priv(rng);
- unsigned long cur;
- s32 delay;
+ wait_for_completion(&priv->completion);
- *data = readl(priv->io_base);
+ *(u32 *)data = readl(priv->io_base);
+ /*
+ * Block any new callers until the RNG has had time to generate new
+ * data.
+ */
cur = jiffies;
delay = cur - priv->expires;
@@ -154,9 +156,7 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
setup_timer(&priv->timer, timeriomem_rng_trigger, (unsigned long)priv);
priv->timeriomem_rng_ops.name = dev_name(&pdev->dev);
- priv->timeriomem_rng_ops.data_present = timeriomem_rng_data_present;
- priv->timeriomem_rng_ops.data_read = timeriomem_rng_data_read;
- priv->timeriomem_rng_ops.priv = (unsigned long)priv;
+ priv->timeriomem_rng_ops.read = timeriomem_rng_read;
priv->io_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(priv->io_base)) {
--
2.12.2.715.g7642488e1d-goog
^ permalink raw reply related
* Re: [PATCH 4/5] crypto/nx: Add P9 NX support for 842 compression engine.
From: Haren Myneni @ 2017-04-05 21:49 UTC (permalink / raw)
To: Michael Ellerman
Cc: herbert, ddstreet, linuxppc-dev, linux-crypto, benh, mikey, suka,
hbabu
In-Reply-To: <87mvbwv3vh.fsf@concordia.ellerman.id.au>
Michael, Thanks for the review and comments.
On 04/04/2017 03:55 AM, Michael Ellerman wrote:
> Hi Haren,
>
> A few comments ...
>
> Haren Myneni <haren@linux.vnet.ibm.com> writes:
>
>> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
>> index 4e5a470..7315621 100644
>> --- a/arch/powerpc/include/asm/vas.h
>> +++ b/arch/powerpc/include/asm/vas.h
>> @@ -19,6 +19,8 @@
>> #define VAS_RX_FIFO_SIZE_MIN (1 << 10) /* 1KB */
>> #define VAS_RX_FIFO_SIZE_MAX (8 << 20) /* 8MB */
>>
>> +#define is_vas_available() (cpu_has_feature(CPU_FTR_ARCH_300))
>
> You shouldn't need that, it should all come from the device tree.
>
>> diff --git a/drivers/crypto/nx/Kconfig b/drivers/crypto/nx/Kconfig
>> index ad7552a..4ad7fdb 100644
>> --- a/drivers/crypto/nx/Kconfig
>> +++ b/drivers/crypto/nx/Kconfig
>> @@ -38,6 +38,7 @@ config CRYPTO_DEV_NX_COMPRESS_PSERIES
>> config CRYPTO_DEV_NX_COMPRESS_POWERNV
>> tristate "Compression acceleration support on PowerNV platform"
>> depends on PPC_POWERNV
>> + select VAS
>
> Don't select symbols that are user visible.
>
> I'm not sure we actually want CONFIG_VAS to be user visible, but
> currently it is so this should be 'depends on VAS'.
>
>> diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
>> index 8737e90..66efd39 100644
>> --- a/drivers/crypto/nx/nx-842-powernv.c
>> +++ b/drivers/crypto/nx/nx-842-powernv.c
>> @@ -554,6 +662,164 @@ static int nx842_powernv_decompress(const unsigned char *in, unsigned int inlen,
>> wmem, CCW_FC_842_DECOMP_CRC);
>> }
>>
>> +
>> +static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
>> + int vasid, int ct)
>> +{
>> + struct vas_window *rxwin, *txwin = NULL;
>> + struct vas_rx_win_attr rxattr;
>> + struct vas_tx_win_attr txattr;
>> + struct nx842_coproc *coproc;
>> + u32 lpid, pid, tid;
>> + u64 rx_fifo;
>> + int ret;
>> +#define RX_FIFO_SIZE 0x8000
>
> Where's that come from?
We use FIFO size in skibbot to allocate FIFO buffer. I should export fifo size as device tree property and use it here.
>
>> + if (of_property_read_u64(dn, "rx-fifo-address", (void *)&rx_fifo)) {
>> + pr_err("ibm,nx-842: Missing rx-fifo-address property\n");
>
> The driver already declares pr_fmt(), so do you need the prefixes on
> these pr_err()s ?
>
>> + return -EINVAL;
>> + }
>> +
>> + if (of_property_read_u32(dn, "lpid", &lpid)) {
>> + pr_err("ibm,nx-842: Missing lpid property\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (of_property_read_u32(dn, "pid", &pid)) {
>> + pr_err("ibm,nx-842: Missing pid property\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (of_property_read_u32(dn, "tid", &tid)) {
>> + pr_err("ibm,nx-842: Missing tid property\n");
>> + return -EINVAL;
>> + }
>> +
>> + vas_init_rx_win_attr(&rxattr, ct);
>> + rxattr.rx_fifo = (void *)rx_fifo;
>> + rxattr.rx_fifo_size = RX_FIFO_SIZE;
>> + rxattr.lnotify_lpid = lpid;
>> + rxattr.lnotify_pid = pid;
>> + rxattr.lnotify_tid = tid;
>> + rxattr.wcreds_max = 64;
>> +
>> + /*
>> + * Open a VAS receice window which is used to configure RxFIFO
>> + * for NX.
>> + */
>> + rxwin = vas_rx_win_open(vasid, ct, &rxattr);
>> + if (IS_ERR(rxwin)) {
>> + pr_err("ibm,nx-842: setting RxFIFO with VAS failed: %ld\n",
>> + PTR_ERR(rxwin));
>> + return PTR_ERR(rxwin);
>> + }
>> +
>> + /*
>> + * Kernel requests will be high priority. So open send
>> + * windows only for high priority RxFIFO entries.
>> + */
>> + if (ct == VAS_COP_TYPE_842_HIPRI) {
>
> This if body looks like it should be a separate function to me.
>
>> + vas_init_tx_win_attr(&txattr, ct);
>> + txattr.lpid = 0; /* lpid is 0 for kernel requests */
>> + txattr.pid = mfspr(SPRN_PID);
>> +
>> + /*
>> + * Open a VAS send window which is used to send request to NX.
>> + */
>> + txwin = vas_tx_win_open(vasid, ct, &txattr);
>> + if (IS_ERR(txwin)) {
>> + pr_err("ibm,nx-842: Can not open TX window: %ld\n",
>> + PTR_ERR(txwin));
>> + ret = PTR_ERR(txwin);
>> + goto err_out;
>> + }
>> + }
>> +
>> + coproc = kmalloc(sizeof(*coproc), GFP_KERNEL);
>> + if (!coproc) {
>> + ret = -ENOMEM;
>> + goto err_out;
>> + }
>
> The error handling would be simpler if you did that earlier, before you
> open the RX/TX windows.
>
>> + coproc->chip_id = chip_id;
>> + coproc->vas.rxwin = rxwin;
>> + coproc->vas.txwin = txwin;
>> +
>> + INIT_LIST_HEAD(&coproc->list);
>> + list_add(&coproc->list, &nx842_coprocs);
>
> That duplicates logic in the non-vas probe, so ideally would be shared
> or in a helper.
>
>> +
>> + return 0;
>> +
>> +err_out:
>> + if (txwin)
>> + vas_win_close(txwin);
>> +
>> + vas_win_close(rxwin);
>> +
>> + return ret;
>> +}
>> +
>> +
>> +static int __init nx842_powernv_probe_vas(struct device_node *dn)
>> +{
>> + struct device_node *nxdn, *np;
>
> There's too many device nodes in this function.
>
>> + int chip_id, vasid, rc;
>> +
>> + chip_id = of_get_ibm_chip_id(dn);
>> + if (chip_id < 0) {
>> + pr_err("ibm,chip-id missing\n");
>> + return -EINVAL;
>> + }
>> +
>> + np = of_find_node_by_name(dn, "vas");
>
> You should always search by compatible when possible. I don't see why
> you wouldn't here.
Compatible property is created with the latest VAS changes. So as suggested by Stewart, will remove search by xscom and use compatible property for VAS.
>
>
>> + if (!np) {
>> + pr_err("ibm,xscom: Missing VAS device node\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (of_property_read_u32(np, "vas-id", &vasid)) {
>> + pr_err("ibm,vas: Missing vas-id device property\n");
>> + of_node_put(np);
>> + return -EINVAL;
>> + }
>> +
>> + of_node_put(np);
>> +
>> + nxdn = of_find_compatible_node(dn, NULL, "ibm,power-nx");
>
> What are you trying to do here?
>
> This will find any node in the device tree that is compatible with
> "ibm,power-nx". It will start searching after dn in the device tree. But
> it doesn't search the children of dn necessarily, is that what you're
> trying to do?
Search has to be with in node. can I use of_get_child_by_name?
>
>> + if (!nxdn) {
>> + pr_err("ibm,xscom: Missing NX device node\n");
>> + return -EINVAL;
>> + }
>> +
>> + np = of_find_node_by_name(nxdn, "ibm,nx-842-high");
>
> Search by name again.
>
>> + if (!np) {
>> + pr_err("ibm,nx-842-high device node is missing\n");
>> + rc = -EINVAL;
>> + goto out_nd_put;
>> + }
>> +
>> + rc = vas_cfg_coproc_info(np, chip_id, vasid, VAS_COP_TYPE_842_HIPRI);
>> + of_node_put(np);
>> + if (rc)
>> + goto out_nd_put;
>> +
>> + np = of_find_node_by_name(nxdn, "ibm,nx-842-normal");
>
> Search by name again.
Do you prefer creating compatible property under these nodes?
>
> Normal vs high should not be encoded in the name, it should be a
> property of the node.
Both 842 and gzip will have normal or high FIFOs and each one will contain rx-fifo-address, lpid, pid, and tid properties. So ibm.nx-842-high and ibm,nx-842-normal device nodes are created.
/proc/device-tree/xscom@603fc00000000/nx@2010000/ibm,nx-842-high
lpid phandle rx-fifo-address
name pid tid
So do you prefer ibm,nx-842/high/
lpid phandle rx-fifo-address
name pid tid
>
>> + if (!np) {
>> + pr_err("ibm,nx-842-normal device node is missing\n");
>> + rc = -EINVAL;
>> + goto out_nd_put;
>> + }
>> +
>> + rc = vas_cfg_coproc_info(np, chip_id, vasid, VAS_COP_TYPE_842);
>> + of_node_put(np);
>> + if (!rc)
>> + return 0;
>> +
>> +out_nd_put:
>> + of_node_put(nxdn);
>> + return rc;
>> +}
>> +
>> static int __init nx842_powernv_probe(struct device_node *dn)
>> {
>> struct nx842_coproc *coproc;
>> @@ -602,11 +868,42 @@ static void nx842_delete_coproc(void)
>> struct nx842_coproc *coproc, *n;
>>
>> list_for_each_entry_safe(coproc, n, &nx842_coprocs, list) {
>> + if (is_vas_available()) {
>
> That should just be a check of coproc->vas.rxwin != NULL or similar.
>
>> + vas_win_close(coproc->vas.rxwin);
>> + /*
>> + * txwin opened only for high priority RxFIFOs
>> + */
>> + if (coproc->vas.txwin)
>> + vas_win_close(coproc->vas.txwin);
>> + }
>
> That should be pulled out into a helper, not in the middle of the loop
> here.
>
>> list_del(&coproc->list);
>> kfree(coproc);
>> }
>> }
>
>
> cheers
>
^ permalink raw reply
* Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
From: Ard Biesheuvel @ 2017-04-05 19:21 UTC (permalink / raw)
To: Matthias Kaehlcke, Herbert Xu
Cc: Greg Hackmann, David S . Miller, Catalin Marinas, Will Deacon,
Robin Murphy, linux-kernel@vger.kernel.org,
linux-crypto@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Grant Grundler,
Michael Davidson
In-Reply-To: <20170405183458.193068-1-mka@chromium.org>
On 5 April 2017 at 19:34, Matthias Kaehlcke <mka@chromium.org> wrote:
> The operand is an integer constant, make the constness explicit by
> adding the modifier. This is needed for clang to generate valid code
> and also works with gcc.
>
> Also change the constraint of the operand from 'I' ("Integer constant
> that is valid as an immediate operand in an ADD instruction", AArch64)
> to 'i' ("An immediate integer operand").
>
> Based-on-patch-from: Greg Hackmann <ghackmann@google.com>
> Signed-off-by: Greg Hackmann <ghackmann@google.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Herbert, could you please pick this up?
> ---
> Changes in v2:
> - Changed operand constraint from I to i
> - Updated commit message
> - Changed 'From' tag to 'Based-on-patch-from'
>
> arch/arm64/crypto/sha1-ce-glue.c | 2 +-
> arch/arm64/crypto/sha2-ce-glue.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
> index aefda9868627..6b520e3f3ab1 100644
> --- a/arch/arm64/crypto/sha1-ce-glue.c
> +++ b/arch/arm64/crypto/sha1-ce-glue.c
> @@ -18,7 +18,7 @@
> #include <linux/module.h>
>
> #define ASM_EXPORT(sym, val) \
> - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> + asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>
> MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
> MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
> diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
> index 7cd587564a41..e3abe11de48c 100644
> --- a/arch/arm64/crypto/sha2-ce-glue.c
> +++ b/arch/arm64/crypto/sha2-ce-glue.c
> @@ -18,7 +18,7 @@
> #include <linux/module.h>
>
> #define ASM_EXPORT(sym, val) \
> - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> + asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
>
> MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto Extensions");
> MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
> --
> 2.12.2.715.g7642488e1d-goog
>
^ permalink raw reply
* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: Michael Davidson @ 2017-04-05 19:11 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Masahiro Yamada, Michal Marek, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
Alexander Potapenko, Dmitry Vyukov, X86 ML,
Linux Kbuild mailing list, Linux Kernel Mailing List,
linux-crypto, linux-raid
In-Reply-To: <20170405190130.GD145051@google.com>
It "works" for the cases that I currently care about but I have to say
that I am uneasy about adding -Werror to the cc-option test in this
way.
Suppose that one of the *other* flags that is implicitly passed to the
compiler by cc-option - eg something that was explicitly specified in
$(KBUILD_CFLAGS) - triggers a warning. In that case all calls to
cc-option will silently fail because of the -Werror and valid options
will not be detected correctly.
If everyone is OK with that because "it shouldn't normally ever
happen" then that is fine, but if does result in a subtle change from
existing behavior (and a trap that I almost immediately fell into
after applying a similar patch).
On Wed, Apr 5, 2017 at 12:01 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi Masahiro,
>
> El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit:
>
>> 2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
>> > Unfortunately, while clang generates a warning about these flags
>> > being unsupported it still exits with a status of 0 so we have
>> > to explicitly disable them instead of just using a cc-option check.
>> >
>> > Signed-off-by: Michael Davidson <md@google.com>
>>
>>
>> Instead, does the following work for you?
>> https://patchwork.kernel.org/patch/9657285/
>
> Thanks for the pointer, I was about to give this change (or rather its
> ancestor) a rework myself :)
>
>> You need to use
>> $(call cc-option, ...)
>> for -falign-jumps=1 and -falign-loops=1
>
> I can confirm that this works.
>
> Thanks
>
> Matthias
^ permalink raw reply
* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: Matthias Kaehlcke @ 2017-04-05 19:01 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
Alexander Potapenko, Dmitry Vyukov, X86 ML,
Linux Kbuild mailing list, Linux Kernel Mailing List,
linux-crypto, linux-raid
In-Reply-To: <CAK7LNAR2efi4e3-st-hP3T7v=Hkb81beKibgYeKzxs3Q3kP0uA@mail.gmail.com>
Hi Masahiro,
El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit:
> 2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
> > Unfortunately, while clang generates a warning about these flags
> > being unsupported it still exits with a status of 0 so we have
> > to explicitly disable them instead of just using a cc-option check.
> >
> > Signed-off-by: Michael Davidson <md@google.com>
>
>
> Instead, does the following work for you?
> https://patchwork.kernel.org/patch/9657285/
Thanks for the pointer, I was about to give this change (or rather its
ancestor) a rework myself :)
> You need to use
> $(call cc-option, ...)
> for -falign-jumps=1 and -falign-loops=1
I can confirm that this works.
Thanks
Matthias
^ permalink raw reply
* [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT
From: Matthias Kaehlcke @ 2017-04-05 18:34 UTC (permalink / raw)
To: Ard Biesheuvel, Greg Hackmann, Herbert Xu, David S . Miller,
Catalin Marinas, Will Deacon, Robin Murphy
Cc: linux-kernel, linux-crypto, linux-arm-kernel, Grant Grundler,
Michael Davidson, Matthias Kaehlcke
The operand is an integer constant, make the constness explicit by
adding the modifier. This is needed for clang to generate valid code
and also works with gcc.
Also change the constraint of the operand from 'I' ("Integer constant
that is valid as an immediate operand in an ADD instruction", AArch64)
to 'i' ("An immediate integer operand").
Based-on-patch-from: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v2:
- Changed operand constraint from I to i
- Updated commit message
- Changed 'From' tag to 'Based-on-patch-from'
arch/arm64/crypto/sha1-ce-glue.c | 2 +-
arch/arm64/crypto/sha2-ce-glue.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
index aefda9868627..6b520e3f3ab1 100644
--- a/arch/arm64/crypto/sha1-ce-glue.c
+++ b/arch/arm64/crypto/sha1-ce-glue.c
@@ -18,7 +18,7 @@
#include <linux/module.h>
#define ASM_EXPORT(sym, val) \
- asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
+ asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
index 7cd587564a41..e3abe11de48c 100644
--- a/arch/arm64/crypto/sha2-ce-glue.c
+++ b/arch/arm64/crypto/sha2-ce-glue.c
@@ -18,7 +18,7 @@
#include <linux/module.h>
#define ASM_EXPORT(sym, val) \
- asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
+ asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto Extensions");
MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
--
2.12.2.715.g7642488e1d-goog
^ permalink raw reply related
* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: Masahiro Yamada @ 2017-04-05 18:08 UTC (permalink / raw)
To: Michael Davidson
Cc: Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Herbert Xu, David S. Miller, Shaohua Li, Alexander Potapenko,
Dmitry Vyukov, Matthias Kaehlcke, X86 ML,
Linux Kbuild mailing list, Linux Kernel Mailing List,
linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-3-md@google.com>
Hi Michael,
2017-03-17 9:15 GMT+09:00 Michael Davidson <md@google.com>:
> Unfortunately, while clang generates a warning about these flags
> being unsupported it still exits with a status of 0 so we have
> to explicitly disable them instead of just using a cc-option check.
>
> Signed-off-by: Michael Davidson <md@google.com>
Instead, does the following work for you?
https://patchwork.kernel.org/patch/9657285/
You need to use
$(call cc-option, ...)
for -falign-jumps=1 and -falign-loops=1
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] crypto: arm64/sha: use %c constraint code in ASM_EXPORT
From: Matthias Kaehlcke @ 2017-04-05 17:43 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Grant Grundler, Catalin Marinas, Will Deacon,
linux-kernel@vger.kernel.org, Greg Hackmann, Michael Davidson,
linux-crypto@vger.kernel.org, Robin Murphy, David S . Miller,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu-7tX6THETVA+M0UrC6rJ4id2AsHi2XfhDEmxCJ_tdwKw@mail.gmail.com>
Hoi Ard!
El Wed, Apr 05, 2017 at 06:08:37PM +0100 Ard Biesheuvel ha dit:
> On 5 April 2017 at 17:56, Matthias Kaehlcke <mka@chromium.org> wrote:
> > From: Greg Hackmann <ghackmann@google.com>
> >
> > The current definition of ASM_EXPORT doesn't work properly with clang,
> > according to https://bugs.llvm.org//show_bug.cgi?id=27250#c3 it relies on
> > gcc specific behavior. Change the constraint from an intermediate to an
> > output expression which works with both gcc and clang.
> >
> > From: Greg Hackmann <ghackmann@google.com>
> > Commit-message-by: Matthias Kaehlcke <mka@chromium.org>
> > Signed-off-by: Greg Hackmann <ghackmann@google.com>
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > arch/arm64/crypto/sha1-ce-glue.c | 2 +-
> > arch/arm64/crypto/sha2-ce-glue.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
> > index aefda9868627..c71e94ba0e43 100644
> > --- a/arch/arm64/crypto/sha1-ce-glue.c
> > +++ b/arch/arm64/crypto/sha1-ce-glue.c
> > @@ -18,7 +18,7 @@
> > #include <linux/module.h>
> >
> > #define ASM_EXPORT(sym, val) \
> > - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> > + asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
> >
> > MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
> > MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
> > diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
> > index 7cd587564a41..381b5fb2dcb2 100644
> > --- a/arch/arm64/crypto/sha2-ce-glue.c
> > +++ b/arch/arm64/crypto/sha2-ce-glue.c
> > @@ -18,7 +18,7 @@
> > #include <linux/module.h>
> >
> > #define ASM_EXPORT(sym, val) \
> > - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> > + asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
> >
> > MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto Extensions");
> > MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
>
> I am fine with this change, although I would really like to add a
> better reference to the commit log. It is *very* difficult to find any
> documentation regarding non-trivial uses of inline asm constraints,
> and if %c0 is the correct syntax, surely we can quote something better
> than a LLVM bugzilla entry? Also, where does the distinction between
> 'intermediate' vs 'output' expression come from?
To be honest assembly is not really my area of expertise and I relayed
the information from the LLVM bug. The link to the gcc doc on
x86(!) operand modifiers posted by Robin Murphy (thanks!) should be
helpful to provide a more accurate commit message.
Thanks
Matthias
^ permalink raw reply
* Re: [PATCH] crypto: arm64/sha: use %c constraint code in ASM_EXPORT
From: Ard Biesheuvel @ 2017-04-05 17:38 UTC (permalink / raw)
To: Robin Murphy
Cc: Herbert Xu, Grant Grundler, Catalin Marinas, Will Deacon,
linux-kernel@vger.kernel.org, Greg Hackmann, Michael Davidson,
Matthias Kaehlcke, linux-crypto@vger.kernel.org, David S . Miller,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <e228a810-3f53-a8d6-29c5-428b9870842b@arm.com>
On 5 April 2017 at 18:21, Robin Murphy <robin.murphy@arm.com> wrote:
> On 05/04/17 18:08, Ard Biesheuvel wrote:
>> Hoi Matthias!
>>
>> On 5 April 2017 at 17:56, Matthias Kaehlcke <mka@chromium.org> wrote:
>>> From: Greg Hackmann <ghackmann@google.com>
>>>
>>> The current definition of ASM_EXPORT doesn't work properly with clang,
>>> according to https://bugs.llvm.org//show_bug.cgi?id=27250#c3 it relies on
>>> gcc specific behavior. Change the constraint from an intermediate to an
>>> output expression which works with both gcc and clang.
>>>
>>> From: Greg Hackmann <ghackmann@google.com>
>>> Commit-message-by: Matthias Kaehlcke <mka@chromium.org>
>>> Signed-off-by: Greg Hackmann <ghackmann@google.com>
>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>> arch/arm64/crypto/sha1-ce-glue.c | 2 +-
>>> arch/arm64/crypto/sha2-ce-glue.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
>>> index aefda9868627..c71e94ba0e43 100644
>>> --- a/arch/arm64/crypto/sha1-ce-glue.c
>>> +++ b/arch/arm64/crypto/sha1-ce-glue.c
>>> @@ -18,7 +18,7 @@
>>> #include <linux/module.h>
>>>
>>> #define ASM_EXPORT(sym, val) \
>>> - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
>>> + asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
>>>
>>> MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
>>> MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
>>> diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
>>> index 7cd587564a41..381b5fb2dcb2 100644
>>> --- a/arch/arm64/crypto/sha2-ce-glue.c
>>> +++ b/arch/arm64/crypto/sha2-ce-glue.c
>>> @@ -18,7 +18,7 @@
>>> #include <linux/module.h>
>>>
>>> #define ASM_EXPORT(sym, val) \
>>> - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
>>> + asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
>>>
>>> MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto Extensions");
>>> MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
>>
>> I am fine with this change, although I would really like to add a
>> better reference to the commit log. It is *very* difficult to find any
>> documentation regarding non-trivial uses of inline asm constraints,
>> and if %c0 is the correct syntax, surely we can quote something better
>> than a LLVM bugzilla entry? Also, where does the distinction between
>> 'intermediate' vs 'output' expression come from?
>
> FWIW, GCC docs do say (under the helpfully-obvious "x86 operand
> modifiers" section[1]):
>
> c Require a constant operand and print the constant
> expression with no punctuation.
>
> Which more or less makes sense in this this context too. As an aside,
> though, since this is emitting a general integer argument to an
> assembler directive, and not an operand to an ADD instruction, how come
> we're using "I" and not "i" as the constraint in the first place?
>
No reason. "I" came to mind when writing the code, and worked as
expected. Perhaps we should just fix that at the same time.
^ permalink raw reply
* Re: [PATCH] crypto: arm64/sha: use %c constraint code in ASM_EXPORT
From: Robin Murphy @ 2017-04-05 17:21 UTC (permalink / raw)
To: Ard Biesheuvel, Matthias Kaehlcke
Cc: Herbert Xu, Grant Grundler, Catalin Marinas, Will Deacon,
linux-kernel@vger.kernel.org, Greg Hackmann, Michael Davidson,
linux-crypto@vger.kernel.org, David S . Miller,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu-7tX6THETVA+M0UrC6rJ4id2AsHi2XfhDEmxCJ_tdwKw@mail.gmail.com>
On 05/04/17 18:08, Ard Biesheuvel wrote:
> Hoi Matthias!
>
> On 5 April 2017 at 17:56, Matthias Kaehlcke <mka@chromium.org> wrote:
>> From: Greg Hackmann <ghackmann@google.com>
>>
>> The current definition of ASM_EXPORT doesn't work properly with clang,
>> according to https://bugs.llvm.org//show_bug.cgi?id=27250#c3 it relies on
>> gcc specific behavior. Change the constraint from an intermediate to an
>> output expression which works with both gcc and clang.
>>
>> From: Greg Hackmann <ghackmann@google.com>
>> Commit-message-by: Matthias Kaehlcke <mka@chromium.org>
>> Signed-off-by: Greg Hackmann <ghackmann@google.com>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> arch/arm64/crypto/sha1-ce-glue.c | 2 +-
>> arch/arm64/crypto/sha2-ce-glue.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
>> index aefda9868627..c71e94ba0e43 100644
>> --- a/arch/arm64/crypto/sha1-ce-glue.c
>> +++ b/arch/arm64/crypto/sha1-ce-glue.c
>> @@ -18,7 +18,7 @@
>> #include <linux/module.h>
>>
>> #define ASM_EXPORT(sym, val) \
>> - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
>> + asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
>>
>> MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
>> MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
>> diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
>> index 7cd587564a41..381b5fb2dcb2 100644
>> --- a/arch/arm64/crypto/sha2-ce-glue.c
>> +++ b/arch/arm64/crypto/sha2-ce-glue.c
>> @@ -18,7 +18,7 @@
>> #include <linux/module.h>
>>
>> #define ASM_EXPORT(sym, val) \
>> - asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
>> + asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
>>
>> MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto Extensions");
>> MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
>
> I am fine with this change, although I would really like to add a
> better reference to the commit log. It is *very* difficult to find any
> documentation regarding non-trivial uses of inline asm constraints,
> and if %c0 is the correct syntax, surely we can quote something better
> than a LLVM bugzilla entry? Also, where does the distinction between
> 'intermediate' vs 'output' expression come from?
FWIW, GCC docs do say (under the helpfully-obvious "x86 operand
modifiers" section[1]):
c Require a constant operand and print the constant
expression with no punctuation.
Which more or less makes sense in this this context too. As an aside,
though, since this is emitting a general integer argument to an
assembler directive, and not an operand to an ADD instruction, how come
we're using "I" and not "i" as the constraint in the first place?
Robin.
[1]:https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#x86-Operand-Modifiers
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ 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