ioctl API for vTPM driver

ioctl API for vTPM driver

[Jarkko Sakkinen]

Hi

Some follow-up question that have popped up while I've started to
write a test program for this feature mainly about ioctl API.  It's
better to be extremly cautious here because we will be stuck with this
ioctl forever.

1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
   vtpm_new_pair? It would be better if they both were either
   VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
   and vtpm_new_pair.
2. Is 'pair' or 'tuple' a better term?
3. Where is the documentation for the ioctl? I don't think I can
   merge this to my next branch before it exists.
4. I have forgotten why the major and minor numbers were returned.
   My guess is that they were returned so that a container could
   replicate the device? This is one reason why documentation is
   mandatory.

/Jarkko

------------------------------------------------------------------------------

Re: ioctl API for vTPM driver

[Jarkko Sakkinen]

On Sun, Mar 06, 2016 at 02:05:37PM +0200, Jarkko Sakkinen wrote:
> Hi
> 
> Some follow-up question that have popped up while I've started to
> write a test program for this feature mainly about ioctl API.  It's
> better to be extremly cautious here because we will be stuck with this
> ioctl forever.
> 
> 1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
>    vtpm_new_pair? It would be better if they both were either
>    VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
>    and vtpm_new_pair.
> 2. Is 'pair' or 'tuple' a better term?
> 3. Where is the documentation for the ioctl? I don't think I can
>    merge this to my next branch before it exists.
> 4. I have forgotten why the major and minor numbers were returned.
>    My guess is that they were returned so that a container could
>    replicate the device? This is one reason why documentation is
>    mandatory.

5. Is there any particular reason why 'tpm_dev_num' couldn't simply be
   'dev_num'?

/Jarkko

------------------------------------------------------------------------------

Re: ioctl API for vTPM driver

[Jarkko Sakkinen]

On Sun, Mar 06, 2016 at 02:11:03PM +0200, Jarkko Sakkinen wrote:
> On Sun, Mar 06, 2016 at 02:05:37PM +0200, Jarkko Sakkinen wrote:
> > Hi
> > 
> > Some follow-up question that have popped up while I've started to
> > write a test program for this feature mainly about ioctl API.  It's
> > better to be extremly cautious here because we will be stuck with this
> > ioctl forever.
> > 
> > 1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
> >    vtpm_new_pair? It would be better if they both were either
> >    VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
> >    and vtpm_new_pair.
> > 2. Is 'pair' or 'tuple' a better term?
> > 3. Where is the documentation for the ioctl? I don't think I can
> >    merge this to my next branch before it exists.
> > 4. I have forgotten why the major and minor numbers were returned.
> >    My guess is that they were returned so that a container could
> >    replicate the device? This is one reason why documentation is
> >    mandatory.
> 
> 5. Is there any particular reason why 'tpm_dev_num' couldn't simply be
>    'dev_num'?

I think you should still send a patch set after documentation has been
completed and we have addressed these items to the following mailing
lists:

* linux-kernel
* linux-api
* linux-doc
* tpmdd

Now these patches have circled only in the tpmdd mailing list which
is not sufficient. I just realized this recently that the relevant
mailing lists were completely missing (while starting to write a
test program).

For patches up to TPM_CHIP_FLAG_VIRTUAL you could downloaded patches
from my repository before posting the patch set since they include
also my reviewed/tested-by's.

Thanks.

/Jarkko

------------------------------------------------------------------------------

Re: ioctl API for vTPM driver

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 2891 bytes --]

Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/06/2016 
07:28:16 AM:

> From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> To: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Date: 03/06/2016 07:29 AM
> Subject: Re: [tpmdd-devel] ioctl API for vTPM driver
> 
> On Sun, Mar 06, 2016 at 02:11:03PM +0200, Jarkko Sakkinen wrote:
> > On Sun, Mar 06, 2016 at 02:05:37PM +0200, Jarkko Sakkinen wrote:
> > > Hi
> > > 
> > > Some follow-up question that have popped up while I've started to
> > > write a test program for this feature mainly about ioctl API.  It's
> > > better to be extremly cautious here because we will be stuck with 
this
> > > ioctl forever.
> > > 
> > > 1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
> > >    vtpm_new_pair? It would be better if they both were either
> > >    VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
> > >    and vtpm_new_pair.

Renamed it to vtpm_new_dev.

> > > 2. Is 'pair' or 'tuple' a better term?
> > > 3. Where is the documentation for the ioctl? I don't think I can
> > >    merge this to my next branch before it exists.

I'll add some later today or tomorrow.

