It was <2017-12-05 wto 14:54>, when Stephan Mueller wrote: > Am Dienstag, 5. Dezember 2017, 13:35:57 CET schrieb Łukasz Stelmach: > > Hi Łukasz, > >> Use memcpy_fromio() instead of custom exynos_rng_copy_random() function >> to retrieve generated numbers from the registers of PRNG. >> >> Remove unnecessary invocation of cpu_relax(). >> >> Signed-off-by: Łukasz Stelmach <l.stelmach@xxxxxxxxxxx> >> --- >> drivers/crypto/exynos-rng.c | 36 +++++------------------------------- >> 1 file changed, 5 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c >> index 894ef93ef5ec..002e9d2a83cc 100644 >> --- a/drivers/crypto/exynos-rng.c >> +++ b/drivers/crypto/exynos-rng.c [...] >> @@ -171,6 +143,8 @@ static int exynos_rng_get_random(struct exynos_rng_dev >> *rng, { >> int retry = EXYNOS_RNG_WAIT_RETRIES; >> >> + *read = min_t(size_t, dlen, EXYNOS_RNG_SEED_SIZE); >> + >> if (rng->type == EXYNOS_PRNG_TYPE4) { >> exynos_rng_writel(rng, EXYNOS_RNG_CONTROL_START, >> EXYNOS_RNG_CONTROL); >> @@ -180,8 +154,8 @@ static int exynos_rng_get_random(struct exynos_rng_dev >> *rng, } >> >> while (!(exynos_rng_readl(rng, >> - EXYNOS_RNG_STATUS) & EXYNOS_RNG_STATUS_RNG_DONE) && --retry) >> - cpu_relax(); >> + EXYNOS_RNG_STATUS) & EXYNOS_RNG_STATUS_RNG_DONE) && >> + --retry); SM> SM> Is this related to the patch? KK> It looks like unrelated change so split it into separate commit with KK> explanation why you are changing the common busy-loop pattern. KK> exynos_rng_readl() uses relaxed versions of readl() so I would expect KK> here cpu_relax(). Yes. As far as I can tell this gives the major part of the performance improvement brought by this patch. The busy loop is not very busy. Every time I checked the loop (w/o cpu_relax()) was executed twice (retry was 98) and the operation was reliable. I don't see why do we need a memory barrier here. On the other hand, I am not sure the whole exynos_rng_get_random() shouldn't be ran under a mutex or a spinlock (I don't see anything like this in the upper layers of the crypto framework). The *_relaxed() I/O operations do not enforce memory Thank you for asking the questions. I will put the above explanations in the commit message. >> >> if (!retry) >> return -ETIMEDOUT; >> @@ -189,7 +163,7 @@ static int exynos_rng_get_random(struct exynos_rng_dev >> *rng, /* Clear status bit */ >> exynos_rng_writel(rng, EXYNOS_RNG_STATUS_RNG_DONE, >> EXYNOS_RNG_STATUS); >> - *read = exynos_rng_copy_random(rng, dst, dlen); >> + memcpy_fromio(dst, rng->mem + EXYNOS_RNG_OUT_BASE, *read); >> >> return 0; >> } Kind regards, -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics
Attachment:
signature.asc
Description: PGP signature