Re: [PATCH] tpm crb: Work around BIOS's that report the wrong ACPI region size

[Davide Guerri <davide.guerri-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

> On 24 Feb 2017, at 1:02 pm, Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 
> On Tue, Feb 21, 2017 at 02:14:24PM -0700, Jason Gunthorpe wrote:
>> The expectation is that the if the CRB cmd/rsp buffer falls within the
>> ACPI region that the entire buffer will be within the reason. Otherwise
>> resource reservation will fail when it crosses regions.
>> 
>> Work around this BIOS bug by limiting the cmd/rsp buffer to the length
>> of the declared ACPI region. BIOS vendors should fix this by making
>> the ACPI and register length declarations consistent.
>> 
>> Reported-by: davide.guerri-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Tested-by: Davide Guerri <davide.guerri-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks again,
 D.

> 
> Thanks for fixing this issue!
> 
> /Jarkko
> 
>> ---
>> drivers/char/tpm/tpm_crb.c | 27 +++++++++++++++++++++++++--
>> 1 file changed, 25 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
>> index a7c870af916c3d..fac1934eedf0bb 100644
>> --- a/drivers/char/tpm/tpm_crb.c
>> +++ b/drivers/char/tpm/tpm_crb.c
>> @@ -243,6 +243,27 @@ 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. Such broken systems
>> + * probably cannot send large TPM commands since the buffer will be truncated.
>> + */
>> +static u64 crb_fixup_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)
>> {
>> @@ -278,14 +299,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_fixup_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_fixup_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);
>> -- 
>> 2.7.4
>> 


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