[Qemu-devel] [PATCH 0/6] bypass tcg memory functions -v2

[Qemu-devel] [PATCH 0/6] bypass tcg memory functions -v2

[Glauber Costa]

Same old story, but addressing immediate concerns.
I'm not handling the overflow issue avi raised, since
it's a separate deal. We can address it in a separate patch

[Qemu-devel] [PATCH 1/6] remove smaller slots if registering a bigger one

[Glauber Costa]

It's like a shark eating a bunch of small fishes:
in some situations (vga linear frame buffer mapping,
for example), we need to register a new slot in place
of older, smaller ones. This patch handles this case

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 11034df..3c12c37 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -582,6 +582,16 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
                 kvm_set_phys_mem(mem_start, mem_size, mem_offset);
 
             return;
+        } else if (start_addr <= mem->start_addr &&
+                   (start_addr + size) >= (mem->start_addr +
+                                           mem->memory_size)) {
+            KVMSlot slot;
+            /* unregister whole slot */
+            memcpy(&slot, mem, sizeof(slot));
+            mem->memory_size = 0;
+            kvm_set_user_memory_region(s, mem);
+
+            kvm_set_phys_mem(start_addr, size, phys_offset);
         } else {
             printf("Registering overlapping slot\n");
             abort();
-- 
1.5.6.5

[Qemu-devel] [PATCH 2/6] re-register whole area upon lfb unmap.

[Glauber Costa]

set phys_offset correctly for the whole vga area when unmapping linear vram
(for vga optimization). We first register the old pieces as unassigned
memory, to make things easier for kvm (and possibly other slot based
implementations in the future). Replacing the region directly would
make the slot management significantly more complex.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/cirrus_vga.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 83c5f40..e4f08ef 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2657,11 +2657,8 @@ static void map_linear_vram(CirrusVGAState *s)
         s->lfb_vram_mapped = 1;
         vga_dirty_log_start((VGAState *)s);
     }
-    else {
-        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000, s->vga_io_memory);
-        cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000, s->vga_io_memory);
-    }
-
+    else
+        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x20000, s->vga_io_memory);
 }
 
 static void unmap_linear_vram(CirrusVGAState *s)
-- 
1.5.6.5

[Qemu-devel] [PATCH 3/6] isolate io handling routine

[Glauber Costa]

introduce cpu_physical_memory_do_io, which handles
the mmio part of cpu_physical_memory_rw. KVM can use
it to do mmio, since mmio is essentially the same for
both KVM and tcg.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 cpu-all.h |    2 +
 exec.c    |   88 ++++++++++++++++++++++++++++++++++---------------------------
 2 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 648264c..d46da05 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -910,6 +910,8 @@ int cpu_register_io_memory(int io_index,
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index);
 CPUReadMemoryFunc **cpu_get_io_memory_read(int io_index);
 
+int cpu_physical_memory_do_io(target_phys_addr_t addr, uint8_t *buf, int l,
+                            int is_write, unsigned long pd);
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write);
 static inline void cpu_physical_memory_read(target_phys_addr_t addr,
diff --git a/exec.c b/exec.c
index 44f6a42..25b403e 100644
--- a/exec.c
+++ b/exec.c
@@ -2891,12 +2891,57 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
 }
 
 #else
