Re: [PATCH RFC] tpm_crb: excluding locality registers for ARM64

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

On Mon, Jul 10, 2017 at 02:52:34PM -0500, Jiandi An wrote:
> 
> Hi Guys,
> 
> In this patch https://patchwork.kernel.org/patch/9562247/
> Are the locality registers in crb_regs_head specific to x86?

IMHO, the ACPI specification for TPM2 has been a disaster. The spec is
too vauge and weird - this buisness with storing addresses inside a
buffer inside a memory map is insane.

This is allowing implementations to do all manner of crazy things that
make no sense at all.

> The quick fix that would make it work for ARM64 is to also exclude
> CRB_FL_CRB_SMC_START.

Well, when the ACPI extension for ACPI_TPM2_COMMAND_BUFFER_WITH_SMC
was defined, it needs to specify how to handle locality. If it is done
via the registers in crb_regs_head then the ACPI spec needs to define
how to locate those registers.

I suspect the test you pointed out is just poorly constructed:

        if (!(priv->flags & CRB_FL_ACPI_START)) {

It should be

   	if (sm == ACPI_TPM2_COMMAND_BUFFER || sm == ACPI_TPM2_MEMORY_MAPPED)

As those are the only two modes to define the register layout in
this way.

But, you will still need to implement locality support for
ACPI_TPM2_COMMAND_BUFFER_WITH_SMC.

> For example, in crb_map_io(),  there is a specific PTT HW bug workaround for
> x86.
> 
> 	/*
> 	 * PTT HW bug w/a: wake up the device to access
> 	 * possibly not retained registers.
> 	 */
> 	ret = crb_cmd_ready(dev, priv);
> 	if (ret)
> 		return ret;
> 
> crb_cmd_ready() does nothing for devices with ACPI-start method as it does
> not support goIdle and cmdReady bits and idle state management is not
> exposed to the host SW.  So I've done similar workaround to bypass by
> additionally excluding CRB_FL_CRB_SMC_START.

Again, I think these tests are backwards, a work around like this
should be a while list, not a black list.. So it should refer to
exactly the sm values the impacted chipsets would use.

Jason

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