diff mbox

[v5,4/8] char: rpmb: provide a user space interface

Message ID 1468873673-21776-5-git-send-email-tomas.winkler@intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Winkler, Tomas July 18, 2016, 8:27 p.m. UTC
The user space API is achieved via two synchronous IOCTL.
Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is performed
by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD where
the whole RPMB sequence including RESULT_READ is supplied by the caller.
The latter is intended for  easier adjusting  of the  applications that
use MMC_IOC_MULTI_CMD ioctl.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: use memdup_user
V3: commit message fix
V4: resend
V5: 1. Add RPMB_IOC_SEQ_CMD API.
    2. Export uapi rpmb.h header
 Documentation/ioctl/ioctl-number.txt |   1 +
 MAINTAINERS                          |   1 +
 drivers/char/rpmb/Kconfig            |   7 +
 drivers/char/rpmb/Makefile           |   1 +
 drivers/char/rpmb/cdev.c             | 269 +++++++++++++++++++++++++++++++++++
 drivers/char/rpmb/core.c             |   9 +-
 drivers/char/rpmb/rpmb-cdev.h        |  25 ++++
 include/linux/rpmb.h                 |  81 ++---------
 include/uapi/linux/Kbuild            |   1 +
 include/uapi/linux/rpmb.h            | 152 ++++++++++++++++++++
 10 files changed, 473 insertions(+), 74 deletions(-)
 create mode 100644 drivers/char/rpmb/cdev.c
 create mode 100644 drivers/char/rpmb/rpmb-cdev.h
 create mode 100644 include/uapi/linux/rpmb.h

Comments

Paul Gortmaker July 18, 2016, 10:15 p.m. UTC | #1
On Mon, Jul 18, 2016 at 4:27 PM, Tomas Winkler <tomas.winkler@intel.com> wrote:
> The user space API is achieved via two synchronous IOCTL.
> Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is performed
> by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD where
> the whole RPMB sequence including RESULT_READ is supplied by the caller.
> The latter is intended for  easier adjusting  of the  applications that
> use MMC_IOC_MULTI_CMD ioctl.
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---

[...]

> diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig
> index c5e6e909efce..6794be9fcc5e 100644
> --- a/drivers/char/rpmb/Kconfig
> +++ b/drivers/char/rpmb/Kconfig
> @@ -6,3 +6,10 @@ config RPMB
>           access RPMB partition.
>
>           If unsure, select N.
> +
> +config RPMB_INTF_DEV
> +       bool "RPMB character device interface /dev/rpmbN"

A bool Kconfig should ideally....

> +       depends on RPMB
> +       help
> +         Say yes here if you want to access RPMB from user space
> +         via character device interface /dev/rpmb%d
> diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile
> index 812b3ed264c0..b5dc087b1299 100644
> --- a/drivers/char/rpmb/Makefile
> +++ b/drivers/char/rpmb/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RPMB) += rpmb.o
>  rpmb-objs += core.o
> +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
>
>  ccflags-y += -D__CHECK_ENDIAN__
> diff --git a/drivers/char/rpmb/cdev.c b/drivers/char/rpmb/cdev.c
> new file mode 100644
> index 000000000000..f3ad3444f76d
> --- /dev/null
> +++ b/drivers/char/rpmb/cdev.c
> @@ -0,0 +1,269 @@
> +/*
> + * Copyright (C) 2015-2016 Intel Corp. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>

....not use module.h or any MODULE_ macros from within it.

Thanks,
Paul.
--

> +#include <linux/fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/compat.h>
> +#include <linux/slab.h>
> +#include <linux/capability.h>
> +
> +#include <linux/rpmb.h>
> +
> +#include "rpmb-cdev.h"
> +
> +static dev_t rpmb_devt;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Winkler, Tomas July 20, 2016, 9:02 a.m. UTC | #2
> 

> On Mon, Jul 18, 2016 at 4:27 PM, Tomas Winkler <tomas.winkler@intel.com>

> wrote:

> > The user space API is achieved via two synchronous IOCTL.

> > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is

> performed

> > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD

> where

> > the whole RPMB sequence including RESULT_READ is supplied by the caller.

> > The latter is intended for  easier adjusting  of the  applications

> > that use MMC_IOC_MULTI_CMD ioctl.

> >

> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

> > ---

> 

> [...]

> 

> > diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig

> > index c5e6e909efce..6794be9fcc5e 100644

> > --- a/drivers/char/rpmb/Kconfig

> > +++ b/drivers/char/rpmb/Kconfig

> > @@ -6,3 +6,10 @@ config RPMB

> >           access RPMB partition.

> >

> >           If unsure, select N.

> > +

> > +config RPMB_INTF_DEV

> > +       bool "RPMB character device interface /dev/rpmbN"

> 

> A bool Kconfig should ideally....

> 

> > +       depends on RPMB

> > +       help

> > +         Say yes here if you want to access RPMB from user space

> > +         via character device interface /dev/rpmb%d

> > diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile

> > index 812b3ed264c0..b5dc087b1299 100644

> > --- a/drivers/char/rpmb/Makefile

> > +++ b/drivers/char/rpmb/Makefile

> > @@ -1,4 +1,5 @@

> >  obj-$(CONFIG_RPMB) += rpmb.o

> >  rpmb-objs += core.o

> > +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o


This is not a builtin, this is an optional part of the module 

> > +#include <linux/module.h>

> 

> ....not use module.h or any MODULE_ macros from within it.


Can be dropped in this case as no macros are used, 
but the pattern Kconfig bool -> no include module.h  you are following has false positive cases.

Thanks
Tomas
Paul Gortmaker July 20, 2016, 2:21 p.m. UTC | #3
[RE: [PATCH v5 4/8] char: rpmb: provide a user space interface] On 20/07/2016 (Wed 09:02) Winkler, Tomas wrote:

> > 
> > On Mon, Jul 18, 2016 at 4:27 PM, Tomas Winkler <tomas.winkler@intel.com>
> > wrote:
> > > The user space API is achieved via two synchronous IOCTL.
> > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > performed
> > > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> > where
> > > the whole RPMB sequence including RESULT_READ is supplied by the caller.
> > > The latter is intended for  easier adjusting  of the  applications
> > > that use MMC_IOC_MULTI_CMD ioctl.
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > ---
> > 
> > [...]
> > 
> > > diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig
> > > index c5e6e909efce..6794be9fcc5e 100644
> > > --- a/drivers/char/rpmb/Kconfig
> > > +++ b/drivers/char/rpmb/Kconfig
> > > @@ -6,3 +6,10 @@ config RPMB
> > >           access RPMB partition.
> > >
> > >           If unsure, select N.
> > > +
> > > +config RPMB_INTF_DEV
> > > +       bool "RPMB character device interface /dev/rpmbN"
> > 
> > A bool Kconfig should ideally....
> > 
> > > +       depends on RPMB
> > > +       help
> > > +         Say yes here if you want to access RPMB from user space
> > > +         via character device interface /dev/rpmb%d
> > > diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile
> > > index 812b3ed264c0..b5dc087b1299 100644
> > > --- a/drivers/char/rpmb/Makefile
> > > +++ b/drivers/char/rpmb/Makefile
> > > @@ -1,4 +1,5 @@
> > >  obj-$(CONFIG_RPMB) += rpmb.o
> > >  rpmb-objs += core.o
> > > +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
> 
> This is not a builtin, this is an optional part of the module 

Object files that are not stand-alone, but linked into a larger object
to create a module generally still don't need module.h if they
themselves specifically aren't registering the module or similar.

The exception is a module that doesn't actively do anything but export
symbols (i.e. a library).  Then somewhere in it there needs to be a
MODULE_LICENSE so that the kernel can audit GPL and taint etc.

> 
> > > +#include <linux/module.h>
> > 
> > ....not use module.h or any MODULE_ macros from within it.
> 
> Can be dropped in this case as no macros are used, 
> but the pattern Kconfig bool -> no include module.h  you are following has false positive cases.

Yes, of course there are other things, like the exception table
searches, and symbol_get / symbol_put, macros defining string
lengths, etc... and I try to watch for those via inspection and
build testing. 

However the majority bool->module.h are there for two reasons:

1) in core code we had to use module.h in the past when export.h
   did not exist.

2) in driver code that largely capitalizes on copying from other
   drivers without explicitly considering modularity.

...and it is worth our while to clean both up, I think.

If you can think of specific false positives that I might not be
aware of, please don't hesitate to call them out.

Thanks,
Paul.
--

> 
> Thanks
> Tomas
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek Aug. 5, 2016, 8:08 p.m. UTC | #4
On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> The user space API is achieved via two synchronous IOCTL.

IOCTLs?

> Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is performed
> by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD where
> the whole RPMB sequence including RESULT_READ is supplied by the caller.
> The latter is intended for  easier adjusting  of the  applications that
> use MMC_IOC_MULTI_CMD ioctl.

Why "  "?

> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

> +
> +static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> +	return __rpmb_ioctl(fp, cmd, (void __user *)arg);
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> +			      unsigned long arg)
> +{
> +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> +}
> +#endif /* CONFIG_COMPAT */

