Re: [Qemu-devel] [PATCH v6 1/4] hw: introduce standard SD host controller

[Igor Mitsyanko]

On 08/06/2012 02:30 PM, Peter Maydell wrote:
> On 6 August 2012 04:25, Peter A. G. Crosthwaite
> <address@hidden> wrote:
>
> > +static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
> > +{
> > +    bool page_aligned = false;
> > +    unsigned int n, begin;
> > +    const uint16_t block_size = s->blksize & 0x0fff;
> > +    uint32_t boundary_chk = 1 << (((s->blksize & 0xf000) >> 12) + 12);
> > +    uint32_t boundary_count = boundary_chk - (s->sdmasysad % boundary_chk);
> > +
> > +    /* XXX: Some sd/mmc drivers (for example, u-boot-slp) do not account for
> > +     * possible stop at page boundary if initial address is not page aligned,
> > +     * allow them to work properly */
> > +    if ((s->sdmasysad % boundary_chk) == 0) {
> > +        page_aligned = true;
> > +    }
> It's not quite clear to me what this comment is indicating. Is it
> a bit of behaviour which is "not specified but behave as hardware
> happens to do because software is accidentally relying on it", or
> are we behaving differently from hardware here?

Spec states that DMA transfer should stop when controller detects a 
carry out of specified address bits and interrupt should be generated to 
stimulate software to update DMA address register. There's no way to 
disable this behaviour, software can only regulate boundary size through 
bits in BLKSIZE register (4K - 512K). That's how it was implemented 
initially, but it caused u-boot mmc driver (which works fine on real 
hardware) to hang.
The reason for hang is that when u-boot performs large continuous data 
transfer (>512K) to an arbitrary (non page-aligned) address, it doesn't 
care about "stop at page boundary" interrupt at all. It just loops 
forefer, waiting for a DMA transfer completion, which will never 
complete if it stopped at page boundary until software updates address 
register.
The fact that it somehow manages to work on hardware got me thinking 
that it only applies to initially aligned addresses.

> > +static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
> > +{
> > +    uint32_t adma1 = 0;
> > +    uint64_t adma2 = 0;
> > +    target_phys_addr_t entry_addr = (target_phys_addr_t)s->admasysaddr;
> > +
> > +    switch (SDHC_DMA_TYPE(s->hostctl)) {
> > +    case SDHC_CTRL_ADMA2_32:
> > +        cpu_physical_memory_read(entry_addr, (uint8_t *)&adma2, sizeof(adma2));
> > +        dscr->addr = (target_phys_addr_t)((adma2 >> 32) & 0xfffffffc);
> > +        dscr->length = (uint16_t)((adma2 >> 16) & 0xFFFF);
> > +        dscr->attr = (uint8_t)(adma2 & 0x3F);
> Does the SDHCI spec define that these words are interpreted like
> this regardless of system endianness, or is this an accidental
> assumption of little-endian behaviour?

Spec never says it explicitly, but it's quite obvious that descriptor 
table has a little endian format. There is even a comment in linux SDHCI 
driver that says:

    /*
     * The spec does not specify endianness of descriptor table.
     * We currently guess that it is LE.
     */

> -- PMM