Re: [PATCH v3 1/3] hw: Model ASPEED's Hash and Crypto Engine

[Cédric Le Goater]

On 3/17/21 2:13 PM, Philippe Mathieu-Daudé wrote:
> 
> 
> On 3/17/21 1:36 PM, Cédric Le Goater wrote:
>> On 3/17/21 12:47 PM, Philippe Mathieu-Daudé wrote:
>>> On 3/12/21 11:57 AM, Joel Stanley wrote:
>>>> The HACE (Hash and Crypto Engine) is a device that offloads MD5, SHA1,
>>>> SHA2, RSA and other cryptographic algorithms.
>>>>
>>>> This initial model implements a subset of the device's functionality;
>>>> currently only direct access (non-scatter gather) hashing.
>>>>
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>> v3:
>>>>  - rebase on upstream to fix meson.build conflict
>>>> v2:
>>>>  - reorder register defines
>>>>  - mask src/dest/len registers according to hardware
>>>> ---
>>>>  include/hw/misc/aspeed_hace.h |  33 ++++
>>>>  hw/misc/aspeed_hace.c         | 312 ++++++++++++++++++++++++++++++++++
>>>>  hw/misc/meson.build           |   1 +
>>>>  3 files changed, 346 insertions(+)
>>>>  create mode 100644 include/hw/misc/aspeed_hace.h
>>>>  create mode 100644 hw/misc/aspeed_hace.c
> 
>>>> +static int do_hash_operation(AspeedHACEState *s, int algo)
>>>> +{
>>>> +    hwaddr src, len, dest;
>>>> +    uint8_t *digest_buf = NULL;
>>>> +    size_t digest_len = 0;
>>>> +    char *src_buf;
>>>> +    int rc;
>>>> +
>>>> +    src = 0x80000000 | s->regs[R_HASH_SRC];
>>>> +    len = s->regs[R_HASH_SRC_LEN];
>>>> +    dest = 0x80000000 | s->regs[R_HASH_DEST];
>>>> +
>>>> +    src_buf = address_space_map(&s->dram_as, src, &len, false,
>>>> +                                MEMTXATTRS_UNSPECIFIED);
>>>
>>> It seems the Aspeed machines aren't using correctly the AS API...
>>> This device shouldn't worry about where it is physically mapped.
>>> IOW its AS is too wide.
>>>
>>> I'm trying to fix this in a series:
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg791085.html
>>
>> The buffers can be anywhere in DRAM
> 
> Exactly. This device only requires direct access to DRAM, not the
> full bus. Regardless the amount of DRAM available, only the bus
> aperture to the DRAM should be passed to this device (MR link),
> then this device use an AS view on it, zero-based.
> 
> Your device becomes independent to where the DRAM is physically
> mapped on the SoC.

ok.

The initial problem was that the RAM was not initialized before 
the SoC was realized and we couldn't use it. things have changed
and we can now pass directly machine->ram to sub models instead of 
the ram_container. That should address your comments on the HACE 
model and on the SMC model. 

I will check what we can do for the mmio_flash address space.

C. 


> 
>> which is mapped at 0x80000000
>> on the AST2600. This is correct and very similar to what we do 
>> in the Aspeed SMC DMA models.
>>
>> I would prefer if we did loads though.
>>
>> Cheers,
>>
>> C.
>>
>>