Description of the ioctl is missing, and it should certainly be
designed in a way that it does not need compat support.
Winkler, Tomas Aug. 7, 2016, 9:44 a.m. UTC | #5
> 
> On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > The user space API is achieved via two synchronous IOCTL.
> 
> IOCTLs?

Will fix
 
> > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> performed
> > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> where
> > the whole RPMB sequence including RESULT_READ is supplied by the caller.
> > The latter is intended for  easier adjusting  of the  applications
> > that use MMC_IOC_MULTI_CMD ioctl.
> 
> Why "  "?
Not sure I there is enough clue in your question. 
> 
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> 
> > +
> > +static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned
> > +long arg) {
> > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > +
> > +#ifdef CONFIG_COMPAT
> > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > +			      unsigned long arg)
> > +{
> > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > +}
> > +#endif /* CONFIG_COMPAT */
> 
> Description of the ioctl is missing, 
Will add. 

and it should certainly be designed in a way
> that it does not need compat support.

The compat_ioctl handler just casts the compat_ptr, I believe this should be done unless the ioctl is globaly registered in fs/compat_ioctl.c, but I might be wrong.
Tomas


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH Aug. 31, 2016, 10:49 a.m. UTC | #6
On Sun, Aug 07, 2016 at 09:44:03AM +0000, Winkler, Tomas wrote:
> > 
> > On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > > The user space API is achieved via two synchronous IOCTL.
> > 
> > IOCTLs?
> 
> Will fix
>  
> > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > performed
> > > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> > where
> > > the whole RPMB sequence including RESULT_READ is supplied by the caller.
> > > The latter is intended for  easier adjusting  of the  applications
> > > that use MMC_IOC_MULTI_CMD ioctl.
> > 
> > Why "  "?
> Not sure I there is enough clue in your question. 
> > 
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > 
> > > +
> > > +static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned
> > > +long arg) {
> > > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > > +
> > > +#ifdef CONFIG_COMPAT
> > > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > > +			      unsigned long arg)
> > > +{
> > > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > > +}
> > > +#endif /* CONFIG_COMPAT */
> > 
> > Description of the ioctl is missing, 
> Will add. 
> 
> and it should certainly be designed in a way
> > that it does not need compat support.
> 
> The compat_ioctl handler just casts the compat_ptr, I believe this
> should be done unless the ioctl is globaly registered in
> fs/compat_ioctl.c, but I might be wrong.

You shouldn't need a compat ioctl for anything new that is added, unless
your api is really messed up.  Please test to be sure, and not use a
compat ioctl at all, it isn't that hard to do.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Winkler, Tomas Sept. 1, 2016, 8:05 p.m. UTC | #7
> 
> On Sun, Aug 07, 2016 at 09:44:03AM +0000, Winkler, Tomas wrote:
> > >
> > > On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > > > The user space API is achieved via two synchronous IOCTL.
> > >
> > > IOCTLs?
> >
> > Will fix
> >
> > > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > > performed
> > > > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> > > where
> > > > the whole RPMB sequence including RESULT_READ is supplied by the
> caller.
> > > > The latter is intended for  easier adjusting  of the  applications
> > > > that use MMC_IOC_MULTI_CMD ioctl.
> > >
> > > Why "  "?
> > Not sure I there is enough clue in your question.
> > >
> > > >
> > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > >
> > > > +
> > > > +static long rpmb_ioctl(struct file *fp, unsigned int cmd,
> > > > +unsigned long arg) {
> > > > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > > > +
> > > > +#ifdef CONFIG_COMPAT
> > > > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > > > +			      unsigned long arg)
> > > > +{
> > > > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > > > +}
> > > > +#endif /* CONFIG_COMPAT */
> > >
> > > Description of the ioctl is missing,
> > Will add.
> >
> > and it should certainly be designed in a way
> > > that it does not need compat support.
> >
> > The compat_ioctl handler just casts the compat_ptr, I believe this
> > should be done unless the ioctl is globaly registered in
> > fs/compat_ioctl.c, but I might be wrong.
> 
> You shouldn't need a compat ioctl for anything new that is added, unless
> your api is really messed up.  Please test to be sure, and not use a compat
> ioctl at all, it isn't that hard to do.