+int cpu_physical_memory_do_io(target_phys_addr_t addr, uint8_t *buf, int l, int is_write, unsigned long pd)
+{
+    int io_index;
+    uint32_t val;
+
+    io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+    if (is_write) {
+        /* XXX: could force cpu_single_env to NULL to avoid
+           potential bugs */
+        if (l >= 4 && ((addr & 3) == 0)) {
+            /* 32 bit write access */
+            val = ldl_p(buf);
+            io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
+            l = 4;
+        } else if (l >= 2 && ((addr & 1) == 0)) {
+            /* 16 bit write access */
+            val = lduw_p(buf);
+            io_mem_write[io_index][1](io_mem_opaque[io_index], addr, val);
+            l = 2;
+        } else {
+            /* 8 bit write access */
+            val = ldub_p(buf);
+            io_mem_write[io_index][0](io_mem_opaque[io_index], addr, val);
+            l = 1;
+        }
+    } else {
+        if (l >= 4 && ((addr & 3) == 0)) {
+            /* 32 bit read access */
+            val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
+            stl_p(buf, val);
+            l = 4;
+        } else if (l >= 2 && ((addr & 1) == 0)) {
+            /* 16 bit read access */
+            val = io_mem_read[io_index][1](io_mem_opaque[io_index], addr);
+            stw_p(buf, val);
+            l = 2;
+        } else {
+            /* 8 bit read access */
+            val = io_mem_read[io_index][0](io_mem_opaque[io_index], addr);
+            stb_p(buf, val);
+            l = 1;
+        }
+    }
+    return l;
+}
+
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write)
 {
-    int l, io_index;
+    int l;
     uint8_t *ptr;
-    uint32_t val;
     target_phys_addr_t page;
     unsigned long pd;
     PhysPageDesc *p;
@@ -2915,27 +2960,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
 
         if (is_write) {
             if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
-                io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
                 if (p)
                     addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-                /* XXX: could force cpu_single_env to NULL to avoid
-                   potential bugs */
-                if (l >= 4 && ((addr & 3) == 0)) {
-                    /* 32 bit write access */
-                    val = ldl_p(buf);
-                    io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
-                    l = 4;
-                } else if (l >= 2 && ((addr & 1) == 0)) {
-                    /* 16 bit write access */
-                    val = lduw_p(buf);
-                    io_mem_write[io_index][1](io_mem_opaque[io_index], addr, val);
-                    l = 2;
-                } else {
-                    /* 8 bit write access */
-                    val = ldub_p(buf);
-                    io_mem_write[io_index][0](io_mem_opaque[io_index], addr, val);
-                    l = 1;
-                }
+                l = cpu_physical_memory_do_io(addr, buf, len, is_write, pd);
             } else {
                 unsigned long addr1;
                 addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
@@ -2953,26 +2980,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
         } else {
             if ((pd & ~TARGET_PAGE_MASK) > IO_MEM_ROM &&
                 !(pd & IO_MEM_ROMD)) {
-                /* I/O case */
-                io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
                 if (p)
                     addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-                if (l >= 4 && ((addr & 3) == 0)) {
-                    /* 32 bit read access */
-                    val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
-                    stl_p(buf, val);
-                    l = 4;
-                } else if (l >= 2 && ((addr & 1) == 0)) {
-                    /* 16 bit read access */
-                    val = io_mem_read[io_index][1](io_mem_opaque[io_index], addr);
-                    stw_p(buf, val);
-                    l = 2;
-                } else {
-                    /* 8 bit read access */
-                    val = io_mem_read[io_index][0](io_mem_opaque[io_index], addr);
-                    stb_p(buf, val);
-                    l = 1;
-                }
+                l = cpu_physical_memory_do_io(addr, buf, len, is_write, pd);
             } else {
                 /* RAM case */
                 ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
-- 
1.5.6.5

[Qemu-devel] [PATCH 4/6] replace cpu_physical_memory_rw

[Glauber Costa]

This patch introduces a kvm version of cpu_physical_memory_rw.
The main motivation is to bypass tcg version, which contains
tcg-specific code, as well as data structures not used by kvm,
such as l1_phys_map.

In this patch, I'm using a runtime selection of which function
to call, but the mid-term goal is to use function pointers in
a way very close to which QEMUAccel used to be.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 exec.c    |   13 +++++++++++--
 kvm-all.c |   44 ++++++++++++++++++++++++++++++++++++++++----
 kvm.h     |    2 ++
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 25b403e..9a52275 100644
--- a/exec.c
+++ b/exec.c
@@ -2937,8 +2937,8 @@ int cpu_physical_memory_do_io(target_phys_addr_t addr, uint8_t *buf, int l, int
     return l;
 }
 
-void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
-                            int len, int is_write)
+static void tcg_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                                   int len, int is_write)
 {
     int l;
     uint8_t *ptr;
@@ -2996,6 +2996,15 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
     }
 }
 
+void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                            int len, int is_write)
+{
+    if (kvm_enabled())
+        kvm_cpu_physical_memory_rw(addr, buf, len, is_write);
+    else
+        tcg_physical_memory_rw(addr, buf, len, is_write);
+}
+
 /* used for ROM loading : can write in RAM and ROM */
 void cpu_physical_memory_write_rom(target_phys_addr_t addr,
                                    const uint8_t *buf, int len)
diff --git a/kvm-all.c b/kvm-all.c
index 3c12c37..90c1413 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -540,7 +540,7 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
 
     mem = kvm_lookup_slot(s, start_addr);
     if (mem) {
-        if ((flags == IO_MEM_UNASSIGNED) || (flags >= TLB_MMIO)) {
+        if ((flags == IO_MEM_UNASSIGNED))  {
             mem->memory_size = 0;
             mem->start_addr = start_addr;
             mem->phys_offset = 0;
@@ -559,6 +559,8 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
                 mem->phys_offset)
                 return;
 
+            if ((phys_offset & ~TARGET_PAGE_MASK) != 0)
+                return;
             /* unregister whole slot */
             memcpy(&slot, mem, sizeof(slot));
             mem->memory_size = 0;
@@ -598,16 +600,21 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
         }
     }
     /* KVM does not need to know about this memory */
