[PATCH] tpm_crb - workaround broken ACPI tables

From: "Safford, David (GE Global Research, US)" <david.safford@ge.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: "linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	"jarkko.sakkinen@linux.intel.com"
	<jarkko.sakkinen@linux.intel.com>,
	"Wiseman, Monty (GE Global Research, US)" <monty.wiseman@ge.com>
Subject: [PATCH] tpm_crb - workaround broken ACPI tables
Date: Thu, 11 Jul 2019 16:44:59 +0000	[thread overview]
Message-ID: <BCA04D5D9A3B764C9B7405BBA4D4A3C035EF7C56@ALPMBAPA12.e2k.ad.ge.com> (raw)
In-Reply-To: <20190711145850.GC25807@ziepe.ca>

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Thursday, July 11, 2019 10:59 AM
> To: Safford, David (GE Global Research, US) <david.safford@ge.com>
> Cc: linux-integrity@vger.kernel.org; jarkko.sakkinen@linux.intel.com;
> Wiseman, Monty (GE Global Research, US) <monty.wiseman@ge.com>
> Subject: EXT: Re: [PATCH] tpm_crb - workaround broken ACPI tables
> 
> On Thu, Jul 11, 2019 at 12:29:30PM +0000, Safford, David (GE Global Research,
> US) wrote:
> > Most x86 desktops and laptops have firmware TPMs which support the CRB
> > interface. Unfortunately, the linux tpm_crb driver depends on
> > perfectly correct ACPI tables, and there are a *lot* of systems out
> > there with broken tpm_crb entries. (Not one of my five tpm_crb systems
> > works with the existing driver.) While it is good to encourage vendors
> > to fix their firmware, many refuse ("It works on Windows"), leaving
> > users in the lurch.
> >
> > This patch adds a kernel parameter "tpm_crb.force=1" that works around
> > the problem in every case I have tested so far. Basically it does two
> > things:
> > 	- it trusts the cmd and resp addresses in the CRB registers
> > 	- it ignores all alleged IO resource conflicts
> >
> > Both workarounds make sense. If there really were an address conflict,
> > or if the register values really were wrong, the device would not be
> > working at all. And testing with this patch has shown that in every
> > case (so far), the problem has been bogus ACPI entries.
> >
> > This patch is against the upstream 5.2 kernel.
> >
> > Signed-off-by: David Safford <david.safford@ge.com>
> 
> I think we need to ask the ioresource and ACPI people how to fix this
> properly and automatically. Maybe some ACPI quirk or maybe we try to
> resorve the resoruce and fall back to forcing or something
> 
> I don't think t a module parameter is the right answer
> 
> Jaason

I would argue that this is the right place to fix the problem, as only the 
tpm_crb driver has the semantic knowledge to get the valid addresses
and sizes from the tpm_crb device registers dynamically. I'm not sure how 
you would fix things in the ACPI subsystem, without a monstrous table for
all the broken systems, or duplicating the driver's code. And I think we
want a module parameter, both for safety, and to encourage vendors to
fix their tables.

dave