compat_ioctl is called anyhow when CONFIG_COMPAT is set, there is no way around it, or I'm missing something?
Actually there is no more than that for the COMPAT support in this code.

Thanks
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH Sept. 1, 2016, 8:46 p.m. UTC | #8
On Thu, Sep 01, 2016 at 08:05:26PM +0000, Winkler, Tomas wrote:
> 
> > 
> > On Sun, Aug 07, 2016 at 09:44:03AM +0000, Winkler, Tomas wrote:
> > > >
> > > > On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > > > > The user space API is achieved via two synchronous IOCTL.
> > > >
> > > > IOCTLs?
> > >
> > > Will fix
> > >
> > > > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > > > performed
> > > > > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> > > > where
> > > > > the whole RPMB sequence including RESULT_READ is supplied by the
> > caller.
> > > > > The latter is intended for  easier adjusting  of the  applications
> > > > > that use MMC_IOC_MULTI_CMD ioctl.
> > > >
> > > > Why "  "?
> > > Not sure I there is enough clue in your question.
> > > >
> > > > >
> > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > >
> > > > > +
> > > > > +static long rpmb_ioctl(struct file *fp, unsigned int cmd,
> > > > > +unsigned long arg) {
> > > > > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > > > > +
> > > > > +#ifdef CONFIG_COMPAT
> > > > > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > > > > +			      unsigned long arg)
> > > > > +{
> > > > > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > > > > +}
> > > > > +#endif /* CONFIG_COMPAT */
> > > >
> > > > Description of the ioctl is missing,
> > > Will add.
> > >
> > > and it should certainly be designed in a way
> > > > that it does not need compat support.
> > >
> > > The compat_ioctl handler just casts the compat_ptr, I believe this
> > > should be done unless the ioctl is globaly registered in
> > > fs/compat_ioctl.c, but I might be wrong.
> > 
> > You shouldn't need a compat ioctl for anything new that is added, unless
> > your api is really messed up.  Please test to be sure, and not use a compat
> > ioctl at all, it isn't that hard to do.
> 
> compat_ioctl is called anyhow when CONFIG_COMPAT is set, there is no
> way around it, or I'm missing something?  Actually there is no more
> than that for the COMPAT support in this code.

If you don't provide a compat_ioctl() all should be fine, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Winkler, Tomas Sept. 4, 2016, 11:35 a.m. UTC | #9
> On Thu, Sep 01, 2016 at 08:05:26PM +0000, Winkler, Tomas wrote:
> >
> > >
> > > On Sun, Aug 07, 2016 at 09:44:03AM +0000, Winkler, Tomas wrote:
> > > > >
> > > > > On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > > > > > The user space API is achieved via two synchronous IOCTL.
> > > > >
> > > > > IOCTLs?
> > > >
> > > > Will fix
> > > >
> > > > > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > > > > performed
> > > > > > by the framework on behalf the user and second,
> > > > > > RPMB_IOC_SEQ_CMD
> > > > > where
> > > > > > the whole RPMB sequence including RESULT_READ is supplied by
> > > > > > the
> > > caller.
> > > > > > The latter is intended for  easier adjusting  of the
> > > > > > applications that use MMC_IOC_MULTI_CMD ioctl.
> > > > >
> > > > > Why "  "?
> > > > Not sure I there is enough clue in your question.
> > > > >
> > > > > >
> > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > >
> > > > > > +
> > > > > > +static long rpmb_ioctl(struct file *fp, unsigned int cmd,
> > > > > > +unsigned long arg) {
> > > > > > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > > > > > +
> > > > > > +#ifdef CONFIG_COMPAT
> > > > > > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > > > > > +			      unsigned long arg)
> > > > > > +{
> > > > > > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > > > > > +}
> > > > > > +#endif /* CONFIG_COMPAT */
> > > > >
> > > > > Description of the ioctl is missing,
> > > > Will add.
> > > >
> > > > and it should certainly be designed in a way
> > > > > that it does not need compat support.
> > > >
> > > > The compat_ioctl handler just casts the compat_ptr, I believe this
> > > > should be done unless the ioctl is globaly registered in
> > > > fs/compat_ioctl.c, but I might be wrong.
> > >
> > > You shouldn't need a compat ioctl for anything new that is added,
> > > unless your api is really messed up.  Please test to be sure, and
> > > not use a compat ioctl at all, it isn't that hard to do.
> >
> > compat_ioctl is called anyhow when CONFIG_COMPAT is set, there is no
> > way around it, or I'm missing something?  Actually there is no more
> > than that for the COMPAT support in this code.
> 
> If you don't provide a compat_ioctl() all should be fine, right?

No,  this doesn't work the driver has to provide compat_ioctl