-    if (flags >= IO_MEM_UNASSIGNED)
+    if (flags == IO_MEM_UNASSIGNED)
         return;
 
-    mem = kvm_alloc_slot(s);
+    if (!mem)
+        mem = kvm_alloc_slot(s);
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->phys_offset = phys_offset;
     mem->flags = 0;
 
-    kvm_set_user_memory_region(s, mem);
+    if (flags < TLB_MMIO)
+        kvm_set_user_memory_region(s, mem);
+    else{
+        mem->phys_offset = flags;
+    }
     /* FIXME deal with errors */
 }
 
@@ -673,3 +680,32 @@ int kvm_has_sync_mmu(void)
 
     return 0;
 }
+
+void kvm_cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                                int len, int is_write)
+{
+    KVMSlot *mem;
+    KVMState *s = kvm_state;
+    int l;
+
+    mem = kvm_lookup_slot(s, addr);
+    if (!mem)
+        return;
+
+    if ((mem->phys_offset & ~TARGET_PAGE_MASK) >= TLB_MMIO) {
+        l = 0;
+        while (len > l)
+            l += cpu_physical_memory_do_io(addr + l, buf + l, len - l, is_write, mem->phys_offset);
+    } else {
+        uint8_t *uaddr = phys_ram_base + mem->phys_offset + (addr - mem->start_addr);
+        l = len;
+        if (addr + len > mem->start_addr + mem->memory_size)
+            l = mem->memory_size - addr;
+        if (!is_write)
+            memcpy(buf, uaddr, l);
+        else
+            memcpy(uaddr, buf, l);
+        if (l != len)
+            kvm_cpu_physical_memory_rw(addr + l, buf + l, len - l, is_write);
+    }
+}
diff --git a/kvm.h b/kvm.h
index efce145..e3e9ca0 100644
--- a/kvm.h
+++ b/kvm.h
@@ -49,6 +49,8 @@ int kvm_has_sync_mmu(void);
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
+void kvm_cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                                int len, int is_write);
 /* internal API */
 
 struct KVMState;
-- 
1.5.6.5

[Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Glauber Costa]

Since now we have our own memory read/write function, we don't
depend on all of tcg data structures anymore. So, instead of filling
them up, bypass it altogether by using kvm_set_phys mem alone.

To do that, we now have to provide our own way to get page
information given the address. (kvm_get_physical_page_desc)

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 exec.c    |   44 +++++++++++++++++++++++++++++---------------
 kvm-all.c |   13 +++++++++++++
 kvm.h     |    2 ++
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/exec.c b/exec.c
index 9a52275..b058501 100644
--- a/exec.c
+++ b/exec.c
@@ -2240,14 +2240,8 @@ static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
         }                                                               \
     } while (0)
 
-/* register physical memory. 'size' must be a multiple of the target
-   page size. If (phys_offset & ~TARGET_PAGE_MASK) != 0, then it is an
-   io memory page.  The address used when calling the IO function is
-   the offset from the start of the region, plus region_offset.  Both
-   start_region and regon_offset are rounded down to a page boundary
-   before calculating this offset.  This should not be a problem unless
-   the low bits of start_addr and region_offset differ.  */
-void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
+
+static void tcg_register_physical_memory_offset(target_phys_addr_t start_addr,
                                          ram_addr_t size,
                                          ram_addr_t phys_offset,
                                          ram_addr_t region_offset)
@@ -2265,8 +2259,6 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
         kqemu_set_phys_mem(start_addr, size, phys_offset);
     }
 #endif
-    if (kvm_enabled())
-        kvm_set_phys_mem(start_addr, size, phys_offset);
 
     region_offset &= TARGET_PAGE_MASK;
     size = (size + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
@@ -2333,15 +2325,37 @@ void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
     }
 }
 