> > > 4. I have forgotten why the major and minor numbers were returned.
> > >    My guess is that they were returned so that a container could
> > >    replicate the device? This is one reason why documentation is
> > >    mandatory.

Yes, it's there so that inside a container a device with that major and 
minor number can be created and device cgroups be setup.

> > 
> > 5. Is there any particular reason why 'tpm_dev_num' couldn't simply be
> >    'dev_num'?

Renamed to 'tpm_num' Updated my git repo.

> 
> I think you should still send a patch set after documentation has been
> completed and we have addressed these items to the following mailing
> lists:
> 
> * linux-kernel
> * linux-api
> * linux-doc
> * tpmdd

The whole patchset as 'v5' ?

> 
> Now these patches have circled only in the tpmdd mailing list which
> is not sufficient. I just realized this recently that the relevant
> mailing lists were completely missing (while starting to write a
> test program).
> 
> For patches up to TPM_CHIP_FLAG_VIRTUAL you could downloaded patches
> from my repository before posting the patch set since they include
> also my reviewed/tested-by's.

Ok, will sync.

    Stefan

> 
> Thanks.
> 
> /Jarkko
> 
> 
------------------------------------------------------------------------------
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
> 



[-- Attachment #1.2: Type: text/html, Size: 4174 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: ioctl API for vTPM driver

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1768 bytes --]

Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/06/2016 
07:28:16 AM:

> 
> On Sun, Mar 06, 2016 at 02:11:03PM +0200, Jarkko Sakkinen wrote:
> > On Sun, Mar 06, 2016 at 02:05:37PM +0200, Jarkko Sakkinen wrote:
> > > Hi
> > > 
> > > Some follow-up question that have popped up while I've started to
> > > write a test program for this feature mainly about ioctl API.  It's
> > > better to be extremly cautious here because we will be stuck with 
this
> > > ioctl forever.
> > > 
> > > 1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
> > >    vtpm_new_pair? It would be better if they both were either
> > >    VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
> > >    and vtpm_new_pair.
> > > 2. Is 'pair' or 'tuple' a better term?
> > > 3. Where is the documentation for the ioctl? I don't think I can
> > >    merge this to my next branch before it exists.
> > > 4. I have forgotten why the major and minor numbers were returned.
> > >    My guess is that they were returned so that a container could
> > >    replicate the device? This is one reason why documentation is
> > >    mandatory.
> > 
> > 5. Is there any particular reason why 'tpm_dev_num' couldn't simply be
> >    'dev_num'?
> 
> I think you should still send a patch set after documentation has been
> completed and we have addressed these items to the following mailing
> lists:


Here's the documentation patch:

https://github.com/stefanberger/linux/commit/bf1d11a67be64e580636c90cb8acbace80e16ab7

If you have suggestions or comments, let me know.

I adjusted the code to reflect the ENOSYS (previously EINVAL) in case of 
unsupported flags or unsupported ioctls. 

Regards,
   Stefan



