Re: [RFC PATCH 3/4] Implement driver for supporting multiple emulated TPMs

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

On Mon, Jan 25, 2016 at 05:46:52PM -0800, Jarkko Sakkinen wrote:
> > The only issue is error unwind, the tpmm version assumes devm works
> > for that, but the vtpm_dev will not be destroyed if the tpm attach
> > fails, do devm is not appropriate.
> 
> At the moment tpmm_chip_alloc() does not use devm for anything. Are you
> speaking about stuff that happens between alloc and register as some of
> the drivers use devm to associate resources to pdev at that point?

Ugh, well I missed that when you made those patches.

You need to put the devm back ASAP, as it is all the drivers now have
broken error handling. eg

static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
                        acpi_handle acpi_dev_handle)
[..]
        chip = tpmm_chip_alloc(dev, &tpm_tis);
[..]
        if (IS_ERR(chip))
	        return PTR_ERR(chip);
			
Obviously leaks chip.

The function is called tpmm_ specifically because it is supposed to
use devm resource unwind, it was a big mistake to remove the devm and
not rename the function or update the comments.

With the change to use a struct device the devm action should have
been a device_put to pair with the device_initialize.

> > As I said before the tpmm stuff was a hack I did to make it easier to
> > migrate the large number of drivers, core code should not be relying
> > on devm like that... One good option here would be unwind that a bit
> > and create a tpm_chip_alloc/tpm_chip_register flow that did not use
> > devm at all. That could be fairly straight forward..
> 
> AFAIK I implemented two phase initialization or are you talking about
> something else?

Right, I was confused because I wrote a different devm thing for TPM
that I discarded when I did the first rework.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140