+/* register physical memory. 'size' must be a multiple of the target
+   page size. If (phys_offset & ~TARGET_PAGE_MASK) != 0, then it is an
+   io memory page.  The address used when calling the IO function is
+   the offset from the start of the region, plus region_offset.  Both
+   start_region and regon_offset are rounded down to a page boundary
+   before calculating this offset.  This should not be a problem unless
+   the low bits of start_addr and region_offset differ.  */
+void cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
+                                         ram_addr_t size,
+                                         ram_addr_t phys_offset,
+                                         ram_addr_t region_offset)
+{
+    if (kvm_enabled())
+        kvm_set_phys_mem(start_addr, size, phys_offset);
+    else
+        tcg_register_physical_memory_offset(start_addr, size, phys_offset, region_offset);
+}
+
 /* XXX: temporary until new memory mapping API */
 ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr)
 {
-    PhysPageDesc *p;
+    if (kvm_enabled()) {
+        return kvm_get_physical_page_desc(addr);
+    } else {
+        PhysPageDesc *p;
 
-    p = phys_page_find(addr >> TARGET_PAGE_BITS);
-    if (!p)
-        return IO_MEM_UNASSIGNED;
-    return p->phys_offset;
+        p = phys_page_find(addr >> TARGET_PAGE_BITS);
+        if (!p)
+            return IO_MEM_UNASSIGNED;
+        return p->phys_offset;
+    }
 }
 
 void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size)
diff --git a/kvm-all.c b/kvm-all.c
index 90c1413..a94ee57 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -709,3 +709,16 @@ void kvm_cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
             kvm_cpu_physical_memory_rw(addr + l, buf + l, len - l, is_write);
     }
 }
+
+ram_addr_t kvm_get_physical_page_desc(target_phys_addr_t addr)
+{
+
+    KVMSlot *mem;
+    KVMState *s = kvm_state;
+    mem = kvm_lookup_slot(s, addr);
+
+    if (!mem)
+        return IO_MEM_UNASSIGNED;
+    else
+        return (addr - mem->start_addr) + mem->phys_offset;
+}
diff --git a/kvm.h b/kvm.h
index e3e9ca0..776cfcf 100644
--- a/kvm.h
+++ b/kvm.h
@@ -51,6 +51,8 @@ int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
 void kvm_cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                                 int len, int is_write);
+
+ram_addr_t kvm_get_physical_page_desc(target_phys_addr_t addr);
 /* internal API */
 
 struct KVMState;
-- 
1.5.6.5

[Qemu-devel] [PATCH 6/6] cache slot lookup

[Glauber Costa]

record slot used in last lookup. For the common mmio case,
we'll usually access the same memory slot repeatedly.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a94ee57..8896cf8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -75,16 +75,25 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
     return NULL;
 }
 
+static KVMSlot *last_slot = NULL;
+
 static KVMSlot *kvm_lookup_slot(KVMState *s, target_phys_addr_t start_addr)
 {
     int i;
 
+
+    if (last_slot && (start_addr >= last_slot->start_addr &&
+            start_addr < (last_slot->start_addr + last_slot->memory_size)))
+        return last_slot;
+
     for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
         KVMSlot *mem = &s->slots[i];
 
         if (start_addr >= mem->start_addr &&
-            start_addr < (mem->start_addr + mem->memory_size))
+            start_addr < (mem->start_addr + mem->memory_size)) {
+            last_slot = mem;
             return mem;
+        }
     }
 
     return NULL;
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Blue Swirl]

On 12/18/08, Glauber Costa <glommer@redhat.com> wrote:
> Since now we have our own memory read/write function, we don't
>  depend on all of tcg data structures anymore. So, instead of filling
>  them up, bypass it altogether by using kvm_set_phys mem alone.
>
>  To do that, we now have to provide our own way to get page
>  information given the address. (kvm_get_physical_page_desc)
>
>  Signed-off-by: Glauber Costa <glommer@redhat.com>

>  +static void tcg_register_physical_memory_offset(target_phys_addr_t start_addr,

I don't think TCG actually has much to do with the function. Would
no_kvm be too ugly?

More philosophically, what is (Qemu /\ ~KVM /\ ~TCG)? I hope I got the
math right.

Re: [Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Laurent Desnogues]

On Fri, Dec 19, 2008 at 8:57 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>
> More philosophically, what is (Qemu /\ ~KVM /\ ~TCG)? I hope I got the
> math right.

Your math is right, but I would add ^~translators, which leaves us with
devices, and various other utilities which in total represent about 50%
of the code base :-)


Laurent

Re: [Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Anthony Liguori]

