From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::22f; helo=mail-wm0-x22f.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x22f.google.com (mail-wm0-x22f.google.com [IPv6:2a00:1450:400c:c09::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 371E220954CBD for ; Mon, 19 Feb 2018 12:01:46 -0800 (PST) Received: by mail-wm0-x22f.google.com with SMTP id f3so17274711wmc.1 for ; Mon, 19 Feb 2018 12:07:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=fMNvxMtxrkMv7szNpickwCm3qR/DzfIPYJItfyaHlTQ=; b=ZqQDedHaTzWk3eyRUtR3stIX0B12xLuqqpG9FYhPgT/E/hO9I8bX1J7poDspF3pJc0 DimBRBZlYF/op+/IK4jF8mcsz4Tk+7+NZ13nQOSMFmNPWG2MmLaK6TMlATeNwSqp2lsX caehroy286ht+Z105GBYZvu6+fEtrOjVw5cxQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=fMNvxMtxrkMv7szNpickwCm3qR/DzfIPYJItfyaHlTQ=; b=pGA9NZPIUHCLzH4hWox80dRLiKTtzkGD3esuN9Dh8l0Pn6/VWCiKLK8J70qBdpOP1I uoVYnt2s+oj9PsTY6J1c+jiRZZ5C8pSzbRPVMOmkg/PlCmJI7vyIY3+qRSFdiUzod7Sl cwS7CO63f9necCgUY5BT3YMcP47XccVUNvy5kVeolVm832NmA53LvhJvHgzVTIHjLuZk HAZQF8GnM6bbKhBdjKcP6vgiakzTXf7EQqPGsMlVzqJ1QW11FvFdiHgVsLNvoMAtUSV/ u/eTQrB/oXUAh8Z+ZIVzGdNN8PJVsPhl/J9M6pqOuiXamiJos+rdPx+0FHifWDesG4c9 u6TQ== X-Gm-Message-State: APf1xPDw5ZG5xn2JEqcxnxsHV/sC7p+InRaNY84kMdNrZ0pAOAMTQ2s9 lli36QiwLRg6MbGU5B/QpdQjLA== X-Google-Smtp-Source: AH8x226ZCV2QIFFBX5Q6QBi8Y37R/jSSlroPfbRd+A9wS15xmDdRdAgaXRQbm6C93KYp4hMYflwLVA== X-Received: by 10.28.47.80 with SMTP id v77mr11710841wmv.23.1519070862346; Mon, 19 Feb 2018 12:07:42 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 186sm3119907wmy.12.2018.02.19.12.07.40 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 19 Feb 2018 12:07:41 -0800 (PST) Date: Mon, 19 Feb 2018 20:07:39 +0000 From: Leif Lindholm To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Joakim Bech Message-ID: <20180219200739.rvwmfll6e4yte7q5@bivouac.eciton.net> References: <20180219094332.19853-1-ard.biesheuvel@linaro.org> <20180219172848.gl33dx5aafy3hcp6@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH] Silicon/Atmel: add support for AtSha204a RNG X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Feb 2018 20:01:47 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Feb 19, 2018 at 06:19:53PM +0000, Ard Biesheuvel wrote: > >> +/** > >> + Produces and returns an RNG value using either the default or specified RNG > >> + algorithm. > >> + > >> + @param[in] This A pointer to the EFI_RNG_PROTOCOL instance. > >> + @param[in] Algorithm A pointer to the EFI_RNG_ALGORITHM that > >> + identifies the RNG algorithm to use. May be > >> + NULL in which case the function will use its > >> + default RNG algorithm. > >> + @param[in] ValueLength The length in bytes of the memory buffer > >> + pointed to by RNGValue. The driver shall > >> + return exactly this numbers of bytes. > >> + @param[out] Value A caller-allocated memory buffer filled by the > >> + driver with the resulting RNG value. > >> + > >> + @retval EFI_SUCCESS The RNG value was returned successfully. > >> + @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm is not > >> + supported by this driver. > >> + @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due to a > >> + hardware or firmware error. > >> + @retval EFI_NOT_READY There is not enough random data available to > >> + satisfy the length requested by > >> + RNGValueLength. > >> + @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is zero. > >> + > >> +**/ > >> +STATIC > >> +EFI_STATUS > >> +EFIAPI > >> +AtSha240aGetRNG ( > >> + IN EFI_RNG_PROTOCOL *This, > >> + IN EFI_RNG_ALGORITHM *Algorithm OPTIONAL, > >> + IN UINTN ValueLength, > >> + OUT UINT8 *Value > >> +) > >> +{ > >> + EFI_STATUS Status; > >> + ATSHA204A_DEV *AtSha204a; > >> + ATSHA204A_I2C_RNG_COMMAND Cmd; > >> + ATSHA204A_I2C_RNG_RESULT Res; > >> + I2C_RNG_REQUEST Req; > >> + I2C_RNG_REQUEST Resp; > > > > Since I just gave Marcin a hard time over non-approved abbreviations, > > I should probably flag these too... Would you mind terribly turning > > them into Command, Result, Request, Response? > > > > No that is fine. > > >> + UINTN Retries; > >> + > >> + if (Algorithm != NULL && !CompareGuid (Algorithm, &gEfiRngAlgorithmRaw)) { > >> + return EFI_UNSUPPORTED; > >> + } > >> + > >> + AtSha204a = ATSHA204A_DEV_FROM_THIS (This); > >> + > >> + Req.OperationCount = 1; > >> + Req.Operation.Flags = 0; > >> + > >> + Cmd.Command = ATSHA204A_COMMAND; > >> + Cmd.Count = sizeof (Cmd) - 1; > >> + Cmd.Opcode = ATSHA204A_OPCODE_RANDOM; > >> + Cmd.Param1 = 0; > >> + Cmd.Param2 = 0; > >> + Cmd.Crc = OPCODE_COMMAND_PACKET_CRC; > >> + > >> + Resp.OperationCount = 1; > >> + Resp.Operation.Flags = I2C_FLAG_READ; > >> + Resp.Operation.LengthInBytes = sizeof (Res); > >> + Resp.Operation.Buffer = (VOID *)&Res; > >> + > >> + Retries = 0; > >> + while (ValueLength > 0) { > >> + // > >> + // The AtSha204a will go back to sleep right in the middle of a transaction > >> + // if it does not complete in ~1.3 seconds. So send the wake sequence for > >> + // each iteration, which consists of a dummy write to slave address 0x0. > >> + // > >> + Req.Operation.LengthInBytes = 0; > >> + Status = AtSha204a->I2cIo->QueueRequest (AtSha204a->I2cIo, 1, NULL, > >> + (VOID *)&Req, NULL); > >> + DEBUG ((DEBUG_INFO, "%a: wake AtSha204a: I2cIo->QueueRequest() - %r\n", > >> + __FUNCTION__, Status)); > >> + > >> + gBS->Stall (2500); // wait 2.5 ms for wake to complete > >> + > >> + Req.Operation.LengthInBytes = sizeof (Cmd); > >> + Req.Operation.Buffer = (VOID *)&Cmd; > >> + Status = AtSha204a->I2cIo->QueueRequest (AtSha204a->I2cIo, 0, NULL, > >> + (VOID *)&Req, NULL); > >> + if (EFI_ERROR (Status)) { > >> + if (++Retries <= MAX_RETRIES) { > >> + continue; > >> + } > >> + DEBUG ((DEBUG_ERROR, "%a: I2C request transfer failed, Status == %r\n", > >> + __FUNCTION__, Status)); > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + gBS->Stall (50 * 1000); // 50 ms max execution time for RANDOM opcode > > > > Is there no way of polling for response complete? > > 50ms per iteration?? :( (or, well, 52.5 in total) > > > > Typical execution time is 11 ms, but it is rare for this routine to > iterate more than once in the first place, so I tried to keep it > simple. (The typical use case of a raw RNG is to seed a DRBG, which > typically takes 32 bytes of seed, which is exactly the output size of > a single invocation) > > I don't mind changing it, though, if you feel strongly about this. Well, decreasing the explicit delay within the same order of magnitude doesn't seem like a very good return on investment. If it was possible to get rid of the explicit delay, I'd be all for putting some effort into that. It's just a bit of a shame with a bandwidth of < 1kb/s. But if that's the hardware limitation, that's the hardware limitation. / Leif