On 01/07/2019 15:06, Peter Maydell wrote:
> On Tue, 18 Jun 2019 at 17:55, 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.
>>
>> The model introduces two custom address spaces for DMAs: one for the
>> AHB window of the FMC flash devices and one for the DRAM. The latter
>> is populated using a "dram" link set from the machine with the RAM
>> container region.
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>> Acked-by: Joel Stanley <j...@jms.id.au>
>> +/*
>> + * 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)
>> +{
>> + MemTxResult result;
>> + 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]) {
>> + result = address_space_read(&s->flash_as, s->regs[R_DMA_FLASH_ADDR],
>> + MEMTXATTRS_UNSPECIFIED,
>> + (uint8_t *)&data, 4);
>
> This does a byte-by-byte read into a local uint32_t...
>
>> + if (result != MEMTX_OK) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash read failed @%08x\n",
>> + __func__, s->regs[R_DMA_FLASH_ADDR]);
>> + return;
>> + }
>> +
>> + /*
>> + * When the DMA is on-going, the DMA registers are updated
>> + * with the current working addresses and length.
>> + */
>> + s->regs[R_DMA_CHECKSUM] += data;
>
> ...which we then use as a (host-endian) 32-bit value.
>
> This will behave differently on big endian and little endian hosts.
> If the h/w behaviour is to to load a 32-bit data type you probably
> want address_space_ldl_le() (or _be() if it's doing a big-endian load).
>
>> + s->regs[R_DMA_FLASH_ADDR] += 4;
>> + s->regs[R_DMA_LEN] -= 4;
>> + }
>> +}
>> +
>> +static void aspeed_smc_dma_rw(AspeedSMCState *s)
>> +{
>> + MemTxResult result;
>> + uint32_t data;
>> +
>> + while (s->regs[R_DMA_LEN]) {
>> + if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
>> + result = address_space_read(&s->dram_as,
>> s->regs[R_DMA_DRAM_ADDR],
>> + MEMTXATTRS_UNSPECIFIED,
>> + (uint8_t *)&data, 4);
>> + if (result != MEMTX_OK) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed
>> @%08x\n",
>> + __func__, s->regs[R_DMA_DRAM_ADDR]);
>> + return;
>> + }
>> +
>> + result = address_space_write(&s->flash_as,
>> + s->regs[R_DMA_FLASH_ADDR],
>> + MEMTXATTRS_UNSPECIFIED,
>> + (uint8_t *)&data, 4);
>> + if (result != MEMTX_OK) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash write failed
>> @%08x\n",
>> + __func__, s->regs[R_DMA_FLASH_ADDR]);
>> + return;
>> + }
>> + } else {
>> + result = address_space_read(&s->flash_as,
>> s->regs[R_DMA_FLASH_ADDR],
>> + MEMTXATTRS_UNSPECIFIED,
>> + (uint8_t *)&data, 4);
>> + if (result != MEMTX_OK) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Flash read failed
>> @%08x\n",
>> + __func__, s->regs[R_DMA_FLASH_ADDR]);
>> + return;
>> + }
>> +
>> + result = address_space_write(&s->dram_as,
>> s->regs[R_DMA_DRAM_ADDR],
>> + MEMTXATTRS_UNSPECIFIED,
>> + (uint8_t *)&data, 4);
>> + if (result != MEMTX_OK) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed
>> @%08x\n",
>> + __func__, s->regs[R_DMA_DRAM_ADDR]);
>> + return;
>> + }
>> + }
>
> Since the code here doesn't do anything with the data the
> address_space_read/write here aren't wrong, but you might
> prefer to use the ldl and stl functions to avoid the casts
> to uint8_t* and need to specify the length.
yes. I will check with the HW guys to know precisely how the values are
read and accumulated.
Thanks,
C.
