> Date: Fri, 8 Jan 2021 10:27:38 -0600 > From: Scott Cheloha <[email protected]> > > On Wed, Jan 06, 2021 at 11:26:27PM +0100, Mark Kettenis wrote: > > > Date: Wed, 6 Jan 2021 16:16:27 -0600 > > > From: Scott Cheloha <[email protected]> > > > > > > On Wed, Jan 06, 2021 at 12:16:13PM -0600, Scott Cheloha wrote: > > > > As mentioned in a prior mail, tpm(4) is the last user of tvtohz(9) in > > > > the tree. > > > > > > > > However, we don't need to use tvtohz(9) in tpm(4) at all. Converting > > > > from milliseconds to ticks is trivial. Using an intermediary timeval > > > > is just pointless indirection. > > > > > > > > With this committed I will be able to remove both tvtohz(9) and > > > > tstohz(9) from the tree in a subsequent patch. > > > > > > Whoops, made a mistake in the prior diff. > > > > > > We want to MAX() the result up to 1 to avoid dividing to zero and > > > blocking indefinitely in e.g. tsleep(9). Previous diff MIN'd the > > > result down to 1, which is very wrong. > > > > To be honest I'd just zap the function completely and instead simply do: > > > > to = TPM_ACCESS_TMO / 10; > > > > and > > > > to = TPM_BURST_TMO / 10; > > That won't work on custom kernels where HZ is not 100, no?
HZ is irrelevant here. These TPM_XXX_TMO defines specify a timeout in microseconds and DELAY(10) just delays for 10us. So you just need to figure out how many times you need to do the 10us delay to reach the timeout specified by TPM_XXX_TMO. Also, this driver is only supported on amd64/i386 at this point (but might show up on arm64 at some point). > > There is no magic happening here. The code is just doing a hardware > > register poll loop in 10us steps. > > > > Hmm, actually it seems the code is broken and uses steps of 10 > > microseconds instead of milliseconds. So instead it should probably > > use: > > > > to = TPM_ACCESS_TMO * 100; > > > > and > > > > to = TPM_BURST_TMO * 100; > > This problem came up in a different thread: > > https://marc.info/?l=openbsd-tech&m=160833962329381&w=2 > > jcs@ said, and I paraphrase, "tpm(4) sucks, we should merge NetBSD's > latest code, which is much nicer now." > > That sounds like a much taller order than I can fill, so I'm just > trying to remove the tvtohz(9) call without changing any behavior, > even if the behavior looks wrong (like using the wrong units). > > I don't even have a tpm(4) device to test. Until I have one I'm > reluctant to do things like expanding the delay time in these loops. > As of now the driver "works" for some subset of people, which is not > nothing. But having illogical code is a problem as well. So I think we should at least attempt to fix it the right way. All it takes is to find someone with a laptop where the driver attaches and have them suspend/resume it.
