Re: [Qemu-devel] [PATCH 04/10] ide: enable preallocated sg lists

[Stefan Hajnoczi]

On Wed, Nov 17, 2010 at 1:05 AM, Alexander Graf <address@hidden> wrote:
> @@ -535,12 +543,25 @@ static int dma_buf_rw(BMDMAState *bm, int is_write)
>         if (bm->cur_prd_len == 0) {
>             /* end of table (with a fail safe of one page) */
>             if (bm->cur_prd_last ||
> -                (bm->cur_addr - bm->addr) >= IDE_PAGE_SIZE)
> -                return 0;
> -            cpu_physical_memory_read(bm->cur_addr, (uint8_t *)&prd, 8);
> -            bm->cur_addr += 8;
> -            prd.addr = le32_to_cpu(prd.addr);
> -            prd.size = le32_to_cpu(prd.size);
> +                (bm->cur_addr - bm->addr) >= IDE_PAGE_SIZE) {
> +                    s->sg.sg -= s->sg.nsg; //XXX

I don't understand why s->sg.sg needs to be modified at all.  Are we stashing
the pointer away accross dma_buf_rw() invocations?

> +                    return 0;
> +                }
> +            if (s->sg_third_party) {
> +                /* We've already parsed the guest RAM PRDT.
> +                 * This is essential for AHCI, where the PRDT is in a 
> different
> +                 * format than in IDE BMDMA.
> +                 */
> +                memcpy((uint8_t *)&prd, s->sg.sg, sizeof(prd));

struct {
    uint32_t addr;
    uint32_t size;
} prd;

typedef struct {
    target_phys_addr_t base;
    target_phys_addr_t len;
} ScatterGatherEntry;

sizeof(target_phys_addr_t) != sizeof(uint32_t) for 64-bit targets

prd should use a proper struct instead of redefining it inline in dma_buf_rw()
and dma_buf_prepare().  That struct could either be a ScatterGatherEntry or
something else that we assign s->sg.sg fields to individually.

Stefan