Intel NUC and fTPM issue on 4.9.2

Commit Message

Winkler, Tomas Feb. 16, 2017, 10:36 p.m. UTC

This is a BIOS issue + kernel ACPI platform driver refusing to handle overlapping memory space. The issue here is that the ACPI platform driver is not created, however this should not prevent  the tpm_crb driver from working as it registers directly the ACPI device. For now you can suppress the message by adding the device in forbidden_id_list in drivers/acpi/acpi_platform.c

I’m already testing a fixed BIOS, not sure whether and when it will be available for your particular device.

Thanks
Tomas

From: Davide Guerri [mailto:[e-mail]]
Sent: Thursday, February 16, 2017 20:40
To: Jason Gunthorpe <[e-mail]>
Cc: [e-mail]
Subject: Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2

Sorry I missed 1 line:


[20417.678952] ACPI resource is [mem 0xfed40000-0xfed4087f flags 0x200]

[20417.678975] map request is is [mem 0xfed40040-0xfed4006f flags 0x200]

[20417.678990] map request is is [mem 0xfed40080-0xfed40fff flags 0x200]

[20417.678996] tpm_crb MSFT0101:00: can't request region for resource [mem 0xfed40080-0xfed40fff]

[20417.688797] tpm_crb: probe of MSFT0101:00 failed with error -16


On 16 February 2017 at 18:39, Davide Guerri <[e-mail]<mailto:[e-mail]>> wrote:

[20417.678975] map request is is [mem 0xfed40040-0xfed4006f flags 0x200]

[20417.678990] map request is is [mem 0xfed40080-0xfed40fff flags 0x200]

[20417.678996] tpm_crb MSFT0101:00: can't request region for resource [mem 0xfed40080-0xfed40fff]

[20417.688797] tpm_crb: probe of MSFT0101:00 failed with error -16



On 16 February 2017 at 18:26, Davide Guerri <[e-mail]<mailto:[e-mail]>> wrote:
No lines including the requested range:


fed10000-fed17fff : pnp 00:06

fed18000-fed18fff : pnp 00:06

fed19000-fed19fff : pnp 00:06

fed20000-fed3ffff : pnp 00:06

fed40000-fed4087f : MSFT0101:00

fed45000-fed8ffff : pnp 00:06

fed90000-fed93fff : pnp 00:06

fee00000-fee00fff : Local APIC

This is the NUC I am using, if that can be useful.


root@vhsv1:~# cat /sys/class/dmi/id/board_name /sys/class/dmi/id/board_version

NUC6i7KYB

H90766-404

I am compiling the module right as we speak, I will get back to you soon.

On 16 February 2017 at 18:19, Jason Gunthorpe <[e-mail]<mailto:[e-mail]>> wrote:
On Thu, Feb 16, 2017 at 06:10:43PM +0000, Davide Guerri wrote:
>    Hey thanks for the prompt reply.
>    I think you are interested in this:
>       fed40000-fed4087f : MSFT0101:00

Are there more lines below that?

Can you apply this patch and report what the results are?




--


[Davide Guerri on about.me]



Davide Guerri
about.me/davide_guerri






--


[Davide Guerri on about.me]



Davide Guerri
about.me/davide_guerri






--


[Davide Guerri on about.me]



Davide Guerri
about.me/davide_guerri
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Comments

Jason Gunthorpe Feb. 17, 2017, 4:43 p.m. UTC | #1

On Thu, Feb 16, 2017 at 10:36:40PM +0000, Winkler, Tomas wrote:
>    This is a BIOS issue + kernel ACPI platform driver refusing to handle
>    overlapping memory space. The issue here is that the ACPI platform
>    driver is not created, however this should not prevent  the tpm_crb
>    driver from working as it registers directly the ACPI device. For now
>    you can suppress the message by adding the device in forbidden_id_list
>    in drivers/acpi/acpi_platform.c
> 
>
>    I'm already testing a fixed BIOS, not sure whether and when it will be
>    available for your particular device.

Do you think we should go ahead with the patch I sent? Or should we
tell users they need a BIOS update?

Jason

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

Jarkko Sakkinen Feb. 17, 2017, 6:28 p.m. UTC | #2

