On 09/18/2018 08:53 PM, Peter Maydell wrote:
> On 31 August 2018 at 11:38, Cédric Le Goater <c...@kaod.org> wrote:
>> The FMC controller on the Aspeed SoCs support DMA to access the flash
>> modules. It can operate in a normal mode, to copy to or from the flash
>> module mapping window, or in a checksum calculation mode, to evaluate
>> the best clock settings for reads.
>>
>> Our primary need is to support the checksum calculation mode and the
>> model only implements synchronous DMA accesses. Something to improve
>> in the future.
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
>>
>> +/*
>> + * Accumulate the result of the reads to provide a checksum that will
>> + * be used to validate the read timing settings.
>> + */
>> +static void aspeed_smc_dma_checksum(AspeedSMCState *s)
>> +{
>> + uint32_t data;
>> +
>> + if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: invalid direction for DMA checksum\n",
>> __func__);
>> + return;
>> + }
>> +
>> + while (s->regs[R_DMA_LEN]) {
>> + cpu_physical_memory_read(s->regs[R_DMA_FLASH_ADDR], &data, 4);
>
> Ideally DMAing devices should not use cpu_physical_memory_*().
> It's better to have the device take a MemoryRegion which it uses to
> create an AddressSpace that it can use address_space_ld*. This (a)
> allows you to correctly model that the DMAing device can't necessarily
> see the same set of devices/memory that the CPU does and (b) detect
> and handle read/write failures.
>
> Commit 112a829f8f0ad is an example of converting a DMA controller from
> old style cpu_physical_memory_* to taking a MemoryRegion and using it.
> (It doesn't do checking for read/write failure, but you should, by
> passing a non-null MemTxResult* final argument.)
OK. I will rework that part.
Thanks,
C.