You would expect something like
if (!f.file->f_op->compat_ioctl) {
                        error = f_op->f.file->f_op->unlocked_ioctl((f.file, cmd, compat_ptr(arg))
}
But there is no such code under  fs/compat_ioctl.c 

The translation has to implemented by the device driver or registered  directly in fs/compat_ioct.c in do_ioctl_trans or ioctl_pointer[]

If compat_ioct is not provided the application is receiving
: ioctl failure -1: Inappropriate ioctl for device
	
Thanks
Tomas

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek Sept. 4, 2016, 8:20 p.m. UTC | #10
On Sun 2016-09-04 11:35:33, Winkler, Tomas wrote:
> 
> > On Thu, Sep 01, 2016 at 08:05:26PM +0000, Winkler, Tomas wrote:
> > >
> > > >
> > > > On Sun, Aug 07, 2016 at 09:44:03AM +0000, Winkler, Tomas wrote:
> > > > > >
> > > > > > On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > > > > > > The user space API is achieved via two synchronous IOCTL.
> > > > > >
> > > > > > IOCTLs?
> > > > >
> > > > > Will fix
> > > > >
> > > > > > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > > > > > performed
> > > > > > > by the framework on behalf the user and second,
> > > > > > > RPMB_IOC_SEQ_CMD
> > > > > > where
> > > > > > > the whole RPMB sequence including RESULT_READ is supplied by
> > > > > > > the
> > > > caller.
> > > > > > > The latter is intended for  easier adjusting  of the
> > > > > > > applications that use MMC_IOC_MULTI_CMD ioctl.
> > > > > >
> > > > > > Why "  "?
> > > > > Not sure I there is enough clue in your question.
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > >
> > > > > > > +
> > > > > > > +static long rpmb_ioctl(struct file *fp, unsigned int cmd,
> > > > > > > +unsigned long arg) {
> > > > > > > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > > > > > > +
> > > > > > > +#ifdef CONFIG_COMPAT
> > > > > > > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > > > > > > +			      unsigned long arg)
> > > > > > > +{
> > > > > > > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > > > > > > +}
> > > > > > > +#endif /* CONFIG_COMPAT */
> > > > > >
> > > > > > Description of the ioctl is missing,
> > > > > Will add.
> > > > >
> > > > > and it should certainly be designed in a way
> > > > > > that it does not need compat support.
> > > > >
> > > > > The compat_ioctl handler just casts the compat_ptr, I believe this
> > > > > should be done unless the ioctl is globaly registered in
> > > > > fs/compat_ioctl.c, but I might be wrong.
> > > >
> > > > You shouldn't need a compat ioctl for anything new that is added,
> > > > unless your api is really messed up.  Please test to be sure, and
> > > > not use a compat ioctl at all, it isn't that hard to do.
> > >
> > > compat_ioctl is called anyhow when CONFIG_COMPAT is set, there is no
> > > way around it, or I'm missing something?  Actually there is no more
> > > than that for the COMPAT support in this code.
> > 
> > If you don't provide a compat_ioctl() all should be fine, right?
> 
> No,  this doesn't work the driver has to provide compat_ioctl
> 
> You would expect something like
> if (!f.file->f_op->compat_ioctl) {
>                         error = f_op->f.file->f_op->unlocked_ioctl((f.file, cmd, compat_ptr(arg))
> }
> But there is no such code under  fs/compat_ioctl.c 
> 
> The translation has to implemented by the device driver or registered  directly in fs/compat_ioct.c in do_ioctl_trans or ioctl_pointer[]
> 
> If compat_ioct is not provided the application is receiving
> : ioctl failure -1: Inappropriate ioctl for device

Care to submit a patch? We should not really have to include
compat_ioctl support if it is already compatible...

Or maybe better provide empty function drivers can fill in when
compatible...?

									Pavel
Winkler, Tomas Sept. 4, 2016, 8:56 p.m. UTC | #11
\
> Subject: Re: [PATCH v5 4/8] char: rpmb: provide a user space interface
> 
> On Sun 2016-09-04 11:35:33, Winkler, Tomas wrote:
> >
> > > On Thu, Sep 01, 2016 at 08:05:26PM +0000, Winkler, Tomas wrote:
> > > >
> > > > >
> > > > > On Sun, Aug 07, 2016 at 09:44:03AM +0000, Winkler, Tomas wrote:
> > > > > > >
> > > > > > > On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> > > > > > > > The user space API is achieved via two synchronous IOCTL.
> > > > > > >
> > > > > > > IOCTLs?
> > > > > >
> > > > > > Will fix
> > > > > >
> > > > > > > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles
> > > > > > > > is
> > > > > > > performed
> > > > > > > > by the framework on behalf the user and second,
> > > > > > > > RPMB_IOC_SEQ_CMD
> > > > > > > where
> > > > > > > > the whole RPMB sequence including RESULT_READ is supplied
> > > > > > > > by the
> > > > > caller.
> > > > > > > > The latter is intended for  easier adjusting  of the
> > > > > > > > applications that use MMC_IOC_MULTI_CMD ioctl.
> > > > > > >
> > > > > > > Why "  "?
> > > > > > Not sure I there is enough clue in your question.
> > > > > > >
> > > > > > > >
> > > > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > > >
> > > > > > > > +
> > > > > > > > +static long rpmb_ioctl(struct file *fp, unsigned int cmd,
> > > > > > > > +unsigned long arg) {
> > > > > > > > +	return __rpmb_ioctl(fp, cmd, (void __user *)arg); }
> > > > > > > > +
> > > > > > > > +#ifdef CONFIG_COMPAT
> > > > > > > > +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> > > > > > > > +			      unsigned long arg) {
> > > > > > > > +	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
> > > > > > > > +}
> > > > > > > > +#endif /* CONFIG_COMPAT */
> > > > > > >
> > > > > > > Description of the ioctl is missing,
> > > > > > Will add.
> > > > > >
> > > > > > and it should certainly be designed in a way
> > > > > > > that it does not need compat support.
> > > > > >
> > > > > > The compat_ioctl handler just casts the compat_ptr, I believe
> > > > > > this should be done unless the ioctl is globaly registered in
> > > > > > fs/compat_ioctl.c, but I might be wrong.
> > > > >
> > > > > You shouldn't need a compat ioctl for anything new that is
> > > > > added, unless your api is really messed up.  Please test to be
> > > > > sure, and not use a compat ioctl at all, it isn't that hard to do.
> > > >
> > > > compat_ioctl is called anyhow when CONFIG_COMPAT is set, there is
> > > > no way around it, or I'm missing something?  Actually there is no
> > > > more than that for the COMPAT support in this code.
> > >
> > > If you don't provide a compat_ioctl() all should be fine, right?
> >
> > No,  this doesn't work the driver has to provide compat_ioctl
> >
> > You would expect something like
> > if (!f.file->f_op->compat_ioctl) {
> >                         error =
> > f_op->f.file->f_op->unlocked_ioctl((f.file, cmd, compat_ptr(arg)) }
> > But there is no such code under  fs/compat_ioctl.c
> >
> > The translation has to implemented by the device driver or registered
> > directly in fs/compat_ioct.c in do_ioctl_trans or ioctl_pointer[]
> >
> > If compat_ioct is not provided the application is receiving
> > : ioctl failure -1: Inappropriate ioctl for device
> 
> Care to submit a patch? We should not really have to include compat_ioctl
> support if it is already compatible...
> 
> Or maybe better provide empty function drivers can fill in when
> compatible...?
> 
I'm not sure it is so simple to generallize, because ioctl selection logic in compat_ioctl has already a default actions and second because 'arg' my not be a pointer. 
Maybe a beter option would be to provide a function in spirit generic_ fs functions let say compat_generic_ioct() that would just cal unlocked_ioctl(fp, cmd, compat_ptr(arg))

Thanks
Tomas


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 9369d3b0f09a..38f57332bf8e 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -320,6 +320,7 @@  Code  Seq#(hex)	Include File		Comments
 0xB1	00-1F	PPPoX			<mailto:mostrows@styx.uwaterloo.ca>
 0xB3	00	linux/mmc/ioctl.h
 0xB4	00-0F	linux/gpio.h		<mailto:linux-gpio@vger.kernel.org>
+0xB5	00-01	linux/uapi/linux/rpmb.h <mailto:linux-mei@linux.intel.com>
 0xC0	00-0F	linux/usb/iowarrior.h
 0xCA	00-0F	uapi/misc/cxl.h
 0xCA	80-8F	uapi/scsi/cxlflash_ioctl.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 02a6f6f10f48..c7c08c6c041b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9711,6 +9711,7 @@  M:	Tomas Winkler <tomas.winkler@intel.com>
 L:	linux-kernel@vger.kernel.org
 S:	Supported
 F:	drivers/char/rpmb/*
+F:	include/uapi/linux/rpmb.h
 F:	include/linux/rpmb.h
 F:	Documentation/ABI/testing/sysfs-class-rpmb
 
diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig
index c5e6e909efce..6794be9fcc5e 100644
--- a/drivers/char/rpmb/Kconfig
+++ b/drivers/char/rpmb/Kconfig
@@ -6,3 +6,10 @@  config RPMB
 	  access RPMB partition.
 
 	  If unsure, select N.
+
+config RPMB_INTF_DEV
+	bool "RPMB character device interface /dev/rpmbN"
+	depends on RPMB
+	help
+	  Say yes here if you want to access RPMB from user space
+	  via character device interface /dev/rpmb%d
diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile
index 812b3ed264c0..b5dc087b1299 100644
--- a/drivers/char/rpmb/Makefile
+++ b/drivers/char/rpmb/Makefile
@@ -1,4 +1,5 @@ 
 obj-$(CONFIG_RPMB) += rpmb.o
 rpmb-objs += core.o
+rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
 
 ccflags-y += -D__CHECK_ENDIAN__
diff --git a/drivers/char/rpmb/cdev.c b/drivers/char/rpmb/cdev.c
new file mode 100644
index 000000000000..f3ad3444f76d
--- /dev/null
+++ b/drivers/char/rpmb/cdev.c
@@ -0,0 +1,269 @@ 
+/*
+ * Copyright (C) 2015-2016 Intel Corp. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/compat.h>
+#include <linux/slab.h>
+#include <linux/capability.h>
+
+#include <linux/rpmb.h>
+
+#include "rpmb-cdev.h"
+
+static dev_t rpmb_devt;
+#define RPMB_MAX_DEVS  MINORMASK
+
+#define RPMB_DEV_OPEN    0  /** single open bit (position) */
+
+/**
+ * rpmb_open - the open function
+ *
+ * @inode: pointer to inode structure
+ * @fp: pointer to file structure
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int rpmb_open(struct inode *inode, struct file *fp)
+{
+	struct rpmb_dev *rdev;
+
+	rdev = container_of(inode->i_cdev, struct rpmb_dev, cdev);
+	if (!rdev)
+		return -ENODEV;
+
+	/* the rpmb is single open! */
+	if (test_and_set_bit(RPMB_DEV_OPEN, &rdev->status))
+		return -EBUSY;
+
+	mutex_lock(&rdev->lock);
+
+	fp->private_data = rdev;
+
+	mutex_unlock(&rdev->lock);
+
+	return nonseekable_open(inode, fp);
+}
+
+static int rpmb_release(struct inode *inode, struct file *fp)
+{
+	struct rpmb_dev *rdev = fp->private_data;
+
+	clear_bit(RPMB_DEV_OPEN, &rdev->status);
+
+	return 0;
+}
+
+static int rpmb_cmd_copy_from_user(struct rpmb_cmd *cmd,
+				   struct rpmb_ioc_cmd __user *ucmd)
+{
+	size_t sz;
+	struct rpmb_frame *frames;
+
+	if (get_user(cmd->flags, &ucmd->flags))
+		return -EFAULT;
+
+	if (get_user(cmd->nframes, &ucmd->nframes))
+		return -EFAULT;
+
+	/* FIXME get real number this is from MMC_IOC_MAX_BYTES */
+	if (cmd->nframes > 256)
+		return -EINVAL;
+
+	sz = cmd->nframes * sizeof(struct rpmb_frame);
+	frames = memdup_user(u64_to_user_ptr(ucmd->frames_ptr), sz);
+	if (IS_ERR(frames))
+		return PTR_ERR(frames);
+
+	cmd->frames = frames;
+	return 0;
+}
+
+static int rpmb_cmd_copy_to_user(struct rpmb_ioc_cmd __user *ucmd,
+				 struct rpmb_cmd *cmd)
+{
+	size_t sz;
+
+	sz = cmd->nframes * sizeof(struct rpmb_frame);
+	if (copy_to_user(u64_to_user_ptr(ucmd->frames_ptr), cmd->frames, sz))
+		return -EFAULT;
+
+	return 0;
+}
+
+static long rpmb_ioctl_seq_cmd(struct rpmb_dev *rdev,
+			       struct rpmb_ioc_seq_cmd __user *ptr)
+{
+	__u64 ncmds;
+	struct rpmb_cmd *cmds;
+	struct rpmb_ioc_cmd __user *ucmds;
+
+	int i;
+	int ret;
+
+	/* The caller must have CAP_SYS_RAWIO, like mmc ioctl */
+	if (!capable(CAP_SYS_RAWIO))
+		return -EPERM;
+
+	/* some archs have issues with 64bit get_user */
+	if (copy_from_user(&ncmds, &ptr->num_of_cmds, sizeof(ncmds)))
+		return -EFAULT;
+
+	if (ncmds > 3) {
+		dev_err(&rdev->dev, "supporting up to 3 packets (%llu)\n",
+			ncmds);
+		return -EINVAL;
+	}
+
+	cmds = kcalloc(ncmds, sizeof(*cmds), GFP_KERNEL);
+	if (!cmds)
+		return -ENOMEM;
+
+	ucmds = (struct rpmb_ioc_cmd __user *)ptr->cmds;
+	for (i = 0; i < ncmds; i++) {
+		ret = rpmb_cmd_copy_from_user(&cmds[i], &ucmds[i]);
+		if (ret)
+			goto out;
+	}
+
+	ret = rpmb_cmd_seq(rdev, cmds, ncmds);
+	if (ret)
+		goto out;
+
+	for (i = 0; i < ncmds; i++) {
+		ret = rpmb_cmd_copy_to_user(&ucmds[i], &cmds[i]);
+		if (ret)
+			goto out;
+	}
+out:
+	for (i = 0; i < ncmds; i++)
+		kfree(cmds[i].frames);
+	kfree(cmds);
+	return ret;
+}
+
+static long rpmb_ioctl_req_cmd(struct rpmb_dev *rdev,
+			       struct rpmb_ioc_req_cmd __user *ptr)
+{
+	struct rpmb_data rpmbd;
+	u64 req_type;
+	int ret;
+
+
+	/* some archs have issues with 64bit get_user */
+	if (copy_from_user(&req_type, &ptr->req_type, sizeof(req_type)))
+		return -EFAULT;
+
+	if (req_type >= U16_MAX)
+		return -EINVAL;
+
+	memset(&rpmbd, 0, sizeof(rpmbd));
+
+	rpmbd.req_type = req_type & 0xFFFF;
+
+	ret = rpmb_cmd_copy_from_user(&rpmbd.icmd, &ptr->icmd);
+	if (ret)
+		goto out;
+
+	ret = rpmb_cmd_copy_from_user(&rpmbd.ocmd, &ptr->ocmd);
+	if (ret)
+		goto out;
+
+	ret = rpmb_cmd_req(rdev, &rpmbd);
+	if (ret)
+		goto out;
+
+	ret = rpmb_cmd_copy_to_user(&ptr->ocmd, &rpmbd.ocmd);
+
+out:
+	kfree(rpmbd.icmd.frames);
+	kfree(rpmbd.ocmd.frames);
+	return ret;
+}
+
+static long __rpmb_ioctl(struct file *fp, unsigned int cmd, void __user *arg)
+{
+	struct rpmb_dev *rdev = fp->private_data;
+
+	switch (cmd) {
+	case RPMB_IOC_REQ_CMD:
+		return rpmb_ioctl_req_cmd(rdev, arg);
+	case RPMB_IOC_SEQ_CMD:
+		return rpmb_ioctl_seq_cmd(rdev, arg);
+	default:
+		dev_err(&rdev->dev, "unsupported ioctl 0x%x.\n", cmd);
+		return -ENOIOCTLCMD;
+	}
+}
+
+static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+	return __rpmb_ioctl(fp, cmd, (void __user *)arg);
+}
+
+#ifdef CONFIG_COMPAT
+static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
+			      unsigned long arg)
+{
+	return	__rpmb_ioctl(fp, cmd, compat_ptr(arg));
+}
+#endif /* CONFIG_COMPAT */
+
+static const struct file_operations rpmb_fops = {
+	.open           = rpmb_open,
+	.release        = rpmb_release,
+	.unlocked_ioctl = rpmb_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl   = rpmb_compat_ioctl,
+#endif
+	.owner          = THIS_MODULE,
+	.llseek         = noop_llseek,
+};
+
+void rpmb_cdev_prepare(struct rpmb_dev *rdev)
+{
+	rdev->dev.devt = MKDEV(MAJOR(rpmb_devt), rdev->id);
+	rdev->cdev.owner = THIS_MODULE;
+	cdev_init(&rdev->cdev, &rpmb_fops);
+}
+
+void rpmb_cdev_add(struct rpmb_dev *rdev)
+{
+	cdev_add(&rdev->cdev, rdev->dev.devt, 1);
+}
+
+void rpmb_cdev_del(struct rpmb_dev *rdev)
+{
+	if (rdev->dev.devt)
+		cdev_del(&rdev->cdev);
+}
+
+int __init rpmb_cdev_init(void)
+{
+	int ret;
+
+	ret = alloc_chrdev_region(&rpmb_devt, 0, RPMB_MAX_DEVS, "rpmb");
+	if (ret < 0)
+		pr_err("unable to allocate char dev region\n");
+
+	return ret;
+}
+
+void __exit rpmb_cdev_exit(void)
+{
+	if (rpmb_devt)
+		unregister_chrdev_region(rpmb_devt, RPMB_MAX_DEVS);
+}
diff --git a/drivers/char/rpmb/core.c b/drivers/char/rpmb/core.c
index 63271c7ed072..f2d244956d83 100644
--- a/drivers/char/rpmb/core.c
+++ b/drivers/char/rpmb/core.c
@@ -20,6 +20,7 @@ 
 #include <linux/slab.h>
 
 #include <linux/rpmb.h>