On Fri, 2017-02-17 at 09:43 -0700, Jason Gunthorpe wrote:
> On Thu, Feb 16, 2017 at 10:36:40PM +0000, Winkler, Tomas wrote:

> >    This is a BIOS issue + kernel ACPI platform driver refusing to

> > handle

> >    overlapping memory space. The issue here is that the ACPI

> > platform

> >    driver is not created, however this should not prevent  the

> > tpm_crb

> >    driver from working as it registers directly the ACPI device.

> > For now

> >    you can suppress the message by adding the device in

> > forbidden_id_list

> >    in drivers/acpi/acpi_platform.c

> > 

> > 

> >    I'm already testing a fixed BIOS, not sure whether and when it

> > will be

> >    available for your particular device.

> 

> Do you think we should go ahead with the patch I sent? Or should we

> tell users they need a BIOS update?

> 

> Jason


I would rather review a patch with proper commit message than just pick
from an email thread in all cases with a proper explanation what the
patch does and why it is the right way to fix the issue.

I got an email privately and identified it as the very same issue. It
was in a NUC and the author said that he had updated the BIOS to the
latest version. That supports the view of doing some kind of workaround
to the driver, perhaps the way you did it.

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

Jason Gunthorpe Feb. 17, 2017, 7:01 p.m. UTC | #3

> I would rather review a patch with proper commit message than just pick
> from an email thread in all cases with a proper explanation what the
> patch does and why it is the right way to fix the issue.

Of course, but I am waiting for Davide to say the approach even works
..

Jason

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

Davide Guerri Feb. 17, 2017, 9:14 p.m. UTC | #4

Hey,
sorry fro the delayed reply.

Jason's patch works. I had no doubt it would, because I tried a qnd patch with manually cropped sizes. 
I have cleaned it up a little bit, removing printks.

Thanks again!

D. 



> On 17 Feb 2017, at 7:01 pm, Jason Gunthorpe <[e-mail]> wrote:
> 
>> I would rather review a patch with proper commit message than just pick
>> from an email thread in all cases with a proper explanation what the
>> patch does and why it is the right way to fix the issue.
> 
> Of course, but I am waiting for Davide to say the approach even works
> ..
> 
> Jason
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Winkler, Tomas Feb. 19, 2017, 10:28 p.m. UTC | #5

I need few days to get into BIOS to understand how we got to here at the first place,
Can you please send me the dmidecode of the NUC (even privately) so I can understand what exactly issue we have. The fixed BIOS I'm testing now is for the Kaby Lake platform.
Davide if would fill a bug for that via intel support page so we can follow up on that.

Thanks
Tomas

From: Davide Guerri [mailto:[e-mail]]
Sent: Friday, February 17, 2017 23:15
To: Jason Gunthorpe <[e-mail]>
Cc: Sakkinen, Jarkko <[e-mail]>; Winkler, Tomas <[e-mail]>; [e-mail]
Subject: Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2

Hey,
sorry fro the delayed reply.

Jason's patch works. I had no doubt it would, because I tried a qnd patch with manually cropped sizes.
I have cleaned it up a little bit, removing printks.

Thanks again!

D.


On 17 Feb 2017, at 7:01 pm, Jason Gunthorpe <[e-mail]<mailto:[e-mail]>> wrote:

I would rather review a patch with proper commit message than just pick
from an email thread in all cases with a proper explanation what the
patch does and why it is the right way to fix the issue.

Of course, but I am waiting for Davide to say the approach even works
..

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

Jarkko Sakkinen Feb. 20, 2017, 10:36 a.m. UTC | #6

On Sun, Feb 19, 2017 at 10:28:08PM +0000, Winkler, Tomas wrote:
>    I need few days to get into BIOS to understand how we got to here at the
>    first place,
> 
>    Can you please send me the dmidecode of the NUC (even privately) so I can
>    understand what exactly issue we have. The fixed BIOS I’m testing now is
>    for the Kaby Lake platform.
> 
>    Davide if would fill a bug for that via intel support page so we can
>    follow up on that.
> 
>     
> 
>    Thanks
> 
>    Tomas

I don't want to be impolite but can you please stop constantly sending
HTML messages to the list and top-posting. This has now happend multiple
times. Thank you.

/Jarkko

