* Re: PIE binaries are no longer mapped below 4 GiB on ppc64le
From: Michal Suchánek @ 2018-10-31 22:41 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho
Cc: Florian Weimer, Lynn A. Boger, linuxppc-dev, linux-mm
In-Reply-To: <87bm79n57l.fsf@linux.ibm.com>
On Wed, 31 Oct 2018 19:04:14 -0300
Tulio Magno Quites Machado Filho <tuliom@ascii.art.br> wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>
> > * Tulio Magno Quites Machado Filho:
> >
> >> I wonder if this is restricted to linker that Golang uses.
> >> Were you able to reproduce the same problem with Binutils'
> >> linker?
> >
> > The example is carefully constructed to use the external linker. It
> > invokes gcc, which then invokes the BFD linker in my case.
>
> Indeed. That question was unnecessary. :-D
>
> > Based on the relocations, I assume there is only so much the linker
> > can do here. I'm amazed that it produces an executable at all, let
> > alone one that runs correctly on some kernel versions!
>
> Agreed. That isn't expected to work. Both the compiler and the
> linker have to generate PIE for it to work.
>
> > I assume that the Go toolchain simply lacks PIE support on
> > ppc64le.
>
> Maybe the support is there, but it doesn't generate PIC by default?
>
golang has -fPIC IIRC. It does not benefit from the GNU toolchian
synergy of always calling the linker with the correct flags
corresponding to the generated code, though. So when gcc flips the
switch default value golang happily produces incompatible objects.
Also I suspect some pieces of stdlib are not compiled with the flags
you pass in for the build so there are always some objects somewhere
that are not compatible.
Thanks
Michal
^ permalink raw reply
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Paul Burton @ 2018-10-31 23:32 UTC (permalink / raw)
To: Guenter Roeck, Trond Myklebust
Cc: linux-mips@linux-mips.org, linux-nfs@vger.kernel.org,
Arnd Bergmann, James Hogan, Jeff Layton,
linux-kernel@vger.kernel.org, Ralf Baechle, David S. Miller,
J. Bruce Fields, netdev@vger.kernel.org, Paul Mackerras,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, Anna Schumaker
In-Reply-To: <20181031220253.GA15505@roeck-us.net>
(Copying SunRPC & net maintainers.)
Hi Guenter,
On Wed, Oct 31, 2018 at 03:02:53PM -0700, Guenter Roeck wrote:
> The alternatives I can see are
> - Do not use cmpxchg64() outside architecture code (ie drop its use from
> the offending driver, and keep doing the same whenever the problem comes
> up again).
> or
> - Introduce something like ARCH_HAS_CMPXCHG64 and use it to determine
> if cmpxchg64 is supported or not.
>
> Any preference ?
My preference would be option 1 - avoiding cmpxchg64() where possible in
generic code. I wouldn't be opposed to the Kconfig option if there are
cases where cmpxchg64() can really help performance though.
The last time I'm aware of this coming up the affected driver was
modified to avoid cmpxchg64() [1].
In this particular case I have no idea why
net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all. It's
essentially reinventing atomic64_fetch_inc() which is already provided
everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At least
for atomic64_* functions the assumption that all access will be
performed using those same functions seems somewhat reasonable.
So how does the below look? Trond?
Thanks,
Paul
[1] https://patchwork.ozlabs.org/cover/891284/
---
diff --git a/include/linux/sunrpc/gss_krb5.h b/include/linux/sunrpc/gss_krb5.h
index 131424cefc6a..02c0412e368c 100644
--- a/include/linux/sunrpc/gss_krb5.h
+++ b/include/linux/sunrpc/gss_krb5.h
@@ -107,8 +107,8 @@ struct krb5_ctx {
u8 Ksess[GSS_KRB5_MAX_KEYLEN]; /* session key */
u8 cksum[GSS_KRB5_MAX_KEYLEN];
s32 endtime;
- u32 seq_send;
- u64 seq_send64;
+ atomic_t seq_send;
+ atomic64_t seq_send64;
struct xdr_netobj mech_used;
u8 initiator_sign[GSS_KRB5_MAX_KEYLEN];
u8 acceptor_sign[GSS_KRB5_MAX_KEYLEN];
@@ -118,9 +118,6 @@ struct krb5_ctx {
u8 acceptor_integ[GSS_KRB5_MAX_KEYLEN];
};
-extern u32 gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx);
-extern u64 gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx);
-
/* The length of the Kerberos GSS token header */
#define GSS_KRB5_TOK_HDR_LEN (16)
diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c
index 7f0424dfa8f6..eab71fc7af3e 100644
--- a/net/sunrpc/auth_gss/gss_krb5_mech.c
+++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
@@ -274,6 +274,7 @@ get_key(const void *p, const void *end,
static int
gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
{
+ u32 seq_send;
int tmp;
p = simple_get_bytes(p, end, &ctx->initiate, sizeof(ctx->initiate));
@@ -315,9 +316,10 @@ gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx->endtime));
if (IS_ERR(p))
goto out_err;
- p = simple_get_bytes(p, end, &ctx->seq_send, sizeof(ctx->seq_send));
+ p = simple_get_bytes(p, end, &seq_send, sizeof(seq_send));
if (IS_ERR(p))
goto out_err;
+ atomic_set(&ctx->seq_send, seq_send);
p = simple_get_netobj(p, end, &ctx->mech_used);
if (IS_ERR(p))
goto out_err;
@@ -607,6 +609,7 @@ static int
gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
gfp_t gfp_mask)
{
+ u64 seq_send64;
int keylen;
p = simple_get_bytes(p, end, &ctx->flags, sizeof(ctx->flags));
@@ -617,14 +620,15 @@ gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx->endtime));
if (IS_ERR(p))
goto out_err;
- p = simple_get_bytes(p, end, &ctx->seq_send64, sizeof(ctx->seq_send64));
+ p = simple_get_bytes(p, end, &seq_send64, sizeof(seq_send64));
if (IS_ERR(p))
goto out_err;
+ atomic64_set(&ctx->seq_send64, seq_send64);
/* set seq_send for use by "older" enctypes */
- ctx->seq_send = ctx->seq_send64;
- if (ctx->seq_send64 != ctx->seq_send) {
- dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n", __func__,
- (unsigned long)ctx->seq_send64, ctx->seq_send);
+ atomic_set(&ctx->seq_send, seq_send64);
+ if (seq_send64 != atomic_read(&ctx->seq_send)) {
+ dprintk("%s: seq_send64 %llx, seq_send %x overflow?\n", __func__,
+ seq_send64, atomic_read(&ctx->seq_send));
p = ERR_PTR(-EINVAL);
goto out_err;
}
diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c b/net/sunrpc/auth_gss/gss_krb5_seal.c
index b4adeb06660b..48fe4a591b54 100644
--- a/net/sunrpc/auth_gss/gss_krb5_seal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
@@ -123,30 +123,6 @@ setup_token_v2(struct krb5_ctx *ctx, struct xdr_netobj *token)
return krb5_hdr;
}
-u32
-gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx)
-{
- u32 old, seq_send = READ_ONCE(ctx->seq_send);
-
- do {
- old = seq_send;
- seq_send = cmpxchg(&ctx->seq_send, old, old + 1);
- } while (old != seq_send);
- return seq_send;
-}
-
-u64
-gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx)
-{
- u64 old, seq_send = READ_ONCE(ctx->seq_send);
-
- do {
- old = seq_send;
- seq_send = cmpxchg64(&ctx->seq_send64, old, old + 1);
- } while (old != seq_send);
- return seq_send;
-}
-
static u32
gss_get_mic_v1(struct krb5_ctx *ctx, struct xdr_buf *text,
struct xdr_netobj *token)
@@ -177,7 +153,7 @@ gss_get_mic_v1(struct krb5_ctx *ctx, struct xdr_buf *text,
memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data, md5cksum.len);
- seq_send = gss_seq_send_fetch_and_inc(ctx);
+ seq_send = atomic_fetch_inc(&ctx->seq_send);
if (krb5_make_seq_num(ctx, ctx->seq, ctx->initiate ? 0 : 0xff,
seq_send, ptr + GSS_KRB5_TOK_HDR_LEN, ptr + 8))
@@ -205,7 +181,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
/* Set up the sequence number. Now 64-bits in clear
* text and w/o direction indicator */
- seq_send_be64 = cpu_to_be64(gss_seq_send64_fetch_and_inc(ctx));
+ seq_send_be64 = cpu_to_be64(atomic64_fetch_inc(&ctx->seq_send64));
memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
if (ctx->initiate) {
diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
index 962fa84e6db1..5cdde6cb703a 100644
--- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
+++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
@@ -228,7 +228,7 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int offset,
memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data, md5cksum.len);
- seq_send = gss_seq_send_fetch_and_inc(kctx);
+ seq_send = atomic_fetch_inc(&kctx->seq_send);
/* XXX would probably be more efficient to compute checksum
* and encrypt at the same time: */
@@ -475,7 +475,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32 offset,
*be16ptr++ = 0;
be64ptr = (__be64 *)be16ptr;
- *be64ptr = cpu_to_be64(gss_seq_send64_fetch_and_inc(kctx));
+ *be64ptr = cpu_to_be64(atomic64_fetch_inc(&kctx->seq_send64));
err = (*kctx->gk5e->encrypt_v2)(kctx, offset, buf, pages);
if (err)
^ permalink raw reply related
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Trond Myklebust @ 2018-11-01 0:17 UTC (permalink / raw)
To: linux@roeck-us.net, paul.burton@mips.com
Cc: linux-mips@linux-mips.org, linux-nfs@vger.kernel.org,
arnd@arndb.de, jhogan@kernel.org, jlayton@kernel.org,
linux-kernel@vger.kernel.org, ralf@linux-mips.org,
davem@davemloft.net, bfields@fieldses.org, paulus@samba.org,
netdev@vger.kernel.org, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, anna.schumaker@netapp.com
In-Reply-To: <20181031233235.qbedw3pinxcuk7me@pburton-laptop>
Hi Paul,
On Wed, 2018-10-31 at 23:32 +0000, Paul Burton wrote:
> (Copying SunRPC & net maintainers.)
>
> Hi Guenter,
>
> On Wed, Oct 31, 2018 at 03:02:53PM -0700, Guenter Roeck wrote:
> > The alternatives I can see are
> > - Do not use cmpxchg64() outside architecture code (ie drop its use
> > from
> > the offending driver, and keep doing the same whenever the
> > problem comes
> > up again).
> > or
> > - Introduce something like ARCH_HAS_CMPXCHG64 and use it to
> > determine
> > if cmpxchg64 is supported or not.
> >
> > Any preference ?
>
> My preference would be option 1 - avoiding cmpxchg64() where possible
> in
> generic code. I wouldn't be opposed to the Kconfig option if there
> are
> cases where cmpxchg64() can really help performance though.
>
> The last time I'm aware of this coming up the affected driver was
> modified to avoid cmpxchg64() [1].
>
> In this particular case I have no idea why
> net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all. It's
> essentially reinventing atomic64_fetch_inc() which is already
> provided
> everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At
> least
> for atomic64_* functions the assumption that all access will be
> performed using those same functions seems somewhat reasonable.
>
> So how does the below look? Trond?
>
My one question (and the reason why I went with cmpxchg() in the first
place) would be about the overflow behaviour for atomic_fetch_inc() and
friends. I believe those functions should be OK on x86, so that when we
overflow the counter, it behaves like an unsigned value and wraps back
around. Is that the case for all architectures?
i.e. are atomic_t/atomic64_t always guaranteed to behave like u32/u64
on increment?
I could not find any documentation that explicitly stated that they
should.
Cheers
Trond
> Thanks,
> Paul
>
> [1] https://patchwork.ozlabs.org/cover/891284/
>
> ---
> diff --git a/include/linux/sunrpc/gss_krb5.h
> b/include/linux/sunrpc/gss_krb5.h
> index 131424cefc6a..02c0412e368c 100644
> --- a/include/linux/sunrpc/gss_krb5.h
> +++ b/include/linux/sunrpc/gss_krb5.h
> @@ -107,8 +107,8 @@ struct krb5_ctx {
> u8 Ksess[GSS_KRB5_MAX_KEYLEN]; /*
> session key */
> u8 cksum[GSS_KRB5_MAX_KEYLEN];
> s32 endtime;
> - u32 seq_send;
> - u64 seq_send64;
> + atomic_t seq_send;
> + atomic64_t seq_send64;
> struct xdr_netobj mech_used;
> u8 initiator_sign[GSS_KRB5_MAX_KEYLEN];
> u8 acceptor_sign[GSS_KRB5_MAX_KEYLEN];
> @@ -118,9 +118,6 @@ struct krb5_ctx {
> u8 acceptor_integ[GSS_KRB5_MAX_KEYLEN];
> };
>
> -extern u32 gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx);
> -extern u64 gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx);
> -
> /* The length of the Kerberos GSS token header */
> #define GSS_KRB5_TOK_HDR_LEN (16)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c
> b/net/sunrpc/auth_gss/gss_krb5_mech.c
> index 7f0424dfa8f6..eab71fc7af3e 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_mech.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
> @@ -274,6 +274,7 @@ get_key(const void *p, const void *end,
> static int
> gss_import_v1_context(const void *p, const void *end, struct
> krb5_ctx *ctx)
> {
> + u32 seq_send;
> int tmp;
>
> p = simple_get_bytes(p, end, &ctx->initiate, sizeof(ctx-
> >initiate));
> @@ -315,9 +316,10 @@ gss_import_v1_context(const void *p, const void
> *end, struct krb5_ctx *ctx)
> p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx-
> >endtime));
> if (IS_ERR(p))
> goto out_err;
> - p = simple_get_bytes(p, end, &ctx->seq_send, sizeof(ctx-
> >seq_send));
> + p = simple_get_bytes(p, end, &seq_send, sizeof(seq_send));
> if (IS_ERR(p))
> goto out_err;
> + atomic_set(&ctx->seq_send, seq_send);
> p = simple_get_netobj(p, end, &ctx->mech_used);
> if (IS_ERR(p))
> goto out_err;
> @@ -607,6 +609,7 @@ static int
> gss_import_v2_context(const void *p, const void *end, struct
> krb5_ctx *ctx,
> gfp_t gfp_mask)
> {
> + u64 seq_send64;
> int keylen;
>
> p = simple_get_bytes(p, end, &ctx->flags, sizeof(ctx->flags));
> @@ -617,14 +620,15 @@ gss_import_v2_context(const void *p, const void
> *end, struct krb5_ctx *ctx,
> p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx-
> >endtime));
> if (IS_ERR(p))
> goto out_err;
> - p = simple_get_bytes(p, end, &ctx->seq_send64, sizeof(ctx-
> >seq_send64));
> + p = simple_get_bytes(p, end, &seq_send64, sizeof(seq_send64));
> if (IS_ERR(p))
> goto out_err;
> + atomic64_set(&ctx->seq_send64, seq_send64);
> /* set seq_send for use by "older" enctypes */
> - ctx->seq_send = ctx->seq_send64;
> - if (ctx->seq_send64 != ctx->seq_send) {
> - dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n",
> __func__,
> - (unsigned long)ctx->seq_send64, ctx->seq_send);
> + atomic_set(&ctx->seq_send, seq_send64);
> + if (seq_send64 != atomic_read(&ctx->seq_send)) {
> + dprintk("%s: seq_send64 %llx, seq_send %x overflow?\n",
> __func__,
> + seq_send64, atomic_read(&ctx->seq_send));
> p = ERR_PTR(-EINVAL);
> goto out_err;
> }
> diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c
> b/net/sunrpc/auth_gss/gss_krb5_seal.c
> index b4adeb06660b..48fe4a591b54 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_seal.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
> @@ -123,30 +123,6 @@ setup_token_v2(struct krb5_ctx *ctx, struct
> xdr_netobj *token)
> return krb5_hdr;
> }
>
> -u32
> -gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx)
> -{
> - u32 old, seq_send = READ_ONCE(ctx->seq_send);
> -
> - do {
> - old = seq_send;
> - seq_send = cmpxchg(&ctx->seq_send, old, old + 1);
> - } while (old != seq_send);
> - return seq_send;
> -}
> -
> -u64
> -gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx)
> -{
> - u64 old, seq_send = READ_ONCE(ctx->seq_send);
> -
> - do {
> - old = seq_send;
> - seq_send = cmpxchg64(&ctx->seq_send64, old, old + 1);
> - } while (old != seq_send);
> - return seq_send;
> -}
> -
> static u32
> gss_get_mic_v1(struct krb5_ctx *ctx, struct xdr_buf *text,
> struct xdr_netobj *token)
> @@ -177,7 +153,7 @@ gss_get_mic_v1(struct krb5_ctx *ctx, struct
> xdr_buf *text,
>
> memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data,
> md5cksum.len);
>
> - seq_send = gss_seq_send_fetch_and_inc(ctx);
> + seq_send = atomic_fetch_inc(&ctx->seq_send);
>
> if (krb5_make_seq_num(ctx, ctx->seq, ctx->initiate ? 0 : 0xff,
> seq_send, ptr + GSS_KRB5_TOK_HDR_LEN, ptr
> + 8))
> @@ -205,7 +181,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct
> xdr_buf *text,
>
> /* Set up the sequence number. Now 64-bits in clear
> * text and w/o direction indicator */
> - seq_send_be64 = cpu_to_be64(gss_seq_send64_fetch_and_inc(ctx));
> + seq_send_be64 = cpu_to_be64(atomic64_fetch_inc(&ctx-
> >seq_send64));
> memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
>
> if (ctx->initiate) {
> diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> index 962fa84e6db1..5cdde6cb703a 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> @@ -228,7 +228,7 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int
> offset,
>
> memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data,
> md5cksum.len);
>
> - seq_send = gss_seq_send_fetch_and_inc(kctx);
> + seq_send = atomic_fetch_inc(&kctx->seq_send);
>
> /* XXX would probably be more efficient to compute checksum
> * and encrypt at the same time: */
> @@ -475,7 +475,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32
> offset,
> *be16ptr++ = 0;
>
> be64ptr = (__be64 *)be16ptr;
> - *be64ptr = cpu_to_be64(gss_seq_send64_fetch_and_inc(kctx));
> + *be64ptr = cpu_to_be64(atomic64_fetch_inc(&kctx->seq_send64));
>
> err = (*kctx->gk5e->encrypt_v2)(kctx, offset, buf, pages);
> if (err)
--
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space
^ permalink raw reply
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Guenter Roeck @ 2018-11-01 1:18 UTC (permalink / raw)
To: Paul Burton, Trond Myklebust
Cc: linux-mips@linux-mips.org, linux-nfs@vger.kernel.org,
Arnd Bergmann, James Hogan, Jeff Layton,
linux-kernel@vger.kernel.org, Ralf Baechle, David S. Miller,
J. Bruce Fields, netdev@vger.kernel.org, Paul Mackerras,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, Anna Schumaker
In-Reply-To: <20181031233235.qbedw3pinxcuk7me@pburton-laptop>
On 10/31/18 4:32 PM, Paul Burton wrote:
> (Copying SunRPC & net maintainers.)
>
> Hi Guenter,
>
> On Wed, Oct 31, 2018 at 03:02:53PM -0700, Guenter Roeck wrote:
>> The alternatives I can see are
>> - Do not use cmpxchg64() outside architecture code (ie drop its use from
>> the offending driver, and keep doing the same whenever the problem comes
>> up again).
>> or
>> - Introduce something like ARCH_HAS_CMPXCHG64 and use it to determine
>> if cmpxchg64 is supported or not.
>>
>> Any preference ?
>
> My preference would be option 1 - avoiding cmpxchg64() where possible in
> generic code. I wouldn't be opposed to the Kconfig option if there are
> cases where cmpxchg64() can really help performance though.
>
> The last time I'm aware of this coming up the affected driver was
> modified to avoid cmpxchg64() [1].
>
> In this particular case I have no idea why
> net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all. It's
> essentially reinventing atomic64_fetch_inc() which is already provided
> everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At least
> for atomic64_* functions the assumption that all access will be
> performed using those same functions seems somewhat reasonable.
>
> So how does the below look? Trond?
>
For my part I agree that this would be a much better solution. The argument
that it is not always absolutely guaranteed that atomics don't wrap doesn't
really hold for me because it looks like they all do. On top of that, there
is an explicit atomic_dec_if_positive() and atomic_fetch_add_unless(),
which to me strongly suggests that they _are_ supposed to wrap.
Given the cost of adding a comparison to each atomic operation to
prevent it from wrapping, anything else would not really make sense to me.
So ... please consider my patch abandoned. Thanks for looking into this!
Guenter
> Thanks,
> Paul
>
> [1] https://patchwork.ozlabs.org/cover/891284/
>
> ---
> diff --git a/include/linux/sunrpc/gss_krb5.h b/include/linux/sunrpc/gss_krb5.h
> index 131424cefc6a..02c0412e368c 100644
> --- a/include/linux/sunrpc/gss_krb5.h
> +++ b/include/linux/sunrpc/gss_krb5.h
> @@ -107,8 +107,8 @@ struct krb5_ctx {
> u8 Ksess[GSS_KRB5_MAX_KEYLEN]; /* session key */
> u8 cksum[GSS_KRB5_MAX_KEYLEN];
> s32 endtime;
> - u32 seq_send;
> - u64 seq_send64;
> + atomic_t seq_send;
> + atomic64_t seq_send64;
> struct xdr_netobj mech_used;
> u8 initiator_sign[GSS_KRB5_MAX_KEYLEN];
> u8 acceptor_sign[GSS_KRB5_MAX_KEYLEN];
> @@ -118,9 +118,6 @@ struct krb5_ctx {
> u8 acceptor_integ[GSS_KRB5_MAX_KEYLEN];
> };
>
> -extern u32 gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx);
> -extern u64 gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx);
> -
> /* The length of the Kerberos GSS token header */
> #define GSS_KRB5_TOK_HDR_LEN (16)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c
> index 7f0424dfa8f6..eab71fc7af3e 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_mech.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
> @@ -274,6 +274,7 @@ get_key(const void *p, const void *end,
> static int
> gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
> {
> + u32 seq_send;
> int tmp;
>
> p = simple_get_bytes(p, end, &ctx->initiate, sizeof(ctx->initiate));
> @@ -315,9 +316,10 @@ gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
> p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx->endtime));
> if (IS_ERR(p))
> goto out_err;
> - p = simple_get_bytes(p, end, &ctx->seq_send, sizeof(ctx->seq_send));
> + p = simple_get_bytes(p, end, &seq_send, sizeof(seq_send));
> if (IS_ERR(p))
> goto out_err;
> + atomic_set(&ctx->seq_send, seq_send);
> p = simple_get_netobj(p, end, &ctx->mech_used);
> if (IS_ERR(p))
> goto out_err;
> @@ -607,6 +609,7 @@ static int
> gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
> gfp_t gfp_mask)
> {
> + u64 seq_send64;
> int keylen;
>
> p = simple_get_bytes(p, end, &ctx->flags, sizeof(ctx->flags));
> @@ -617,14 +620,15 @@ gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
> p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx->endtime));
> if (IS_ERR(p))
> goto out_err;
> - p = simple_get_bytes(p, end, &ctx->seq_send64, sizeof(ctx->seq_send64));
> + p = simple_get_bytes(p, end, &seq_send64, sizeof(seq_send64));
> if (IS_ERR(p))
> goto out_err;
> + atomic64_set(&ctx->seq_send64, seq_send64);
> /* set seq_send for use by "older" enctypes */
> - ctx->seq_send = ctx->seq_send64;
> - if (ctx->seq_send64 != ctx->seq_send) {
> - dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n", __func__,
> - (unsigned long)ctx->seq_send64, ctx->seq_send);
> + atomic_set(&ctx->seq_send, seq_send64);
> + if (seq_send64 != atomic_read(&ctx->seq_send)) {
> + dprintk("%s: seq_send64 %llx, seq_send %x overflow?\n", __func__,
> + seq_send64, atomic_read(&ctx->seq_send));
> p = ERR_PTR(-EINVAL);
> goto out_err;
> }
> diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c b/net/sunrpc/auth_gss/gss_krb5_seal.c
> index b4adeb06660b..48fe4a591b54 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_seal.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
> @@ -123,30 +123,6 @@ setup_token_v2(struct krb5_ctx *ctx, struct xdr_netobj *token)
> return krb5_hdr;
> }
>
> -u32
> -gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx)
> -{
> - u32 old, seq_send = READ_ONCE(ctx->seq_send);
> -
> - do {
> - old = seq_send;
> - seq_send = cmpxchg(&ctx->seq_send, old, old + 1);
> - } while (old != seq_send);
> - return seq_send;
> -}
> -
> -u64
> -gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx)
> -{
> - u64 old, seq_send = READ_ONCE(ctx->seq_send);
> -
> - do {
> - old = seq_send;
> - seq_send = cmpxchg64(&ctx->seq_send64, old, old + 1);
> - } while (old != seq_send);
> - return seq_send;
> -}
> -
> static u32
> gss_get_mic_v1(struct krb5_ctx *ctx, struct xdr_buf *text,
> struct xdr_netobj *token)
> @@ -177,7 +153,7 @@ gss_get_mic_v1(struct krb5_ctx *ctx, struct xdr_buf *text,
>
> memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data, md5cksum.len);
>
> - seq_send = gss_seq_send_fetch_and_inc(ctx);
> + seq_send = atomic_fetch_inc(&ctx->seq_send);
>
> if (krb5_make_seq_num(ctx, ctx->seq, ctx->initiate ? 0 : 0xff,
> seq_send, ptr + GSS_KRB5_TOK_HDR_LEN, ptr + 8))
> @@ -205,7 +181,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
>
> /* Set up the sequence number. Now 64-bits in clear
> * text and w/o direction indicator */
> - seq_send_be64 = cpu_to_be64(gss_seq_send64_fetch_and_inc(ctx));
> + seq_send_be64 = cpu_to_be64(atomic64_fetch_inc(&ctx->seq_send64));
> memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
>
> if (ctx->initiate) {
> diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> index 962fa84e6db1..5cdde6cb703a 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> @@ -228,7 +228,7 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int offset,
>
> memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data, md5cksum.len);
>
> - seq_send = gss_seq_send_fetch_and_inc(kctx);
> + seq_send = atomic_fetch_inc(&kctx->seq_send);
>
> /* XXX would probably be more efficient to compute checksum
> * and encrypt at the same time: */
> @@ -475,7 +475,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32 offset,
> *be16ptr++ = 0;
>
> be64ptr = (__be64 *)be16ptr;
> - *be64ptr = cpu_to_be64(gss_seq_send64_fetch_and_inc(kctx));
> + *be64ptr = cpu_to_be64(atomic64_fetch_inc(&kctx->seq_send64));
>
> err = (*kctx->gk5e->encrypt_v2)(kctx, offset, buf, pages);
> if (err)
>
^ permalink raw reply
* Re:Re:chargers and adapters in European Market.
From: Lisa Gao @ 2018-11-01 1:21 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/html, Size: 556 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] Guarded Userspace Access Prevention on Radix
From: Russell Currey @ 2018-11-01 3:54 UTC (permalink / raw)
To: LEROY Christophe; +Cc: mikey, linuxppc-dev, npiggin
In-Reply-To: <20181031175845.Horde.o95YpPKYLOaCZUubOS1NRw1@messagerie.si.c-s.fr>
On Wed, 2018-10-31 at 17:58 +0100, LEROY Christophe wrote:
> Russell Currey <ruscur@russell.cc> a écrit :
>
> > On Fri, 2018-10-26 at 18:29 +0200, LEROY Christophe wrote:
> > > Russell Currey <ruscur@russell.cc> a écrit :
> > >
> > > > Guarded Userspace Access Prevention is a security mechanism
> > > > that
> > > > prevents
> > > > the kernel from being able to read and write userspace
> > > > addresses
> > > > outside of
> > > > the allowed paths, most commonly copy_{to/from}_user().
> > > >
> > > > At present, the only CPU that supports this is POWER9, and only
> > > > while using
> > > > the Radix MMU. Privileged reads and writes cannot access user
> > > > data
> > > > when
> > > > key 0 of the AMR is set. This is described in the "Radix Tree
> > > > Translation
> > > > Storage Protection" section of the POWER ISA as of version 3.0.
> > >
> > > It is not right that only power9 can support that.
> >
> > It's true that not only P9 can support it, but there are more
> > considerations under hash than radix, implementing this for radix
> > is a
> > first step.
>
> I don't know much about hash, but I was talking about the 8xx which
> is
> a nohash ppc32. I'll see next week if I can do something with it on
> top of your serie.
My small brain saw the number 8 and assumed you were talking about
POWER8, I didn't know what 8xx was until now.
Working on a refactor to make things a bit more generic, and removing
the radix name and dependency from the config option.
>
> > > The 8xx has mmu access protection registers which can serve the
> > > same
> > > purpose. Today on the 8xx kernel space is group 0 and user space
> > > is
> > > group 1. Group 0 is set to "page defined access permission" in
> > > MD_AP
> > > and MI_AP registers, and group 1 is set to "all accesses done
> > > with
> > > supervisor rights". By setting group 1 to "user and supervisor
> > > interpretation swapped" we can forbid kernel access to user space
> > > while still allowing user access to it. Then by simply changing
> > > group
> > > 1 mode at dedicated places we can lock/unlock kernel access to
> > > user
> > > space.
> > >
> > > Could you implement something as generic as possible having that
> > > in
> > > mind for a future patch ?
> >
> > I don't think anything in this series is particularly problematic
> > in
> > relation to future work for hash. I am interested in doing a hash
> > implementation in future too.
>
> I think we have to look at that carrefuly to avoid uggly ifdefs
>
> Christophe
>
> > - Russell
> >
> > > Christophe
> > >
> > > > GUAP code sets key 0 of the AMR (thus disabling accesses of
> > > > user
> > > > data)
> > > > early during boot, and only ever "unlocks" access prior to
> > > > certain
> > > > operations, like copy_{to/from}_user(), futex ops,
> > > > etc. Setting
> > > > this does
> > > > not prevent unprivileged access, so userspace can operate fine
> > > > while access
> > > > is locked.
> > > >
> > > > There is a performance impact, although I don't consider it
> > > > heavy. Running
> > > > a worst-case benchmark of a 1GB copy 1 byte at a time (and thus
> > > > constant
> > > > read(1) write(1) syscalls), I found enabling GUAP to be 3.5%
> > > > slower
> > > > than
> > > > when disabled. In most cases, the difference is
> > > > negligible. The
> > > > main
> > > > performance impact is the mtspr instruction, which is quite
> > > > slow.
> > > >
> > > > There are a few caveats with this series that could be improved
> > > > upon in
> > > > future. Right now there is no saving and restoring of the AMR
> > > > value -
> > > > there is no userspace exploitation of the AMR on Radix in
> > > > POWER9,
> > > > but if
> > > > this were to change in future, saving and restoring the value
> > > > would
> > > > be
> > > > necessary.
> > > >
> > > > No attempt to optimise cases of repeated calls - for example,
> > > > if
> > > > some
> > > > code was repeatedly calling copy_to_user() for small sizes very
> > > > frequently,
> > > > it would be slower than the equivalent of wrapping that code in
> > > > an
> > > > unlock
> > > > and lock and only having to modify the AMR once.
> > > >
> > > > There are some interesting cases that I've attempted to handle,
> > > > such as if
> > > > the AMR is unlocked (i.e. because a copy_{to_from}_user is in
> > > > progress)...
> > > >
> > > > - and an exception is taken, the kernel would then be
> > > > running
> > > > with the
> > > > AMR unlocked and freely able to access userspace again. I
> > > > am
> > > > working
> > > > around this by storing a flag in the PACA to indicate if
> > > > the
> > > > AMR is
> > > > unlocked (to save a costly SPR read), and if so, locking
> > > > the
> > > > AMR in
> > > > the exception entry path and unlocking it on the way out.
> > > >
> > > > - and gets context switched out, goes into a path that
> > > > locks
> > > > the AMR,
> > > > then context switches back, access will be disabled and
> > > > will
> > > > fault.
> > > > As a result, I context switch the AMR between tasks as if
> > > > it
> > > > was used
> > > > by userspace like hash (which already implements this).
> > > >
> > > > Another consideration is use of the isync instruction. Without
> > > > an
> > > > isync
> > > > following the mtspr instruction, there is no guarantee that the
> > > > change
> > > > takes effect. The issue is that isync is very slow, and so I
> > > > tried
> > > > to
> > > > avoid them wherever necessary. In this series, the only place
> > > > an
> > > > isync
> > > > gets used is after *unlocking* the AMR, because if an access
> > > > takes
> > > > place
> > > > and access is still prevented, the kernel will fault.
> > > >
> > > > On the flipside, a slight delay in unlocking caused by skipping
> > > > an
> > > > isync
> > > > potentially allows a small window of vulnerability. It is my
> > > > opinion
> > > > that this window is practically impossible to exploit, but if
> > > > someone
> > > > thinks otherwise, please do share.
> > > >
> > > > This series is my first attempt at POWER assembly so all
> > > > feedback
> > > > is very
> > > > welcome.
> > > >
> > > > The official theme song of this series can be found here:
> > > > https://www.youtube.com/watch?v=QjTrnKAcYjE
> > > >
> > > > Russell Currey (5):
> > > > powerpc/64s: Guarded Userspace Access Prevention
> > > > powerpc/futex: GUAP support for futex ops
> > > > powerpc/lib: checksum GUAP support
> > > > powerpc/64s: Disable GUAP with nosmap option
> > > > powerpc/64s: Document that PPC supports nosmap
> > > >
> > > > .../admin-guide/kernel-parameters.txt | 2 +-
> > > > arch/powerpc/include/asm/exception-64e.h | 3 +
> > > > arch/powerpc/include/asm/exception-64s.h | 19 ++++++-
> > > > arch/powerpc/include/asm/futex.h | 6 ++
> > > > arch/powerpc/include/asm/mmu.h | 7 +++
> > > > arch/powerpc/include/asm/paca.h | 3 +
> > > > arch/powerpc/include/asm/reg.h | 1 +
> > > > arch/powerpc/include/asm/uaccess.h | 57
> > > > ++++++++++++++++---
> > > > arch/powerpc/kernel/asm-offsets.c | 1 +
> > > > arch/powerpc/kernel/dt_cpu_ftrs.c | 4 ++
> > > > arch/powerpc/kernel/entry_64.S | 17 +++++-
> > > > arch/powerpc/lib/checksum_wrappers.c | 6 +-
> > > > arch/powerpc/mm/fault.c | 9 +++
> > > > arch/powerpc/mm/init_64.c | 15 +++++
> > > > arch/powerpc/mm/pgtable-radix.c | 2 +
> > > > arch/powerpc/mm/pkeys.c | 7 ++-
> > > > arch/powerpc/platforms/Kconfig.cputype | 15 +++++
> > > > 17 files changed, 158 insertions(+), 16 deletions(-)
> > > >
> > > > --
> > > > 2.19.1
>
>
^ permalink raw reply
* Re: PIE binaries are no longer mapped below 4 GiB on ppc64le
From: Michael Ellerman @ 2018-11-01 3:55 UTC (permalink / raw)
To: Florian Weimer, linuxppc-dev; +Cc: linux-mm, keescook, amodra
In-Reply-To: <87k1lyf2x3.fsf@oldenburg.str.redhat.com>
Hi Florian,
Florian Weimer <fweimer@redhat.com> writes:
> We tried to use Go to build PIE binaries, and while the Go toolchain is
> definitely not ready (it produces text relocations and problematic
> relocations in general), it exposed what could be an accidental
> userspace ABI change.
>
> With our 4.10-derived kernel, PIE binaries are mapped below 4 GiB, so
> relocations like R_PPC64_ADDR16_HA work:
>
> 21f00000-220d0000 r-xp 00000000 fd:00 36593493 /root/extld
> 220d0000-220e0000 r--p 001c0000 fd:00 36593493 /root/extld
> 220e0000-22100000 rw-p 001d0000 fd:00 36593493 /root/extld
...
>
> With a 4.18-derived kernel (with the hashed mm), we get this instead:
>
> 120e60000-121030000 rw-p 00000000 fd:00 102447141 /root/extld
> 121030000-121060000 rw-p 001c0000 fd:00 102447141 /root/extld
> 121060000-121080000 rw-p 00000000 00:00 0
I assume that's caused by:
47ebb09d5485 ("powerpc: move ELF_ET_DYN_BASE to 4GB / 4MB")
Which did roughly:
-#define ELF_ET_DYN_BASE 0x20000000
+#define ELF_ET_DYN_BASE (is_32bit_task() ? 0x000400000UL : \
+ 0x100000000UL)
And went into 4.13.
> ...
> I'm not entirely sure what to make of this, but I'm worried that this
> could be a regression that matters to userspace.
It was a deliberate change, and it seemed to not break anything so we
merged it. But obviously we didn't test widely enough.
So I guess it clearly can matter to userspace, and it used to work, so
therefore it is a regression.
But at the same time we haven't had any other reports of breakage, so is
this somehow specific to something Go is doing? Or did we just get lucky
up until now? Or is no one actually testing on Power? ;)
cheers
^ permalink raw reply
* Re: [PATCH] raid6/ppc: Fix build for clang
From: Michael Ellerman @ 2018-11-01 4:38 UTC (permalink / raw)
To: Nick Desaulniers, joel; +Cc: jji, linuxppc-dev, LKML, Arnd Bergmann, Kees Cook
In-Reply-To: <CAKwvOdm8rFNEG39_6f5voZo3nU1g9eC6M8uxeQPVE8=JUoB3mg@mail.gmail.com>
Nick Desaulniers <ndesaulniers@google.com> writes:
> On Tue, Oct 30, 2018 at 8:28 PM Joel Stanley <joel@jms.id.au> wrote:
>>
>> We cannot build these files with clang as it does not allow altivec
>> instructions in assembly when -msoft-float is passed.
>>
>> Jinsong Ji <jji@us.ibm.com> wrote:
>> > We currently disable Altivec/VSX support when enabling soft-float. So
>> > any usage of vector builtins will break.
>> >
>> > Enable Altivec/VSX with soft-float may need quite some clean up work, so
>> > I guess this is currently a limitation.
>> >
>> > Removing -msoft-float will make it work (and we are lucky that no
>> > floating point instructions will be generated as well).
>>
>> This is a workaround until the issue is resolved in clang.
>>
>> Link: https://bugs.llvm.org/show_bug.cgi?id=31177
>> Link: https://github.com/ClangBuiltLinux/linux/issues/239
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Hopefully we can have this fixed up soon. I'll try to see who works
> on Clang+PPC here and see if they can help move that bug along. Thanks
> for the patch!
>
>> ---
>> lib/raid6/Makefile | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile
>> index 2f8b61dfd9b0..3a844e6fd01c 100644
>> --- a/lib/raid6/Makefile
>> +++ b/lib/raid6/Makefile
>> @@ -18,6 +18,21 @@ quiet_cmd_unroll = UNROLL $@
>>
>> ifeq ($(CONFIG_ALTIVEC),y)
>> altivec_flags := -maltivec $(call cc-option,-mabi=altivec)
>> +
>> +ifdef CONFIG_CC_IS_CLANG
>
> Note that the top level makefile detects clang via:
>
> ifeq ($(cc-name),clang)
>
> Does that not work here? I'd prefer to keep compiler detection in
> Makefiles consistent, if it works?
cc-name is being removed in favor of CONFIG_CC_IS_CLANG:
https://lore.kernel.org/lkml/1540905994-6073-1-git-send-email-yamada.masahiro@socionext.com/
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: remove dead npu-dma code
From: Alexey Kardashevskiy @ 2018-11-01 4:41 UTC (permalink / raw)
To: Christoph Hellwig, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: James Bottomley, linuxppc-dev, linux-kernel, Andrew Morton,
Paul E. McKenney, Linus Torvalds
In-Reply-To: <20181030133122.GA31102@lst.de>
On 31/10/2018 00:31, Christoph Hellwig wrote:
> This code has never been unused in the kernel since it was merged, and
> there has been no attempt that I could find to even submit users for
> it. Besides the general policy of not keep 1000+ lines of dead
> code, it helps cleaning up the DMA code and making powerpc user common
> infrastructure.
>
> This effectively reverts commit 5d2aa710 ("powerpc/powernv: Add support
> for Nvlink NPUs").
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>
> [this is a follow up to the brief maintainer summit discussion]
>
> arch/powerpc/include/asm/pci.h | 3 -
> arch/powerpc/include/asm/powernv.h | 23 -
> arch/powerpc/platforms/powernv/Makefile | 2 +-
> arch/powerpc/platforms/powernv/npu-dma.c | 989 ----------------------
> arch/powerpc/platforms/powernv/pci-ioda.c | 243 ------
> arch/powerpc/platforms/powernv/pci.h | 11 -
> 6 files changed, 1 insertion(+), 1270 deletions(-)
> delete mode 100644 arch/powerpc/platforms/powernv/npu-dma.c
>
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 2af9ded80540..a01d2e3d6ff9 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -127,7 +127,4 @@ extern void pcibios_scan_phb(struct pci_controller *hose);
>
> #endif /* __KERNEL__ */
>
> -extern struct pci_dev *pnv_pci_get_gpu_dev(struct pci_dev *npdev);
> -extern struct pci_dev *pnv_pci_get_npu_dev(struct pci_dev *gpdev, int index);
> -
> #endif /* __ASM_POWERPC_PCI_H */
> diff --git a/arch/powerpc/include/asm/powernv.h b/arch/powerpc/include/asm/powernv.h
> index 2f3ff7a27881..4848a6b3c6b2 100644
> --- a/arch/powerpc/include/asm/powernv.h
> +++ b/arch/powerpc/include/asm/powernv.h
> @@ -11,33 +11,10 @@
> #define _ASM_POWERNV_H
>
> #ifdef CONFIG_PPC_POWERNV
> -#define NPU2_WRITE 1
> extern void powernv_set_nmmu_ptcr(unsigned long ptcr);
> -extern struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
> - unsigned long flags,
> - void (*cb)(struct npu_context *, void *),
> - void *priv);
> -extern void pnv_npu2_destroy_context(struct npu_context *context,
> - struct pci_dev *gpdev);
> -extern int pnv_npu2_handle_fault(struct npu_context *context, uintptr_t *ea,
> - unsigned long *flags, unsigned long *status,
> - int count);
> -
> void pnv_tm_init(void);
> #else
> static inline void powernv_set_nmmu_ptcr(unsigned long ptcr) { }
> -static inline struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
> - unsigned long flags,
> - struct npu_context *(*cb)(struct npu_context *, void *),
> - void *priv) { return ERR_PTR(-ENODEV); }
> -static inline void pnv_npu2_destroy_context(struct npu_context *context,
> - struct pci_dev *gpdev) { }
> -
> -static inline int pnv_npu2_handle_fault(struct npu_context *context,
> - uintptr_t *ea, unsigned long *flags,
> - unsigned long *status, int count) {
> - return -ENODEV;
> -}
>
> static inline void pnv_tm_init(void) { }
> static inline void pnv_power9_force_smt4(void) { }
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index b540ce8eec55..2b13e9dd137c 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -6,7 +6,7 @@ obj-y += opal-msglog.o opal-hmi.o opal-power.o opal-irqchip.o
> obj-y += opal-kmsg.o opal-powercap.o opal-psr.o opal-sensor-groups.o
>
> obj-$(CONFIG_SMP) += smp.o subcore.o subcore-asm.o
> -obj-$(CONFIG_PCI) += pci.o pci-ioda.o npu-dma.o pci-ioda-tce.o
> +obj-$(CONFIG_PCI) += pci.o pci-ioda.o pci-ioda-tce.o
> obj-$(CONFIG_CXL_BASE) += pci-cxl.o
> obj-$(CONFIG_EEH) += eeh-powernv.o
> obj-$(CONFIG_PPC_SCOM) += opal-xscom.o
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> deleted file mode 100644
> index 6f60e0931922..000000000000
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ /dev/null
> @@ -1,989 +0,0 @@
> -/*
> - * This file implements the DMA operations for NVLink devices. The NPU
> - * devices all point to the same iommu table as the parent PCI device.
> - *
> - * Copyright Alistair Popple, IBM Corporation 2015.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of version 2 of the GNU General Public
> - * License as published by the Free Software Foundation.
> - */
> -
> -#include <linux/slab.h>
> -#include <linux/mmu_notifier.h>
> -#include <linux/mmu_context.h>
> -#include <linux/of.h>
> -#include <linux/export.h>
> -#include <linux/pci.h>
> -#include <linux/memblock.h>
> -#include <linux/iommu.h>
> -#include <linux/sizes.h>
> -
> -#include <asm/debugfs.h>
> -#include <asm/tlb.h>
> -#include <asm/powernv.h>
> -#include <asm/reg.h>
> -#include <asm/opal.h>
> -#include <asm/io.h>
> -#include <asm/iommu.h>
> -#include <asm/pnv-pci.h>
> -#include <asm/msi_bitmap.h>
> -#include <asm/opal.h>
> -
> -#include "powernv.h"
> -#include "pci.h"
> -
> -#define npu_to_phb(x) container_of(x, struct pnv_phb, npu)
> -
> -/*
> - * spinlock to protect initialisation of an npu_context for a particular
> - * mm_struct.
> - */
> -static DEFINE_SPINLOCK(npu_context_lock);
> -
> -/*
> - * Other types of TCE cache invalidation are not functional in the
> - * hardware.
> - */
> -static struct pci_dev *get_pci_dev(struct device_node *dn)
> -{
> - struct pci_dn *pdn = PCI_DN(dn);
> -
> - return pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus),
> - pdn->busno, pdn->devfn);
> -}
> -
> -/* Given a NPU device get the associated PCI device. */
> -struct pci_dev *pnv_pci_get_gpu_dev(struct pci_dev *npdev)
> -{
> - struct device_node *dn;
> - struct pci_dev *gpdev;
> -
> - if (WARN_ON(!npdev))
> - return NULL;
> -
> - if (WARN_ON(!npdev->dev.of_node))
> - return NULL;
> -
> - /* Get assoicated PCI device */
> - dn = of_parse_phandle(npdev->dev.of_node, "ibm,gpu", 0);
> - if (!dn)
> - return NULL;
> -
> - gpdev = get_pci_dev(dn);
> - of_node_put(dn);
> -
> - return gpdev;
> -}
> -EXPORT_SYMBOL(pnv_pci_get_gpu_dev);
> -
> -/* Given the real PCI device get a linked NPU device. */
> -struct pci_dev *pnv_pci_get_npu_dev(struct pci_dev *gpdev, int index)
> -{
> - struct device_node *dn;
> - struct pci_dev *npdev;
> -
> - if (WARN_ON(!gpdev))
> - return NULL;
> -
> - /* Not all PCI devices have device-tree nodes */
> - if (!gpdev->dev.of_node)
> - return NULL;
> -
> - /* Get assoicated PCI device */
> - dn = of_parse_phandle(gpdev->dev.of_node, "ibm,npu", index);
> - if (!dn)
> - return NULL;
> -
> - npdev = get_pci_dev(dn);
> - of_node_put(dn);
> -
> - return npdev;
> -}
> -EXPORT_SYMBOL(pnv_pci_get_npu_dev);
> -
> -#define NPU_DMA_OP_UNSUPPORTED() \
> - dev_err_once(dev, "%s operation unsupported for NVLink devices\n", \
> - __func__)
> -
> -static void *dma_npu_alloc(struct device *dev, size_t size,
> - dma_addr_t *dma_handle, gfp_t flag,
> - unsigned long attrs)
> -{
> - NPU_DMA_OP_UNSUPPORTED();
> - return NULL;
> -}
> -
> -static void dma_npu_free(struct device *dev, size_t size,
> - void *vaddr, dma_addr_t dma_handle,
> - unsigned long attrs)
> -{
> - NPU_DMA_OP_UNSUPPORTED();
> -}
> -
> -static dma_addr_t dma_npu_map_page(struct device *dev, struct page *page,
> - unsigned long offset, size_t size,
> - enum dma_data_direction direction,
> - unsigned long attrs)
> -{
> - NPU_DMA_OP_UNSUPPORTED();
> - return 0;
> -}
> -
> -static int dma_npu_map_sg(struct device *dev, struct scatterlist *sglist,
> - int nelems, enum dma_data_direction direction,
> - unsigned long attrs)
> -{
> - NPU_DMA_OP_UNSUPPORTED();
> - return 0;
> -}
> -
> -static int dma_npu_dma_supported(struct device *dev, u64 mask)
> -{
> - NPU_DMA_OP_UNSUPPORTED();
> - return 0;
> -}
> -
> -static u64 dma_npu_get_required_mask(struct device *dev)
> -{
> - NPU_DMA_OP_UNSUPPORTED();
> - return 0;
> -}
> -
> -static const struct dma_map_ops dma_npu_ops = {
> - .map_page = dma_npu_map_page,
> - .map_sg = dma_npu_map_sg,
> - .alloc = dma_npu_alloc,
> - .free = dma_npu_free,
> - .dma_supported = dma_npu_dma_supported,
> - .get_required_mask = dma_npu_get_required_mask,
> -};
> -
> -/*
> - * Returns the PE assoicated with the PCI device of the given
> - * NPU. Returns the linked pci device if pci_dev != NULL.
> - */
> -static struct pnv_ioda_pe *get_gpu_pci_dev_and_pe(struct pnv_ioda_pe *npe,
> - struct pci_dev **gpdev)
> -{
> - struct pnv_phb *phb;
> - struct pci_controller *hose;
> - struct pci_dev *pdev;
> - struct pnv_ioda_pe *pe;
> - struct pci_dn *pdn;
> -
> - pdev = pnv_pci_get_gpu_dev(npe->pdev);
> - if (!pdev)
> - return NULL;
> -
> - pdn = pci_get_pdn(pdev);
> - if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
> - return NULL;
> -
> - hose = pci_bus_to_host(pdev->bus);
> - phb = hose->private_data;
> - pe = &phb->ioda.pe_array[pdn->pe_number];
> -
> - if (gpdev)
> - *gpdev = pdev;
> -
> - return pe;
> -}
> -
> -long pnv_npu_set_window(struct pnv_ioda_pe *npe, int num,
> - struct iommu_table *tbl)
> -{
> - struct pnv_phb *phb = npe->phb;
> - int64_t rc;
> - const unsigned long size = tbl->it_indirect_levels ?
> - tbl->it_level_size : tbl->it_size;
> - const __u64 start_addr = tbl->it_offset << tbl->it_page_shift;
> - const __u64 win_size = tbl->it_size << tbl->it_page_shift;
> -
> - pe_info(npe, "Setting up window %llx..%llx pg=%lx\n",
> - start_addr, start_addr + win_size - 1,
> - IOMMU_PAGE_SIZE(tbl));
> -
> - rc = opal_pci_map_pe_dma_window(phb->opal_id,
> - npe->pe_number,
> - npe->pe_number,
> - tbl->it_indirect_levels + 1,
> - __pa(tbl->it_base),
> - size << 3,
> - IOMMU_PAGE_SIZE(tbl));
> - if (rc) {
> - pe_err(npe, "Failed to configure TCE table, err %lld\n", rc);
> - return rc;
> - }
> - pnv_pci_ioda2_tce_invalidate_entire(phb, false);
> -
> - /* Add the table to the list so its TCE cache will get invalidated */
> - pnv_pci_link_table_and_group(phb->hose->node, num,
> - tbl, &npe->table_group);
> -
> - return 0;
> -}
> -
> -long pnv_npu_unset_window(struct pnv_ioda_pe *npe, int num)
> -{
> - struct pnv_phb *phb = npe->phb;
> - int64_t rc;
> -
> - pe_info(npe, "Removing DMA window\n");
> -
> - rc = opal_pci_map_pe_dma_window(phb->opal_id, npe->pe_number,
> - npe->pe_number,
> - 0/* levels */, 0/* table address */,
> - 0/* table size */, 0/* page size */);
> - if (rc) {
> - pe_err(npe, "Unmapping failed, ret = %lld\n", rc);
> - return rc;
> - }
> - pnv_pci_ioda2_tce_invalidate_entire(phb, false);
> -
> - pnv_pci_unlink_table_and_group(npe->table_group.tables[num],
> - &npe->table_group);
> -
> - return 0;
> -}
> -
> -/*
> - * Enables 32 bit DMA on NPU.
> - */
> -static void pnv_npu_dma_set_32(struct pnv_ioda_pe *npe)
> -{
> - struct pci_dev *gpdev;
> - struct pnv_ioda_pe *gpe;
> - int64_t rc;
> -
> - /*
> - * Find the assoicated PCI devices and get the dma window
> - * information from there.
> - */
> - if (!npe->pdev || !(npe->flags & PNV_IODA_PE_DEV))
> - return;
> -
> - gpe = get_gpu_pci_dev_and_pe(npe, &gpdev);
> - if (!gpe)
> - return;
> -
> - rc = pnv_npu_set_window(npe, 0, gpe->table_group.tables[0]);
> -
> - /*
> - * We don't initialise npu_pe->tce32_table as we always use
> - * dma_npu_ops which are nops.
> - */
> - set_dma_ops(&npe->pdev->dev, &dma_npu_ops);
> -}
> -
> -/*
> - * Enables bypass mode on the NPU. The NPU only supports one
> - * window per link, so bypass needs to be explicitly enabled or
> - * disabled. Unlike for a PHB3 bypass and non-bypass modes can't be
> - * active at the same time.
> - */
> -static int pnv_npu_dma_set_bypass(struct pnv_ioda_pe *npe)
> -{
> - struct pnv_phb *phb = npe->phb;
> - int64_t rc = 0;
> - phys_addr_t top = memblock_end_of_DRAM();
> -
> - if (phb->type != PNV_PHB_NPU_NVLINK || !npe->pdev)
> - return -EINVAL;
> -
> - rc = pnv_npu_unset_window(npe, 0);
> - if (rc != OPAL_SUCCESS)
> - return rc;
> -
> - /* Enable the bypass window */
> -
> - top = roundup_pow_of_two(top);
> - dev_info(&npe->pdev->dev, "Enabling bypass for PE %x\n",
> - npe->pe_number);
> - rc = opal_pci_map_pe_dma_window_real(phb->opal_id,
> - npe->pe_number, npe->pe_number,
> - 0 /* bypass base */, top);
> -
> - if (rc == OPAL_SUCCESS)
> - pnv_pci_ioda2_tce_invalidate_entire(phb, false);
> -
> - return rc;
> -}
> -
> -void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass)
> -{
> - int i;
> - struct pnv_phb *phb;
> - struct pci_dn *pdn;
> - struct pnv_ioda_pe *npe;
> - struct pci_dev *npdev;
> -
> - for (i = 0; ; ++i) {
> - npdev = pnv_pci_get_npu_dev(gpdev, i);
> -
> - if (!npdev)
> - break;
> -
> - pdn = pci_get_pdn(npdev);
> - if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
> - return;
> -
> - phb = pci_bus_to_host(npdev->bus)->private_data;
> -
> - /* We only do bypass if it's enabled on the linked device */
> - npe = &phb->ioda.pe_array[pdn->pe_number];
> -
> - if (bypass) {
> - dev_info(&npdev->dev,
> - "Using 64-bit DMA iommu bypass\n");
> - pnv_npu_dma_set_bypass(npe);
> - } else {
> - dev_info(&npdev->dev, "Using 32-bit DMA via iommu\n");
> - pnv_npu_dma_set_32(npe);
> - }
> - }
> -}
> -
> -/* Switch ownership from platform code to external user (e.g. VFIO) */
> -void pnv_npu_take_ownership(struct pnv_ioda_pe *npe)
> -{
> - struct pnv_phb *phb = npe->phb;
> - int64_t rc;
> -
> - /*
> - * Note: NPU has just a single TVE in the hardware which means that
> - * while used by the kernel, it can have either 32bit window or
> - * DMA bypass but never both. So we deconfigure 32bit window only
> - * if it was enabled at the moment of ownership change.
> - */
> - if (npe->table_group.tables[0]) {
> - pnv_npu_unset_window(npe, 0);
> - return;
> - }
> -
> - /* Disable bypass */
> - rc = opal_pci_map_pe_dma_window_real(phb->opal_id,
> - npe->pe_number, npe->pe_number,
> - 0 /* bypass base */, 0);
> - if (rc) {
> - pe_err(npe, "Failed to disable bypass, err %lld\n", rc);
> - return;
> - }
> - pnv_pci_ioda2_tce_invalidate_entire(npe->phb, false);
> -}
> -
> -struct pnv_ioda_pe *pnv_pci_npu_setup_iommu(struct pnv_ioda_pe *npe)
> -{
> - struct pnv_phb *phb = npe->phb;
> - struct pci_bus *pbus = phb->hose->bus;
> - struct pci_dev *npdev, *gpdev = NULL, *gptmp;
> - struct pnv_ioda_pe *gpe = get_gpu_pci_dev_and_pe(npe, &gpdev);
> -
> - if (!gpe || !gpdev)
> - return NULL;
> -
> - list_for_each_entry(npdev, &pbus->devices, bus_list) {
> - gptmp = pnv_pci_get_gpu_dev(npdev);
> -
> - if (gptmp != gpdev)
> - continue;
> -
> - pe_info(gpe, "Attached NPU %s\n", dev_name(&npdev->dev));
> - iommu_group_add_device(gpe->table_group.group, &npdev->dev);
> - }
> -
> - return gpe;
> -}
> -
> -/* Maximum number of nvlinks per npu */
> -#define NV_MAX_LINKS 6
> -
> -/* Maximum index of npu2 hosts in the system. Always < NV_MAX_NPUS */
> -static int max_npu2_index;
> -
> -struct npu_context {
> - struct mm_struct *mm;
> - struct pci_dev *npdev[NV_MAX_NPUS][NV_MAX_LINKS];
> - struct mmu_notifier mn;
> - struct kref kref;
> - bool nmmu_flush;
> -
> - /* Callback to stop translation requests on a given GPU */
> - void (*release_cb)(struct npu_context *context, void *priv);
> -
> - /*
> - * Private pointer passed to the above callback for usage by
> - * device drivers.
> - */
> - void *priv;
> -};
> -
> -struct mmio_atsd_reg {
> - struct npu *npu;
> - int reg;
> -};
> -
> -/*
> - * Find a free MMIO ATSD register and mark it in use. Return -ENOSPC
> - * if none are available.
> - */
> -static int get_mmio_atsd_reg(struct npu *npu)
> -{
> - int i;
> -
> - for (i = 0; i < npu->mmio_atsd_count; i++) {
> - if (!test_bit(i, &npu->mmio_atsd_usage))
> - if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage))
> - return i;
> - }
> -
> - return -ENOSPC;
> -}
> -
> -static void put_mmio_atsd_reg(struct npu *npu, int reg)
> -{
> - clear_bit_unlock(reg, &npu->mmio_atsd_usage);
> -}
> -
> -/* MMIO ATSD register offsets */
> -#define XTS_ATSD_LAUNCH 0
> -#define XTS_ATSD_AVA 1
> -#define XTS_ATSD_STAT 2
> -
> -static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize)
> -{
> - unsigned long launch = 0;
> -
> - if (psize == MMU_PAGE_COUNT) {
> - /* IS set to invalidate entire matching PID */
> - launch |= PPC_BIT(12);
> - } else {
> - /* AP set to invalidate region of psize */
> - launch |= (u64)mmu_get_ap(psize) << PPC_BITLSHIFT(17);
> - }
> -
> - /* PRS set to process-scoped */
> - launch |= PPC_BIT(13);
> -
> - /* PID */
> - launch |= pid << PPC_BITLSHIFT(38);
> -
> - /* Leave "No flush" (bit 39) 0 so every ATSD performs a flush */
> -
> - return launch;
> -}
> -
> -static void mmio_atsd_regs_write(struct mmio_atsd_reg
> - mmio_atsd_reg[NV_MAX_NPUS], unsigned long offset,
> - unsigned long val)
> -{
> - struct npu *npu;
> - int i, reg;
> -
> - for (i = 0; i <= max_npu2_index; i++) {
> - reg = mmio_atsd_reg[i].reg;
> - if (reg < 0)
> - continue;
> -
> - npu = mmio_atsd_reg[i].npu;
> - __raw_writeq_be(val, npu->mmio_atsd_regs[reg] + offset);
> - }
> -}
> -
> -static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> - unsigned long pid)
> -{
> - unsigned long launch = get_atsd_launch_val(pid, MMU_PAGE_COUNT);
> -
> - /* Invalidating the entire process doesn't use a va */
> - mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
> -}
> -
> -static void mmio_invalidate_range(struct mmio_atsd_reg
> - mmio_atsd_reg[NV_MAX_NPUS], unsigned long pid,
> - unsigned long start, unsigned long psize)
> -{
> - unsigned long launch = get_atsd_launch_val(pid, psize);
> -
> - /* Write all VAs first */
> - mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_AVA, start);
> -
> - /* Issue one barrier for all address writes */
> - eieio();
> -
> - /* Launch */
> - mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
> -}
> -
> -#define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
> -
> -static void mmio_invalidate_wait(
> - struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
> -{
> - struct npu *npu;
> - int i, reg;
> -
> - /* Wait for all invalidations to complete */
> - for (i = 0; i <= max_npu2_index; i++) {
> - if (mmio_atsd_reg[i].reg < 0)
> - continue;
> -
> - /* Wait for completion */
> - npu = mmio_atsd_reg[i].npu;
> - reg = mmio_atsd_reg[i].reg;
> - while (__raw_readq(npu->mmio_atsd_regs[reg] + XTS_ATSD_STAT))
> - cpu_relax();
> - }
> -}
> -
> -/*
> - * Acquires all the address translation shootdown (ATSD) registers required to
> - * launch an ATSD on all links this npu_context is active on.
> - */
> -static void acquire_atsd_reg(struct npu_context *npu_context,
> - struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
> -{
> - int i, j;
> - struct npu *npu;
> - struct pci_dev *npdev;
> - struct pnv_phb *nphb;
> -
> - for (i = 0; i <= max_npu2_index; i++) {
> - mmio_atsd_reg[i].reg = -1;
> - for (j = 0; j < NV_MAX_LINKS; j++) {
> - /*
> - * There are no ordering requirements with respect to
> - * the setup of struct npu_context, but to ensure
> - * consistent behaviour we need to ensure npdev[][] is
> - * only read once.
> - */
> - npdev = READ_ONCE(npu_context->npdev[i][j]);
> - if (!npdev)
> - continue;
> -
> - nphb = pci_bus_to_host(npdev->bus)->private_data;
> - npu = &nphb->npu;
> - mmio_atsd_reg[i].npu = npu;
> - mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
> - while (mmio_atsd_reg[i].reg < 0) {
> - mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
> - cpu_relax();
> - }
> - break;
> - }
> - }
> -}
> -
> -/*
> - * Release previously acquired ATSD registers. To avoid deadlocks the registers
> - * must be released in the same order they were acquired above in
> - * acquire_atsd_reg.
> - */
> -static void release_atsd_reg(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
> -{
> - int i;
> -
> - for (i = 0; i <= max_npu2_index; i++) {
> - /*
> - * We can't rely on npu_context->npdev[][] being the same here
> - * as when acquire_atsd_reg() was called, hence we use the
> - * values stored in mmio_atsd_reg during the acquire phase
> - * rather than re-reading npdev[][].
> - */
> - if (mmio_atsd_reg[i].reg < 0)
> - continue;
> -
> - put_mmio_atsd_reg(mmio_atsd_reg[i].npu, mmio_atsd_reg[i].reg);
> - }
> -}
> -
> -/*
> - * Invalidate a virtual address range
> - */
> -static void mmio_invalidate(struct npu_context *npu_context,
> - unsigned long start, unsigned long size)
> -{
> - struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
> - unsigned long pid = npu_context->mm->context.id;
> - unsigned long atsd_start = 0;
> - unsigned long end = start + size - 1;
> - int atsd_psize = MMU_PAGE_COUNT;
> -
> - /*
> - * Convert the input range into one of the supported sizes. If the range
> - * doesn't fit, use the next larger supported size. Invalidation latency
> - * is high, so over-invalidation is preferred to issuing multiple
> - * invalidates.
> - *
> - * A 4K page size isn't supported by NPU/GPU ATS, so that case is
> - * ignored.
> - */
> - if (size == SZ_64K) {
> - atsd_start = start;
> - atsd_psize = MMU_PAGE_64K;
> - } else if (ALIGN_DOWN(start, SZ_2M) == ALIGN_DOWN(end, SZ_2M)) {
> - atsd_start = ALIGN_DOWN(start, SZ_2M);
> - atsd_psize = MMU_PAGE_2M;
> - } else if (ALIGN_DOWN(start, SZ_1G) == ALIGN_DOWN(end, SZ_1G)) {
> - atsd_start = ALIGN_DOWN(start, SZ_1G);
> - atsd_psize = MMU_PAGE_1G;
> - }
> -
> - if (npu_context->nmmu_flush)
> - /*
> - * Unfortunately the nest mmu does not support flushing specific
> - * addresses so we have to flush the whole mm once before
> - * shooting down the GPU translation.
> - */
> - flush_all_mm(npu_context->mm);
> -
> - /*
> - * Loop over all the NPUs this process is active on and launch
> - * an invalidate.
> - */
> - acquire_atsd_reg(npu_context, mmio_atsd_reg);
> -
> - if (atsd_psize == MMU_PAGE_COUNT)
> - mmio_invalidate_pid(mmio_atsd_reg, pid);
> - else
> - mmio_invalidate_range(mmio_atsd_reg, pid, atsd_start,
> - atsd_psize);
> -
> - mmio_invalidate_wait(mmio_atsd_reg);
> -
> - /*
> - * The GPU requires two flush ATSDs to ensure all entries have been
> - * flushed. We use PID 0 as it will never be used for a process on the
> - * GPU.
> - */
> - mmio_invalidate_pid(mmio_atsd_reg, 0);
> - mmio_invalidate_wait(mmio_atsd_reg);
> - mmio_invalidate_pid(mmio_atsd_reg, 0);
> - mmio_invalidate_wait(mmio_atsd_reg);
> -
> - release_atsd_reg(mmio_atsd_reg);
> -}
> -
> -static void pnv_npu2_mn_release(struct mmu_notifier *mn,
> - struct mm_struct *mm)
> -{
> - struct npu_context *npu_context = mn_to_npu_context(mn);
> -
> - /* Call into device driver to stop requests to the NMMU */
> - if (npu_context->release_cb)
> - npu_context->release_cb(npu_context, npu_context->priv);
> -
> - /*
> - * There should be no more translation requests for this PID, but we
> - * need to ensure any entries for it are removed from the TLB.
> - */
> - mmio_invalidate(npu_context, 0, ~0UL);
> -}
> -
> -static void pnv_npu2_mn_change_pte(struct mmu_notifier *mn,
> - struct mm_struct *mm,
> - unsigned long address,
> - pte_t pte)
> -{
> - struct npu_context *npu_context = mn_to_npu_context(mn);
> - mmio_invalidate(npu_context, address, PAGE_SIZE);
> -}
> -
> -static void pnv_npu2_mn_invalidate_range(struct mmu_notifier *mn,
> - struct mm_struct *mm,
> - unsigned long start, unsigned long end)
> -{
> - struct npu_context *npu_context = mn_to_npu_context(mn);
> - mmio_invalidate(npu_context, start, end - start);
> -}
> -
> -static const struct mmu_notifier_ops nv_nmmu_notifier_ops = {
> - .release = pnv_npu2_mn_release,
> - .change_pte = pnv_npu2_mn_change_pte,
> - .invalidate_range = pnv_npu2_mn_invalidate_range,
> -};
> -
> -/*
> - * Call into OPAL to setup the nmmu context for the current task in
> - * the NPU. This must be called to setup the context tables before the
> - * GPU issues ATRs. pdev should be a pointed to PCIe GPU device.
> - *
> - * A release callback should be registered to allow a device driver to
> - * be notified that it should not launch any new translation requests
> - * as the final TLB invalidate is about to occur.
> - *
> - * Returns an error if there no contexts are currently available or a
> - * npu_context which should be passed to pnv_npu2_handle_fault().
> - *
> - * mmap_sem must be held in write mode and must not be called from interrupt
> - * context.
> - */
> -struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
> - unsigned long flags,
> - void (*cb)(struct npu_context *, void *),
> - void *priv)
> -{
> - int rc;
> - u32 nvlink_index;
> - struct device_node *nvlink_dn;
> - struct mm_struct *mm = current->mm;
> - struct pnv_phb *nphb;
> - struct npu *npu;
> - struct npu_context *npu_context;
> -
> - /*
> - * At present we don't support GPUs connected to multiple NPUs and I'm
> - * not sure the hardware does either.
> - */
> - struct pci_dev *npdev = pnv_pci_get_npu_dev(gpdev, 0);
> -
> - if (!firmware_has_feature(FW_FEATURE_OPAL))
> - return ERR_PTR(-ENODEV);
> -
> - if (!npdev)
> - /* No nvlink associated with this GPU device */
> - return ERR_PTR(-ENODEV);
> -
> - nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
> - if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
> - &nvlink_index)))
> - return ERR_PTR(-ENODEV);
> -
> - if (!mm || mm->context.id == 0) {
> - /*
> - * Kernel thread contexts are not supported and context id 0 is
> - * reserved on the GPU.
> - */
> - return ERR_PTR(-EINVAL);
> - }
> -
> - nphb = pci_bus_to_host(npdev->bus)->private_data;
> - npu = &nphb->npu;
> -
> - /*
> - * Setup the NPU context table for a particular GPU. These need to be
> - * per-GPU as we need the tables to filter ATSDs when there are no
> - * active contexts on a particular GPU. It is safe for these to be
> - * called concurrently with destroy as the OPAL call takes appropriate
> - * locks and refcounts on init/destroy.
> - */
> - rc = opal_npu_init_context(nphb->opal_id, mm->context.id, flags,
> - PCI_DEVID(gpdev->bus->number, gpdev->devfn));
> - if (rc < 0)
> - return ERR_PTR(-ENOSPC);
> -
> - /*
> - * We store the npu pci device so we can more easily get at the
> - * associated npus.
> - */
> - spin_lock(&npu_context_lock);
> - npu_context = mm->context.npu_context;
> - if (npu_context) {
> - if (npu_context->release_cb != cb ||
> - npu_context->priv != priv) {
> - spin_unlock(&npu_context_lock);
> - opal_npu_destroy_context(nphb->opal_id, mm->context.id,
> - PCI_DEVID(gpdev->bus->number,
> - gpdev->devfn));
> - return ERR_PTR(-EINVAL);
> - }
> -
> - WARN_ON(!kref_get_unless_zero(&npu_context->kref));
> - }
> - spin_unlock(&npu_context_lock);
> -
> - if (!npu_context) {
> - /*
> - * We can set up these fields without holding the
> - * npu_context_lock as the npu_context hasn't been returned to
> - * the caller meaning it can't be destroyed. Parallel allocation
> - * is protected against by mmap_sem.
> - */
> - rc = -ENOMEM;
> - npu_context = kzalloc(sizeof(struct npu_context), GFP_KERNEL);
> - if (npu_context) {
> - kref_init(&npu_context->kref);
> - npu_context->mm = mm;
> - npu_context->mn.ops = &nv_nmmu_notifier_ops;
> - rc = __mmu_notifier_register(&npu_context->mn, mm);
> - }
> -
> - if (rc) {
> - kfree(npu_context);
> - opal_npu_destroy_context(nphb->opal_id, mm->context.id,
> - PCI_DEVID(gpdev->bus->number,
> - gpdev->devfn));
> - return ERR_PTR(rc);
> - }
> -
> - mm->context.npu_context = npu_context;
> - }
> -
> - npu_context->release_cb = cb;
> - npu_context->priv = priv;
> -
> - /*
> - * npdev is a pci_dev pointer setup by the PCI code. We assign it to
> - * npdev[][] to indicate to the mmu notifiers that an invalidation
> - * should also be sent over this nvlink. The notifiers don't use any
> - * other fields in npu_context, so we just need to ensure that when they
> - * deference npu_context->npdev[][] it is either a valid pointer or
> - * NULL.
> - */
> - WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
> -
> - if (!nphb->npu.nmmu_flush) {
> - /*
> - * If we're not explicitly flushing ourselves we need to mark
> - * the thread for global flushes
> - */
> - npu_context->nmmu_flush = false;
> - mm_context_add_copro(mm);
> - } else
> - npu_context->nmmu_flush = true;
> -
> - return npu_context;
> -}
> -EXPORT_SYMBOL(pnv_npu2_init_context);
> -
> -static void pnv_npu2_release_context(struct kref *kref)
> -{
> - struct npu_context *npu_context =
> - container_of(kref, struct npu_context, kref);
> -
> - if (!npu_context->nmmu_flush)
> - mm_context_remove_copro(npu_context->mm);
> -
> - npu_context->mm->context.npu_context = NULL;
> -}
> -
> -/*
> - * Destroy a context on the given GPU. May free the npu_context if it is no
> - * longer active on any GPUs. Must not be called from interrupt context.
> - */
> -void pnv_npu2_destroy_context(struct npu_context *npu_context,
> - struct pci_dev *gpdev)
> -{
> - int removed;
> - struct pnv_phb *nphb;
> - struct npu *npu;
> - struct pci_dev *npdev = pnv_pci_get_npu_dev(gpdev, 0);
> - struct device_node *nvlink_dn;
> - u32 nvlink_index;
> -
> - if (WARN_ON(!npdev))
> - return;
> -
> - if (!firmware_has_feature(FW_FEATURE_OPAL))
> - return;
> -
> - nphb = pci_bus_to_host(npdev->bus)->private_data;
> - npu = &nphb->npu;
> - nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
> - if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
> - &nvlink_index)))
> - return;
> - WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], NULL);
> - opal_npu_destroy_context(nphb->opal_id, npu_context->mm->context.id,
> - PCI_DEVID(gpdev->bus->number, gpdev->devfn));
> - spin_lock(&npu_context_lock);
> - removed = kref_put(&npu_context->kref, pnv_npu2_release_context);
> - spin_unlock(&npu_context_lock);
> -
> - /*
> - * We need to do this outside of pnv_npu2_release_context so that it is
> - * outside the spinlock as mmu_notifier_destroy uses SRCU.
> - */
> - if (removed) {
> - mmu_notifier_unregister(&npu_context->mn,
> - npu_context->mm);
> -
> - kfree(npu_context);
> - }
> -
> -}
> -EXPORT_SYMBOL(pnv_npu2_destroy_context);
> -
> -/*
> - * Assumes mmap_sem is held for the contexts associated mm.
> - */
> -int pnv_npu2_handle_fault(struct npu_context *context, uintptr_t *ea,
> - unsigned long *flags, unsigned long *status, int count)
> -{
> - u64 rc = 0, result = 0;
> - int i, is_write;
> - struct page *page[1];
> -
> - /* mmap_sem should be held so the struct_mm must be present */
> - struct mm_struct *mm = context->mm;
> -
> - if (!firmware_has_feature(FW_FEATURE_OPAL))
> - return -ENODEV;
> -
> - WARN_ON(!rwsem_is_locked(&mm->mmap_sem));
> -
> - for (i = 0; i < count; i++) {
> - is_write = flags[i] & NPU2_WRITE;
> - rc = get_user_pages_remote(NULL, mm, ea[i], 1,
> - is_write ? FOLL_WRITE : 0,
> - page, NULL, NULL);
> -
> - /*
> - * To support virtualised environments we will have to do an
> - * access to the page to ensure it gets faulted into the
> - * hypervisor. For the moment virtualisation is not supported in
> - * other areas so leave the access out.
> - */
> - if (rc != 1) {
> - status[i] = rc;
> - result = -EFAULT;
> - continue;
> - }
> -
> - status[i] = 0;
> - put_page(page[0]);
> - }
> -
> - return result;
> -}
> -EXPORT_SYMBOL(pnv_npu2_handle_fault);
> -
> -int pnv_npu2_init(struct pnv_phb *phb)
> -{
> - unsigned int i;
> - u64 mmio_atsd;
> - struct device_node *dn;
> - struct pci_dev *gpdev;
> - static int npu_index;
> - uint64_t rc = 0;
> -
> - phb->npu.nmmu_flush =
> - of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
> - for_each_child_of_node(phb->hose->dn, dn) {
> - gpdev = pnv_pci_get_gpu_dev(get_pci_dev(dn));
> - if (gpdev) {
> - rc = opal_npu_map_lpar(phb->opal_id,
> - PCI_DEVID(gpdev->bus->number, gpdev->devfn),
> - 0, 0);
I heavily hacked on these (draft):
https://patchwork.ozlabs.org/project/kvm-ppc/list/?series=70791
and the result is going to be used for VFIO purposes (draft too):
https://patchwork.ozlabs.org/project/kvm-ppc/list/?series=70793
Other bits can go away from npu-dma but I still need some to make
KVM+VFIO work for GPUs, this is without any vendor driver on the host
side whatsoever.
> - if (rc)
> - dev_err(&gpdev->dev,
> - "Error %lld mapping device to LPAR\n",
> - rc);
> - }
> - }
> -
> - for (i = 0; !of_property_read_u64_index(phb->hose->dn, "ibm,mmio-atsd",
> - i, &mmio_atsd); i++)
> - phb->npu.mmio_atsd_regs[i] = ioremap(mmio_atsd, 32);
> -
> - pr_info("NPU%lld: Found %d MMIO ATSD registers", phb->opal_id, i);
> - phb->npu.mmio_atsd_count = i;
> - phb->npu.mmio_atsd_usage = 0;
> - npu_index++;
> - if (WARN_ON(npu_index >= NV_MAX_NPUS))
> - return -ENOSPC;
> - max_npu2_index = npu_index;
> - phb->npu.index = npu_index;
> -
> - return 0;
> -}
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index cde710297a4e..f480e3fc9ebb 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1203,75 +1203,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
> return pe;
> }
>
> -static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
> -{
> - int pe_num, found_pe = false, rc;
> - long rid;
> - struct pnv_ioda_pe *pe;
> - struct pci_dev *gpu_pdev;
> - struct pci_dn *npu_pdn;
> - struct pci_controller *hose = pci_bus_to_host(npu_pdev->bus);
> - struct pnv_phb *phb = hose->private_data;
> -
> - /*
> - * Due to a hardware errata PE#0 on the NPU is reserved for
> - * error handling. This means we only have three PEs remaining
> - * which need to be assigned to four links, implying some
> - * links must share PEs.
> - *
> - * To achieve this we assign PEs such that NPUs linking the
> - * same GPU get assigned the same PE.
> - */
> - gpu_pdev = pnv_pci_get_gpu_dev(npu_pdev);
> - for (pe_num = 0; pe_num < phb->ioda.total_pe_num; pe_num++) {
> - pe = &phb->ioda.pe_array[pe_num];
> - if (!pe->pdev)
> - continue;
> -
> - if (pnv_pci_get_gpu_dev(pe->pdev) == gpu_pdev) {
> - /*
> - * This device has the same peer GPU so should
> - * be assigned the same PE as the existing
> - * peer NPU.
> - */
> - dev_info(&npu_pdev->dev,
> - "Associating to existing PE %x\n", pe_num);
> - pci_dev_get(npu_pdev);
> - npu_pdn = pci_get_pdn(npu_pdev);
> - rid = npu_pdev->bus->number << 8 | npu_pdn->devfn;
> - npu_pdn->pe_number = pe_num;
> - phb->ioda.pe_rmap[rid] = pe->pe_number;
> -
> - /* Map the PE to this link */
> - rc = opal_pci_set_pe(phb->opal_id, pe_num, rid,
> - OpalPciBusAll,
> - OPAL_COMPARE_RID_DEVICE_NUMBER,
> - OPAL_COMPARE_RID_FUNCTION_NUMBER,
> - OPAL_MAP_PE);
> - WARN_ON(rc != OPAL_SUCCESS);
> - found_pe = true;
> - break;
> - }
> - }
> -
> - if (!found_pe)
> - /*
> - * Could not find an existing PE so allocate a new
> - * one.
> - */
> - return pnv_ioda_setup_dev_PE(npu_pdev);
> - else
> - return pe;
> -}
> -
> -static void pnv_ioda_setup_npu_PEs(struct pci_bus *bus)
> -{
> - struct pci_dev *pdev;
> -
> - list_for_each_entry(pdev, &bus->devices, bus_list)
> - pnv_ioda_setup_npu_PE(pdev);
> -}
> -
> static void pnv_pci_ioda_setup_PEs(void)
> {
> struct pci_controller *hose, *tmp;
> @@ -1281,13 +1212,6 @@ static void pnv_pci_ioda_setup_PEs(void)
>
> list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> phb = hose->private_data;
> - if (phb->type == PNV_PHB_NPU_NVLINK) {
> - /* PE#0 is needed for error reporting */
> - pnv_ioda_reserve_pe(phb, 0);
> - pnv_ioda_setup_npu_PEs(hose->bus);
> - if (phb->model == PNV_PHB_MODEL_NPU2)
> - pnv_npu2_init(phb);
> - }
> if (phb->type == PNV_PHB_NPU_OCAPI) {
> bus = hose->bus;
> list_for_each_entry(pdev, &bus->devices, bus_list)
> @@ -1895,9 +1819,6 @@ static int pnv_pci_ioda_dma_set_mask(struct pci_dev *pdev, u64 dma_mask)
> }
> *pdev->dev.dma_mask = dma_mask;
>
> - /* Update peer npu devices */
> - pnv_npu_try_dma_set_bypass(pdev, bypass);
> -
> return 0;
> }
>
> @@ -2143,14 +2064,6 @@ static void pnv_pci_ioda2_tce_invalidate(struct iommu_table *tbl,
> }
> }
>
> -void pnv_pci_ioda2_tce_invalidate_entire(struct pnv_phb *phb, bool rm)
> -{
> - if (phb->model == PNV_PHB_MODEL_NPU || phb->model == PNV_PHB_MODEL_PHB3)
> - pnv_pci_phb3_tce_invalidate_entire(phb, rm);
> - else
> - opal_pci_tce_kill(phb->opal_id, OPAL_PCI_TCE_KILL, 0, 0, 0, 0);
> -}
> -
> static int pnv_ioda2_tce_build(struct iommu_table *tbl, long index,
> long npages, unsigned long uaddr,
> enum dma_data_direction direction,
> @@ -2639,137 +2552,6 @@ static struct iommu_table_group_ops pnv_pci_ioda2_ops = {
> .take_ownership = pnv_ioda2_take_ownership,
> .release_ownership = pnv_ioda2_release_ownership,
> };
> -
> -static int gpe_table_group_to_npe_cb(struct device *dev, void *opaque)
> -{
> - struct pci_controller *hose;
> - struct pnv_phb *phb;
> - struct pnv_ioda_pe **ptmppe = opaque;
> - struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> - struct pci_dn *pdn = pci_get_pdn(pdev);
> -
> - if (!pdn || pdn->pe_number == IODA_INVALID_PE)
> - return 0;
> -
> - hose = pci_bus_to_host(pdev->bus);
> - phb = hose->private_data;
> - if (phb->type != PNV_PHB_NPU_NVLINK)
> - return 0;
> -
> - *ptmppe = &phb->ioda.pe_array[pdn->pe_number];
> -
> - return 1;
> -}
> -
> -/*
> - * This returns PE of associated NPU.
> - * This assumes that NPU is in the same IOMMU group with GPU and there is
> - * no other PEs.
> - */
> -static struct pnv_ioda_pe *gpe_table_group_to_npe(
> - struct iommu_table_group *table_group)
> -{
> - struct pnv_ioda_pe *npe = NULL;
> - int ret = iommu_group_for_each_dev(table_group->group, &npe,
> - gpe_table_group_to_npe_cb);
> -
> - BUG_ON(!ret || !npe);
> -
> - return npe;
> -}
> -
> -static long pnv_pci_ioda2_npu_set_window(struct iommu_table_group *table_group,
> - int num, struct iommu_table *tbl)
> -{
> - struct pnv_ioda_pe *npe = gpe_table_group_to_npe(table_group);
> - int num2 = (num == 0) ? 1 : 0;
> - long ret = pnv_pci_ioda2_set_window(table_group, num, tbl);
> -
> - if (ret)
> - return ret;
> -
> - if (table_group->tables[num2])
> - pnv_npu_unset_window(npe, num2);
> -
> - ret = pnv_npu_set_window(npe, num, tbl);
> - if (ret) {
> - pnv_pci_ioda2_unset_window(table_group, num);
> - if (table_group->tables[num2])
> - pnv_npu_set_window(npe, num2,
> - table_group->tables[num2]);
> - }
> -
> - return ret;
> -}
> -
> -static long pnv_pci_ioda2_npu_unset_window(
> - struct iommu_table_group *table_group,
> - int num)
> -{
> - struct pnv_ioda_pe *npe = gpe_table_group_to_npe(table_group);
> - int num2 = (num == 0) ? 1 : 0;
> - long ret = pnv_pci_ioda2_unset_window(table_group, num);
> -
> - if (ret)
> - return ret;
> -
> - if (!npe->table_group.tables[num])
> - return 0;
> -
> - ret = pnv_npu_unset_window(npe, num);
> - if (ret)
> - return ret;
> -
> - if (table_group->tables[num2])
> - ret = pnv_npu_set_window(npe, num2, table_group->tables[num2]);
> -
> - return ret;
> -}
> -
> -static void pnv_ioda2_npu_take_ownership(struct iommu_table_group *table_group)
> -{
> - /*
> - * Detach NPU first as pnv_ioda2_take_ownership() will destroy
> - * the iommu_table if 32bit DMA is enabled.
> - */
> - pnv_npu_take_ownership(gpe_table_group_to_npe(table_group));
> - pnv_ioda2_take_ownership(table_group);
> -}
> -
> -static struct iommu_table_group_ops pnv_pci_ioda2_npu_ops = {
> - .get_table_size = pnv_pci_ioda2_get_table_size,
> - .create_table = pnv_pci_ioda2_create_table_userspace,
> - .set_window = pnv_pci_ioda2_npu_set_window,
> - .unset_window = pnv_pci_ioda2_npu_unset_window,
> - .take_ownership = pnv_ioda2_npu_take_ownership,
> - .release_ownership = pnv_ioda2_release_ownership,
These are called from the VFIO SPAPR TCE IOMMU subdriver when certain
GPUs are passed through with VFIO:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/vfio/vfio_iommu_spapr_tce.c
> -};
> -
> -static void pnv_pci_ioda_setup_iommu_api(void)
> -{
> - struct pci_controller *hose, *tmp;
> - struct pnv_phb *phb;
> - struct pnv_ioda_pe *pe, *gpe;
> -
> - /*
> - * Now we have all PHBs discovered, time to add NPU devices to
> - * the corresponding IOMMU groups.
> - */
> - list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> - phb = hose->private_data;
> -
> - if (phb->type != PNV_PHB_NPU_NVLINK)
> - continue;
> -
> - list_for_each_entry(pe, &phb->ioda.pe_list, list) {
> - gpe = pnv_pci_npu_setup_iommu(pe);
> - if (gpe)
> - gpe->table_group.ops = &pnv_pci_ioda2_npu_ops;
> - }
> - }
> -}
> -#else /* !CONFIG_IOMMU_API */
> -static void pnv_pci_ioda_setup_iommu_api(void) { };
> #endif
>
> static unsigned long pnv_ioda_parse_tce_sizes(struct pnv_phb *phb)
> @@ -3266,7 +3048,6 @@ static void pnv_pci_enable_bridges(void)
> static void pnv_pci_ioda_fixup(void)
> {
> pnv_pci_ioda_setup_PEs();
> - pnv_pci_ioda_setup_iommu_api();
> pnv_pci_ioda_create_dbgfs();
>
> pnv_pci_enable_bridges();
> @@ -3713,27 +3494,6 @@ static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
> .shutdown = pnv_pci_ioda_shutdown,
> };
>
> -static int pnv_npu_dma_set_mask(struct pci_dev *npdev, u64 dma_mask)
> -{
> - dev_err_once(&npdev->dev,
> - "%s operation unsupported for NVLink devices\n",
> - __func__);
> - return -EPERM;
> -}
> -
> -static const struct pci_controller_ops pnv_npu_ioda_controller_ops = {
> - .dma_dev_setup = pnv_pci_dma_dev_setup,
> -#ifdef CONFIG_PCI_MSI
> - .setup_msi_irqs = pnv_setup_msi_irqs,
> - .teardown_msi_irqs = pnv_teardown_msi_irqs,
> -#endif
> - .enable_device_hook = pnv_pci_enable_device_hook,
> - .window_alignment = pnv_pci_window_alignment,
> - .reset_secondary_bus = pnv_pci_reset_secondary_bus,
> - .dma_set_mask = pnv_npu_dma_set_mask,
> - .shutdown = pnv_pci_ioda_shutdown,
> -};
> -
> static const struct pci_controller_ops pnv_npu_ocapi_ioda_controller_ops = {
> .enable_device_hook = pnv_pci_enable_device_hook,
> .window_alignment = pnv_pci_window_alignment,
> @@ -3955,9 +3715,6 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
> ppc_md.pcibios_fixup = pnv_pci_ioda_fixup;
>
> switch (phb->type) {
> - case PNV_PHB_NPU_NVLINK:
> - hose->controller_ops = pnv_npu_ioda_controller_ops;
> - break;
> case PNV_PHB_NPU_OCAPI:
> hose->controller_ops = pnv_npu_ocapi_ioda_controller_ops;
> break;
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 8b37b28e3831..54f2935b7ac5 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -231,17 +231,6 @@ extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
> #define pe_info(pe, fmt, ...) \
> pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__)
>
> -/* Nvlink functions */
> -extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
> -extern void pnv_pci_ioda2_tce_invalidate_entire(struct pnv_phb *phb, bool rm);
> -extern struct pnv_ioda_pe *pnv_pci_npu_setup_iommu(struct pnv_ioda_pe *npe);
> -extern long pnv_npu_set_window(struct pnv_ioda_pe *npe, int num,
> - struct iommu_table *tbl);
> -extern long pnv_npu_unset_window(struct pnv_ioda_pe *npe, int num);
> -extern void pnv_npu_take_ownership(struct pnv_ioda_pe *npe);
> -extern void pnv_npu_release_ownership(struct pnv_ioda_pe *npe);
> -extern int pnv_npu2_init(struct pnv_phb *phb);
> -
> /* pci-ioda-tce.c */
> #define POWERNV_IOMMU_DEFAULT_LEVELS 1
> #define POWERNV_IOMMU_MAX_LEVELS 5
>
--
Alexey
^ permalink raw reply
* [PATCH] powerpc/mm/64s: Fix preempt warning in slb_allocate_kernel()
From: Michael Ellerman @ 2018-11-01 5:21 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin
With preempt enabled we see warnings in do_slb_fault():
BUG: using smp_processor_id() in preemptible [00000000] code: kworker/u33:0/98
futex hash table entries: 4096 (order: 3, 524288 bytes)
caller is do_slb_fault+0x204/0x230
CPU: 5 PID: 98 Comm: kworker/u33:0 Not tainted 4.19.0-rc3-gcc-7.3.1-00022-g1936f094e164 #138
Call Trace:
dump_stack+0xb4/0x104 (unreliable)
check_preemption_disabled+0x148/0x150
do_slb_fault+0x204/0x230
data_access_slb_common+0x138/0x180
This is caused by the get_paca() in slb_allocate_kernel(), which
includes a call to debug_smp_processor_id().
slb_allocate_kernel() can only be called from do_slb_fault(), and in
that path interrupts are hard disabled and so we can't be preempted,
but we can't update the preempt flags (in thread_info) because that
could cause an SLB fault.
So just use local_paca which is safe and doesn't cause the warning.
Fixes: 48e7b7695745 ("powerpc/64s/hash: Convert SLB miss handlers to C")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/slb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index c3fdf2969d9f..2f5c0a10fac1 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c
@@ -715,7 +715,7 @@ static long slb_allocate_kernel(unsigned long ea, unsigned long id)
return -EFAULT;
if (ea < H_VMALLOC_END)
- flags = get_paca()->vmalloc_sllp;
+ flags = local_paca->vmalloc_sllp;
else
flags = SLB_VSID_KERNEL | mmu_psize_defs[mmu_io_psize].sllp;
} else {
--
2.17.2
^ permalink raw reply related
* Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
From: Christoph Hellwig @ 2018-11-01 5:48 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arch, linux-scsi, Linux Kbuild mailing list, linux-pci,
Linux Kernel Mailing List, Dominik Brodowski, Russell King,
Alex Bounine, linuxppc-dev, Christoph Hellwig, linux-arm-kernel
In-Reply-To: <CAK7LNASUArREw1AWUzcOoQCe5=4jyifbs+x2CdCK+qXJ+woAdw@mail.gmail.com>
On Thu, Nov 01, 2018 at 01:05:26AM +0900, Masahiro Yamada wrote:
> > How about letting CONFIG_ARM to select HAVE_PCI ?
> >
>
>
> I applied 1/9, 3/9, 4/9, 5/9.
> (I think 2/9 should be squashed to 9/9)
>
> As Russell pointed out, we need to avoid
> the unmet dependency.
Yes, I think the HAVE_PCI is probably the nicest way, but we'll
need to wait what Russell as the maintainer wants.
> Are you planning to send
> the updated version for 6/9 through - 9/9 ?
>
> If so, could you please rebase 6/9
> so that it is cleanly applicable ?
Will do once I find some time after rc1.
^ permalink raw reply
* Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
From: Trond Myklebust @ 2018-11-01 6:30 UTC (permalink / raw)
To: linux@roeck-us.net, paul.burton@mips.com
Cc: linux-mips@linux-mips.org, linux-nfs@vger.kernel.org,
arnd@arndb.de, jhogan@kernel.org, jlayton@kernel.org,
linux-kernel@vger.kernel.org, ralf@linux-mips.org,
davem@davemloft.net, bfields@fieldses.org, paulus@samba.org,
netdev@vger.kernel.org, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, anna.schumaker@netapp.com
In-Reply-To: <291af20b-820e-e848-cf75-730024612117@roeck-us.net>
On Wed, 2018-10-31 at 18:18 -0700, Guenter Roeck wrote:
> On 10/31/18 4:32 PM, Paul Burton wrote:
> > (Copying SunRPC & net maintainers.)
> >
> > Hi Guenter,
> >
> > On Wed, Oct 31, 2018 at 03:02:53PM -0700, Guenter Roeck wrote:
> > > The alternatives I can see are
> > > - Do not use cmpxchg64() outside architecture code (ie drop its
> > > use from
> > > the offending driver, and keep doing the same whenever the
> > > problem comes
> > > up again).
> > > or
> > > - Introduce something like ARCH_HAS_CMPXCHG64 and use it to
> > > determine
> > > if cmpxchg64 is supported or not.
> > >
> > > Any preference ?
> >
> > My preference would be option 1 - avoiding cmpxchg64() where
> > possible in
> > generic code. I wouldn't be opposed to the Kconfig option if there
> > are
> > cases where cmpxchg64() can really help performance though.
> >
> > The last time I'm aware of this coming up the affected driver was
> > modified to avoid cmpxchg64() [1].
> >
> > In this particular case I have no idea why
> > net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all.
> > It's
> > essentially reinventing atomic64_fetch_inc() which is already
> > provided
> > everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At
> > least
> > for atomic64_* functions the assumption that all access will be
> > performed using those same functions seems somewhat reasonable.
> >
> > So how does the below look? Trond?
> >
>
> For my part I agree that this would be a much better solution. The
> argument
> that it is not always absolutely guaranteed that atomics don't wrap
> doesn't
> really hold for me because it looks like they all do. On top of that,
> there
> is an explicit atomic_dec_if_positive() and
> atomic_fetch_add_unless(),
> which to me strongly suggests that they _are_ supposed to wrap.
> Given the cost of adding a comparison to each atomic operation to
> prevent it from wrapping, anything else would not really make sense
> to me.
That's a hypothesis, not a proven fact. There are architectures out
there that do not wrap signed integers, hence my question.
> So ... please consider my patch abandoned. Thanks for looking into
> this!
>
> Guenter
>
> > Thanks,
> > Paul
> >
> > [1] https://patchwork.ozlabs.org/cover/891284/
> >
> > ---
> > diff --git a/include/linux/sunrpc/gss_krb5.h
> > b/include/linux/sunrpc/gss_krb5.h
> > index 131424cefc6a..02c0412e368c 100644
> > --- a/include/linux/sunrpc/gss_krb5.h
> > +++ b/include/linux/sunrpc/gss_krb5.h
> > @@ -107,8 +107,8 @@ struct krb5_ctx {
> > u8 Ksess[GSS_KRB5_MAX_KEYLEN]; /* session key
> > */
> > u8 cksum[GSS_KRB5_MAX_KEYLEN];
> > s32 endtime;
> > - u32 seq_send;
> > - u64 seq_send64;
> > + atomic_t seq_send;
> > + atomic64_t seq_send64;
> > struct xdr_netobj mech_used;
> > u8 initiator_sign[GSS_KRB5_MAX_KEYLEN];
> > u8 acceptor_sign[GSS_KRB5_MAX_KEYLEN];
> > @@ -118,9 +118,6 @@ struct krb5_ctx {
> > u8 acceptor_integ[GSS_KRB5_MAX_KEYLEN];
> > };
> >
> > -extern u32 gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx);
> > -extern u64 gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx);
> > -
> > /* The length of the Kerberos GSS token header */
> > #define GSS_KRB5_TOK_HDR_LEN (16)
> >
> > diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c
> > b/net/sunrpc/auth_gss/gss_krb5_mech.c
> > index 7f0424dfa8f6..eab71fc7af3e 100644
> > --- a/net/sunrpc/auth_gss/gss_krb5_mech.c
> > +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
> > @@ -274,6 +274,7 @@ get_key(const void *p, const void *end,
> > static int
> > gss_import_v1_context(const void *p, const void *end, struct
> > krb5_ctx *ctx)
> > {
> > + u32 seq_send;
> > int tmp;
> >
> > p = simple_get_bytes(p, end, &ctx->initiate, sizeof(ctx-
> > >initiate));
> > @@ -315,9 +316,10 @@ gss_import_v1_context(const void *p, const
> > void *end, struct krb5_ctx *ctx)
> > p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx-
> > >endtime));
> > if (IS_ERR(p))
> > goto out_err;
> > - p = simple_get_bytes(p, end, &ctx->seq_send, sizeof(ctx-
> > >seq_send));
> > + p = simple_get_bytes(p, end, &seq_send, sizeof(seq_send));
> > if (IS_ERR(p))
> > goto out_err;
> > + atomic_set(&ctx->seq_send, seq_send);
> > p = simple_get_netobj(p, end, &ctx->mech_used);
> > if (IS_ERR(p))
> > goto out_err;
> > @@ -607,6 +609,7 @@ static int
> > gss_import_v2_context(const void *p, const void *end, struct
> > krb5_ctx *ctx,
> > gfp_t gfp_mask)
> > {
> > + u64 seq_send64;
> > int keylen;
> >
> > p = simple_get_bytes(p, end, &ctx->flags, sizeof(ctx->flags));
> > @@ -617,14 +620,15 @@ gss_import_v2_context(const void *p, const
> > void *end, struct krb5_ctx *ctx,
> > p = simple_get_bytes(p, end, &ctx->endtime, sizeof(ctx-
> > >endtime));
> > if (IS_ERR(p))
> > goto out_err;
> > - p = simple_get_bytes(p, end, &ctx->seq_send64, sizeof(ctx-
> > >seq_send64));
> > + p = simple_get_bytes(p, end, &seq_send64, sizeof(seq_send64));
> > if (IS_ERR(p))
> > goto out_err;
> > + atomic64_set(&ctx->seq_send64, seq_send64);
> > /* set seq_send for use by "older" enctypes */
> > - ctx->seq_send = ctx->seq_send64;
> > - if (ctx->seq_send64 != ctx->seq_send) {
> > - dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n",
> > __func__,
> > - (unsigned long)ctx->seq_send64, ctx->seq_send);
> > + atomic_set(&ctx->seq_send, seq_send64);
> > + if (seq_send64 != atomic_read(&ctx->seq_send)) {
> > + dprintk("%s: seq_send64 %llx, seq_send %x overflow?\n",
> > __func__,
> > + seq_send64, atomic_read(&ctx->seq_send));
> > p = ERR_PTR(-EINVAL);
> > goto out_err;
> > }
> > diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c
> > b/net/sunrpc/auth_gss/gss_krb5_seal.c
> > index b4adeb06660b..48fe4a591b54 100644
> > --- a/net/sunrpc/auth_gss/gss_krb5_seal.c
> > +++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
> > @@ -123,30 +123,6 @@ setup_token_v2(struct krb5_ctx *ctx, struct
> > xdr_netobj *token)
> > return krb5_hdr;
> > }
> >
> > -u32
> > -gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx)
> > -{
> > - u32 old, seq_send = READ_ONCE(ctx->seq_send);
> > -
> > - do {
> > - old = seq_send;
> > - seq_send = cmpxchg(&ctx->seq_send, old, old + 1);
> > - } while (old != seq_send);
> > - return seq_send;
> > -}
> > -
> > -u64
> > -gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx)
> > -{
> > - u64 old, seq_send = READ_ONCE(ctx->seq_send);
> > -
> > - do {
> > - old = seq_send;
> > - seq_send = cmpxchg64(&ctx->seq_send64, old, old + 1);
> > - } while (old != seq_send);
> > - return seq_send;
> > -}
> > -
> > static u32
> > gss_get_mic_v1(struct krb5_ctx *ctx, struct xdr_buf *text,
> > struct xdr_netobj *token)
> > @@ -177,7 +153,7 @@ gss_get_mic_v1(struct krb5_ctx *ctx, struct
> > xdr_buf *text,
> >
> > memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data,
> > md5cksum.len);
> >
> > - seq_send = gss_seq_send_fetch_and_inc(ctx);
> > + seq_send = atomic_fetch_inc(&ctx->seq_send);
> >
> > if (krb5_make_seq_num(ctx, ctx->seq, ctx->initiate ? 0 : 0xff,
> > seq_send, ptr + GSS_KRB5_TOK_HDR_LEN, ptr
> > + 8))
> > @@ -205,7 +181,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct
> > xdr_buf *text,
> >
> > /* Set up the sequence number. Now 64-bits in clear
> > * text and w/o direction indicator */
> > - seq_send_be64 = cpu_to_be64(gss_seq_send64_fetch_and_inc(ctx));
> > + seq_send_be64 = cpu_to_be64(atomic64_fetch_inc(&ctx-
> > >seq_send64));
> > memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
> >
> > if (ctx->initiate) {
> > diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > index 962fa84e6db1..5cdde6cb703a 100644
> > --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > @@ -228,7 +228,7 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int
> > offset,
> >
> > memcpy(ptr + GSS_KRB5_TOK_HDR_LEN, md5cksum.data,
> > md5cksum.len);
> >
> > - seq_send = gss_seq_send_fetch_and_inc(kctx);
> > + seq_send = atomic_fetch_inc(&kctx->seq_send);
> >
> > /* XXX would probably be more efficient to compute checksum
> > * and encrypt at the same time: */
> > @@ -475,7 +475,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32
> > offset,
> > *be16ptr++ = 0;
> >
> > be64ptr = (__be64 *)be16ptr;
> > - *be64ptr = cpu_to_be64(gss_seq_send64_fetch_and_inc(kctx));
> > + *be64ptr = cpu_to_be64(atomic64_fetch_inc(&kctx->seq_send64));
> >
> > err = (*kctx->gk5e->encrypt_v2)(kctx, offset, buf, pages);
> > if (err)
> >
--
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space
^ permalink raw reply
* Re: [RFC PATCH v1 1/4] kvmppc: HMM backend driver to manage pages of secure guest
From: Balbir Singh @ 2018-11-01 6:43 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, kvm-ppc, benh, linux-mm, jglisse, aneesh.kumar, paulus,
linuxppc-dev
In-Reply-To: <20181022051837.1165-2-bharata@linux.ibm.com>
On Mon, Oct 22, 2018 at 10:48:34AM +0530, Bharata B Rao wrote:
> HMM driver for KVM PPC to manage page transitions of
> secure guest via H_SVM_PAGE_IN and H_SVM_PAGE_OUT hcalls.
>
> H_SVM_PAGE_IN: Move the content of a normal page to secure page
> H_SVM_PAGE_OUT: Move the content of a secure page to normal page
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/include/asm/hvcall.h | 7 +-
> arch/powerpc/include/asm/kvm_host.h | 15 +
> arch/powerpc/include/asm/kvm_ppc.h | 28 ++
> arch/powerpc/include/asm/ucall-api.h | 20 ++
> arch/powerpc/kvm/Makefile | 3 +
> arch/powerpc/kvm/book3s_hv.c | 38 ++
> arch/powerpc/kvm/book3s_hv_hmm.c | 514 +++++++++++++++++++++++++++
> 7 files changed, 624 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/ucall-api.h
> create mode 100644 arch/powerpc/kvm/book3s_hv_hmm.c
>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index a0b17f9f1ea4..89e6b70c1857 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -158,6 +158,9 @@
> /* Each control block has to be on a 4K boundary */
> #define H_CB_ALIGNMENT 4096
>
> +/* Flags for H_SVM_PAGE_IN */
> +#define H_PAGE_IN_SHARED 0x1
> +
> /* pSeries hypervisor opcodes */
> #define H_REMOVE 0x04
> #define H_ENTER 0x08
> @@ -295,7 +298,9 @@
> #define H_INT_ESB 0x3C8
> #define H_INT_SYNC 0x3CC
> #define H_INT_RESET 0x3D0
> -#define MAX_HCALL_OPCODE H_INT_RESET
> +#define H_SVM_PAGE_IN 0x3D4
> +#define H_SVM_PAGE_OUT 0x3D8
> +#define MAX_HCALL_OPCODE H_SVM_PAGE_OUT
>
> /* H_VIOCTL functions */
> #define H_GET_VIOA_DUMP_SIZE 0x01
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 906bcbdfd2a1..194e6e0ff239 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -310,6 +310,9 @@ struct kvm_arch {
> struct kvmppc_passthru_irqmap *pimap;
> #endif
> struct kvmppc_ops *kvm_ops;
> +#ifdef CONFIG_PPC_SVM
> + struct hlist_head *hmm_hash;
> +#endif
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> /* This array can grow quite large, keep it at the end */
> struct kvmppc_vcore *vcores[KVM_MAX_VCORES];
> @@ -830,4 +833,16 @@ static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
>
> +#ifdef CONFIG_PPC_SVM
> +struct kvmppc_hmm_device {
> + struct hmm_device *device;
> + struct hmm_devmem *devmem;
> + unsigned long *pfn_bitmap;
> +};
> +
> +extern int kvmppc_hmm_init(void);
> +extern void kvmppc_hmm_free(void);
> +extern int kvmppc_hmm_hash_create(struct kvm *kvm);
> +extern void kvmppc_hmm_hash_destroy(struct kvm *kvm);
> +#endif
> #endif /* __POWERPC_KVM_HOST_H__ */
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index e991821dd7fa..ba81a07e2bdf 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -906,4 +906,32 @@ static inline ulong kvmppc_get_ea_indexed(struct kvm_vcpu *vcpu, int ra, int rb)
>
> extern void xics_wake_cpu(int cpu);
>
> +#ifdef CONFIG_PPC_SVM
> +extern unsigned long kvmppc_h_svm_page_in(struct kvm *kvm,
> + unsigned int lpid,
> + unsigned long gra,
> + unsigned long flags,
> + unsigned long page_shift);
> +extern unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
> + unsigned int lpid,
> + unsigned long gra,
> + unsigned long flags,
> + unsigned long page_shift);
> +#else
> +static inline unsigned long
> +kvmppc_h_svm_page_in(struct kvm *kvm, unsigned int lpid,
> + unsigned long gra, unsigned long flags,
> + unsigned long page_shift)
> +{
> + return H_UNSUPPORTED;
> +}
> +
> +static inline unsigned long
> +kvmppc_h_svm_page_out(struct kvm *kvm, unsigned int lpid,
> + unsigned long gra, unsigned long flags,
> + unsigned long page_shift)
> +{
> + return H_UNSUPPORTED;
> +}
> +#endif
> #endif /* __POWERPC_KVM_PPC_H__ */
> diff --git a/arch/powerpc/include/asm/ucall-api.h b/arch/powerpc/include/asm/ucall-api.h
> new file mode 100644
> index 000000000000..2c12f514f8ab
> --- /dev/null
> +++ b/arch/powerpc/include/asm/ucall-api.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_UCALL_API_H
> +#define _ASM_POWERPC_UCALL_API_H
> +
> +#define U_SUCCESS 0
> +
> +/*
> + * TODO: Dummy uvcalls, will be replaced by real calls
> + */
> +static inline int uv_page_in(u64 lpid, u64 dw0, u64 dw1, u64 dw2, u64 dw3)
> +{
> + return U_SUCCESS;
> +}
> +
> +static inline int uv_page_out(u64 lpid, u64 dw0, u64 dw1, u64 dw2, u64 dw3)
> +{
> + return U_SUCCESS;
> +}
> +
> +#endif /* _ASM_POWERPC_UCALL_API_H */
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index f872c04bb5b1..6945ffc18679 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -77,6 +77,9 @@ kvm-hv-y += \
> book3s_64_mmu_hv.o \
> book3s_64_mmu_radix.o
>
> +kvm-hv-$(CONFIG_PPC_SVM) += \
> + book3s_hv_hmm.o
> +
> kvm-hv-$(CONFIG_PPC_TRANSACTIONAL_MEM) += \
> book3s_hv_tm.o
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 3e3a71594e63..05084eb8aadd 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -73,6 +73,7 @@
> #include <asm/opal.h>
> #include <asm/xics.h>
> #include <asm/xive.h>
> +#include <asm/kvm_host.h>
>
> #include "book3s.h"
>
> @@ -935,6 +936,20 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> if (ret == H_TOO_HARD)
> return RESUME_HOST;
> break;
> + case H_SVM_PAGE_IN:
> + ret = kvmppc_h_svm_page_in(vcpu->kvm,
> + kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6),
> + kvmppc_get_gpr(vcpu, 7));
> + break;
> + case H_SVM_PAGE_OUT:
> + ret = kvmppc_h_svm_page_out(vcpu->kvm,
> + kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6),
> + kvmppc_get_gpr(vcpu, 7));
Won't just touching the page cause page_out? I wonder if you could
avoid code duplication by touch addr. So under mmap_sem if you
did get_user_pages() and put_page(), it might simplify all the
additional code you've got.
> + break;
> default:
> return RESUME_HOST;
> }
> @@ -961,6 +976,8 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
> case H_IPOLL:
> case H_XIRR_X:
> #endif
> + case H_SVM_PAGE_IN:
> + case H_SVM_PAGE_OUT:
> return 1;
> }
>
> @@ -3938,6 +3955,13 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
> return -ENOMEM;
> kvm->arch.lpid = lpid;
>
> +#ifdef CONFIG_PPC_SVM
> + ret = kvmppc_hmm_hash_create(kvm);
> + if (ret) {
> + kvmppc_free_lpid(kvm->arch.lpid);
> + return ret;
> + }
> +#endif
> kvmppc_alloc_host_rm_ops();
>
> /*
> @@ -4073,6 +4097,9 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm)
>
> kvmppc_free_vcores(kvm);
>
> +#ifdef CONFIG_PPC_SVM
> + kvmppc_hmm_hash_destroy(kvm);
> +#endif
> kvmppc_free_lpid(kvm->arch.lpid);
>
> if (kvm_is_radix(kvm))
> @@ -4384,6 +4411,8 @@ static unsigned int default_hcall_list[] = {
> H_XIRR,
> H_XIRR_X,
> #endif
> + H_SVM_PAGE_IN,
> + H_SVM_PAGE_OUT,
> 0
> };
>
> @@ -4596,11 +4625,20 @@ static int kvmppc_book3s_init_hv(void)
> no_mixing_hpt_and_radix = true;
> }
>
> +#ifdef CONFIG_PPC_SVM
> + r = kvmppc_hmm_init();
> + if (r < 0)
> + pr_err("KVM-HV: kvmppc_hmm_init failed %d\n", r);
> +#endif
> +
> return r;
> }
>
> static void kvmppc_book3s_exit_hv(void)
> {
> +#ifdef CONFIG_PPC_SVM
> + kvmppc_hmm_free();
> +#endif
> kvmppc_free_host_rm_ops();
> if (kvmppc_radix_possible())
> kvmppc_radix_exit();
> diff --git a/arch/powerpc/kvm/book3s_hv_hmm.c b/arch/powerpc/kvm/book3s_hv_hmm.c
> new file mode 100644
> index 000000000000..a2ee3163a312
> --- /dev/null
> +++ b/arch/powerpc/kvm/book3s_hv_hmm.c
> @@ -0,0 +1,514 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HMM driver to manage page migration between normal and secure
> + * memory.
> + *
> + * Based on Jérôme Glisse's HMM dummy driver.
> + *
> + * Copyright 2018 Bharata B Rao, IBM Corp. <bharata@linux.ibm.com>
> + */
> +
> +/*
> + * A pseries guest can be run as a secure guest on Ultravisor-enabled
> + * POWER platforms. On such platforms, this driver will be used to manage
> + * the movement of guest pages between the normal memory managed by
> + * hypervisor (HV) and secure memory managed by Ultravisor (UV).
> + *
> + * Private ZONE_DEVICE memory equal to the amount of secure memory
> + * available in the platform for running secure guests is created
> + * via a HMM device. The movement of pages between normal and secure
> + * memory is done by ->alloc_and_copy() callback routine of migrate_vma().
> + *
> + * The page-in or page-out requests from UV will come to HV as hcalls and
> + * HV will call back into UV via uvcalls to satisfy these page requests.
> + *
> + * For each page that gets moved into secure memory, a HMM PFN is used
> + * on the HV side and HMM migration PTE corresponding to that PFN would be
> + * populated in the QEMU page tables. A per-guest hash table is created to
> + * manage the pool of HMM PFNs. Guest real address is used as key to index
> + * into the hash table and choose a free HMM PFN.
> + */
> +
> +#include <linux/hmm.h>
> +#include <linux/kvm_host.h>
> +#include <linux/sched/mm.h>
> +#include <asm/ucall-api.h>
> +
> +static struct kvmppc_hmm_device *kvmppc_hmm;
> +spinlock_t kvmppc_hmm_lock;
> +
> +#define KVMPPC_HMM_HASH_BITS 10
> +#define KVMPPC_HMM_HASH_SIZE (1 << KVMPPC_HMM_HASH_BITS)
> +
> +struct kvmppc_hmm_pfn_entry {
> + struct hlist_node hlist;
> + unsigned long addr;
> + unsigned long hmm_pfn;
> +};
> +
> +struct kvmppc_hmm_page_pvt {
> + struct hlist_head *hmm_hash;
> + unsigned int lpid;
> + unsigned long gpa;
> +};
> +
> +struct kvmppc_hmm_migrate_args {
> + struct hlist_head *hmm_hash;
> + unsigned int lpid;
> + unsigned long gpa;
> + unsigned long page_shift;
> +};
> +
> +int kvmppc_hmm_hash_create(struct kvm *kvm)
> +{
> + int i;
> +
> + kvm->arch.hmm_hash = kzalloc(KVMPPC_HMM_HASH_SIZE *
> + sizeof(struct hlist_head), GFP_KERNEL);
> + if (!kvm->arch.hmm_hash)
> + return -ENOMEM;
> +
> + for (i = 0; i < KVMPPC_HMM_HASH_SIZE; i++)
> + INIT_HLIST_HEAD(&kvm->arch.hmm_hash[i]);
> + return 0;
> +}
> +
> +/*
> + * Cleanup the HMM pages hash table when guest terminates
> + *
> + * Iterate over all the HMM pages hash list entries and release
> + * reference on them. The actual freeing of the entry happens
> + * via hmm_devmem_ops.free path.
> + */
> +void kvmppc_hmm_hash_destroy(struct kvm *kvm)
> +{
> + int i;
> + struct kvmppc_hmm_pfn_entry *p;
> + struct page *hmm_page;
> +
> + for (i = 0; i < KVMPPC_HMM_HASH_SIZE; i++) {
> + while (!hlist_empty(&kvm->arch.hmm_hash[i])) {
> + p = hlist_entry(kvm->arch.hmm_hash[i].first,
> + struct kvmppc_hmm_pfn_entry,
> + hlist);
> + hmm_page = pfn_to_page(p->hmm_pfn);
> + put_page(hmm_page);
> + }
> + }
> + kfree(kvm->arch.hmm_hash);
> +}
> +
> +static u64 kvmppc_hmm_pfn_hash_fn(u64 addr)
> +{
> + return hash_64(addr, KVMPPC_HMM_HASH_BITS);
> +}
> +
> +static void
> +kvmppc_hmm_hash_free_pfn(struct hlist_head *hmm_hash, unsigned long gpa)
> +{
> + struct kvmppc_hmm_pfn_entry *p;
> + struct hlist_head *list;
> +
> + list = &hmm_hash[kvmppc_hmm_pfn_hash_fn(gpa)];
> + hlist_for_each_entry(p, list, hlist) {
> + if (p->addr == gpa) {
> + hlist_del(&p->hlist);
> + kfree(p);
> + return;
> + }
> + }
> +}
> +
> +/*
> + * Get a free HMM PFN from the pool
> + *
> + * Called when a normal page is moved to secure memory (UV_PAGE_IN). HMM
> + * PFN will be used to keep track of the secure page on HV side.
> + */
> +static struct page *kvmppc_hmm_get_page(struct hlist_head *hmm_hash,
> + unsigned long gpa, unsigned int lpid)
> +{
> + struct page *dpage = NULL;
> + unsigned long bit;
> + unsigned long nr_pfns = kvmppc_hmm->devmem->pfn_last -
> + kvmppc_hmm->devmem->pfn_first;
> + struct hlist_head *list;
> + struct kvmppc_hmm_pfn_entry *p;
> + bool found = false;
> + unsigned long flags;
> + struct kvmppc_hmm_page_pvt *pvt;
> +
> + spin_lock_irqsave(&kvmppc_hmm_lock, flags);
> + list = &hmm_hash[kvmppc_hmm_pfn_hash_fn(gpa)];
> + hlist_for_each_entry(p, list, hlist) {
> + if (p->addr == gpa) {
> + found = true;
> + break;
> + }
> + }
> + if (!found) {
> + p = kzalloc(sizeof(struct kvmppc_hmm_pfn_entry), GFP_ATOMIC);
> + if (!p) {
> + spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
> + return NULL;
> + }
> + p->addr = gpa;
> + bit = find_first_zero_bit(kvmppc_hmm->pfn_bitmap, nr_pfns);
> + if (bit >= nr_pfns) {
> + kfree(p);
Why not consider resizing the bitmap, from the code below, it looks
like this should never happen - BUG_ON()?
> + spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
> + return NULL;
> + }
> + bitmap_set(kvmppc_hmm->pfn_bitmap, bit, 1);
> + p->hmm_pfn = bit + kvmppc_hmm->devmem->pfn_first;
> + INIT_HLIST_NODE(&p->hlist);
> + hlist_add_head(&p->hlist, list);
> + } else {
> + spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
> + return NULL;
For found=true, why do we return NULL?
> + }
> + dpage = pfn_to_page(p->hmm_pfn);
> +
> + if (!trylock_page(dpage)) {
> + bitmap_clear(kvmppc_hmm->pfn_bitmap,
> + p->hmm_pfn - kvmppc_hmm->devmem->pfn_first, 1);
> + hlist_del(&p->hlist);
> + kfree(p);
> + spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
> + return NULL;
What race are you avoiding here?
> + }
> + spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
> +
> + pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC);
> + pvt->hmm_hash = hmm_hash;
> + pvt->gpa = gpa;
> + pvt->lpid = lpid;
> + hmm_devmem_page_set_drvdata(dpage, (unsigned long)pvt);
Do we care about when this store is seen?
> +
> + get_page(dpage);
> + return dpage;
> +}
> +
> +/*
> + * Release the HMM PFN back to the pool
> + *
> + * Called when secure page becomes a normal page during UV_PAGE_OUT.
> + */
> +static void kvmppc_hmm_put_page(struct page *page)
> +{
> + unsigned long pfn = page_to_pfn(page);
> + unsigned long flags;
> + struct kvmppc_hmm_page_pvt *pvt;
> +
> + pvt = (struct kvmppc_hmm_page_pvt *)hmm_devmem_page_get_drvdata(page);
> + hmm_devmem_page_set_drvdata(page, 0);
> +
> + spin_lock_irqsave(&kvmppc_hmm_lock, flags);
> + bitmap_clear(kvmppc_hmm->pfn_bitmap,
> + pfn - kvmppc_hmm->devmem->pfn_first, 1);
> + kvmppc_hmm_hash_free_pfn(pvt->hmm_hash, pvt->gpa);
Don't we need a put_page here?
> + spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
> + kfree(pvt);
> +}
> +
> +static void
> +kvmppc_hmm_migrate_alloc_and_copy(struct vm_area_struct *vma,
> + const unsigned long *src_pfns,
> + unsigned long *dst_pfns,
> + unsigned long start,
> + unsigned long end,
> + void *private)
> +{
> + unsigned long addr;
> + struct kvmppc_hmm_migrate_args *args = private;
> + unsigned long page_size = 1UL << args->page_shift;
> +
> + for (addr = start; addr < end;
> + addr += page_size, src_pfns++, dst_pfns++) {
> + struct page *spage = migrate_pfn_to_page(*src_pfns);
> + struct page *dpage;
> + unsigned long pfn = *src_pfns >> MIGRATE_PFN_SHIFT;
> +
> + *dst_pfns = 0;
> + if (!spage && !(*src_pfns & MIGRATE_PFN_MIGRATE))
> + continue;
> +
> + if (spage && !(*src_pfns & MIGRATE_PFN_MIGRATE))
> + continue;
> +
Isn't this the same as !(*src_pfns & MIGRATE_PFN_MIGRATE) continue?
That probably indicates the page is backed by a file. Do such pages
ever get migrated?
> + dpage = kvmppc_hmm_get_page(args->hmm_hash, args->gpa,
> + args->lpid);
> + if (!dpage)
> + continue;
Aaah.. I see if the page is already in secure memory - kvmppc_hmm_get_page()
returns NULL and no migration is done! dpage is the struct page for the
secure memory.
> +
> + if (spage)
> + uv_page_in(args->lpid, pfn << args->page_shift,
> + args->gpa, 0, args->page_shift);
> +
> + *dst_pfns = migrate_pfn(page_to_pfn(dpage)) |
> + MIGRATE_PFN_DEVICE | MIGRATE_PFN_LOCKED;
> + }
> +}
> +
> +static void
> +kvmppc_hmm_migrate_finalize_and_map(struct vm_area_struct *vma,
> + const unsigned long *src_pfns,
> + const unsigned long *dst_pfns,
> + unsigned long start,
> + unsigned long end,
> + void *private)
> +{
> +}
> +
> +static const struct migrate_vma_ops kvmppc_hmm_migrate_ops = {
> + .alloc_and_copy = kvmppc_hmm_migrate_alloc_and_copy,
> + .finalize_and_map = kvmppc_hmm_migrate_finalize_and_map,
> +};
> +
> +static unsigned long kvmppc_gpa_to_hva(struct kvm *kvm, unsigned long gpa,
> + unsigned long page_shift)
> +{
> + unsigned long gfn, hva;
> + struct kvm_memory_slot *memslot;
> +
> + gfn = gpa >> page_shift;
> + memslot = gfn_to_memslot(kvm, gfn);
> + hva = gfn_to_hva_memslot(memslot, gfn);
> +
> + return hva;
> +}
This doesn't sound like the rightplace to have this function
> +
> +/*
> + * Move page from normal memory to secure memory.
> + */
> +unsigned long
> +kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
> + unsigned long flags, unsigned long page_shift)
> +{
> + unsigned long addr, end;
> + unsigned long src_pfn, dst_pfn;
> + struct kvmppc_hmm_migrate_args args;
> + struct mm_struct *mm = get_task_mm(current);
> + struct vm_area_struct *vma;
> + int ret = H_SUCCESS;
> +
> + if (page_shift != PAGE_SHIFT)
> + return H_P3;
So no large page support?
> +
> + addr = kvmppc_gpa_to_hva(kvm, gpa, page_shift);
> + if (!addr)
> + return H_PARAMETER;
So addr == 0 is invalid?
> + end = addr + (1UL << page_shift);
> +
> + if (flags)
> + return H_P2;
> +
> + args.hmm_hash = kvm->arch.hmm_hash;
> + args.lpid = kvm->arch.lpid;
> + args.gpa = gpa;
> + args.page_shift = page_shift;
> +
> + down_read(&mm->mmap_sem);
> + vma = find_vma_intersection(mm, addr, end);
> + if (!vma || vma->vm_start > addr || vma->vm_end < end) {
> + ret = H_PARAMETER;
> + goto out;
> + }
> + ret = migrate_vma(&kvmppc_hmm_migrate_ops, vma, addr, end,
> + &src_pfn, &dst_pfn, &args);
> + if (ret < 0)
> + ret = H_PARAMETER;
> +out:
> + up_read(&mm->mmap_sem);
> + return ret;
> +}
> +
> +static void
> +kvmppc_hmm_fault_migrate_alloc_and_copy(struct vm_area_struct *vma,
> + const unsigned long *src_pfn,
> + unsigned long *dst_pfn,
> + unsigned long start,
> + unsigned long end,
> + void *private)
> +{
> + struct page *dpage, *spage;
> + struct kvmppc_hmm_page_pvt *pvt;
> + unsigned long pfn;
> + int ret = U_SUCCESS;
> +
> + *dst_pfn = MIGRATE_PFN_ERROR;
> + spage = migrate_pfn_to_page(*src_pfn);
> + if (!spage || !(*src_pfn & MIGRATE_PFN_MIGRATE))
> + return;
> + if (!is_zone_device_page(spage))
> + return;
> + dpage = hmm_vma_alloc_locked_page(vma, start);
> + if (!dpage)
We've probably already OOM'd here :)
> + return;
> + pvt = (struct kvmppc_hmm_page_pvt *)
> + hmm_devmem_page_get_drvdata(spage);
> +
> + pfn = page_to_pfn(dpage);
> + ret = uv_page_out(pvt->lpid, pfn << PAGE_SHIFT,
Do we need pfn << PAGE_SHIFT?
> + pvt->gpa, 0, PAGE_SHIFT);
> + if (ret == U_SUCCESS)
> + *dst_pfn = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
> +}
> +
> +static void
> +kvmppc_hmm_fault_migrate_finalize_and_map(struct vm_area_struct *vma,
> + const unsigned long *src_pfns,
> + const unsigned long *dst_pfns,
> + unsigned long start,
> + unsigned long end,
> + void *private)
> +{
> +}
> +
> +static const struct migrate_vma_ops kvmppc_hmm_fault_migrate_ops = {
> + .alloc_and_copy = kvmppc_hmm_fault_migrate_alloc_and_copy,
> + .finalize_and_map = kvmppc_hmm_fault_migrate_finalize_and_map,
> +};
> +
> +/*
> + * Fault handler callback when HV touches any page that has been
> + * moved to secure memory, we ask UV to give back the page by
> + * issuing a UV_PAGE_OUT uvcall.
> + */
> +static int kvmppc_hmm_devmem_fault(struct hmm_devmem *devmem,
> + struct vm_area_struct *vma,
> + unsigned long addr,
> + const struct page *page,
> + unsigned int flags,
> + pmd_t *pmdp)
> +{
> + unsigned long end = addr + PAGE_SIZE;
> + unsigned long src_pfn, dst_pfn = 0;
> +
> + if (migrate_vma(&kvmppc_hmm_fault_migrate_ops, vma, addr, end,
> + &src_pfn, &dst_pfn, NULL))
> + return VM_FAULT_SIGBUS;
> + if (dst_pfn == MIGRATE_PFN_ERROR)
> + return VM_FAULT_SIGBUS;
> + return 0;
> +}
> +
> +static void kvmppc_hmm_devmem_free(struct hmm_devmem *devmem,
> + struct page *page)
> +{
> + kvmppc_hmm_put_page(page);
> +}
> +
> +static const struct hmm_devmem_ops kvmppc_hmm_devmem_ops = {
> + .free = kvmppc_hmm_devmem_free,
> + .fault = kvmppc_hmm_devmem_fault,
> +};
> +
> +/*
> + * Move page from secure memory to normal memory.
> + */
> +unsigned long
> +kvmppc_h_svm_page_out(struct kvm *kvm, unsigned long gpa,
> + unsigned long flags, unsigned long page_shift)
> +{
> + unsigned long addr, end;
> + struct mm_struct *mm = get_task_mm(current);
> + struct vm_area_struct *vma;
> + unsigned long src_pfn, dst_pfn = 0;
> + int ret = H_SUCCESS;
> +
> + if (page_shift != PAGE_SHIFT)
> + return H_P4;
> +
> + addr = kvmppc_gpa_to_hva(kvm, gpa, page_shift);
> + if (!addr)
> + return H_P2;
> + end = addr + (1UL << page_shift);
> +
> + down_read(&mm->mmap_sem);
> + vma = find_vma_intersection(mm, addr, end);
> + if (!vma || vma->vm_start > addr || vma->vm_end < end) {
> + ret = H_PARAMETER;
> + goto out;
> + }
> + ret = migrate_vma(&kvmppc_hmm_fault_migrate_ops, vma, addr, end,
> + &src_pfn, &dst_pfn, NULL);
> + if (ret < 0)
> + ret = H_PARAMETER;
> +out:
> + up_read(&mm->mmap_sem);
> + return ret;
> +}
> +
> +/*
> + * TODO: Number of secure pages and the page size order would probably come
> + * via DT or via some uvcall. Return 8G for now.
> + */
> +static unsigned long kvmppc_get_secmem_size(void)
> +{
> + return (1UL << 33);
> +}
> +
> +static int kvmppc_hmm_pages_init(void)
> +{
> + unsigned long nr_pfns = kvmppc_hmm->devmem->pfn_last -
> + kvmppc_hmm->devmem->pfn_first;
> +
> + kvmppc_hmm->pfn_bitmap = kcalloc(BITS_TO_LONGS(nr_pfns),
> + sizeof(unsigned long), GFP_KERNEL);
> + if (!kvmppc_hmm->pfn_bitmap)
> + return -ENOMEM;
> +
> + spin_lock_init(&kvmppc_hmm_lock);
> +
> + return 0;
> +}
> +
> +int kvmppc_hmm_init(void)
> +{
> + int ret = 0;
> + unsigned long size = kvmppc_get_secmem_size();
Can you split secmem to secure_mem?
> +
> + kvmppc_hmm = kzalloc(sizeof(*kvmppc_hmm), GFP_KERNEL);
> + if (!kvmppc_hmm) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + kvmppc_hmm->device = hmm_device_new(NULL);
> + if (IS_ERR(kvmppc_hmm->device)) {
> + ret = PTR_ERR(kvmppc_hmm->device);
> + goto out_free;
> + }
> +
> + kvmppc_hmm->devmem = hmm_devmem_add(&kvmppc_hmm_devmem_ops,
> + &kvmppc_hmm->device->device, size);
IIUC, there is just one HMM device for all the secure memory in the
system?
> + if (IS_ERR(kvmppc_hmm->devmem)) {
> + ret = PTR_ERR(kvmppc_hmm->devmem);
> + goto out_device;
> + }
> + ret = kvmppc_hmm_pages_init();
> + if (ret < 0)
> + goto out_devmem;
> +
> + return ret;
> +
> +out_devmem:
> + hmm_devmem_remove(kvmppc_hmm->devmem);
> +out_device:
> + hmm_device_put(kvmppc_hmm->device);
> +out_free:
> + kfree(kvmppc_hmm);
> + kvmppc_hmm = NULL;
> +out:
> + return ret;
> +}
> +
> +void kvmppc_hmm_free(void)
> +{
> + kfree(kvmppc_hmm->pfn_bitmap);
> + hmm_devmem_remove(kvmppc_hmm->devmem);
> + hmm_device_put(kvmppc_hmm->device);
> + kfree(kvmppc_hmm);
> + kvmppc_hmm = NULL;
> +}
Balbir Singh.
^ permalink raw reply
* Re: PIE binaries are no longer mapped below 4 GiB on ppc64le
From: Alan Modra @ 2018-11-01 6:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Florian Weimer, linux-mm, linuxppc-dev, keescook
In-Reply-To: <87lg6dfo3t.fsf@concordia.ellerman.id.au>
On Thu, Nov 01, 2018 at 02:55:34PM +1100, Michael Ellerman wrote:
> Hi Florian,
>
> Florian Weimer <fweimer@redhat.com> writes:
> > We tried to use Go to build PIE binaries, and while the Go toolchain is
> > definitely not ready (it produces text relocations and problematic
> > relocations in general), it exposed what could be an accidental
> > userspace ABI change.
> >
> > With our 4.10-derived kernel, PIE binaries are mapped below 4 GiB, so
> > relocations like R_PPC64_ADDR16_HA work:
> >
> > 21f00000-220d0000 r-xp 00000000 fd:00 36593493 /root/extld
> > 220d0000-220e0000 r--p 001c0000 fd:00 36593493 /root/extld
> > 220e0000-22100000 rw-p 001d0000 fd:00 36593493 /root/extld
> ...
> >
> > With a 4.18-derived kernel (with the hashed mm), we get this instead:
> >
> > 120e60000-121030000 rw-p 00000000 fd:00 102447141 /root/extld
> > 121030000-121060000 rw-p 001c0000 fd:00 102447141 /root/extld
> > 121060000-121080000 rw-p 00000000 00:00 0
>
> I assume that's caused by:
>
> 47ebb09d5485 ("powerpc: move ELF_ET_DYN_BASE to 4GB / 4MB")
>
> Which did roughly:
>
> -#define ELF_ET_DYN_BASE 0x20000000
> +#define ELF_ET_DYN_BASE (is_32bit_task() ? 0x000400000UL : \
> + 0x100000000UL)
>
> And went into 4.13.
>
> > ...
> > I'm not entirely sure what to make of this, but I'm worried that this
> > could be a regression that matters to userspace.
>
> It was a deliberate change, and it seemed to not break anything so we
> merged it. But obviously we didn't test widely enough.
>
> So I guess it clearly can matter to userspace, and it used to work, so
> therefore it is a regression.
>
> But at the same time we haven't had any other reports of breakage, so is
> this somehow specific to something Go is doing? Or did we just get lucky
> up until now? Or is no one actually testing on Power? ;)
Mapping PIEs above 4G should be fine. It works for gcc C and C++
after all. The problem is that ppc64le Go is generating code not
suitable for a PIE. Dynamic text relocations are evidence of non-PIC
object files.
Quoting Lynn Boger <boger@us.ibm.com>:
"When building a pie binary with golang, they should be using
-buildmode=pie and not just pass -pie to the linker".
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply
* [RFC PATCH for 4.21 09/16] powerpc: Wire up cpu_opv system call
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: Joel Fernandes, Dave Watson, Will Deacon, Andi Kleen,
Paul Mackerras, H . Peter Anvin, Chris Lameter, Russell King,
Ingo Molnar, Michael Kerrisk, Catalin Marinas, Paul Turner,
Josh Triplett, Steven Rostedt, Ben Maurer, Mathieu Desnoyers,
Thomas Gleixner, linux-api, linuxppc-dev, linux-kernel,
Andy Lutomirski, Andrew Morton, Linus Torvalds
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/uapi/asm/unistd.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 01b5171ea189..8f58710f5e8b 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -394,3 +394,4 @@ SYSCALL(pkey_free)
SYSCALL(pkey_mprotect)
SYSCALL(rseq)
COMPAT_SYS(io_pgetevents)
+SYSCALL(cpu_opv)
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 985534d0b448..112e2c54750a 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -400,5 +400,6 @@
#define __NR_pkey_mprotect 386
#define __NR_rseq 387
#define __NR_io_pgetevents 388
+#define __NR_cpu_opv 389
#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
From: Russell King - ARM Linux @ 2018-11-01 10:24 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-arch, linux-scsi, Linux Kbuild mailing list, linux-pci,
Linux Kernel Mailing List, Dominik Brodowski, Alex Bounine,
linuxppc-dev, Christoph Hellwig, linux-arm-kernel
In-Reply-To: <CAK7LNATZ8LagrTrTae_QCQbSuTW7_y6sDrdTHfTyONPrsYDs4A@mail.gmail.com>
On Fri, Oct 19, 2018 at 09:58:46PM +0900, Masahiro Yamada wrote:
> On Fri, Oct 19, 2018 at 9:23 PM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>
> > > index a68b34183107..b185794549be 100644
> > > --- a/arch/arm/mach-pxa/Kconfig
> > > +++ b/arch/arm/mach-pxa/Kconfig
> > > @@ -125,7 +125,7 @@ config MACH_ARMCORE
> > > bool "CompuLab CM-X255/CM-X270 modules"
> > > select ARCH_HAS_DMA_SET_COHERENT_MASK if PCI
> > > select IWMMXT
> > > - select MIGHT_HAVE_PCI
> > > + select HAVE_PCI
> > > select NEED_MACH_IO_H if PCI
> > > select PXA25x
> > > select PXA27x
> >
> > This is wrong. "MIGHT_HAVE_PCI" is _not_ the same as "HAVE_PCI" - we
> > have a bunch of platforms that mandatorily have PCI and these select
> > PCI directly. "MIGHT_HAVE_PCI" controls the _visibility_ of the PCI
> > menu option, but does not prevent it being selected. Your patch will
> > cause Kconfig to complain for those which mandatorily have PCI but
> > do not set HAVE_PCI.
>
>
> Good catch!
> But, adding a bunch of 'select HAVE_PCI' along with 'select PCI' is ugly.
>
> Do you have any suggestion?
>
> How about letting CONFIG_ARM to select HAVE_PCI ?
Well, the situation we have on ARM is rather optimal - when the
rest of the configuration supports PCI, PCI is made visible. When
there's no hardware support for PCI, PCI is hidden. When PCI is
mandatory, PCI is selected and mostly always hidden.
It seems that with this consolidation, we lose that - we end up
with PCI being visible for every ARM config, not only those where
PCI is "impossible" but also for those where PCI is forcefully
selected.
That said, offering PCI for platforms that do not have any possibility
of PCI hardware shouldn't cause any compile issues, and would increase
build coverage - but I'd say it wouldn't _usefully_ increase build
coverage.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [RFC PATCH v1 2/4] kvmppc: Add support for shared pages in HMM driver
From: Balbir Singh @ 2018-11-01 10:45 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, kvm-ppc, benh, linux-mm, jglisse, aneesh.kumar, paulus,
linuxppc-dev
In-Reply-To: <20181022051837.1165-3-bharata@linux.ibm.com>
On Mon, Oct 22, 2018 at 10:48:35AM +0530, Bharata B Rao wrote:
> A secure guest will share some of its pages with hypervisor (Eg. virtio
> bounce buffers etc). Support shared pages in HMM driver.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_hmm.c | 69 ++++++++++++++++++++++++++++++--
> 1 file changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_hmm.c b/arch/powerpc/kvm/book3s_hv_hmm.c
> index a2ee3163a312..09b8e19b7605 100644
> --- a/arch/powerpc/kvm/book3s_hv_hmm.c
> +++ b/arch/powerpc/kvm/book3s_hv_hmm.c
> @@ -50,6 +50,7 @@ struct kvmppc_hmm_page_pvt {
> struct hlist_head *hmm_hash;
> unsigned int lpid;
> unsigned long gpa;
> + bool skip_page_out;
> };
>
> struct kvmppc_hmm_migrate_args {
> @@ -278,6 +279,65 @@ static unsigned long kvmppc_gpa_to_hva(struct kvm *kvm, unsigned long gpa,
> return hva;
> }
>
> +/*
> + * Shares the page with HV, thus making it a normal page.
> + *
> + * - If the page is already secure, then provision a new page and share
> + * - If the page is a normal page, share the existing page
> + *
> + * In the former case, uses the HMM fault handler to release the HMM page.
> + */
> +static unsigned long
> +kvmppc_share_page(struct kvm *kvm, unsigned long gpa,
> + unsigned long addr, unsigned long page_shift)
> +{
> +
So this is a special flag passed via the hypercall to say
this page can be skipped from page_out from secure memory?
Who has the master copy of the page at this point?
In which case the question is
Why did we get a fault on the page which resulted in the
fault migration ops being called?
What category of pages are considered shared?
Balbir
^ permalink raw reply
* Re: [RFC PATCH v1 3/4] kvmppc: H_SVM_INIT_START and H_SVM_INIT_DONE hcalls
From: Balbir Singh @ 2018-11-01 10:49 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, kvm-ppc, benh, linux-mm, jglisse, aneesh.kumar, paulus,
linuxppc-dev
In-Reply-To: <20181022051837.1165-4-bharata@linux.ibm.com>
On Mon, Oct 22, 2018 at 10:48:36AM +0530, Bharata B Rao wrote:
> H_SVM_INIT_START: Initiate securing a VM
> H_SVM_INIT_DONE: Conclude securing a VM
>
> During early guest init, these hcalls will be issued by UV.
> As part of these hcalls, [un]register memslots with UV.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/include/asm/hvcall.h | 4 ++-
> arch/powerpc/include/asm/kvm_host.h | 1 +
> arch/powerpc/include/asm/ucall-api.h | 6 ++++
> arch/powerpc/kvm/book3s_hv.c | 54 ++++++++++++++++++++++++++++
> 4 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index 89e6b70c1857..6091276fef07 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -300,7 +300,9 @@
> #define H_INT_RESET 0x3D0
> #define H_SVM_PAGE_IN 0x3D4
> #define H_SVM_PAGE_OUT 0x3D8
> -#define MAX_HCALL_OPCODE H_SVM_PAGE_OUT
> +#define H_SVM_INIT_START 0x3DC
> +#define H_SVM_INIT_DONE 0x3E0
> +#define MAX_HCALL_OPCODE H_SVM_INIT_DONE
>
> /* H_VIOCTL functions */
> #define H_GET_VIOA_DUMP_SIZE 0x01
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 194e6e0ff239..267f8c568bc3 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -292,6 +292,7 @@ struct kvm_arch {
> struct dentry *debugfs_dir;
> struct dentry *htab_dentry;
> struct kvm_resize_hpt *resize_hpt; /* protected by kvm->lock */
> + bool svm_init_start; /* Indicates H_SVM_INIT_START has been called */
> #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
> struct mutex hpt_mutex;
> diff --git a/arch/powerpc/include/asm/ucall-api.h b/arch/powerpc/include/asm/ucall-api.h
> index 2c12f514f8ab..9ddfcf541211 100644
> --- a/arch/powerpc/include/asm/ucall-api.h
> +++ b/arch/powerpc/include/asm/ucall-api.h
> @@ -17,4 +17,10 @@ static inline int uv_page_out(u64 lpid, u64 dw0, u64 dw1, u64 dw2, u64 dw3)
> return U_SUCCESS;
> }
>
> +static inline int uv_register_mem_slot(u64 lpid, u64 dw0, u64 dw1, u64 dw2,
> + u64 dw3)
> +{
> + return 0;
> +}
> +
> #endif /* _ASM_POWERPC_UCALL_API_H */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 05084eb8aadd..47f366f634fd 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -819,6 +819,50 @@ static int kvmppc_get_yield_count(struct kvm_vcpu *vcpu)
> return yield_count;
> }
>
> +#ifdef CONFIG_PPC_SVM
> +#include <asm/ucall-api.h>
> +/*
> + * TODO: Check if memslots related calls here need to be called
> + * under any lock.
> + */
> +static unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
> +{
> + struct kvm_memslots *slots;
> + struct kvm_memory_slot *memslot;
> + int ret;
> +
> + slots = kvm_memslots(kvm);
> + kvm_for_each_memslot(memslot, slots) {
> + ret = uv_register_mem_slot(kvm->arch.lpid,
> + memslot->base_gfn << PAGE_SHIFT,
> + memslot->npages * PAGE_SIZE,
> + 0, memslot->id);
For every memslot their is a corresponding registration in the ultravisor?
Is there a corresponding teardown?
> + if (ret < 0)
> + return H_PARAMETER;
> + }
> + kvm->arch.svm_init_start = true;
> + return H_SUCCESS;
> +}
> +
> +static unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)
> +{
> + if (kvm->arch.svm_init_start)
> + return H_SUCCESS;
> + else
> + return H_UNSUPPORTED;
> +}
> +#else
> +static unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
> +{
> + return H_UNSUPPORTED;
> +}
> +
> +static unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)
> +{
> + return H_UNSUPPORTED;
> +}
> +#endif
> +
> int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> {
> unsigned long req = kvmppc_get_gpr(vcpu, 3);
> @@ -950,6 +994,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> kvmppc_get_gpr(vcpu, 6),
> kvmppc_get_gpr(vcpu, 7));
> break;
> + case H_SVM_INIT_START:
> + ret = kvmppc_h_svm_init_start(vcpu->kvm);
> + break;
> + case H_SVM_INIT_DONE:
> + ret = kvmppc_h_svm_init_done(vcpu->kvm);
> + break;
> default:
> return RESUME_HOST;
> }
> @@ -978,6 +1028,8 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
> #endif
> case H_SVM_PAGE_IN:
> case H_SVM_PAGE_OUT:
> + case H_SVM_INIT_START:
> + case H_SVM_INIT_DONE:
> return 1;
> }
>
> @@ -4413,6 +4465,8 @@ static unsigned int default_hcall_list[] = {
> #endif
> H_SVM_PAGE_IN,
> H_SVM_PAGE_OUT,
> + H_SVM_INIT_START,
> + H_SVM_INIT_DONE,
> 0
> };
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 01/21] of: Add cpu node iterator for_each_of_cpu_node()
From: Michael Ellerman @ 2018-11-01 10:52 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linuxppc-dev, Linux Kernel Mailing List, Frank Rowand,
Tyrel Datwyler
In-Reply-To: <CABGGisw_bd+bJedZ_T2e-dxmRnXf2N_gGBZj8u+QVGQxj_gc+Q@mail.gmail.com>
Rob Herring <robh@kernel.org> writes:
> On Tue, Oct 30, 2018 at 9:20 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>> > Hi Rob,
>> >
>> > Sorry I missed this when you posted it.
>> >
>> > Rob Herring <robh@kernel.org> writes:
>> >> Iterating thru cpu nodes is a common pattern. Create a common iterator
>> >> which can find child nodes either by node name or device_type == cpu.
>> >> Using the former will allow for eventually dropping device_type
>> >> properties which are deprecated for FDT.
>> >
>> > Device trees we see on powerpc generally don't (never?) use "cpu" as the
>> > node name for CPU nodes. And many of those device trees come from
>> > firmware, so we can't update them.
>> >
>> > So dropping support for device_type is a non-starter from our POV.
>>
>> ps. presumably that's what you meant by deprecated *for FDT*.
>
> Right.
>
>> But anyway just wanted to make sure we are on the same page.
>
> Yes, I was aware at least older powerpc DTs don't use 'cpu' for node names.
Actually newer ones too, see below :)
And there's code out there that expects this, so we can't realistically
change it any time soon :/
https://github.com/ibm-power-utilities/powerpc-utils/blob/master/src/drmgr/common_cpu.c#L186
https://github.com/ibm-power-utilities/powerpc-utils/blob/master/src/ppc64_cpu.c#L344
cheers
$ ls -d1 /proc/device-tree/cpus/PowerPC\,POWER9@*
/proc/device-tree/cpus/PowerPC,POWER9@14
/proc/device-tree/cpus/PowerPC,POWER9@1c
/proc/device-tree/cpus/PowerPC,POWER9@34
/proc/device-tree/cpus/PowerPC,POWER9@3c
/proc/device-tree/cpus/PowerPC,POWER9@4
/proc/device-tree/cpus/PowerPC,POWER9@48
/proc/device-tree/cpus/PowerPC,POWER9@54
/proc/device-tree/cpus/PowerPC,POWER9@804
/proc/device-tree/cpus/PowerPC,POWER9@80c
/proc/device-tree/cpus/PowerPC,POWER9@814
/proc/device-tree/cpus/PowerPC,POWER9@81c
/proc/device-tree/cpus/PowerPC,POWER9@834
/proc/device-tree/cpus/PowerPC,POWER9@83c
/proc/device-tree/cpus/PowerPC,POWER9@844
/proc/device-tree/cpus/PowerPC,POWER9@84c
/proc/device-tree/cpus/PowerPC,POWER9@c
^ permalink raw reply
* Re: PIE binaries are no longer mapped below 4 GiB on ppc64le
From: Florian Weimer @ 2018-11-01 11:20 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linux-mm, linuxppc-dev, keescook, amodra
In-Reply-To: <87lg6dfo3t.fsf@concordia.ellerman.id.au>
* Michael Ellerman:
> Hi Florian,
>
> Florian Weimer <fweimer@redhat.com> writes:
>> We tried to use Go to build PIE binaries, and while the Go toolchain is
>> definitely not ready (it produces text relocations and problematic
>> relocations in general), it exposed what could be an accidental
>> userspace ABI change.
>>
>> With our 4.10-derived kernel, PIE binaries are mapped below 4 GiB, so
>> relocations like R_PPC64_ADDR16_HA work:
>>
>> 21f00000-220d0000 r-xp 00000000 fd:00 36593493 /root/extld
>> 220d0000-220e0000 r--p 001c0000 fd:00 36593493 /root/extld
>> 220e0000-22100000 rw-p 001d0000 fd:00 36593493 /root/extld
> ...
>>
>> With a 4.18-derived kernel (with the hashed mm), we get this instead:
>>
>> 120e60000-121030000 rw-p 00000000 fd:00 102447141 /root/extld
>> 121030000-121060000 rw-p 001c0000 fd:00 102447141 /root/extld
>> 121060000-121080000 rw-p 00000000 00:00 0
>
> I assume that's caused by:
>
> 47ebb09d5485 ("powerpc: move ELF_ET_DYN_BASE to 4GB / 4MB")
>
> Which did roughly:
>
> -#define ELF_ET_DYN_BASE 0x20000000
> +#define ELF_ET_DYN_BASE (is_32bit_task() ? 0x000400000UL : \
> + 0x100000000UL)
>
> And went into 4.13.
>
>> ...
>> I'm not entirely sure what to make of this, but I'm worried that this
>> could be a regression that matters to userspace.
>
> It was a deliberate change, and it seemed to not break anything so we
> merged it. But obviously we didn't test widely enough.
* Michael Ellerman:
>> I'm not entirely sure what to make of this, but I'm worried that this
>> could be a regression that matters to userspace.
>
> It was a deliberate change, and it seemed to not break anything so we
> merged it. But obviously we didn't test widely enough.
Thanks for moving back the discussion to kernel matters. 8-)
> So I guess it clearly can matter to userspace, and it used to work, so
> therefore it is a regression.
Is there a knob to get back the old base address?
> But at the same time we haven't had any other reports of breakage, so is
> this somehow specific to something Go is doing?
Go uses 32-bit run-time relocations which (I think) were primarily
designed as link-time relocations for programs mapped under 4 GiB. It's
amazing that the binaries work at all under old kernels. On other
targets, the link editor refuses to produce an executable, or may even
produce a binary which crashes at run time.
> Or did we just get lucky up until now? Or is no one actually testing
> on Power? ;)
I'm not too worried about it. It looks like a well-understood change to
me. The glibc dynamic linker prints a reasonably informative error
message (in the sense that it doesn't crash without printing anything).
I think we can wait and see if someone comes up with a more compelling
case for backwards compatibility than the broken Go binaries (which we
will rebuild anyway because we don't want text relocations). I assume
that it will be possible to add a personality flag if it ever proves
necessary—or maybe map the executable below 4 GiB in case of ASLR is
disabled, so that people have at least a workaround to get old binaries
going again.
But right now, that doesn't seem necessary.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH] ppc4xx: ocm: fix errnous "failed to create file" errors
From: Michael Ellerman @ 2018-11-01 11:45 UTC (permalink / raw)
To: Christian Lamparter, linuxppc-dev; +Cc: Paul Mackerras, Markus Elfring
In-Reply-To: <20181028161702.9300-1-chunkeey@gmail.com>
Hi Christian,
Christian Lamparter <chunkeey@gmail.com> writes:
> Previously, the kernel would complain:
>
> | debugfs ppc4xx ocm: failed to create file
>
> But the "info" file was still created and working. This
> is because debugfs_create_file() returns a pointer to a
> struct *dentry on success or -ENODEV when debugfs isn't
> available. This patch fixes both the debugfs_create_dir()
> and debugfs_create_file() check, so this will work as
> expected.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
>
> diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
> index 69d9f60d9fe5..e616c8636303 100644
> --- a/arch/powerpc/platforms/4xx/ocm.c
> +++ b/arch/powerpc/platforms/4xx/ocm.c
> @@ -293,13 +293,14 @@ static int ocm_debugfs_init(void)
> {
> struct dentry *junk;
>
> - junk = debugfs_create_dir("ppc4xx_ocm", 0);
> - if (!junk) {
> + junk = debugfs_create_dir("ppc4xx_ocm", NULL);
> + if (IS_ERR_OR_NULL(junk)) {
> printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create dir\n");
> return -1;
> }
>
> - if (debugfs_create_file("info", 0644, junk, NULL, &ocm_debugfs_fops)) {
> + if (IS_ERR_OR_NULL(debugfs_create_file("info", 0644, junk, NULL,
> + &ocm_debugfs_fops))) {
> printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create file\n");
> return -1;
> }
Typically we would just make the whole function not compile when DEBUG_FS
is disabled, eg:
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index f5bbd4563342..c5293a28545c 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -286,6 +286,7 @@ static const struct file_operations ocm_debugfs_fops = {
.release = single_release,
};
+#ifdef CONFIG_DEBUG_FS
static int ocm_debugfs_init(void)
{
struct dentry *junk;
@@ -303,6 +304,9 @@ static int ocm_debugfs_init(void)
return 0;
}
+#else
+static int ocm_debugfs_init(void) { return 0; }
+#endif
void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align,
int flags, const char *owner)
I don't really mind, but I think the #ifdef approach is simpler to
reason about.
cheers
^ permalink raw reply related
* Re: selftests: powerpc: Fix warning for security subdir
From: Michael Ellerman @ 2018-11-01 12:46 UTC (permalink / raw)
To: Joel Stanley, linuxppc-dev
In-Reply-To: <20181022120926.12797-1-joel@jms.id.au>
On Mon, 2018-10-22 at 12:09:26 UTC, Joel Stanley wrote:
> typing 'make' inside tools/testing/selftests/powerpc gave a build
> warning:
>
> BUILD_TARGET=tools/testing/selftests/powerpc/security; mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C security all
> make[1]: Entering directory 'tools/testing/selftests/powerpc/security'
> ../../lib.mk:20: ../../../../scripts/subarch.include: No such file or directory
> make[1]: *** No rule to make target '../../../../scripts/subarch.include'.
> make[1]: Failed to remake makefile '../../../../scripts/subarch.include'.
>
> The build is one level deeper than lib.mk thinks it is. Set top_srcdir
> to set things straight.
>
> Note that the test program is still built.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a0aebae07f211c05d64be886e56d35
cheers
^ permalink raw reply
* Re: selftests/powerpc: Relax L1d miss targets for rfi_flush test
From: Michael Ellerman @ 2018-11-01 12:46 UTC (permalink / raw)
To: Naveen N. Rao, Joel Stanley; +Cc: linuxppc-dev
In-Reply-To: <20181023080456.2558-1-naveen.n.rao@linux.vnet.ibm.com>
On Tue, 2018-10-23 at 08:04:56 UTC, "Naveen N. Rao" wrote:
> When running the rfi_flush test, if the system is loaded, we see two
> issues:
> 1. The L1d misses when rfi_flush is disabled increase significantly due
> to other workloads interfering with the cache.
> 2. The L1d misses when rfi_flush is enabled sometimes goes slightly
> below the expected number of misses.
>
> To address these, let's relax the expected number of L1d misses:
> 1. When rfi_flush is disabled, we allow upto half the expected number of
> the misses for when rfi_flush is enabled.
> 2. When rfi_flush is enabled, we allow ~1% lower number of cache misses.
>
> Reported-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Tested-by: Joel Stanley <joel@jms.id.au>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a95ecac5cb2fc8a8ee606991384d33
cheers
^ permalink raw reply
* Re: [1/5] selftests/powerpc/ptrace: Fix out-of-tree build
From: Michael Ellerman @ 2018-11-01 12:46 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: joel
In-Reply-To: <20181029112353.15805-1-mpe@ellerman.id.au>
On Mon, 2018-10-29 at 11:23:49 UTC, Michael Ellerman wrote:
> From: Joel Stanley <joel@jms.id.au>
>
> We should use TEST_GEN_PROGS, not TEST_PROGS. That tells the selftests
> makefile (lib.mk) that those tests are generated (built), and so it
> adds the $(OUTPUT) prefix for us, making the out-of-tree build work
> correctly.
>
> It also means we don't need our own clean rule, lib.mk does it.
>
> We also have to update the ptrace-pkey and core-pkey rules to use
> $(OUTPUT).
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Series applied to powerpc next.
https://git.kernel.org/powerpc/c/c39b79082a38a4f8c801790edecbbb
cheers
^ permalink raw reply
* Re: powerpc/xmon: Relax frame size for clang
From: Michael Ellerman @ 2018-11-01 12:46 UTC (permalink / raw)
To: Joel Stanley, Naveen N . Rao, linuxppc-dev; +Cc: Nick Desaulniers
In-Reply-To: <20181031010934.3627-1-joel@jms.id.au>
On Wed, 2018-10-31 at 01:09:34 UTC, Joel Stanley wrote:
> When building with clang (8 trunk, 7.0 release) the frame size limit is
> hit:
>
> arch/powerpc/xmon/xmon.c:452:12: warning: stack frame size of 2576
> bytes in function 'xmon_core' [-Wframe-larger-than=]
>
> Some investigation by Naveen indicates this is due to clang saving the
> addresses to printf format strings on the stack.
>
> While this issue is investigated, bump up the frame size limit for xmon
> when building with clang.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/252
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/9c87156cce5a63735d1218f0096a65
cheers
^ 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