Hello everyone,
Dear Hao,
The first aim of the patch you mentioned is not to shorten TPM timings,
as no
timing is changed at all on this patch, but to count time more
accurately.
If your TPM needs more time to perform a specific operation, it has
nothing to
do with this patch, but rather with its specified timings in the
kernel. The
timings for your Atmel TPM were certainly wrong before 4.14 to begin
with, they
were not counted accurately and your TPM working was potentially a
"positive"
side effect of the imprecise time counting (ie non-specified delays
were added
every time msleep was used instead of usleep_range).
TPM polling should not be affected because we use a different way to
count
timings in the kernel.
If you see issues with your Atmel TPM, I'd suggest to make the changes
in the
tpm_atmel code only (tpm_infineon would be a suitable example to look
at) and
discuss the timing values with Atmel TPMs experts, but reverting this
patch and
affecting all the other TPMs seems like a misunderstanding.
Thanks,
Hamza ATTAK.
On Sat, Sep 12, 2020 at 10:14, Paul Menzel <pmenzel@xxxxxxxxxxxxx>
wrote:
Dear Hao,
Thank you for the reply.
Am 12.09.20 um 10:10 schrieb Hao Wu:
Thanks for quick responses over this report.
Thank you for the quick follow-up.
Hao, I wouldn’t expect a longer timeout causing the TPM to be
queried less frequently, but I do not know the code well.
From our understanding, the TPM queries might be retried due to some
reason, increase timeout 3x would lower the query frequency to 1/3.
The explanation might be wrong, but the fact looks like the timeout
matters. Unfortunately, engineers from Rubrik are not experts over
the TPM driver code neither :(
Be careful about making this a global change. It could reduce
the TPM performance by 3x. We don't want to affect all TPMs to
fix a bug in an old TPM 1.2 chip from one vendor.
Linux has a no regression policy, so the performance penalty
wouldn’t matter. Unfortunately, the regression was only noticed
several years after being introduced in Linux v4.14-rc2.
So does that mean we are good to apply the global change ? Or we need
to discuss about the actual fix further?
To get a fix into the stable series, it first needs to be applied to
the master branch. I guess you tested also with Linux master, right?
Please add the explanation from your email to Greg into the git
commit message, format the patch with `git format-patch -1 -o
outgoing` and send it with `git send-email outgoing/*` to the
addresses listed for the subsystem in `MAINTAINERS` and the people
listed in the commit introducing the regression.
Then it can be properly reviewed and discussed.
Kind regards,
Paul