[-- Attachment #1.2: Type: text/html, Size: 2506 bytes --]

[-- Attachment #2: Type: text/plain, Size: 267 bytes --]

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: ioctl API for vTPM driver

[Jason Gunthorpe]

On Sun, Mar 06, 2016 at 07:40:40PM -0500, Stefan Berger wrote:

>    If you have suggestions or comments, let me know.
>    I adjusted the code to reflect the ENOSYS (previously EINVAL) in case
>    of unsupported flags or unsupported ioctls.

Please don't use ENOSYS, the desire is that is only used by
systemcalls.

Jason

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

Re: ioctl API for vTPM driver

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 578 bytes --]

Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote on 03/07/2016 
03:32:06 PM:


> 
> On Sun, Mar 06, 2016 at 07:40:40PM -0500, Stefan Berger wrote:
> 
> >    If you have suggestions or comments, let me know.
> >    I adjusted the code to reflect the ENOSYS (previously EINVAL) in 
case
> >    of unsupported flags or unsupported ioctls.
> 
> Please don't use ENOSYS, the desire is that is only used by
> systemcalls.

Ok, found alternatives: ENOTSUPP for the flag and ENOIOCTLCMD for the 
unsupported ioctl ?

   Stefan



[-- Attachment #1.2: Type: text/html, Size: 805 bytes --]

[-- Attachment #2: Type: text/plain, Size: 267 bytes --]

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: ioctl API for vTPM driver

[Jason Gunthorpe]

On Mon, Mar 07, 2016 at 04:12:45PM -0500, Stefan Berger wrote:
>    Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote on 03/07/2016
>    03:32:06 PM:
>    >
>    > On Sun, Mar 06, 2016 at 07:40:40PM -0500, Stefan Berger wrote:
>    >
>    > >    If you have suggestions or comments, let me know.
>    > >    I adjusted the code to reflect the ENOSYS (previously EINVAL) in
>    case
>    > >    of unsupported flags or unsupported ioctls.
>    >
>    > Please don't use ENOSYS, the desire is that is only used by
>    > systemcalls.

>    Ok, found alternatives: ENOTSUPP for the flag ENOIOCTLCMD for the
>    unsupported ioctl ?

ENOIOCTLCMD might be OK, just make sure it gets converted to ENOTTY:

390  * Is it an unrecognized ioctl? The correct returns are either
391  * ENOTTY (final) or ENOIOCTLCMD ("I don't know this one, try a
392  * fallback"). ENOIOCTLCMD gets turned into ENOTTY by the ioctl
393  * code before returning.

ENOTSUPP is for NFS internal use only, can't be exported to user
space.

Try EOPNOTSUPP

Jason

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

Re: ioctl API for vTPM driver

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1496 bytes --]

Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote on 03/07/2016 
04:34:53 PM:

> Subject: Re: [tpmdd-devel] ioctl API for vTPM driver
> 
> On Mon, Mar 07, 2016 at 04:12:45PM -0500, Stefan Berger wrote:
> >    Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote on 
03/07/2016
> >    03:32:06 PM:
> >    >
> >    > On Sun, Mar 06, 2016 at 07:40:40PM -0500, Stefan Berger wrote:
> >    >
> >    > >    If you have suggestions or comments, let me know.
> >    > >    I adjusted the code to reflect the ENOSYS (previously 
EINVAL) in
> >    case
> >    > >    of unsupported flags or unsupported ioctls.
> >    >
> >    > Please don't use ENOSYS, the desire is that is only used by
> >    > systemcalls.
> 
> >    Ok, found alternatives: ENOTSUPP for the flag ENOIOCTLCMD for the
> >    unsupported ioctl ?
> 
> ENOIOCTLCMD might be OK, just make sure it gets converted to ENOTTY:
> 
> 390  * Is it an unrecognized ioctl? The correct returns are either
> 391  * ENOTTY (final) or ENOIOCTLCMD ("I don't know this one, try a
> 392  * fallback"). ENOIOCTLCMD gets turned into ENOTTY by the ioctl
> 393  * code before returning.
> 
> ENOTSUPP is for NFS internal use only, can't be exported to user
> space.

It's found in more then 300 files using that errno...

http://lxr.free-electrons.com/ident?i=ENOTSUPP

> 
> Try EOPNOTSUPP

Ok, I will use that one.

   Stefan

> 
> Jason
> 



[-- Attachment #1.2: Type: text/html, Size: 2293 bytes --]

[-- Attachment #2: Type: text/plain, Size: 267 bytes --]

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: ioctl API for vTPM driver

[Jason Gunthorpe]

On Mon, Mar 07, 2016 at 05:24:03PM -0500, Stefan Berger wrote:

>    It's found in more then 300 files using that errno...
>    [1]http://lxr.free-electrons.com/ident?i=ENOTSUPP

No idea if those leak to user space or not, but ENOTSUPP is not
exposed to user space:

grep -r 524 /usr/include/bits*

So there is nothing a user space app can do with it.

Jason

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

Re: ioctl API for vTPM driver

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 2091 bytes --]

Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/06/2016 
07:28:16 AM:


> 
> On Sun, Mar 06, 2016 at 02:11:03PM +0200, Jarkko Sakkinen wrote:
> > On Sun, Mar 06, 2016 at 02:05:37PM +0200, Jarkko Sakkinen wrote:
> > > Hi
> > > 
> > > Some follow-up question that have popped up while I've started to
> > > write a test program for this feature mainly about ioctl API.  It's
> > > better to be extremly cautious here because we will be stuck with 
this
> > > ioctl forever.
> > > 
> > > 1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
> > >    vtpm_new_pair? It would be better if they both were either
> > >    VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
> > >    and vtpm_new_pair.
> > > 2. Is 'pair' or 'tuple' a better term?
> > > 3. Where is the documentation for the ioctl? I don't think I can
> > >    merge this to my next branch before it exists.
> > > 4. I have forgotten why the major and minor numbers were returned.
> > >    My guess is that they were returned so that a container could
> > >    replicate the device? This is one reason why documentation is
> > >    mandatory.
> > 
> > 5. Is there any particular reason why 'tpm_dev_num' couldn't simply be
> >    'dev_num'?
> 
> I think you should still send a patch set after documentation has been
> completed and we have addressed these items to the following mailing
> lists:
> 
> * linux-kernel
> * linux-api
> * linux-doc
> * tpmdd
> 
> Now these patches have circled only in the tpmdd mailing list which
> is not sufficient. I just realized this recently that the relevant
> mailing lists were completely missing (while starting to write a
> test program).
> 
> For patches up to TPM_CHIP_FLAG_VIRTUAL you could downloaded patches
> from my repository before posting the patch set since they include
> also my reviewed/tested-by's.

I will post a v6 tomorrow again with the 3 vtpm related patches cc'ed to 
the above mailing lists. V6 addresses the errno's Jason has been 
suggesting.

   Stefan



[-- Attachment #1.2: Type: text/html, Size: 2659 bytes --]

[-- Attachment #2: Type: text/plain, Size: 267 bytes --]

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: ioctl API for vTPM driver

[Jarkko Sakkinen]

On Sun, Mar 06, 2016 at 11:21:42AM -0500, Stefan Berger wrote:
>    > Now these patches have circled only in the tpmdd mailing list which
>    > is not sufficient. I just realized this recently that the relevant
>    > mailing lists were completely missing (while starting to write a
>    > test program).
>    >
>    > For patches up to TPM_CHIP_FLAG_VIRTUAL you could downloaded patches
>    > from my repository before posting the patch set since they include
>    > also my reviewed/tested-by's.
> 
>    Ok, will sync.

In the mean time I'll continue porting Peters (Hüwe) Python script to
run on top of vtpm so that I can run my smoke tests [1] by using TPM 2.0
simulator.

>        Stefan

[1] git://git.infradead.org/users/jjs/tpm2-scripts.git

/Jarkko

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval

Re: ioctl API for vTPM driver

[Jarkko Sakkinen]

On Tue, Mar 08, 2016 at 12:24:10PM -0500, Stefan Berger wrote:
>    Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/06/2016
>    07:28:16 AM:
> 
>    >
>    > On Sun, Mar 06, 2016 at 02:11:03PM +0200, Jarkko Sakkinen wrote:
>    > > On Sun, Mar 06, 2016 at 02:05:37PM +0200, Jarkko Sakkinen wrote:
>    > > > Hi
>    > > >
>    > > > Some follow-up question that have popped up while I've started to
>    > > > write a test program for this feature mainly about ioctl API.  It's
>    > > > better to be extremly cautious here because we will be stuck with
>    this
>    > > > ioctl forever.
>    > > >
>    > > > 1. Why the ioctl name is VTPM_NEW_DEV but the struct name is
>    > > >    vtpm_new_pair? It would be better if they both were either
>    > > >    VTPM_NEW_DEV and vtpm_new_dev or alternatively VTPM_NEW_PAIR
>    > > >    and vtpm_new_pair.
>    > > > 2. Is 'pair' or 'tuple' a better term?
>    > > > 3. Where is the documentation for the ioctl? I don't think I can
>    > > >    merge this to my next branch before it exists.
>    > > > 4. I have forgotten why the major and minor numbers were returned.
>    > > >    My guess is that they were returned so that a container could
>    > > >    replicate the device? This is one reason why documentation is
>    > > >    mandatory.
>    > >
>    > > 5. Is there any particular reason why 'tpm_dev_num' couldn't simply be
>    > >    'dev_num'?
>    >
>    > I think you should still send a patch set after documentation has been
>    > completed and we have addressed these items to the following mailing
>    > lists:
>    >
>    > * linux-kernel
>    > * linux-api
>    > * linux-doc
>    > * tpmdd
>    >
>    > Now these patches have circled only in the tpmdd mailing list which
>    > is not sufficient. I just realized this recently that the relevant
>    > mailing lists were completely missing (while starting to write a
>    > test program).
>    >
>    > For patches up to TPM_CHIP_FLAG_VIRTUAL you could downloaded patches
>    > from my repository before posting the patch set since they include
>    > also my reviewed/tested-by's.
> 
>    I will post a v6 tomorrow again with the 3 vtpm related patches cc'ed to
>    the above mailing lists. V6 addresses the errno's Jason has been
>    suggesting.

I've used today improving this:

git://git.infradead.org/users/jjs/buildroot-tpmdd.git

By doing

make tpmdd_defconfig
make

You get a file called 'tpmdd.img' that you can flash to a USB stick. The
image is both legacy and UEFI bootable. The kernel is my latest master
and perf is included for tracing.

The compiled kernel has vtpm enabled.

There's a scripts called board/tpmdd/image.sh that does the image
generation magic. I've only tested it in Ubuntu environment.

As the next step I'll finish my test script for vtpm and use it and
TPM 2.0 simulator to test your functionality.

>       Stefan

/Jarkko

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval