* [PATCH v3 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 4:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222041739.27899-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Some required information is not exposed to userspace currently (eg. the
PASID), pass this information back, along with other information which
is currently communicated via sysfs, which saves some parsing effort in
userspace.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 17 +++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d9aa407db06a..90df1be5ef3f 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
return rc;
}
+static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
+ struct ocxl_ioctl_metadata __user *uarg)
+{
+ struct ocxl_ioctl_metadata arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+ arg.version = 0;
+
+ arg.afu_version_major = ctx->afu->config.version_major;
+ arg.afu_version_minor = ctx->afu->config.version_minor;
+ arg.pasid = ctx->pasid;
+ arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
+ arg.global_mmio_size = ctx->afu->config.global_mmio_size;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
+ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
irq_fd.eventfd);
break;
+ case OCXL_IOCTL_GET_METADATA:
+ rc = afu_ioctl_get_metadata(ctx,
+ (struct ocxl_ioctl_metadata __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 4b0b0b756f3e..0af83d80fb3e 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -32,6 +32,22 @@ struct ocxl_ioctl_attach {
__u64 reserved3;
};
+struct ocxl_ioctl_metadata {
+ __u16 version; // struct version, always backwards compatible
+
+ // Version 0 fields
+ __u8 afu_version_major;
+ __u8 afu_version_minor;
+ __u32 pasid; // PASID assigned to the current context
+
+ __u64 pp_mmio_size; // Per PASID MMIO size
+ __u64 global_mmio_size;
+
+ // End version 0 fields
+
+ __u64 reserved[13]; // Total of 16*u64
+};
+
struct ocxl_ioctl_irq_fd {
__u64 irq_offset;
__s32 eventfd;
@@ -45,5 +61,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_ALLOC _IOR(OCXL_MAGIC, 0x11, __u64)
#define OCXL_IOCTL_IRQ_FREE _IOW(OCXL_MAGIC, 0x12, __u64)
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
+#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_metadata)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
* [PATCH v3 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Alastair D'Silva @ 2018-02-22 4:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222041739.27899-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Documentation/accelerators/ocxl.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index 4f7af841d935..ddcc58d01cfb 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -152,6 +152,11 @@ OCXL_IOCTL_IRQ_SET_FD:
Associate an event fd to an AFU interrupt so that the user process
can be notified when the AFU sends an interrupt.
+OCXL_IOCTL_GET_METADATA:
+
+ Obtains configuration information from the card, such at the size of
+ MMIO areas, the AFU version, and the PASID for the current context.
+
mmap
----
--
2.14.3
^ permalink raw reply related
* [PATCH v3 0/2] Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 4:17 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222030334.14060-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
This series adds a new IOCTL for OpenCAPI, which exposes adapter
metadata to userspace.
---
v1->v2:
- Document the new IOCTL
v2->v3:
- Rename metadata struct
- Document a couple of struct members that are not immediately obvious
Alastair D'Silva (2):
ocxl: Add get_metadata IOCTL to share OCXL information to userspace
ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
Documentation/accelerators/ocxl.rst | 5 +++++
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 17 +++++++++++++++++
3 files changed, 49 insertions(+)
--
2.14.3
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:51 UTC (permalink / raw)
To: Balbir Singh, Frederic Barrat
Cc: Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan
In-Reply-To: <CAKTCnzkwwmbU4vzrmzW=eNnr2QW+rj=OUcAdOVxz+qKGC=9Q9A@mail.gmail.com>
On Thu, 2018-02-22 at 14:46 +1100, Balbir Singh wrote:
<snip>
> lpc_size could be added. It's currently useless to the library, but
> > doesn't
> > hurt. The one which was giving me troubles on a previous version of
> > this
> > patch was the lpc numa node ID, since that was experimental code
> > and felt
> > out of place considering what's been upstreamed in skiboot and
> > linux so far.
> >
>
> Yeah, I think metadata will evolve for a while till it settle's down.
> Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program
> calling an older kernel will never work, since the size of that
> struct
> will always be larger than what the OS supports and our
> copy_to_user()
> will fail. The other option is for the user program to try all
> possible versions till one succeeds, that is bad as well. I think
> there are a few ways around it, if we care about this combination.
>
> Balbir Singh.
>
We have a number of reserved members at the end of the struct which can
be re-purposed for future information (with a corresponding bump of the
version number).
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:48 UTC (permalink / raw)
To: Balbir Singh
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH, Andrew Donnellan
In-Reply-To: <CAKTCnzmmUGPNk1ggpWYQjiDLcAR_c7cqdZyPU+6rKGC6AcNW7A@mail.gmail.com>
On Thu, 2018-02-22 at 14:41 +1100, Balbir Singh wrote:
> On Thu, Feb 22, 2018 at 10:32 AM, Alastair D'Silva <alastair@au1.ibm.
> com> wrote:
> >
> > On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
> > > On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.i
> > > bm.c
> > > om> wrote:
> > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > >
> > > > Some required information is not exposed to userspace currently
> > > > (eg. the
> > > > PASID), pass this information back, along with other
> > > > information
> > > > which
> > > > is currently communicated via sysfs, which saves some parsing
> > > > effort in
> > > > userspace.
> > > >
> > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > > ---
> > > > drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
> > > > include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
> > > > 2 files changed, 49 insertions(+)
> > > >
> > > > diff --git a/drivers/misc/ocxl/file.c
> > > > b/drivers/misc/ocxl/file.c
> > > > index d9aa407db06a..11514a8444e5 100644
> > > > --- a/drivers/misc/ocxl/file.c
> > > > +++ b/drivers/misc/ocxl/file.c
> > > > @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct
> > > > ocxl_context *ctx,
> > > > return rc;
> > > > }
> > > >
> > > > +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
> > > > + struct ocxl_ioctl_get_metadata __user *uarg)
> > >
> > > Why do we call this metadata? Isn't this an afu_descriptor?
> > >
> >
> > It's metadata for the descriptor.
>
> I meant metadata is too generic, could we have other types of
> metadata in OCXL?
>
I don't believe so, we would instead expand the scope of this IOCTL
using version & space available from the reserved fields.
> >
> > > > +{
> > > > + struct ocxl_ioctl_get_metadata arg;
> > > > +
> > > > + memset(&arg, 0, sizeof(arg));
> > > > +
> > > > + arg.version = 0;
> > >
> > > Does it make sense to have version 0? Even if does, you can
> > > afford
> > > to skip initialization due to the memset above. I prefer that
> > > versions
> > > start with 1
> > >
> >
> > Setting it to 0 is for the reader, not the compiler. I'm not clear
> > on
> > the benefit of starting the version at 1, could you clarify?
>
> How do I distinguish between version number never set and 0?
>
The version number is always set. If the IOCTL doesn't exist, the ioctl
call will error instead.
> >
> > > > +
> > > > + arg.afu_version_major = ctx->afu->config.version_major;
> > > > + arg.afu_version_minor = ctx->afu->config.version_minor;
> > > > + arg.pasid = ctx->pasid;
> > > > + arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
> > > > + arg.global_mmio_size = ctx->afu-
> > > > >config.global_mmio_size;
> > > > +
> > > > + if (copy_to_user(uarg, &arg, sizeof(arg)))
> > > > + return -EFAULT;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH"
> > > > : \
> > > > x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC"
> > > > : \
> > > > x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE"
> > > > : \
> > > > x == OCXL_IOCTL_IRQ_SET_FD ?
> > > > "IRQ_SET_FD"
> > > > : \
> > > > + x == OCXL_IOCTL_GET_METADATA ?
> > > > "GET_METADATA" : \
> > > > "UNKNOWN")
> > > >
> > > > static long afu_ioctl(struct file *file, unsigned int cmd,
> > > > @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file,
> > > > unsigned int cmd,
> > > > irq_fd.eventfd);
> > > > break;
> > > >
> > > > + case OCXL_IOCTL_GET_METADATA:
> > > > + rc = afu_ioctl_get_metadata(ctx,
> > > > + (struct ocxl_ioctl_get_metadata
> > > > __user *) args);
> > > > + break;
> > > > +
> > > > default:
> > > > rc = -EINVAL;
> > > > }
> > > > diff --git a/include/uapi/misc/ocxl.h
> > > > b/include/uapi/misc/ocxl.h
> > > > index 4b0b0b756f3e..16e1f48ce280 100644
> > > > --- a/include/uapi/misc/ocxl.h
> > > > +++ b/include/uapi/misc/ocxl.h
> > > > @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
> > > > __u64 reserved3;
> > > > };
> > > >
> > > > +/*
> > > > + * Version contains the version of the struct.
> > > > + * Versions will always be backwards compatible, that is, new
> > > > versions will not
> > > > + * alter existing fields
> > > > + */
> > > > +struct ocxl_ioctl_get_metadata {
> > >
> > > This sounds more like a function name, do we need it to be
> > > _get_metdata?
> > >
> >
> > It pretty much is a function, it returns to userspace metadata
> > about
> > the descriptor being operated on.
> >
>
> It has a verb indicating action
I misunderstood, I had named the struct to match the IOCTL, but that
isn't necessary. I'll update it in the next patch.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Andrew Donnellan @ 2018-02-22 3:47 UTC (permalink / raw)
To: Balbir Singh, Alastair D'Silva
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH
In-Reply-To: <CAKTCnzmmUGPNk1ggpWYQjiDLcAR_c7cqdZyPU+6rKGC6AcNW7A@mail.gmail.com>
On 22/02/18 14:41, Balbir Singh wrote:
>> Setting it to 0 is for the reader, not the compiler. I'm not clear on
>> the benefit of starting the version at 1, could you clarify?
>
> How do I distinguish between version number never set and 0?
The ioctl won't exist without a version number set.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Balbir Singh @ 2018-02-22 3:46 UTC (permalink / raw)
To: Frederic Barrat
Cc: Alastair D'Silva, Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan,
Alastair D'Silva
In-Reply-To: <c6017f9f-67f0-10eb-1eed-527c9c5298ba@linux.vnet.ibm.com>
On Wed, Feb 21, 2018 at 10:25 PM, Frederic Barrat
<fbarrat@linux.vnet.ibm.com> wrote:
>
>
> Le 21/02/2018 =C3=A0 07:43, Balbir Singh a =C3=A9crit :
>>
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.com>
>> wrote:
>>>
>>> From: Alastair D'Silva <alastair@d-silva.org>
>>>
>>> Some required information is not exposed to userspace currently (eg. th=
e
>>> PASID), pass this information back, along with other information which
>>> is currently communicated via sysfs, which saves some parsing effort in
>>> userspace.
>>>
>>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>>> ---
>>> drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
>>> include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
>>> 2 files changed, 49 insertions(+)
>>>
>>> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
>>> index d9aa407db06a..11514a8444e5 100644
>>> --- a/drivers/misc/ocxl/file.c
>>> +++ b/drivers/misc/ocxl/file.c
>>> @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context
>>> *ctx,
>>> return rc;
>>> }
>>>
>>> +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
>>> + struct ocxl_ioctl_get_metadata __user *uarg)
>>
>>
>> Why do we call this metadata? Isn't this an afu_descriptor?
>>
>>> +{
>>> + struct ocxl_ioctl_get_metadata arg;
>>> +
>>> + memset(&arg, 0, sizeof(arg));
>>> +
>>> + arg.version =3D 0;
>>
>>
>> Does it make sense to have version 0? Even if does, you can afford
>> to skip initialization due to the memset above. I prefer that versions
>> start with 1
>>
>>> +
>>> + arg.afu_version_major =3D ctx->afu->config.version_major;
>>> + arg.afu_version_minor =3D ctx->afu->config.version_minor;
>>> + arg.pasid =3D ctx->pasid;
>>> + arg.pp_mmio_size =3D ctx->afu->config.pp_mmio_stride;
>>> + arg.global_mmio_size =3D ctx->afu->config.global_mmio_size;
>>> +
>>> + if (copy_to_user(uarg, &arg, sizeof(arg)))
>>> + return -EFAULT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> #define CMD_STR(x) (x =3D=3D OCXL_IOCTL_ATTACH ? "ATTACH" :
>>> \
>>> x =3D=3D OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" :
>>> \
>>> x =3D=3D OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" :
>>> \
>>> x =3D=3D OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" =
:
>>> \
>>> + x =3D=3D OCXL_IOCTL_GET_METADATA ? "GET_METADAT=
A" : \
>>> "UNKNOWN")
>>>
>>> static long afu_ioctl(struct file *file, unsigned int cmd,
>>> @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned
>>> int cmd,
>>> irq_fd.eventfd);
>>> break;
>>>
>>> + case OCXL_IOCTL_GET_METADATA:
>>> + rc =3D afu_ioctl_get_metadata(ctx,
>>> + (struct ocxl_ioctl_get_metadata __user =
*)
>>> args);
>>> + break;
>>> +
>>> default:
>>> rc =3D -EINVAL;
>>> }
>>> diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>>> index 4b0b0b756f3e..16e1f48ce280 100644
>>> --- a/include/uapi/misc/ocxl.h
>>> +++ b/include/uapi/misc/ocxl.h
>>> @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
>>> __u64 reserved3;
>>> };
>>>
>>> +/*
>>> + * Version contains the version of the struct.
>>> + * Versions will always be backwards compatible, that is, new versions
>>> will not
>>> + * alter existing fields
>>> + */
>>> +struct ocxl_ioctl_get_metadata {
>>
>>
>> This sounds more like a function name, do we need it to be _get_metdata?
>>
>>> + __u16 version;
>>> +
>>> + // Version 0 fields
>>> + __u8 afu_version_major;
>>> + __u8 afu_version_minor;
>>> + __u32 pasid;
>>> +
>>> + __u64 pp_mmio_size;
>>> + __u64 global_mmio_size;
>>> +
>>
>>
>> Should we document the fields? pp_ stands for per process, but is not
>> very clear at first look. Why do we care to return only the size, what
>> about lpc size?
>
>
> My bad, I forgot to mention it before. There's a somewhat high-level
> description which needs updating in:
> Documentation/accelerators/ocxl.rst
Thanks, that's helpful
>
> It doesn't go down to the level of the structure members, but at least al=
l
> ioctl commands should have a brief description.
>
> lpc_size could be added. It's currently useless to the library, but doesn=
't
> hurt. The one which was giving me troubles on a previous version of this
> patch was the lpc numa node ID, since that was experimental code and felt
> out of place considering what's been upstreamed in skiboot and linux so f=
ar.
>
Yeah, I think metadata will evolve for a while till it settle's down.
Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program
calling an older kernel will never work, since the size of that struct
will always be larger than what the OS supports and our copy_to_user()
will fail. The other option is for the user program to try all
possible versions till one succeeds, that is bad as well. I think
there are a few ways around it, if we care about this combination.
Balbir Singh.
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Balbir Singh @ 2018-02-22 3:41 UTC (permalink / raw)
To: Alastair D'Silva
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH, Andrew Donnellan
In-Reply-To: <1519255934.2867.3.camel@au1.ibm.com>
On Thu, Feb 22, 2018 at 10:32 AM, Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.c
>> om> wrote:
>> > From: Alastair D'Silva <alastair@d-silva.org>
>> >
>> > Some required information is not exposed to userspace currently
>> > (eg. the
>> > PASID), pass this information back, along with other information
>> > which
>> > is currently communicated via sysfs, which saves some parsing
>> > effort in
>> > userspace.
>> >
>> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>> > ---
>> > drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
>> > include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
>> > 2 files changed, 49 insertions(+)
>> >
>> > diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
>> > index d9aa407db06a..11514a8444e5 100644
>> > --- a/drivers/misc/ocxl/file.c
>> > +++ b/drivers/misc/ocxl/file.c
>> > @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct
>> > ocxl_context *ctx,
>> > return rc;
>> > }
>> >
>> > +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
>> > + struct ocxl_ioctl_get_metadata __user *uarg)
>>
>> Why do we call this metadata? Isn't this an afu_descriptor?
>>
>
> It's metadata for the descriptor.
I meant metadata is too generic, could we have other types of metadata in OCXL?
>
>> > +{
>> > + struct ocxl_ioctl_get_metadata arg;
>> > +
>> > + memset(&arg, 0, sizeof(arg));
>> > +
>> > + arg.version = 0;
>>
>> Does it make sense to have version 0? Even if does, you can afford
>> to skip initialization due to the memset above. I prefer that
>> versions
>> start with 1
>>
>
> Setting it to 0 is for the reader, not the compiler. I'm not clear on
> the benefit of starting the version at 1, could you clarify?
How do I distinguish between version number never set and 0?
>
>> > +
>> > + arg.afu_version_major = ctx->afu->config.version_major;
>> > + arg.afu_version_minor = ctx->afu->config.version_minor;
>> > + arg.pasid = ctx->pasid;
>> > + arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
>> > + arg.global_mmio_size = ctx->afu->config.global_mmio_size;
>> > +
>> > + if (copy_to_user(uarg, &arg, sizeof(arg)))
>> > + return -EFAULT;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH"
>> > : \
>> > x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC"
>> > : \
>> > x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE"
>> > : \
>> > x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD"
>> > : \
>> > + x == OCXL_IOCTL_GET_METADATA ?
>> > "GET_METADATA" : \
>> > "UNKNOWN")
>> >
>> > static long afu_ioctl(struct file *file, unsigned int cmd,
>> > @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file,
>> > unsigned int cmd,
>> > irq_fd.eventfd);
>> > break;
>> >
>> > + case OCXL_IOCTL_GET_METADATA:
>> > + rc = afu_ioctl_get_metadata(ctx,
>> > + (struct ocxl_ioctl_get_metadata
>> > __user *) args);
>> > + break;
>> > +
>> > default:
>> > rc = -EINVAL;
>> > }
>> > diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>> > index 4b0b0b756f3e..16e1f48ce280 100644
>> > --- a/include/uapi/misc/ocxl.h
>> > +++ b/include/uapi/misc/ocxl.h
>> > @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
>> > __u64 reserved3;
>> > };
>> >
>> > +/*
>> > + * Version contains the version of the struct.
>> > + * Versions will always be backwards compatible, that is, new
>> > versions will not
>> > + * alter existing fields
>> > + */
>> > +struct ocxl_ioctl_get_metadata {
>>
>> This sounds more like a function name, do we need it to be
>> _get_metdata?
>>
>
> It pretty much is a function, it returns to userspace metadata about
> the descriptor being operated on.
>
It has a verb indicating action
>> > + __u16 version;
>> > +
>> > + // Version 0 fields
>> > + __u8 afu_version_major;
>> > + __u8 afu_version_minor;
>> > + __u32 pasid;
>> > +
>> > + __u64 pp_mmio_size;
>> > + __u64 global_mmio_size;
>> > +
>>
>> Should we document the fields? pp_ stands for per process, but is not
>> very clear at first look. Why do we care to return only the size,
>> what
>> about lpc size?
>>
>
> Yes, I would rather call it per_pasid_mmio_size, but consistency with
> the rest of the driver (& exposed sysfs entries) is also important.
>
Balbir Singh.
^ permalink raw reply
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Michael Ellerman @ 2018-02-22 3:38 UTC (permalink / raw)
To: Naveen N. Rao, Mark Lord
Cc: Alexei Starovoitov, Daniel Borkmann, Kees Cook, linuxppc-dev,
Andy Lutomirski, Will Drewry
In-Reply-To: <1519221778.inx201t3s9.naveen@linux.ibm.com>
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> Mark Lord wrote:
>> On 18-02-21 07:52 AM, Mark Lord wrote:
>>> On 18-02-21 03:35 AM, Naveen N. Rao wrote:
>> ..
>>>> Looks good to me, but I am not able to apply this patch. There seems to be whitespace damage.
>>>
>>> Here (attached) is a clean copy.
>>
>> Again, this time with the commit message included!
>
> Thanks. However...
> I am able to apply this using 'patch', but not with 'git am' since the
> headers are missing. FWIW, the usual workflow is to make the changes and
> commit it into your repository using 'git commit' and then use 'git
> format-patch' to generate a patch file that you can then post.
>
> I'll defer to Michael on whether he is ok to process this as it is.
The main thing is that it's caught by patchwork[1], otherwise I tend to
miss it. In this case the initial patch was caught by patchwork, so
that's fine.
I fixed up the white space and other issues before applying.
In general I'm happy to do that for new/infrequent committers, not so
much for people who send lots of patches and/or are paid to do so :)
cheers
1: http://patchwork.ozlabs.org/patch/875890/
^ permalink raw reply
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Michael Ellerman @ 2018-02-22 3:34 UTC (permalink / raw)
To: Daniel Borkmann, Mark Lord, Naveen N. Rao, Alexei Starovoitov,
Kees Cook, Andy Lutomirski, Will Drewry
Cc: linuxppc-dev
In-Reply-To: <03bbd3d6-3f67-795a-afa3-2feee367991a@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 02/21/2018 01:33 AM, Michael Ellerman wrote:
>> Mark Lord <mlord@pobox.com> writes:
>>
>>> I am using SECCOMP to filter syscalls on a ppc32 platform,
>>> and noticed that the JIT compiler was failing on the BPF
>>> even though the interpreter was working fine.
>>>
>>> The issue was that the compiler was missing one of the instructions
>>> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>>>
>>> Signed-Off-By: Mark Lord <mlord@pobox.com>
>>
>> Thanks.
>>
>> What is the failure mode without this patch? I assume when you have the
>> JIT enabled the seccomp filter fails to load entirely? Or do we have
>> logic to fall back to the interpreter?
>
> The logic for all JITs is that once a BPF insn cannot be JITed then it falls
> back to BPF interpreter. Here, since ppc32 is cBPF the path is: cBPF insn ->
> cBPF JIT -> fail -> migrate cBPF to eBPF -> run in eBPF interpreter. In the
> case where interpreter is compiled out (CONFIG_BPF_JIT_ALWAYS_ON), then the
> filter is rejected.
OK thanks.
>> Either way we should probably back port to stable. I assume this has
>> been broken since we originally added 32-bit support, so:
>
> Note that arm32 before it was converted to be an eBPF JIT (eBPF JITs do handle
> seccomp BPF in any case) was the only cBPF JIT that had it 'JIT-able', so
> currently, other cBPF ones like sparc32 or mips32 don't translate it either.
> Meaning, it would be nice to have as feature; I wouldn't necessarily frame
> it as a bug though (or at least a stable-urgent one, imho, but I leave that
> to you, of course).
OK, I'll just add the Fixes tag for our reference and we can always back
port it to stable later if we want to.
cheers
^ permalink raw reply
* Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
From: Michael Ellerman @ 2018-02-22 3:22 UTC (permalink / raw)
To: Masahiro Yamada, Arnd Bergmann
Cc: Rich Felker, Kernel Hardening, X86 ML, Paul Mackerras,
H. Peter Anvin, sparclinux, Sam Ravnborg, Yoshinori Sato,
Jonathan Corbet, Richard Weinberger, Linux-sh list,
Linus Torvalds, Ingo Molnar, Emese Revfy, Kees Cook, uml-devel,
Linux Kbuild mailing list, Peter Oberparleiter, Jeff Dike,
user-mode-linux-user, Thomas Gleixner, Michal Marek,
Ulf Magnusson, Greg Kroah-Hartman, Randy Dunlap,
open list:DOCUMENTATION, Linux Kernel Mailing List, linuxppc-dev,
David S. Miller
In-Reply-To: <CAK7LNAR3OMh9Q9ZfaBq=FpSJ-+DT5-RH_20ohV-iu34pX9hFKw@mail.gmail.com>
Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
<snip>
>
> (Case 3)
> Compiler flag -foo is sensitive to endian-ness.
>
>
> config CC_NEEDS_BIG_ENDIAN
> def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
>
> config CC_NEEDS_LITTLE_ENDIAN
> def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
>
> config CC_HAS_FOO
> bool
> default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
> default $(cc-option -foo)
We may do something like this on powerpc, where we have 32/64-bit and
big/little endian (on 64-bit) and then some ABI options that we
set/unset depending on endian.
The above looks like it could work though.
cheers
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Michael Ellerman @ 2018-02-22 3:19 UTC (permalink / raw)
To: Alastair D'Silva, Balbir Singh
Cc: Arnd Bergmann, frederic.barrat, Greg KH,
linux-kernel@vger.kernel.org, linuxppc-dev, Andrew Donnellan
In-Reply-To: <1519255934.2867.3.camel@au1.ibm.com>
"Alastair D'Silva" <alastair@au1.ibm.com> writes:
> On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.c
>> om> wrote:
>> > From: Alastair D'Silva <alastair@d-silva.org>
>> >
...
>> > diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>> > index 4b0b0b756f3e..16e1f48ce280 100644
>> > --- a/include/uapi/misc/ocxl.h
>> > +++ b/include/uapi/misc/ocxl.h
>> > @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
>> > __u64 reserved3;
>> > };
>> >
>> > +/*
>> > + * Version contains the version of the struct.
>> > + * Versions will always be backwards compatible, that is, new
>> > versions will not
>> > + * alter existing fields
>> > + */
>> > +struct ocxl_ioctl_get_metadata {
>>
>> This sounds more like a function name, do we need it to be
>> _get_metdata?
>>
>
> It pretty much is a function, it returns to userspace metadata about
> the descriptor being operated on.
It's not a function, it's a struct?
Outside of "management English" "get" is a verb, so using it in the name
of the struct is confusing, it should be a noun phrase.
cheers
^ permalink raw reply
* [PATCH v2 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:03 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222030334.14060-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Some required information is not exposed to userspace currently (eg. the
PASID), pass this information back, along with other information which
is currently communicated via sysfs, which saves some parsing effort in
userspace.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d9aa407db06a..11514a8444e5 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
return rc;
}
+static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
+ struct ocxl_ioctl_get_metadata __user *uarg)
+{
+ struct ocxl_ioctl_get_metadata arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+ arg.version = 0;
+
+ arg.afu_version_major = ctx->afu->config.version_major;
+ arg.afu_version_minor = ctx->afu->config.version_minor;
+ arg.pasid = ctx->pasid;
+ arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
+ arg.global_mmio_size = ctx->afu->config.global_mmio_size;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
+ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
irq_fd.eventfd);
break;
+ case OCXL_IOCTL_GET_METADATA:
+ rc = afu_ioctl_get_metadata(ctx,
+ (struct ocxl_ioctl_get_metadata __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 4b0b0b756f3e..16e1f48ce280 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
__u64 reserved3;
};
+/*
+ * Version contains the version of the struct.
+ * Versions will always be backwards compatible, that is, new versions will not
+ * alter existing fields
+ */
+struct ocxl_ioctl_get_metadata {
+ __u16 version;
+
+ // Version 0 fields
+ __u8 afu_version_major;
+ __u8 afu_version_minor;
+ __u32 pasid;
+
+ __u64 pp_mmio_size;
+ __u64 global_mmio_size;
+
+ // End version 0 fields
+
+ __u64 reserved[13]; // Total of 16*u64
+};
+
struct ocxl_ioctl_irq_fd {
__u64 irq_offset;
__s32 eventfd;
@@ -45,5 +66,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_ALLOC _IOR(OCXL_MAGIC, 0x11, __u64)
#define OCXL_IOCTL_IRQ_FREE _IOW(OCXL_MAGIC, 0x12, __u64)
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
+#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_get_metadata)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
* [PATCH v2 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Alastair D'Silva @ 2018-02-22 3:03 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
In-Reply-To: <20180222030334.14060-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Documentation/accelerators/ocxl.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst
index 4f7af841d935..ddcc58d01cfb 100644
--- a/Documentation/accelerators/ocxl.rst
+++ b/Documentation/accelerators/ocxl.rst
@@ -152,6 +152,11 @@ OCXL_IOCTL_IRQ_SET_FD:
Associate an event fd to an AFU interrupt so that the user process
can be notified when the AFU sends an interrupt.
+OCXL_IOCTL_GET_METADATA:
+
+ Obtains configuration information from the card, such at the size of
+ MMIO areas, the AFU version, and the PASID for the current context.
+
mmap
----
--
2.14.3
^ permalink raw reply related
* [PATCH v2 0/2] Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-22 3:03 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
From: Alastair D'Silva <alastair@d-silva.org>
This series adds a new IOCTL for OpenCAPI, which exposes adapter
metadata to userspace.
---
v1->v2:
- Document the new IOCTL
Alastair D'Silva (2):
ocxl: Add get_metadata IOCTL to share OCXL information to userspace
ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
Documentation/accelerators/ocxl.rst | 5 +++++
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
3 files changed, 54 insertions(+)
--
2.14.3
^ permalink raw reply
* [PATCH v12 22/22] selftests/vm: Fix deadlock in protection_keys.c
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
The sig_chld() handler calls dprintf2() taking care of setting
dprint_in_signal so that sigsafe_printf() won't call printf().
Unfortunately, this precaution is is negated by dprintf_level(), which
has a call to fflush().
This function acquires a lock, which means that if the signal interrupts an
ongoing fflush() the process will deadlock. At least on powerpc this is
easy to trigger, resulting in the following backtrace when attaching to the
frozen process:
(gdb) bt
#0 0x00007fff9f96c7d8 in __lll_lock_wait_private () from /lib64/power8/libc.so.6
#1 0x00007fff9f8cba4c in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#2 0x00007fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#3 0x00007fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#4 0x00000000100504f8 in sig_chld (x=17) at protection_keys.c:283
#5 <signal handler called>
#6 0x00007fff9f8cb8ac in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#7 0x00007fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#8 0x00007fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#9 0x0000000010050b50 in pkey_get (pkey=7, flags=0) at protection_keys.c:379
#10 0x0000000010050dc0 in pkey_disable_set (pkey=7, flags=2) at protection_keys.c:423
#11 0x0000000010051414 in pkey_write_deny (pkey=7) at protection_keys.c:486
#12 0x00000000100556bc in test_ptrace_of_child (ptr=0x7fff9f7f0000, pkey=7) at protection_keys.c:1288
#13 0x0000000010055f60 in run_tests_once () at protection_keys.c:1414
#14 0x00000000100561a4 in main () at protection_keys.c:1459
The fix is to refrain from calling fflush() when inside a signal handler.
The output may not be as pretty but at least the testcase will be able to
move on.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
---
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index 67f9b0f..7240598 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -128,7 +128,8 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf_level(level, args...) do { \
if (level <= DEBUG_LEVEL) \
sigsafe_printf(args); \
- fflush(NULL); \
+ if (!dprint_in_signal) \
+ fflush(NULL); \
} while (0)
#define dprintf0(args...) dprintf_level(0, args)
#define dprintf1(args...) dprintf_level(1, args)
--
1.7.1
^ permalink raw reply related
* [PATCH v12 21/22] selftests/vm: sub-page allocator
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
introduce a new allocator that allocates 4k hardware-pages to back
64k linux-page. This allocator is only applicable on powerpc.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 30 ++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 42c068a..1b06e59 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -766,6 +766,35 @@ void free_pkey_malloc(void *ptr)
return ptr;
}
+void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 pkey)
+{
+#ifdef __powerpc64__
+ void *ptr;
+ int ret;
+
+ dprintf1("doing %s(size=%ld, prot=0x%x, pkey=%d)\n", __func__,
+ size, prot, pkey);
+ pkey_assert(pkey < NR_PKEYS);
+ ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ pkey_assert(ptr != (void *)-1);
+
+ ret = syscall(__NR_subpage_prot, ptr, size, NULL);
+ if (ret) {
+ perror("subpage_perm");
+ return PTR_ERR_ENOTSUP;
+ }
+
+ ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
+ pkey_assert(!ret);
+ record_pkey_malloc(ptr, size);
+
+ dprintf1("%s() for pkey %d @ %p\n", __func__, pkey, ptr);
+ return ptr;
+#else /* __powerpc64__ */
+ return PTR_ERR_ENOTSUP;
+#endif /* __powerpc64__ */
+}
+
void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
{
int ret;
@@ -888,6 +917,7 @@ void setup_hugetlbfs(void)
void *(*pkey_malloc[])(long size, int prot, u16 pkey) = {
malloc_pkey_with_mprotect,
+ malloc_pkey_with_mprotect_subpage,
malloc_pkey_anon_huge,
malloc_pkey_hugetlb
/* can not do direct with the pkey_mprotect() API:
--
1.7.1
^ permalink raw reply related
* [PATCH v12 20/22] selftests/vm: testcases must restore pkey-permissions
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
Generally the signal handler restores the state of the pkey register
before returning. However there are times when the read/write operation
can legitamely fail without invoking the signal handler. Eg: A
sys_read() operaton to a write-protected page should be disallowed. In
such a case the state of the pkey register is not restored to its
original state. The test case is responsible for restoring the key
register state to its original value.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 437ee74..42c068a 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1003,6 +1003,7 @@ void test_read_of_write_disabled_region(int *ptr, u16 pkey)
ptr_contents = read_ptr(ptr);
dprintf1("*ptr: %d\n", ptr_contents);
dprintf1("\n");
+ pkey_write_allow(pkey);
}
void test_read_of_access_disabled_region(int *ptr, u16 pkey)
{
@@ -1082,6 +1083,7 @@ void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
ret = read(test_fd, ptr, 1);
dprintf1("read ret: %d\n", ret);
pkey_assert(ret);
+ pkey_access_allow(pkey);
}
void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
{
@@ -1094,6 +1096,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
if (ret < 0 && (DEBUG_LEVEL > 0))
perror("verbose read result (OK for this to be bad)");
pkey_assert(ret);
+ pkey_write_allow(pkey);
}
void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
@@ -1113,6 +1116,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
vmsplice_ret = vmsplice(pipe_fds[1], &iov, 1, SPLICE_F_GIFT);
dprintf1("vmsplice() ret: %d\n", vmsplice_ret);
pkey_assert(vmsplice_ret == -1);
+ pkey_access_allow(pkey);
close(pipe_fds[0]);
close(pipe_fds[1]);
@@ -1133,6 +1137,7 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey)
if (DEBUG_LEVEL > 0)
perror("futex");
dprintf1("futex() ret: %d\n", futex_ret);
+ pkey_write_allow(pkey);
}
/* Assumes that all pkeys other than 'pkey' are unallocated */
--
1.7.1
^ permalink raw reply related
* [PATCH v12 19/22] selftests/vm: detect write violation on a mapped access-denied-key page
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
detect write-violation on a page to which access-disabled
key is associated much after the page is mapped.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index b685e26..437ee74 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1059,6 +1059,18 @@ void test_write_of_access_disabled_region(int *ptr, u16 pkey)
*ptr = __LINE__;
expected_pkey_fault(pkey);
}
+
+void test_write_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling access; after accessing the page, "
+ " to PKEY[%02d], doing write\n", pkey);
+ pkey_access_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
{
int ret;
@@ -1343,6 +1355,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
test_write_of_write_disabled_region,
test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
+ test_write_of_access_disabled_region_with_page_already_mapped,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
test_kernel_gup_of_access_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v12 18/22] selftests/vm: associate key on a mapped page and detect write violation
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
detect write-violation on a page to which write-disabled
key is associated much after the page is mapped.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index dba9162..b685e26 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1034,6 +1034,17 @@ void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
expected_pkey_fault(pkey);
}
+void test_write_of_write_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling write access; after accessing the page, "
+ "to PKEY[%02d], doing write\n", pkey);
+ pkey_write_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1330,6 +1341,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
test_read_of_access_disabled_region,
test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
+ test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v12 17/22] selftests/vm: associate key on a mapped page and detect access violation
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
detect access-violation on a page to which access-disabled
key is associated much after the page is mapped.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index aae6771..dba9162 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1016,6 +1016,24 @@ void test_read_of_access_disabled_region(int *ptr, u16 pkey)
dprintf1("*ptr: %d\n", ptr_contents);
expected_pkey_fault(pkey);
}
+
+void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ int ptr_contents;
+
+ dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",
+ pkey, ptr);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("reading ptr before disabling the read : %d\n",
+ ptr_contents);
+ rdpkey_reg();
+ pkey_access_deny(pkey);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("*ptr: %d\n", ptr_contents);
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1310,6 +1328,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
void (*pkey_tests[])(int *ptr, u16 pkey) = {
test_read_of_write_disabled_region,
test_read_of_access_disabled_region,
+ test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v12 16/22] selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
The maximum number of keys that can be allocated has to
take into consideration, that some keys are reserved by
the architecture for specific purpose. Hence cannot
be allocated.
Fix the assertion in test_pkey_alloc_exhaust()
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/pkey-helpers.h | 14 ++++++++++++++
tools/testing/selftests/vm/protection_keys.c | 9 ++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index 88ef58f..67f9b0f 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -416,4 +416,18 @@ static inline int get_start_key(void)
#endif
}
+static inline int arch_reserved_keys(void)
+{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+ return NR_RESERVED_PKEYS;
+#elif __powerpc64__ /* arch */
+ if (sysconf(_SC_PAGESIZE) == 4096)
+ return NR_RESERVED_PKEYS_4K;
+ else
+ return NR_RESERVED_PKEYS_64K;
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
+}
+
#endif /* _PKEYS_HELPER_H */
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 58da5a0..aae6771 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1167,12 +1167,11 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)
pkey_assert(i < NR_PKEYS*2);
/*
- * There are 16 pkeys supported in hardware. One is taken
- * up for the default (0) and another can be taken up by
- * an execute-only mapping. Ensure that we can allocate
- * at least 14 (16-2).
+ * There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys()
+ * are reserved. One can be taken up by an execute-only mapping.
+ * Ensure that we can allocate at least the remaining.
*/
- pkey_assert(i >= NR_PKEYS-2);
+ pkey_assert(i >= (NR_PKEYS-arch_reserved_keys()-1));
for (i = 0; i < nr_allocated_pkeys; i++) {
err = sys_pkey_free(allocated_pkeys[i]);
--
1.7.1
^ permalink raw reply related
* [PATCH v12 15/22] selftests/vm: powerpc implementation to check support for pkey
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
pkey subsystem is supported if the hardware and kernel has support.
We determine that by checking if allocation of a key succeeds or not.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/pkey-helpers.h | 22 ++++++++++++++++------
tools/testing/selftests/vm/protection_keys.c | 9 +++++----
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index c47aead..88ef58f 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -258,7 +258,7 @@ static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
#define X86_FEATURE_PKU (1<<3) /* Protection Keys for Userspace */
#define X86_FEATURE_OSPKE (1<<4) /* OS Protection Keys Enable */
-static inline int cpu_has_pku(void)
+static inline bool is_pkey_supported(void)
{
unsigned int eax;
unsigned int ebx;
@@ -271,13 +271,13 @@ static inline int cpu_has_pku(void)
if (!(ecx & X86_FEATURE_PKU)) {
dprintf2("cpu does not have PKU\n");
- return 0;
+ return false;
}
if (!(ecx & X86_FEATURE_OSPKE)) {
dprintf2("cpu does not have OSPKE\n");
- return 0;
+ return false;
}
- return 1;
+ return true;
}
#define XSTATE_PKEY_BIT (9)
@@ -323,9 +323,19 @@ static inline void __page_o_noops(void)
#elif __powerpc64__ /* arch */
#define PAGE_SIZE (0x1UL << 16)
-static inline int cpu_has_pku(void)
+static inline bool is_pkey_supported(void)
{
- return 1;
+ /*
+ * No simple way to determine this.
+ * lets try allocating a key and see if it succeeds.
+ */
+ int ret = sys_pkey_alloc(0, 0);
+
+ if (ret > 0) {
+ sys_pkey_free(ret);
+ return true;
+ }
+ return false;
}
/* 8-bytes of instruction * 16384bytes = 1 page */
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index e82bd88..58da5a0 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1299,8 +1299,8 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
int size = PAGE_SIZE;
int sret;
- if (cpu_has_pku()) {
- dprintf1("SKIP: %s: no CPU support\n", __func__);
+ if (is_pkey_supported()) {
+ dprintf1("SKIP: %s: no CPU/kernel support\n", __func__);
return;
}
@@ -1362,12 +1362,13 @@ void run_tests_once(void)
int main(void)
{
int nr_iterations = 22;
+ int pkey_supported = is_pkey_supported();
setup_handlers();
- printf("has pkey: %d\n", cpu_has_pku());
+ printf("has pkey: %s\n", pkey_supported ? "Yes" : "No");
- if (!cpu_has_pku()) {
+ if (!pkey_supported) {
int size = PAGE_SIZE;
int *ptr;
--
1.7.1
^ permalink raw reply related
* [PATCH v12 14/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
When a key is freed, the key is no more effective.
Clear the bits corresponding to the pkey in the shadow
register. Otherwise it will carry some spurious bits
which can trigger false-positive asserts.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index c4c73e6..e82bd88 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -586,7 +586,8 @@ int sys_pkey_free(unsigned long pkey)
int ret = syscall(SYS_pkey_free, pkey);
if (!ret)
- shadow_pkey_reg &= reset_bits(pkey, PKEY_DISABLE_ACCESS);
+ shadow_pkey_reg &= reset_bits(pkey,
+ PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);
dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
return ret;
}
--
1.7.1
^ permalink raw reply related
* [PATCH v12 13/22] selftests/vm: powerpc implementation for generic abstraction
From: Ram Pai @ 2018-02-22 1:55 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1519264541-7621-1-git-send-email-linuxram@us.ibm.com>
Introduce powerpc implementation for the various
abstractions.
cc: Dave Hansen <dave.hansen@intel.com>
cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/pkey-helpers.h | 128 ++++++++++++++++++++++----
tools/testing/selftests/vm/protection_keys.c | 42 +++++---
2 files changed, 136 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index c8f5739..c47aead 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -18,27 +18,51 @@
#define u16 uint16_t
#define u32 uint32_t
#define u64 uint64_t
-#define pkey_reg_t u32
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
+#ifdef __i386__ /* arch */
#define SYS_mprotect_key 380
-#define SYS_pkey_alloc 381
-#define SYS_pkey_free 382
+#define SYS_pkey_alloc 381
+#define SYS_pkey_free 382
#define REG_IP_IDX REG_EIP
-#define si_pkey_offset 0x14
-#else
+#elif __x86_64__
#define SYS_mprotect_key 329
-#define SYS_pkey_alloc 330
-#define SYS_pkey_free 331
+#define SYS_pkey_alloc 330
+#define SYS_pkey_free 331
#define REG_IP_IDX REG_RIP
-#define si_pkey_offset 0x20
-#endif
+#endif /* __x86_64__ */
+
+#define NR_PKEYS 16
+#define NR_RESERVED_PKEYS 1
+#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS 0x1
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<21)
+#define pkey_reg_t u32
-#define NR_PKEYS 16
-#define PKEY_BITS_PER_PKEY 2
-#define PKEY_DISABLE_ACCESS 0x1
-#define PKEY_DISABLE_WRITE 0x2
-#define HPAGE_SIZE (1UL<<21)
+#elif __powerpc64__ /* arch */
+
+#define SYS_mprotect_key 386
+#define SYS_pkey_alloc 384
+#define SYS_pkey_free 385
+#define REG_IP_IDX PT_NIP
+#define REG_TRAPNO PT_TRAP
+#define gregs gp_regs
+#define fpregs fp_regs
+
+#define NR_PKEYS 32
+#define NR_RESERVED_PKEYS_4K 26
+#define NR_RESERVED_PKEYS_64K 3
+#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS 0x3 /* disable read and write */
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<24)
+#define pkey_reg_t u64
+
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
#ifndef DEBUG_LEVEL
#define DEBUG_LEVEL 0
@@ -47,7 +71,11 @@
static inline u32 pkey_to_shift(int pkey)
{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
return pkey * PKEY_BITS_PER_PKEY;
+#elif __powerpc64__ /* arch */
+ return (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
+#endif /* arch */
}
static inline pkey_reg_t reset_bits(int pkey, pkey_reg_t bits)
@@ -111,6 +139,7 @@ static inline void sigsafe_printf(const char *format, ...)
extern pkey_reg_t shadow_pkey_reg;
static inline pkey_reg_t __rdpkey_reg(void)
{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
unsigned int eax, edx;
unsigned int ecx = 0;
pkey_reg_t pkey_reg;
@@ -118,7 +147,13 @@ static inline pkey_reg_t __rdpkey_reg(void)
asm volatile(".byte 0x0f,0x01,0xee\n\t"
: "=a" (eax), "=d" (edx)
: "c" (ecx));
- pkey_reg = eax;
+#elif __powerpc64__ /* arch */
+ pkey_reg_t eax;
+ pkey_reg_t pkey_reg;
+
+ asm volatile("mfspr %0, 0xd" : "=r" ((pkey_reg_t)(eax)));
+#endif /* arch */
+ pkey_reg = (pkey_reg_t)eax;
return pkey_reg;
}
@@ -138,6 +173,7 @@ static inline pkey_reg_t _rdpkey_reg(int line)
static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
{
pkey_reg_t eax = pkey_reg;
+#if defined(__i386__) || defined(__x86_64__) /* arch */
pkey_reg_t ecx = 0;
pkey_reg_t edx = 0;
@@ -146,6 +182,14 @@ static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
asm volatile(".byte 0x0f,0x01,0xef\n\t"
: : "a" (eax), "c" (ecx), "d" (edx));
assert(pkey_reg == __rdpkey_reg());
+
+#elif __powerpc64__ /* arch */
+ dprintf4("%s() changing %llx to %llx\n",
+ __func__, __rdpkey_reg(), pkey_reg);
+ asm volatile("mtspr 0xd, %0" : : "r" ((unsigned long)(eax)) : "memory");
+#endif /* arch */
+ dprintf4("%s() pkey register after changing %016lx to %016lx\n",
+ __func__, __rdpkey_reg(), pkey_reg);
}
static inline void wrpkey_reg(pkey_reg_t pkey_reg)
@@ -192,6 +236,8 @@ static inline void __pkey_write_allow(int pkey, int do_allow_write)
dprintf4("pkey_reg now: %08x\n", rdpkey_reg());
}
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
#define PAGE_SIZE 4096
#define MB (1<<20)
@@ -274,8 +320,18 @@ static inline void __page_o_noops(void)
/* 8-bytes of instruction * 512 bytes = 1 page */
asm(".rept 512 ; nopl 0x7eeeeeee(%eax) ; .endr");
}
+#elif __powerpc64__ /* arch */
-#endif /* _PKEYS_HELPER_H */
+#define PAGE_SIZE (0x1UL << 16)
+static inline int cpu_has_pku(void)
+{
+ return 1;
+}
+
+/* 8-bytes of instruction * 16384bytes = 1 page */
+#define __page_o_noops() asm(".rept 16384 ; nop; .endr")
+
+#endif /* arch */
#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
#define ALIGN_UP(x, align_to) (((x) + ((align_to)-1)) & ~((align_to)-1))
@@ -307,11 +363,47 @@ static inline void __page_o_noops(void)
static inline int open_hugepage_file(int flag)
{
- return open("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages",
+ int fd;
+
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+ fd = open("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages",
O_RDONLY);
+#elif __powerpc64__ /* arch */
+ fd = open("/sys/kernel/mm/hugepages/hugepages-16384kB/nr_hugepages",
+ O_RDONLY);
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
+ return fd;
}
static inline int get_start_key(void)
{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
return 1;
+#elif __powerpc64__ /* arch */
+ return 0;
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
+}
+
+static inline u32 *siginfo_get_pkey_ptr(siginfo_t *si)
+{
+#ifdef si_pkey
+ return &si->si_pkey;
+#else
+
+#ifdef __i386__ /* arch */
+#define si_pkey_offset 0x14
+#elif __x86_64__ /*arch */
+#define si_pkey_offset 0x1C
+#elif __powerpc64__ /*arch */
+#define si_pkey_offset 0x1C
+#endif /*arch */
+
+ return (u32 *)(((u8 *)si) + si_pkey_offset);
+#endif
}
+
+#endif /* _PKEYS_HELPER_H */
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 6fdd8f5..c4c73e6 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -186,17 +186,18 @@ void dump_mem(void *dumpme, int len_bytes)
int pkey_faults;
int last_si_pkey = -1;
+void pkey_access_allow(int pkey);
void signal_handler(int signum, siginfo_t *si, void *vucontext)
{
ucontext_t *uctxt = vucontext;
int trapno;
unsigned long ip;
char *fpregs;
+#if defined(__i386__) || defined(__x86_64__) /* arch */
pkey_reg_t *pkey_reg_ptr;
- u64 siginfo_pkey;
+#endif /* defined(__i386__) || defined(__x86_64__) */
+ u32 siginfo_pkey;
u32 *si_pkey_ptr;
- int pkey_reg_offset;
- fpregset_t fpregset;
dprint_in_signal = 1;
dprintf1(">>>>===============SIGSEGV============================\n");
@@ -206,12 +207,14 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO];
ip = uctxt->uc_mcontext.gregs[REG_IP_IDX];
- fpregset = uctxt->uc_mcontext.fpregs;
- fpregs = (void *)fpregset;
+ fpregs = (char *) uctxt->uc_mcontext.fpregs;
dprintf2("%s() trapno: %d ip: 0x%016lx info->si_code: %s/%d\n",
__func__, trapno, ip, si_code_str(si->si_code),
si->si_code);
+
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
#ifdef __i386__
/*
* 32-bit has some extra padding so that userspace can tell whether
@@ -219,22 +222,23 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
* state. We just assume that it is here.
*/
fpregs += 0x70;
-#endif
- pkey_reg_offset = pkey_reg_xstate_offset();
- pkey_reg_ptr = (void *)(&fpregs[pkey_reg_offset]);
+#endif /* __i386__ */
- dprintf1("siginfo: %p\n", si);
- dprintf1(" fpregs: %p\n", fpregs);
+ pkey_reg_ptr = (void *)(&fpregs[pkey_reg_xstate_offset()]);
/*
- * If we got a PKEY fault, we *HAVE* to have at least one bit set in
+ * If we got a key fault, we *HAVE* to have at least one bit set in
* here.
*/
dprintf1("pkey_reg_xstate_offset: %d\n", pkey_reg_xstate_offset());
if (DEBUG_LEVEL > 4)
dump_mem(pkey_reg_ptr - 128, 256);
pkey_assert(*pkey_reg_ptr);
+#endif /* defined(__i386__) || defined(__x86_64__) */
- si_pkey_ptr = (u32 *)(((u8 *)si) + si_pkey_offset);
+ dprintf1("siginfo: %p\n", si);
+ dprintf1(" fpregs: %p\n", fpregs);
+
+ si_pkey_ptr = siginfo_get_pkey_ptr(si);
dprintf1("si_pkey_ptr: %p\n", si_pkey_ptr);
dump_mem(si_pkey_ptr - 8, 24);
siginfo_pkey = *si_pkey_ptr;
@@ -248,19 +252,25 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
exit(4);
}
- dprintf1("signal pkey_reg from xsave: %016lx\n", *pkey_reg_ptr);
/*
* need __rdpkey_reg() version so we do not do shadow_pkey_reg
* checking
*/
dprintf1("signal pkey_reg from pkey_reg: %016lx\n", __rdpkey_reg());
- dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
- *(u64 *)pkey_reg_ptr = 0x00000000;
+ dprintf1("pkey from siginfo: %lx\n", siginfo_pkey);
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+ dprintf1("signal pkey_reg from xsave: %016lx\n", *pkey_reg_ptr);
+ *(u64 *)pkey_reg_ptr &= reset_bits(siginfo_pkey,
+ PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);
+#elif __powerpc64__
+ pkey_access_allow(siginfo_pkey);
+#endif
+ shadow_pkey_reg &= reset_bits(siginfo_pkey,
+ PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);
dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
"to continue\n");
pkey_faults++;
dprintf1("<<<<==================================================\n");
- return;
}
int wait_all_children(void)
--
1.7.1
^ permalink raw reply related
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