> 
>     
> 
>    From: Davide Guerri [mailto:[e-mail]]
>    Sent: Friday, February 17, 2017 23:15
>    To: Jason Gunthorpe <[e-mail]>
>    Cc: Sakkinen, Jarkko <[e-mail]>; Winkler, Tomas
>    <[e-mail]>; [e-mail]
>    Subject: Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2
> 
>     
> 
>    Hey,
> 
>    sorry fro the delayed reply.
> 
>     
> 
>    Jason's patch works. I had no doubt it would, because I tried a qnd patch
>    with manually cropped sizes. 
> 
>    I have cleaned it up a little bit, removing printks.
> 
>     
> 
>    Thanks again!
> 
>     
> 
>    D. 
> 
>     
> 
>     
> 
>      On 17 Feb 2017, at 7:01 pm, Jason Gunthorpe
>      <[e-mail]> wrote:
> 
>       
> 
>        I would rather review a patch with proper commit message than just
>        pick
>        from an email thread in all cases with a proper explanation what the
>        patch does and why it is the right way to fix the issue.
> 
>      Of course, but I am waiting for Davide to say the approach even works
>      ..
> 
>      Jason
> 
>     

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

> _______________________________________________
> tpmdd-devel mailing list
> [e-mail]
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel


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

Jarkko Sakkinen Feb. 20, 2017, 10:47 a.m. UTC | #7

On Fri, Feb 17, 2017 at 12:01:25PM -0700, Jason Gunthorpe wrote:
> > I would rather review a patch with proper commit message than just pick
> > from an email thread in all cases with a proper explanation what the
> > patch does and why it is the right way to fix the issue.
> 
> Of course, but I am waiting for Davide to say the approach even works
> ..
> 
> Jason

I'm not sure what would be a good function name but maybe something
that would state that this is not status quo but workaround for a
bug. Since I do not have any better suggestion keep the current
name if nothing comes up.

Probably a dev_err message would make sense here to state in klog
that something is wrong in the BIOS.

Since there are multiple people complaining about the issue I will
take this workaround even if the BIOS is fixed.

/Jarkko

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

Winkler, Tomas April 18, 2017, 2:49 p.m. UTC | #8

> -----Original Message-----
> From: Jarkko Sakkinen [mailto:[e-mail]]
> Sent: Monday, February 20, 2017 12:48
> To: Jason Gunthorpe <[e-mail]>
> Cc: Sakkinen, Jarkko <[e-mail]>; tpmdd-
> [e-mail]; [e-mail]
> Subject: Re: [tpmdd-devel] Intel NUC and fTPM issue on 4.9.2
> 
> On Fri, Feb 17, 2017 at 12:01:25PM -0700, Jason Gunthorpe wrote:
> > > I would rather review a patch with proper commit message than just
> > > pick from an email thread in all cases with a proper explanation
> > > what the patch does and why it is the right way to fix the issue.
> >
> > Of course, but I am waiting for Davide to say the approach even works
> > ..
> >
> > Jason
> 
> I'm not sure what would be a good function name but maybe something that
> would state that this is not status quo but workaround for a bug. Since I do not
> have any better suggestion keep the current name if nothing comes up.
> 
> Probably a dev_err message would make sense here to state in klog that
> something is wrong in the BIOS.
> 
> Since there are multiple people complaining about the issue I will take this
> workaround even if the BIOS is fixed.
> 
> /Jarkko


There is finally a BIOS fix 
https://downloadcenter.intel.com/download/26698/NUCs-BIOS-Update-KYSKLi70-86A- 
0045 and newer

Thanks
Tomas


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

Patch

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index a7c870af916c3d..acc54a03d6025d 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -233,6 +233,8 @@  static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
                .flags  = IORESOURCE_MEM,
        };

+       printk("map request is is %pr\n",&new_res);
+
        /* Detect a 64 bit address on a 32 bit system */
        if (start != new_res.start)
                return (void __iomem *) ERR_PTR(-EINVAL);
@@ -267,6 +269,8 @@  static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
                return -EINVAL;
        }

+       printk("ACPI resource is %pr\n",&io_res);
+
        priv->iobase = devm_ioremap_resource(dev, &io_res);
        if (IS_ERR(priv->iobase))
                return PTR_ERR(priv->iobase);