diff mbox

[04/10] ide: enable preallocated sg lists

Message ID 1289955937-24121-5-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

Alexander Graf Nov. 17, 2010, 1:05 a.m. UTC 
The AHCI core does all the SG handling for us, so we need to allow it
to keep its own layouts.

This patch adds hooks into the IDE code to allow for preallocated SG lists.

Signed-off-by: Roland Elek <elek.roland@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ide/core.c     |   38 +++++++++++++++++++++++++++++++-------
 hw/ide/internal.h |    1 +
 2 files changed, 32 insertions(+), 7 deletions(-)

Comments

Gerd Hoffmann Nov. 17, 2010, 8:59 a.m. UTC | #1 
Hi,

> +            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));
> +                s->sg.sg++;
> +            } else {
> +                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);
> +            }

Does it make sense to handle this via IDEBusOps too?

cheers,
   Gerd
Stefan Hajnoczi Nov. 17, 2010, 9:21 a.m. UTC | #2 
On Wed, Nov 17, 2010 at 1:05 AM, Alexander Graf <agraf@suse.de> 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
diff mbox

Patch

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 276b853..4c15ea2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -444,8 +444,16 @@  static int dma_buf_prepare(BMDMAState *bm, int is_write)
         uint32_t addr;
         uint32_t size;
     } prd;
-    int l, len;
+    int l, len, i;
 
+    if (s->sg_third_party) {
+        /* We already have an sglist. */
+        s->io_buffer_size = 0;
+        for (i=0; i < s->sg.nsg; i++) {
+            s->io_buffer_size += s->sg.sg[i].len;
+        }
+        return s->io_buffer_size != 0;
+    }
     qemu_sglist_init(&s->sg, s->nsector / (IDE_PAGE_SIZE / 512) + 1);
     s->io_buffer_size = 0;
     for(;;) {
@@ -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
+                    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));
+                s->sg.sg++;
+            } else {
+                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);
+            }
+
+
             len = prd.size & 0xfffe;
             if (len == 0)
                 len = 0x10000;
@@ -2715,6 +2736,9 @@  int ide_init_drive(IDEState *s, BlockDriverState *bs,
     } else {
         pstrcpy(s->version, sizeof(s->version), QEMU_VERSION);
     }
+
+    s->sg_third_party = 0;
+
     ide_reset(s);
     bdrv_set_removable(bs, s->drive_kind == IDE_CD);
     return 0;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 19e5efb..005283c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -422,6 +422,7 @@  struct IDEState {
     /* ATA DMA state */
     int io_buffer_size;
     QEMUSGList sg;
+    int sg_third_party;
     /* PIO transfer handling */
     int req_nb_sectors; /* number of sectors per interrupt */
     EndTransferFunc *end_transfer_func;