Blue Swirl wrote:
> On 12/18/08, Glauber Costa <glommer@redhat.com> wrote:
>   
>> Since now we have our own memory read/write function, we don't
>>  depend on all of tcg data structures anymore. So, instead of filling
>>  them up, bypass it altogether by using kvm_set_phys mem alone.
>>
>>  To do that, we now have to provide our own way to get page
>>  information given the address. (kvm_get_physical_page_desc)
>>
>>  Signed-off-by: Glauber Costa <glommer@redhat.com>
>>     
>
>   
>>  +static void tcg_register_physical_memory_offset(target_phys_addr_t start_addr,
>>     
>
> I don't think TCG actually has much to do with the function.

It really does though.  The way physical memory is registered and 
managed is TCG specific right now.  It has deep hooks for invalidating 
TranslationBlock's, and the table structure is designed to be conducive 
to the access patterns of TCG.

If you think of a higher level CPU API, I think registering physical 
memory and reading/writing physical memory would end up being part of 
that API.

Regards,

Anthony Liguori

>  Would
> no_kvm be too ugly?
>
> More philosophically, what is (Qemu /\ ~KVM /\ ~TCG)? I hope I got the
> math right.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: [Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Blue Swirl]

On 12/19/08, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Blue Swirl wrote:
>
> > On 12/18/08, Glauber Costa <glommer@redhat.com> wrote:
> >
> >
> > > Since now we have our own memory read/write function, we don't
> > >  depend on all of tcg data structures anymore. So, instead of filling
> > >  them up, bypass it altogether by using kvm_set_phys mem alone.
> > >
> > >  To do that, we now have to provide our own way to get page
> > >  information given the address. (kvm_get_physical_page_desc)
> > >
> > >  Signed-off-by: Glauber Costa <glommer@redhat.com>
> > >
> > >
> >
> >
> >
> > >  +static void
> tcg_register_physical_memory_offset(target_phys_addr_t
> start_addr,
> > >
> > >
> >
> > I don't think TCG actually has much to do with the function.
> >
>
>  It really does though.  The way physical memory is registered and managed
> is TCG specific right now.  It has deep hooks for invalidating
> TranslationBlock's, and the table structure is designed to be conducive to
> the access patterns of TCG.

Yes, but also dyngen stuff used the same structures, so it's a bit
more generic than TCG-only.

>  If you think of a higher level CPU API, I think registering physical memory
> and reading/writing physical memory would end up being part of that API.

Thanks, I was looking for something like this. CPU emulator is more
than just TCG or dyngen and it is also ~KVM. So how about
cpu_emu_register_physical_memory_offset?

Also noaccel_register_physical_memory_offset would fit.

Re: [Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Glauber Costa]

>>  It really does though.  The way physical memory is registered and managed
>> is TCG specific right now.  It has deep hooks for invalidating
>> TranslationBlock's, and the table structure is designed to be conducive to
>> the access patterns of TCG.
>
> Yes, but also dyngen stuff used the same structures, so it's a bit
> more generic than TCG-only.
yeah, but dyngen is gone now. So tcg really stands for "whatever qemu
does", in here.

>
>>  If you think of a higher level CPU API, I think registering physical memory
>> and reading/writing physical memory would end up being part of that API.
>
> Thanks, I was looking for something like this. CPU emulator is more
> than just TCG or dyngen and it is also ~KVM. So how about
> cpu_emu_register_physical_memory_offset?
>
> Also noaccel_register_physical_memory_offset would fit.
After this revision, I must say I'm less happy with the name "accel".
It's really a cpu model. I'm even thinking about changing the name of it
to QEMUCPUModel or whatever. (suggestions welcome)

So "default" or "qemu" would be good choices IMHO.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH 5/6] hook cpu_register_physical_mem

[Blue Swirl]

On 12/22/08, Glauber Costa <glommer@gmail.com> wrote:
> >>  It really does though.  The way physical memory is registered and managed
>  >> is TCG specific right now.  It has deep hooks for invalidating
>  >> TranslationBlock's, and the table structure is designed to be conducive to
>  >> the access patterns of TCG.
>  >
>  > Yes, but also dyngen stuff used the same structures, so it's a bit
>  > more generic than TCG-only.
>
> yeah, but dyngen is gone now. So tcg really stands for "whatever qemu
>  does", in here.

But something new can still come and replace TCG.

>  >>  If you think of a higher level CPU API, I think registering physical memory
>  >> and reading/writing physical memory would end up being part of that API.
>  >
>  > Thanks, I was looking for something like this. CPU emulator is more
>  > than just TCG or dyngen and it is also ~KVM. So how about
>  > cpu_emu_register_physical_memory_offset?
>  >
>  > Also noaccel_register_physical_memory_offset would fit.
>
> After this revision, I must say I'm less happy with the name "accel".
>  It's really a cpu model. I'm even thinking about changing the name of it
>  to QEMUCPUModel or whatever. (suggestions welcome)
>
>  So "default" or "qemu" would be good choices IMHO.

I vote for "default".

[Qemu-devel] [PATCH 0/6] Bypass tcg memory functions -v1.0-2009

[Glauber Costa]

This series is not very different from the last one that did it.
Just bringing it back from my vacations. Idea is to replace tcg
memory functions with kvm's, selectable at runtime. Right now
we use a conditional selection. In the future, we might use
function pointers to allow for easy coupling of any hypervisor.

[Qemu-devel] [PATCH 1/6] remove smaller slots if registering a bigger one

[Glauber Costa]

It's like a shark eating a bunch of small fishes:
in some situations (vga linear frame buffer mapping,
for example), we need to register a new slot in place
of older, smaller ones. This patch handles this case

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9fb295c..53aca0a 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -582,6 +582,16 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
                 kvm_set_phys_mem(mem_start, mem_size, mem_offset);
 
             return;
+        } else if (start_addr <= mem->start_addr &&
+                   (start_addr + size) >= (mem->start_addr +
+                                           mem->memory_size)) {
+            KVMSlot slot;
+            /* unregister whole slot */
+            memcpy(&slot, mem, sizeof(slot));
+            mem->memory_size = 0;
+            kvm_set_user_memory_region(s, mem);
+
+            kvm_set_phys_mem(start_addr, size, phys_offset);
         } else {
             printf("Registering overlapping slot\n");
             abort();
-- 
1.5.6.5

[Qemu-devel] [PATCH 2/6] re-register whole area upon lfb unmap.

[Glauber Costa]

set phys_offset correctly for the whole vga area when unmapping linear vram
(for vga optimization). We first register the old pieces as unassigned
memory, to make things easier for kvm (and possibly other slot based
implementations in the future). Replacing the region directly would
make the slot management significantly more complex.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/cirrus_vga.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index ef939ae..75faafc 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2646,11 +2646,8 @@ static void map_linear_vram(CirrusVGAState *s)
         s->lfb_vram_mapped = 1;
         vga_dirty_log_start((VGAState *)s);
     }
-    else {
-        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000, s->vga_io_memory);
-        cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000, s->vga_io_memory);
-    }
-
+    else
+        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x20000, s->vga_io_memory);
 }
 
 static void unmap_linear_vram(CirrusVGAState *s)
-- 
1.5.6.5

[Qemu-devel] [PATCH 3/6] isolate io handling routine

[Glauber Costa]

introduce cpu_physical_memory_do_io, which handles
the mmio part of cpu_physical_memory_rw. KVM can use
it to do mmio, since mmio is essentially the same for
both KVM and tcg.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 cpu-all.h |    2 +
 exec.c    |   88 ++++++++++++++++++++++++++++++++++---------------------------
 2 files changed, 51 insertions(+), 39 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index ee0a6e3..0c4432a 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -911,6 +911,8 @@ int cpu_register_io_memory(int io_index,
 CPUWriteMemoryFunc **cpu_get_io_memory_write(int io_index);
 CPUReadMemoryFunc **cpu_get_io_memory_read(int io_index);
 
+int cpu_physical_memory_do_io(target_phys_addr_t addr, uint8_t *buf, int l,
+                            int is_write, unsigned long pd);
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write);
 static inline void cpu_physical_memory_read(target_phys_addr_t addr,
diff --git a/exec.c b/exec.c
index faa6333..f72fb96 100644
--- a/exec.c
+++ b/exec.c
@@ -2911,12 +2911,57 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
 }
 
 #else
+int cpu_physical_memory_do_io(target_phys_addr_t addr, uint8_t *buf, int l, int is_write, unsigned long pd)
+{
+    int io_index;
+    uint32_t val;
+
+    io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+    if (is_write) {
+        /* XXX: could force cpu_single_env to NULL to avoid
+           potential bugs */
+        if (l >= 4 && ((addr & 3) == 0)) {
+            /* 32 bit write access */
+            val = ldl_p(buf);
+            io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
+            l = 4;
+        } else if (l >= 2 && ((addr & 1) == 0)) {
+            /* 16 bit write access */
+            val = lduw_p(buf);
+            io_mem_write[io_index][1](io_mem_opaque[io_index], addr, val);
+            l = 2;
+        } else {
+            /* 8 bit write access */
+            val = ldub_p(buf);
+            io_mem_write[io_index][0](io_mem_opaque[io_index], addr, val);
+            l = 1;
+        }
+    } else {
+        if (l >= 4 && ((addr & 3) == 0)) {
+            /* 32 bit read access */
+            val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
+            stl_p(buf, val);
+            l = 4;
+        } else if (l >= 2 && ((addr & 1) == 0)) {
+            /* 16 bit read access */
+            val = io_mem_read[io_index][1](io_mem_opaque[io_index], addr);
+            stw_p(buf, val);
+            l = 2;
+        } else {
+            /* 8 bit read access */
+            val = io_mem_read[io_index][0](io_mem_opaque[io_index], addr);
+            stb_p(buf, val);
+            l = 1;
+        }
+    }
+    return l;
+}
+
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write)
 {
-    int l, io_index;
+    int l;
     uint8_t *ptr;
-    uint32_t val;
     target_phys_addr_t page;
     unsigned long pd;
     PhysPageDesc *p;
@@ -2935,27 +2980,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
 
         if (is_write) {
             if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
-                io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
                 if (p)
                     addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-                /* XXX: could force cpu_single_env to NULL to avoid
-                   potential bugs */
-                if (l >= 4 && ((addr & 3) == 0)) {
-                    /* 32 bit write access */
-                    val = ldl_p(buf);
-                    io_mem_write[io_index][2](io_mem_opaque[io_index], addr, val);
-                    l = 4;
-                } else if (l >= 2 && ((addr & 1) == 0)) {
-                    /* 16 bit write access */
-                    val = lduw_p(buf);
-                    io_mem_write[io_index][1](io_mem_opaque[io_index], addr, val);
-                    l = 2;
-                } else {
-                    /* 8 bit write access */
-                    val = ldub_p(buf);
-                    io_mem_write[io_index][0](io_mem_opaque[io_index], addr, val);
-                    l = 1;
-                }
+                l = cpu_physical_memory_do_io(addr, buf, len, is_write, pd);
             } else {
                 unsigned long addr1;
                 addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
@@ -2973,26 +3000,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
         } else {
             if ((pd & ~TARGET_PAGE_MASK) > IO_MEM_ROM &&
                 !(pd & IO_MEM_ROMD)) {
-                /* I/O case */
-                io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
                 if (p)
                     addr = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
-                if (l >= 4 && ((addr & 3) == 0)) {
-                    /* 32 bit read access */
-                    val = io_mem_read[io_index][2](io_mem_opaque[io_index], addr);
-                    stl_p(buf, val);
-                    l = 4;
-                } else if (l >= 2 && ((addr & 1) == 0)) {
-                    /* 16 bit read access */
-                    val = io_mem_read[io_index][1](io_mem_opaque[io_index], addr);
-                    stw_p(buf, val);
-                    l = 2;
-                } else {
-                    /* 8 bit read access */
-                    val = io_mem_read[io_index][0](io_mem_opaque[io_index], addr);
-                    stb_p(buf, val);
-                    l = 1;
-                }
+                l = cpu_physical_memory_do_io(addr, buf, len, is_write, pd);
             } else {
                 /* RAM case */
                 ptr = phys_ram_base + (pd & TARGET_PAGE_MASK) +
-- 
1.5.6.5

[Qemu-devel] [PATCH 4/6] replace cpu_physical_memory_rw

[Glauber Costa]

This patch introduces a kvm version of cpu_physical_memory_rw.
The main motivation is to bypass tcg version, which contains
tcg-specific code, as well as data structures not used by kvm,
such as l1_phys_map.

In this patch, I'm using a runtime selection of which function
to call, but the mid-term goal is to use function pointers in
a way very close to which QEMUAccel used to be.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 exec.c    |   13 +++++++++++--
 kvm-all.c |   44 ++++++++++++++++++++++++++++++++++++++++----
 kvm.h     |    2 ++
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index f72fb96..20ac972 100644
--- a/exec.c
+++ b/exec.c
@@ -2957,8 +2957,8 @@ int cpu_physical_memory_do_io(target_phys_addr_t addr, uint8_t *buf, int l, int
     return l;
 }
 
-void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
-                            int len, int is_write)
+static void default_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                                   int len, int is_write)
 {
     int l;
     uint8_t *ptr;
@@ -3016,6 +3016,15 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
     }
 }
 
+void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                            int len, int is_write)
+{
+    if (kvm_enabled())
+        kvm_cpu_physical_memory_rw(addr, buf, len, is_write);
+    else
+        default_physical_memory_rw(addr, buf, len, is_write);
+}
+
 /* used for ROM loading : can write in RAM and ROM */
 void cpu_physical_memory_write_rom(target_phys_addr_t addr,
                                    const uint8_t *buf, int len)
diff --git a/kvm-all.c b/kvm-all.c
index 53aca0a..9cbb2f9 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -540,7 +540,7 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
 
     mem = kvm_lookup_slot(s, start_addr);
     if (mem) {
-        if ((flags == IO_MEM_UNASSIGNED) || (flags >= TLB_MMIO)) {
+        if ((flags == IO_MEM_UNASSIGNED))  {
             mem->memory_size = 0;
             mem->start_addr = start_addr;
             mem->phys_offset = 0;
@@ -559,6 +559,8 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
                 mem->phys_offset)
                 return;
 
+            if ((phys_offset & ~TARGET_PAGE_MASK) != 0)
+                return;
             /* unregister whole slot */
             memcpy(&slot, mem, sizeof(slot));
             mem->memory_size = 0;
@@ -598,16 +600,21 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
         }
     }
     /* KVM does not need to know about this memory */
-    if (flags >= IO_MEM_UNASSIGNED)
+    if (flags == IO_MEM_UNASSIGNED)
         return;
 
-    mem = kvm_alloc_slot(s);
+    if (!mem)
+        mem = kvm_alloc_slot(s);
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->phys_offset = phys_offset;
     mem->flags = 0;
 
-    kvm_set_user_memory_region(s, mem);
+    if (flags < TLB_MMIO)
+        kvm_set_user_memory_region(s, mem);
+    else{
+        mem->phys_offset = flags;
+    }
     /* FIXME deal with errors */
 }
 
@@ -673,3 +680,32 @@ int kvm_has_sync_mmu(void)
 
     return 0;
 }
+
+void kvm_cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                                int len, int is_write)
+{
+    KVMSlot *mem;
+    KVMState *s = kvm_state;
+    int l;
+
+    mem = kvm_lookup_slot(s, addr);
+    if (!mem)
+        return;
+
+    if ((mem->phys_offset & ~TARGET_PAGE_MASK) >= TLB_MMIO) {
+        l = 0;
+        while (len > l)
+            l += cpu_physical_memory_do_io(addr + l, buf + l, len - l, is_write, mem->phys_offset);
+    } else {
+        uint8_t *uaddr = phys_ram_base + mem->phys_offset + (addr - mem->start_addr);
+        l = len;
+        if (addr + len > mem->start_addr + mem->memory_size)
+            l = mem->memory_size - addr;
+        if (!is_write)
+            memcpy(buf, uaddr, l);
+        else
+            memcpy(uaddr, buf, l);
+        if (l != len)
+            kvm_cpu_physical_memory_rw(addr + l, buf + l, len - l, is_write);
+    }
+}
diff --git a/kvm.h b/kvm.h
index efce145..e3e9ca0 100644
--- a/kvm.h
+++ b/kvm.h
@@ -49,6 +49,8 @@ int kvm_has_sync_mmu(void);
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
+void kvm_cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
+                                int len, int is_write);
 /* internal API */
 
 struct KVMState;
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH 4/6] replace cpu_physical_memory_rw

[Avi Kivity]

Glauber Costa wrote:
> This patch introduces a kvm version of cpu_physical_memory_rw.
> The main motivation is to bypass tcg version, which contains
> tcg-specific code, as well as data structures not used by kvm,
> such as l1_phys_map.
>
> In this patch, I'm using a runtime selection of which function
> to call, but the mid-term goal is to use function pointers in
> a way very close to which QEMUAccel used to be.
>   

You will need to trivially adjust this for cpu_physical_memory_map().

You're still allocating the page descriptors, right?

My feeling is that qemu could long term drop phys_ram_base and move to a 
slot based structure; most lookups will hit on the first access (the 
largest slot).  The main motivation is to enable memory hotplug.  Of 
course this shouldn't interfere with this patchset going in.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 4/6] replace cpu_physical_memory_rw

[Glauber Costa]

On Tue, Jan 20, 2009 at 6:24 PM, Avi Kivity <avi@redhat.com> wrote:
> Glauber Costa wrote:
>>
>> This patch introduces a kvm version of cpu_physical_memory_rw.
>> The main motivation is to bypass tcg version, which contains
>> tcg-specific code, as well as data structures not used by kvm,
>> such as l1_phys_map.
>>
>> In this patch, I'm using a runtime selection of which function
>> to call, but the mid-term goal is to use function pointers in
>> a way very close to which QEMUAccel used to be.
>>
>
> You will need to trivially adjust this for cpu_physical_memory_map().
Was it merged yet?

>
> You're still allocating the page descriptors, right?
By page descriptors you mean the l1_phys_map? If so, yes, because of
the way cpu_exec works. We can
live without them right now. With other patches I have queued up, we
get closer to this goal.

>
> My feeling is that qemu could long term drop phys_ram_base and move to a
> slot based structure; most lookups will hit on the first access (the largest
> slot).  The main motivation is to enable memory hotplug.  Of course this
> shouldn't interfere with this patchset going in.
>

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."