diff mbox

Intel NUC and fTPM issue on 4.9.2

Message ID 20170216190329.GC7257@obsidianresearch.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jason Gunthorpe Feb. 16, 2017, 7:03 p.m. UTC 
On Thu, Feb 16, 2017 at 06:40:02PM +0000, Davide Guerri wrote:
>    Sorry I missed 1 line:
> 
>    [20417.678952] ACPI resource is [mem 0xfed40000-0xfed4087f flags 0x200]

The BIOS is broken.. That range declared in the ACPI is too small

Or the cmd_size is too big:

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

I have no idea which.

If we trust the ACPI table then the cmd_size is 2047 bytes

If we trust the register then it is 3967..

Try this?


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

Comments

Davide Guerri Feb. 16, 2017, 7:11 p.m. UTC | #1 
Yes.
I tried to crop the 2 sizes manually and it worked.
I tested a couple of user space commands and it seems to be working.

Later I will try with your patch.

Thanks!



Sent from my iPhone
> On 16 Feb 2017, at 19:03, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote:
> 
>> On Thu, Feb 16, 2017 at 06:40:02PM +0000, Davide Guerri wrote:
>>   Sorry I missed 1 line:
>> 
>>   [20417.678952] ACPI resource is [mem 0xfed40000-0xfed4087f flags 0x200]
> 
> The BIOS is broken.. That range declared in the ACPI is too small
> 
> Or the cmd_size is too big:
> 
>>   [20417.678990] map request is is [mem 0xfed40080-0xfed40fff flags 0x200]
> 
> I have no idea which.
> 
> If we trust the ACPI table then the cmd_size is 2047 bytes
> 
> If we trust the register then it is 3967..
> 
> Try this?
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index a7c870af916c3d..2d16cc4aa0f43b 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);
> @@ -243,6 +245,26 @@ static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
>    return priv->iobase + (new_res.start - io_res->start);
> }
> 
> +/*
> + * Work around broken BIOSs that return inconsistent values from the ACPI
> + * region vs the registers. Trust the ACPI region.
> + */
> +static u64 crb_clamp_cmd_size(struct device *dev, struct resource *io_res,
> +                  u64 start, u64 size)
> +{
> +    if (io_res->start > start || io_res->end < start)
> +        return size;
> +
> +    if (start + size - 1 <= io_res->end)
> +        return size;
> +
> +    dev_err(dev,
> +        FW_BUG "ACPI region does not cover the entire command/response buffer. %pr vs %llx %llx\n",
> +        io_res, start, size);
> +
> +    return io_res->end - start + 1;
> +}
> +
> static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
>              struct acpi_table_tpm2 *buf)
> {
> @@ -267,6 +289,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);
> @@ -278,14 +302,16 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
> 
>    cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) |
>          (u64) ioread32(&priv->cca->cmd_pa_low);
> -    cmd_size = ioread32(&priv->cca->cmd_size);
> +    cmd_size = crb_clamp_cmd_size(dev, &io_res, cmd_pa,
> +                      ioread32(&priv->cca->cmd_size));
>    priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size);
>    if (IS_ERR(priv->cmd))
>        return PTR_ERR(priv->cmd);
> 
>    memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
>    rsp_pa = le64_to_cpu(rsp_pa);
> -    rsp_size = ioread32(&priv->cca->rsp_size);
> +    rsp_size = crb_clamp_cmd_size(dev, &io_res, rsp_pa,
> +                      ioread32(&priv->cca->rsp_size));
> 
>    if (cmd_pa != rsp_pa) {
>        priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);

------------------------------------------------------------------------------
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. 16, 2017, 8 p.m. UTC | #2 
On Thu, Feb 16, 2017 at 12:03:29PM -0700, Jason Gunthorpe wrote:
> On Thu, Feb 16, 2017 at 06:40:02PM +0000, Davide Guerri wrote:
> >    Sorry I missed 1 line:
> > 
> >    [20417.678952] ACPI resource is [mem 0xfed40000-0xfed4087f flags 0x200]
> 
> The BIOS is broken.. That range declared in the ACPI is too small

The range declare in ACPI is too small. It should be one full page.

/Jarkko

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

Patch

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index a7c870af916c3d..2d16cc4aa0f43b 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);
@@ -243,6 +245,26 @@  static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
 	return priv->iobase + (new_res.start - io_res->start);
 }
 
+/*
+ * Work around broken BIOSs that return inconsistent values from the ACPI
+ * region vs the registers. Trust the ACPI region.
+ */
+static u64 crb_clamp_cmd_size(struct device *dev, struct resource *io_res,
+			      u64 start, u64 size)
+{
+	if (io_res->start > start || io_res->end < start)
+		return size;
+
+	if (start + size - 1 <= io_res->end)
+		return size;
+
+	dev_err(dev,
+		FW_BUG "ACPI region does not cover the entire command/response buffer. %pr vs %llx %llx\n",
+		io_res, start, size);
+
+	return io_res->end - start + 1;
+}
+
 static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 		      struct acpi_table_tpm2 *buf)
 {
@@ -267,6 +289,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);
@@ -278,14 +302,16 @@  static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
 
 	cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) |
 		  (u64) ioread32(&priv->cca->cmd_pa_low);
-	cmd_size = ioread32(&priv->cca->cmd_size);
+	cmd_size = crb_clamp_cmd_size(dev, &io_res, cmd_pa,
+				      ioread32(&priv->cca->cmd_size));
 	priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size);
 	if (IS_ERR(priv->cmd))
 		return PTR_ERR(priv->cmd);
 
 	memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
 	rsp_pa = le64_to_cpu(rsp_pa);
-	rsp_size = ioread32(&priv->cca->rsp_size);
+	rsp_size = crb_clamp_cmd_size(dev, &io_res, rsp_pa,
+				      ioread32(&priv->cca->rsp_size));
 
 	if (cmd_pa != rsp_pa) {
 		priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);