+#include "rpmb-cdev.h"
 
 static DEFINE_IDA(rpmb_ida);
 
@@ -390,6 +391,7 @@  int rpmb_dev_unregister(struct device *dev)
 	rpmb_dev_put(rdev);
 
 	mutex_lock(&rdev->lock);
+	rpmb_cdev_del(rdev);
 	device_del(&rdev->dev);
 	mutex_unlock(&rdev->lock);
 
@@ -440,10 +442,14 @@  struct rpmb_dev *rpmb_dev_register(struct device *dev,
 	rdev->dev.parent = dev;
 	rdev->dev.groups = rpmb_attr_groups;
 
+	rpmb_cdev_prepare(rdev);
+
 	ret = device_register(&rdev->dev);
 	if (ret)
 		goto exit;
 
+	rpmb_cdev_add(rdev);
+
 	dev_dbg(&rdev->dev, "registered disk\n");
 
 	return rdev;
@@ -460,11 +466,12 @@  static int __init rpmb_init(void)
 {
 	ida_init(&rpmb_ida);
 	class_register(&rpmb_class);
-	return 0;
+	return rpmb_cdev_init();
 }
 
 static void __exit rpmb_exit(void)
 {
+	rpmb_cdev_exit();
 	class_unregister(&rpmb_class);
 	ida_destroy(&rpmb_ida);
 }
diff --git a/drivers/char/rpmb/rpmb-cdev.h b/drivers/char/rpmb/rpmb-cdev.h
new file mode 100644
index 000000000000..5fb41e586ba9
--- /dev/null
+++ b/drivers/char/rpmb/rpmb-cdev.h
@@ -0,0 +1,25 @@ 
+/*
+ * Copyright (C) 2015-2016 Intel Corp. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#ifdef CONFIG_RPMB_INTF_DEV
+int __init rpmb_cdev_init(void);
+void __exit rpmb_cdev_exit(void);
+void rpmb_cdev_prepare(struct rpmb_dev *rdev);
+void rpmb_cdev_add(struct rpmb_dev *rdev);
+void rpmb_cdev_del(struct rpmb_dev *rdev);
+#else
+static inline int __init rpmb_cdev_init(void) { return 0; }
+static inline void __exit rpmb_cdev_exit(void) {}
+static inline void rpmb_cdev_prepare(struct rpmb_dev *rdev) {}
+static inline void rpmb_cdev_add(struct rpmb_dev *rdev) {}
+static inline void rpmb_cdev_del(struct rpmb_dev *rdev) {}
+#endif /* CONFIG_RPMB_INTF_DEV */
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index c6e49ade92e5..c383d8d027e9 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -15,79 +15,11 @@ 
 
 #include <linux/types.h>
 #include <linux/device.h>
+#include <linux/cdev.h>
+#include <uapi/linux/rpmb.h>
 #include <linux/kref.h>
 
 /**
- * struct rpmb_frame - rpmb frame as defined by specs
- *
- * @stuff        : stuff bytes
- * @key_mac      : The authentication key or the message authentication
- *                 code (MAC) depending on the request/response type.
- *                 The MAC will be delivered in the last (or the only)
- *                 block of data.
- * @data         : Data to be written or read by signed access.
- * @nonce        : Random number generated by the host for the requests
- *                 and copied to the response by the RPMB engine.
- * @write_counter: Counter value for the total amount of the successful
- *                 authenticated data write requests made by the host.
- * @addr         : Address of the data to be programmed to or read
- *                 from the RPMB. Address is the serial number of
- *                 the accessed block (half sector 256B).
- * @block_count  : Number of blocks (half sectors, 256B) requested to be
- *                 read/programmed.
- * @result       : Includes information about the status of the write counter
- *                 (valid, expired) and result of the access made to the RPMB.
- * @req_resp     : Defines the type of request and response to/from the memory.
- */
-struct rpmb_frame {
-	u8     stuff[196];
-	u8     key_mac[32];
-	u8     data[256];
-	u8     nonce[16];
-	__be32 write_counter;
-	__be16 addr;
-	__be16 block_count;
-	__be16 result;
-	__be16 req_resp;
-} __packed;
-
-#define RPMB_PROGRAM_KEY       0x1    /* Program RPMB Authentication Key */
-#define RPMB_GET_WRITE_COUNTER 0x2    /* Read RPMB write counter */
-#define RPMB_WRITE_DATA        0x3    /* Write data to RPMB partition */
-#define RPMB_READ_DATA         0x4    /* Read data from RPMB partition */
-#define RPMB_RESULT_READ       0x5    /* Read result request  (Internal) */
-
-#define RPMB_REQ2RESP(_OP) ((_OP) << 8)
-#define RPMB_RESP2REQ(_OP) ((_OP) >> 8)
-
-/**
- * enum rpmb_op_result - rpmb operation results
- *
- * @RPMB_ERR_OK      : operation successful
- * @RPMB_ERR_GENERAL : general failure
- * @RPMB_ERR_AUTH    : mac doesn't match or ac calculation failure
- * @RPMB_ERR_COUNTER : counter doesn't match or counter increment failure
- * @RPMB_ERR_ADDRESS : address out of range or wrong address alignment
- * @RPMB_ERR_WRITE   : data, counter, or result write failure
- * @RPMB_ERR_READ    : data, counter, or result read failure
- * @RPMB_ERR_NO_KEY  : authentication key not yet programmed
- *
- * @RPMB_ERR_COUNTER_EXPIRED:  counter expired
- */
-enum rpmb_op_result {
-	RPMB_ERR_OK      = 0x0000,
-	RPMB_ERR_GENERAL = 0x0001,
-	RPMB_ERR_AUTH    = 0x0002,
-	RPMB_ERR_COUNTER = 0x0003,
-	RPMB_ERR_ADDRESS = 0x0004,
-	RPMB_ERR_WRITE   = 0x0005,
-	RPMB_ERR_READ    = 0x0006,
-	RPMB_ERR_NO_KEY  = 0x0007,
-
-	RPMB_ERR_COUNTER_EXPIRED = 0x0080
-};
-
-/**
  * enum rpmb_type - type of underlaying storage technology
  *
  * @RPMB_TYPE_ANY   : any type used for search only
@@ -104,9 +36,6 @@  enum rpmb_type {
 
 extern struct class rpmb_class;
 
-#define RPMB_F_WRITE     BIT(0)
-#define RPMB_F_REL_WRITE BIT(1)
-
 /**
  * struct rpmb_cmd: rpmb access command
  *
@@ -160,12 +89,18 @@  struct rpmb_ops {
  * @lock       : the device lock
  * @dev        : device
  * @id         : device id
+ * @cdev       : character dev
+ * @status     : device status
  * @ops        : operation exported by block layer
  */
 struct rpmb_dev {
 	struct mutex lock; /* device serialization lock */
 	struct device dev;
 	int    id;
+#ifdef CONFIG_RPMB_INTF_DEV
+	struct cdev cdev;
+	unsigned long status;
+#endif /* CONFIG_RPMB_INTF_DEV */
 	const struct rpmb_ops *ops;
 };
 
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 8bdae34d1f9a..4a02adb48b7a 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -360,6 +360,7 @@  header-y += rio_mport_cdev.h
 header-y += romfs_fs.h
 header-y += rose.h
 header-y += route.h
+header-y += rpmb.h
 header-y += rtc.h
 header-y += rtnetlink.h
 header-y += scc.h
diff --git a/include/uapi/linux/rpmb.h b/include/uapi/linux/rpmb.h
new file mode 100644
index 000000000000..a6fa78e03c4f
--- /dev/null
+++ b/include/uapi/linux/rpmb.h
@@ -0,0 +1,152 @@ 
+/*
+ * Copyright (C) 2015-2016, Intel Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation;
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _UAPI_LINUX_RPMB_H_
+#define _UAPI_LINUX_RPMB_H_
+
+#include <linux/types.h>
+
+/**
+ * struct rpmb_frame - rpmb frame as defined by specs
+ *
+ * @stuff        : stuff bytes
+ * @key_mac      : The authentication key or the message authentication
+ *                 code (MAC) depending on the request/response type.
+ *                 The MAC will be delivered in the last (or the only)
+ *                 block of data.
+ * @data         : Data to be written or read by signed access.
+ * @nonce        : Random number generated by the host for the requests
+ *                 and copied to the response by the RPMB engine.
+ * @write_counter: Counter value for the total amount of the successful
+ *                 authenticated data write requests made by the host.
+ * @addr         : Address of the data to be programmed to or read
+ *                 from the RPMB. Address is the serial number of
+ *                 the accessed block (half sector 256B).
+ * @block_count  : Number of blocks (half sectors, 256B) requested to be
+ *                 read/programmed.
+ * @result       : Includes information about the status of the write counter
+ *                 (valid, expired) and result of the access made to the RPMB.
+ * @req_resp     : Defines the type of request and response to/from the memory.
+ */
+struct rpmb_frame {
+	__u8   stuff[196];
+	__u8   key_mac[32];
+	__u8   data[256];
+	__u8   nonce[16];
+	__be32 write_counter;
+	__be16 addr;
+	__be16 block_count;
+	__be16 result;
+	__be16 req_resp;
+} __attribute__((packed));
+
+#define RPMB_PROGRAM_KEY       0x1    /* Program RPMB Authentication Key */
+#define RPMB_GET_WRITE_COUNTER 0x2    /* Read RPMB write counter */
+#define RPMB_WRITE_DATA        0x3    /* Write data to RPMB partition */
+#define RPMB_READ_DATA         0x4    /* Read data from RPMB partition */
+#define RPMB_RESULT_READ       0x5    /* Read result request  (Internal) */
+
+#define RPMB_REQ2RESP(_OP) ((_OP) << 8)
+#define RPMB_RESP2REQ(_OP) ((_OP) >> 8)
+
+/* length of the part of the frame used for HMAC computation */
+#define hmac_data_len \
+	(sizeof(struct rpmb_frame) - offsetof(struct rpmb_frame, data))
+
+/**
+ * enum rpmb_op_result - rpmb operation results
+ *
+ * @RPMB_ERR_OK:       operation successful
+ * @RPMB_ERR_GENERAL:  general failure
+ * @RPMB_ERR_AUTH:     mac doesn't match or ac calculation failure
+ * @RPMB_ERR_COUNTER:  counter doesn't match or counter increment failure
+ * @RPMB_ERR_ADDRESS:  address out of range or wrong address alignment
+ * @RPMB_ERR_WRITE:    data, counter, or result write failure
+ * @RPMB_ERR_READ:     data, counter, or result read failure
+ * @RPMB_ERR_NO_KEY:   authentication key not yet programmed
+ *
+ * @RPMB_ERR_COUNTER_EXPIRED:  counter expired
+ */
+enum rpmb_op_result {
+	RPMB_ERR_OK      = 0x0000,
+	RPMB_ERR_GENERAL = 0x0001,
+	RPMB_ERR_AUTH    = 0x0002,
+	RPMB_ERR_COUNTER = 0x0003,
+	RPMB_ERR_ADDRESS = 0x0004,
+	RPMB_ERR_WRITE   = 0x0005,
+	RPMB_ERR_READ    = 0x0006,
+	RPMB_ERR_NO_KEY  = 0x0007,
+
+	RPMB_ERR_COUNTER_EXPIRED = 0x0080
+};
+
+#define RPMB_F_WRITE     (1UL << 0)
+#define RPMB_F_REL_WRITE (1UL << 1)
+
+/**
+ * struct rpmb_cmd: rpmb access command
+ *
+ * @flags:   command flags
+ *      0 - read command
+ *      1 - write commnad RPMB_F_WRITE
+ *      2 -  reliable write RPMB_F_REL_WRITE
+ * @nframes: number of rpmb frames in the command
+ * @frames_ptr:  a pointer to the list of rpmb frames
+ */
+struct rpmb_ioc_cmd {
+	__u32 flags;
+	__u32 nframes;
+	__aligned_u64 frames_ptr;
+};
+
+#define rpmb_ioc_cmd_set_frames(_cmd, _ptr) \
+	(_cmd).frames_ptr = (__aligned_u64)(intptr_t)(_ptr)
+
+#define rpmb_ioc_cmd_set(_cmd, _flags, _ptr, _n) do {         \
+	(_cmd).flags = (_flags);                              \
+	(_cmd).nframes = (_n);                                \
+	(_cmd).frames_ptr = (__aligned_u64)(intptr_t)(_ptr);  \
+} while (0)
+
+/**
+ * struct rpmb_ioc_req_cmd - rpmb operation request command
+ *
+ * @req_type: request type:  must match the in frame req_resp
+ *            program key
+ *            get write counter
+ *            write data
+ *            read data
+ * @icmd: input command
+ * @ocmd: output/result command
+ */
+struct rpmb_ioc_req_cmd {
+	__u64 req_type;
+	struct rpmb_ioc_cmd icmd;
+	struct rpmb_ioc_cmd ocmd;
+};
+
+/**
+ * struct rpmb_ioc_seq_cmd - rpmb command sequence
+ *
+ * @num_of_cmds: number of commands
+ * @cmds: list of rpmb commands
+ */
+struct rpmb_ioc_seq_cmd {
+	__u64 num_of_cmds;
+	struct rpmb_ioc_cmd cmds[0];
+};
+
+#define RPMB_IOC_REQ_CMD _IOWR(0xB5, 0, struct rpmb_ioc_req_cmd)
+#define RPMB_IOC_SEQ_CMD _IOWR(0xB5, 1, struct rpmb_ioc_seq_cmd)
+
+#endif /* _UAPI_LINUX_RPMB_H_ */