On Wed, Feb 10, 2016 at 12:15:44AM -0500, Stefan Berger wrote:
> Redid that now.
>
> https://github.com/stefanberger/linux/commit/83019eaab2cf5eb33f2665efdf9d2a117ed703b2
Yeah, I think you've got that now.
Just a few other random commments..
This isn't great:
vtpm_dev->dev_num = find_first_zero_bit(dev_mask, VTPM_NUM_DEVICES);
We shouldn't artificially limit the number of devices if
virtualization is the target. Use an idr, or figure out how to get rid
of it. Since it is only used here:
dev_set_name(&vtpm_dev->dev, "vtpms%d", vtpm_dev->dev_num);
Maybe it could be adjusted to use chip->dev_num instead.
This:
INIT_WORK(&vtpm_dev->work, vtpm_dev_work);
should be near the kzalloc
Drop the 1 line functions (vtpm_dev_work_start, vtpm_delete_vtpm_dev,
Use u32's here:
u8 req_buf[TPM_BUFSIZE]; /* request buffer */
and related to ensure the buffer is sensibly aligned
Don't log on user space inputs, use debug or something:
dev_err(&vtpm_dev->dev,
"Invalid size in recv: count=%zd, req_len=%zd\n",
count, len);
This:
vtpm_dev_work_stop(vtpm_dev);
Doesn't kill a running work thread - more synchronization is needed
for that corner case
Make sure that devm is working for your specific case when this
happens:
err:
/* did not register chip */
vtpm_dev->chip = NULL;
Instrument the core, devm should free the tpm chip when user space
closes the fd. If not the core needs to provide a non-devm alloc for
this driver. (easy)
This spin lock:
spin_lock(&driver_lock);
vtpm_dev->dev_num = find_first_zero_bit(dev_mask, VTPM_NUM_DEVICES);
[..]
err = device_register(&vtpm_dev->dev); /* does get_device */
spin_unlock(&driver_lock);
I'm not comfortable with how big a region that is. If you keep
dev_mask, then only lock around the dev_mask manipulation not other
stuff. Relock to undo
fops_write should fail if op_recv isn't waiting for a buffer.
Looks pretty good really
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=272487151&iu=/4140
_______________________________________________
tpmdd-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel