2013-10-03 00:10:55

by Ashley Lai

[permalink] [raw]
Subject: Re: [tpmdd-devel] [PATCH 02/13] tpm atmel: Call request_region with the correct base

On Wed, 2013-10-02 at 00:00 +0200, Peter Hüwe wrote:
> Am Montag, 23. September 2013, 20:14:32 schrieb Jason Gunthorpe:
>
> > Commit e0dd03caf20d040a0a86b6bd74028ec9bda545f5
>
> > changed the code path here so that ateml_get_base_addr
>
> > no longer directly altered the tpm_vendor_specific
>
> > structure, and instead placed the base address on the stack.
>
> >
>
> > The commit missed updating the request_region call, which
>
> > would have resulted in request_region being called with 0
>
> > as the base address.
>
Good catch!!

> >
>
> > I don't know if request_region(0, ..) will fail, if so the
>
> > driver has been broken since 2006 and we should remove it
>
> > from the tree as it has no users.
>
>
>
> The Atmel TPM depends on PPC64 || HAS_IOPORT
>
>
>
> So request_region is defined as
>
> #define request_region(start,n,name)
> __request_region(&ioport_resource, (start), (n), (name), 0)
>
> http://lxr.free-electrons.com/source/include/linux/ioport.h?a=powerpc#L174
>
>
>
> with the final check in
>
> __request_resource(struct resource *root, struct resource *new)
>
> ...
>
> 204 if (end < start)
>
> 205 return root;
>
> 206 if (start < root->start)
>
> 207 return root;
>
> 208 if (end > root->end)
>
> 209 return root;
>
>
>
>
>
> with root being ioport_resource, ioport_resource->start = 0 and
> new->start being 0
>
> so at least the basic check should work.
>
>
>
> Nevertheless it most probably does request and reserve the wrong
> region and if anything else has or wants to request the region it
> might cause a problem.
>
Would be interesting to see what problem this may cause. Unfortunately I
don't have a machine to test this.

>
>
> Does anyone (Ashley? IBM Fellows? Ted?) have access to one of those
> ancient tpms and maybe a machine to use test them?
>
See above. I will keep looking and will let you know if I can find one.

>
>
>
>
>
>
> I somewhat have the feeling that we should maybe begin to deprecate
> the vendor specific 1.1 tpms...
>
I agree. If we have a machine to test and it fails then we know we don't
have a user for this.

>
>
>
>
>
>
> Reviewed-by: Peter Huewe <[email protected]>
>
> Signed-off-by: Peter Huewe <[email protected]>
>
>
>
> Staged here
> https://github.com/PeterHuewe/linux-tpmdd for-james
>
>
>
>
> Peter
>
>
>
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk
> _______________________________________________ tpmdd-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tpmdd-devel


2013-10-03 04:37:19

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [tpmdd-devel] [PATCH 02/13] tpm atmel: Call request_region with the correct base

On Wed, Oct 02, 2013 at 07:11:14PM -0500, Ashley D Lai wrote:

> > I somewhat have the feeling that we should maybe begin to deprecate
> > the vendor specific 1.1 tpms...

> I agree. If we have a machine to test and it fails then we know we don't
> have a user for this.

Is this driver is only used on IBM systems? If so, will IBM provide
support for those systems on RHEL7? If not the driver can probably
safely be dropped.

The trouble with these old drivers is that they don't follow modern
conventions (there are several little bugs at least) and nobody can
test them to safely fix them.

If you do find hardware, we can at least take a solid run at sprucing
up the testable drivers which should give them more life..

Jason

2013-10-04 17:21:47

by Joel Schopp

[permalink] [raw]
Subject: Re: [tpmdd-devel] [PATCH 02/13] tpm atmel: Call request_region with the correct base

On 10/02/2013 11:36 PM, Jason Gunthorpe wrote:
> On Wed, Oct 02, 2013 at 07:11:14PM -0500, Ashley D Lai wrote:
>
>>> I somewhat have the feeling that we should maybe begin to deprecate
>>> the vendor specific 1.1 tpms...
>
>> I agree. If we have a machine to test and it fails then we know we don't
>> have a user for this.
>
> Is this driver is only used on IBM systems? If so, will IBM provide
> support for those systems on RHEL7? If not the driver can probably
> safely be dropped.
>
> The trouble with these old drivers is that they don't follow modern
> conventions (there are several little bugs at least) and nobody can
> test them to safely fix them.
>
> If you do find hardware, we can at least take a solid run at sprucing
> up the testable drivers which should give them more life..
>
> Jason

We don't have any plans to support 1.1 tpms in any new major
distribution releases. I am in support of deprecating 1.1 tpms and just
supporting 1.2 and 2.0 going forward.

This is the direction we are taking with TrouSerS as well for what it's
worth. So that's at least one major user you won't see complain about
missing tpm 1.1 support.

-Joel