Re: [PATCH RFC] tpm_crb: Fix AMD Zen on-chip fTPM detection

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Fri, Jun 16, 2017 at 09:41:38PM +0200, Manuel Lauss wrote:
> On Fri, Jun 16, 2017 at 9:25 PM, Jason Gunthorpe
> <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> > On Fri, Jun 16, 2017 at 08:29:51PM +0200, Manuel Lauss wrote:
> >> This RFC patch fixes 2 issues which prevent the fTPM device from being initialized
> >> by the tpm_crb driver:
> >>
> >> 1) use devm_ioremap() instead of devm_ioremap_resource() to fix the following error
> >> due to it not allowing overlapping resources:
> >>
> >> tpm_crb MSFT0101:00: can't request region for resource [mem 0xdd84f000-0xdd84ffff]
> >> tpm_crb: probe of MSFT0101:00 failed with error -16
> >
> > No, we can't do this, it breaks other situations that rely on
> > request_resource.
> >
> > We already put a work around for a very similar problem on a different
> > system, do you have commit?
> >
> > commit b4e2eb0651ac3180a942d378b040c5cc045113ee
> > Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > Date:   Tue Feb 21 14:14:24 2017 -0700
> >
> >     tpm crb: Work around BIOS's that report the wrong ACPI region size
> >
> 
> Yes, that was actually the third problem I encountered on 4.11.5, but this
> patch does not fix point 1) above.
> 
> /proc/iomem looks like this before the probe attempt:
> dd759000-dd868fff : ACPI Non-volatile Storage
>   dd84b000-dd84bfff : MSFT0101:00
>   dd84f000-dd84ffff : MSFT0101:00
> 
> I have no idea yet why devm_request_mem_region() fails here. Is it because
> the ACPI NVS parent is already marked busy by the previous mapping
> of b000-bfff?

Hum. I wonder what does

static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
                      struct acpi_table_tpm2 *buf)
{
        ret = acpi_dev_get_resources(device, &resources, crb_check_resource,
                                     &io_res);

return in io_res for this arrangment? I'm guessing it isn't
dd759000-dd868fff ?

The issue might be that crb_check_resource/iomem assumes that there is
a single contiguous io region, while this device seems to have two of
them..

If so then the required patch is to iomap all of the io regsions that
crb_check_resource finds, and search all of them in
crb_map_res. Currently it only does the first one.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot