| Message ID | 1324304024-11220-14-git-send-email-avi@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2011-12-19 15:13, Avi Kivity wrote: > Drop the use of cpu_register_phys_memory_client() in favour of the new > MemoryListener API. The new API simplifies the caller, since there is no > need to deal with splitting and merging slots; however this is not exploited > in this patch. This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. Jan > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > kvm-all.c | 107 ++++++++++++++++++++++++++++++++++++++++--------------------- > 1 files changed, 70 insertions(+), 37 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 4f58ae8..138e0a2 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -27,6 +27,7 @@ > #include "gdbstub.h" > #include "kvm.h" > #include "bswap.h" > +#include "memory.h" > > /* This check must be after config-host.h is included */ > #ifdef CONFIG_EVENTFD > @@ -289,16 +290,28 @@ static int kvm_dirty_pages_log_change(target_phys_addr_t phys_addr, > return kvm_slot_dirty_pages_log_change(mem, log_dirty); > } > > -static int kvm_log_start(CPUPhysMemoryClient *client, > - target_phys_addr_t phys_addr, ram_addr_t size) > +static void kvm_log_start(MemoryListener *listener, > + MemoryRegionSection *section) > { > - return kvm_dirty_pages_log_change(phys_addr, size, true); > + int r; > + > + r = kvm_dirty_pages_log_change(section->offset_within_address_space, > + section->size, true); > + if (r < 0) { > + abort(); > + } > } > > -static int kvm_log_stop(CPUPhysMemoryClient *client, > - target_phys_addr_t phys_addr, ram_addr_t size) > +static void kvm_log_stop(MemoryListener *listener, > + MemoryRegionSection *section) > { > - return kvm_dirty_pages_log_change(phys_addr, size, false); > + int r; > + > + r = kvm_dirty_pages_log_change(section->offset_within_address_space, > + section->size, false); > + if (r < 0) { > + abort(); > + } > } > > static int kvm_set_migration_log(int enable) > @@ -519,13 +532,15 @@ static int kvm_check_many_ioeventfds(void) > return NULL; > } > > -static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, > - ram_addr_t phys_offset, bool log_dirty) > +static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) > { > KVMState *s = kvm_state; > - ram_addr_t flags = phys_offset & ~TARGET_PAGE_MASK; > KVMSlot *mem, old; > int err; > + MemoryRegion *mr = section->mr; > + bool log_dirty = memory_region_is_logging(mr); > + target_phys_addr_t start_addr = section->offset_within_address_space; > + ram_addr_t size = section->size; > void *ram = NULL; > > /* kvm works in page size chunks, but the function may be called > @@ -533,20 +548,19 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, > size = TARGET_PAGE_ALIGN(size); > start_addr = TARGET_PAGE_ALIGN(start_addr); > > - /* KVM does not support read-only slots */ > - phys_offset &= ~IO_MEM_ROM; > - > - if ((phys_offset & ~TARGET_PAGE_MASK) == IO_MEM_RAM) { > - ram = qemu_safe_ram_ptr(phys_offset); > + if (!memory_region_is_ram(mr)) { > + return; > } > > + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; > + > while (1) { > mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + size); > if (!mem) { > break; > } > > - if (flags < IO_MEM_UNASSIGNED && start_addr >= mem->start_addr && > + if (add && start_addr >= mem->start_addr && > (start_addr + size <= mem->start_addr + mem->memory_size) && > (ram - start_addr == mem->ram - mem->start_addr)) { > /* The new slot fits into the existing one and comes with > @@ -575,8 +589,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, > * slot comes around later, we will fail (not seen in practice so far) > * - and actually require a recent KVM version. */ > if (s->broken_set_mem_region && > - old.start_addr == start_addr && old.memory_size < size && > - flags < IO_MEM_UNASSIGNED) { > + old.start_addr == start_addr && old.memory_size < size && add) { > mem = kvm_alloc_slot(s); > mem->memory_size = old.memory_size; > mem->start_addr = old.start_addr; > @@ -591,7 +604,6 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, > } > > start_addr += old.memory_size; > - phys_offset += old.memory_size; > ram += old.memory_size; > size -= old.memory_size; > continue; > @@ -642,8 +654,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, > if (!size) { > return; > } > - /* KVM does not need to know about this memory */ > - if (flags >= IO_MEM_UNASSIGNED) { > + if (!add) { > return; > } > mem = kvm_alloc_slot(s); > @@ -660,33 +671,55 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, > } > } > > -static void kvm_client_set_memory(struct CPUPhysMemoryClient *client, > - target_phys_addr_t start_addr, > - ram_addr_t size, ram_addr_t phys_offset, > - bool log_dirty) > +static void kvm_region_add(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + kvm_set_phys_mem(section, true); > +} > + > +static void kvm_region_del(MemoryListener *listener, > + MemoryRegionSection *section) > { > - kvm_set_phys_mem(start_addr, size, phys_offset, log_dirty); > + kvm_set_phys_mem(section, false); > +} > + > +static void kvm_log_sync(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + target_phys_addr_t start = section->offset_within_address_space; > + target_phys_addr_t end = start + section->size; > + int r; > + > + r = kvm_physical_sync_dirty_bitmap(start, end); > + if (r < 0) { > + abort(); > + } > } > > -static int kvm_client_sync_dirty_bitmap(struct CPUPhysMemoryClient *client, > - target_phys_addr_t start_addr, > - target_phys_addr_t end_addr) > +static void kvm_log_global_start(struct MemoryListener *listener) > { > - return kvm_physical_sync_dirty_bitmap(start_addr, end_addr); > + int r; > + > + r = kvm_set_migration_log(1); > + assert(r >= 0); > } > > -static int kvm_client_migration_log(struct CPUPhysMemoryClient *client, > - int enable) > +static void kvm_log_global_stop(struct MemoryListener *listener) > { > - return kvm_set_migration_log(enable); > + int r; > + > + r = kvm_set_migration_log(0); > + assert(r >= 0); > } > > -static CPUPhysMemoryClient kvm_cpu_phys_memory_client = { > - .set_memory = kvm_client_set_memory, > - .sync_dirty_bitmap = kvm_client_sync_dirty_bitmap, > - .migration_log = kvm_client_migration_log, > +static MemoryListener kvm_memory_listener = { > + .region_add = kvm_region_add, > + .region_del = kvm_region_del, > .log_start = kvm_log_start, > .log_stop = kvm_log_stop, > + .log_sync = kvm_log_sync, > + .log_global_start = kvm_log_global_start, > + .log_global_stop = kvm_log_global_stop, > }; > > static void kvm_handle_interrupt(CPUState *env, int mask) > @@ -794,7 +827,7 @@ int kvm_init(void) > } > > kvm_state = s; > - cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client); > + memory_listener_register(&kvm_memory_listener); > > s->many_ioeventfds = kvm_check_many_ioeventfds(); >
On 2012-01-15 11:49, Jan Kiszka wrote: > On 2011-12-19 15:13, Avi Kivity wrote: >> Drop the use of cpu_register_phys_memory_client() in favour of the new >> MemoryListener API. The new API simplifies the caller, since there is no >> need to deal with splitting and merging slots; however this is not exploited >> in this patch. > > This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. In fact, it breaks all vga types in that scenario. Jan
On 01/15/2012 12:52 PM, Jan Kiszka wrote: > On 2012-01-15 11:49, Jan Kiszka wrote: > > On 2011-12-19 15:13, Avi Kivity wrote: > >> Drop the use of cpu_register_phys_memory_client() in favour of the new > >> MemoryListener API. The new API simplifies the caller, since there is no > >> need to deal with splitting and merging slots; however this is not exploited > >> in this patch. > > > > This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. > > In fact, it breaks all vga types in that scenario. > An F14 guest works here. More info, please.
On 2012-01-15 13:35, Avi Kivity wrote: > On 01/15/2012 12:52 PM, Jan Kiszka wrote: >> On 2012-01-15 11:49, Jan Kiszka wrote: >>> On 2011-12-19 15:13, Avi Kivity wrote: >>>> Drop the use of cpu_register_phys_memory_client() in favour of the new >>>> MemoryListener API. The new API simplifies the caller, since there is no >>>> need to deal with splitting and merging slots; however this is not exploited >>>> in this patch. >>> >>> This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. >> >> In fact, it breaks all vga types in that scenario. >> > > An F14 guest works here. More info, please. Just try to boot an openSUSE live image (or installation). Grub output is corrupted, obviously dirty logging is not properly set up in that graphic mode. Jan
On 01/15/2012 02:40 PM, Jan Kiszka wrote: > On 2012-01-15 13:35, Avi Kivity wrote: > > On 01/15/2012 12:52 PM, Jan Kiszka wrote: > >> On 2012-01-15 11:49, Jan Kiszka wrote: > >>> On 2011-12-19 15:13, Avi Kivity wrote: > >>>> Drop the use of cpu_register_phys_memory_client() in favour of the new > >>>> MemoryListener API. The new API simplifies the caller, since there is no > >>>> need to deal with splitting and merging slots; however this is not exploited > >>>> in this patch. > >>> > >>> This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. > >> > >> In fact, it breaks all vga types in that scenario. > >> > > > > An F14 guest works here. More info, please. > > Just try to boot an openSUSE live image (or installation). Grub output > is corrupted, obviously dirty logging is not properly set up in that > graphic mode. > Downloading now.
On 01/15/2012 02:49 PM, Avi Kivity wrote: > On 01/15/2012 02:40 PM, Jan Kiszka wrote: > > On 2012-01-15 13:35, Avi Kivity wrote: > > > On 01/15/2012 12:52 PM, Jan Kiszka wrote: > > >> On 2012-01-15 11:49, Jan Kiszka wrote: > > >>> On 2011-12-19 15:13, Avi Kivity wrote: > > >>>> Drop the use of cpu_register_phys_memory_client() in favour of the new > > >>>> MemoryListener API. The new API simplifies the caller, since there is no > > >>>> need to deal with splitting and merging slots; however this is not exploited > > >>>> in this patch. > > >>> > > >>> This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. > > >> > > >> In fact, it breaks all vga types in that scenario. > > >> > > > > > > An F14 guest works here. More info, please. > > > > Just try to boot an openSUSE live image (or installation). Grub output > > is corrupted, obviously dirty logging is not properly set up in that > > graphic mode. > > > > Downloading now. > Wait, isn't opensuse grub2 based? Which version should I test?
On 2012-01-15 13:50, Avi Kivity wrote: > On 01/15/2012 02:49 PM, Avi Kivity wrote: >> On 01/15/2012 02:40 PM, Jan Kiszka wrote: >>> On 2012-01-15 13:35, Avi Kivity wrote: >>>> On 01/15/2012 12:52 PM, Jan Kiszka wrote: >>>>> On 2012-01-15 11:49, Jan Kiszka wrote: >>>>>> On 2011-12-19 15:13, Avi Kivity wrote: >>>>>>> Drop the use of cpu_register_phys_memory_client() in favour of the new >>>>>>> MemoryListener API. The new API simplifies the caller, since there is no >>>>>>> need to deal with splitting and merging slots; however this is not exploited >>>>>>> in this patch. >>>>>> >>>>>> This breaks graphical grub1 with cirrus-vga in KVM mode. Dunno why yet. >>>>> >>>>> In fact, it breaks all vga types in that scenario. >>>>> >>>> >>>> An F14 guest works here. More info, please. >>> >>> Just try to boot an openSUSE live image (or installation). Grub output >>> is corrupted, obviously dirty logging is not properly set up in that >>> graphic mode. >>> >> >> Downloading now. >> > > Wait, isn't opensuse grub2 based? Which version should I test? > My test case is 11.4-based, but I think to remember 12.1 is also still grub1 (luckily...). Jan
diff --git a/kvm-all.c b/kvm-all.c index 4f58ae8..138e0a2 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -27,6 +27,7 @@ #include "gdbstub.h" #include "kvm.h" #include "bswap.h" +#include "memory.h" /* This check must be after config-host.h is included */ #ifdef CONFIG_EVENTFD @@ -289,16 +290,28 @@ static int kvm_dirty_pages_log_change(target_phys_addr_t phys_addr, return kvm_slot_dirty_pages_log_change(mem, log_dirty); } -static int kvm_log_start(CPUPhysMemoryClient *client, - target_phys_addr_t phys_addr, ram_addr_t size) +static void kvm_log_start(MemoryListener *listener, + MemoryRegionSection *section) { - return kvm_dirty_pages_log_change(phys_addr, size, true); + int r; + + r = kvm_dirty_pages_log_change(section->offset_within_address_space, + section->size, true); + if (r < 0) { + abort(); + } } -static int kvm_log_stop(CPUPhysMemoryClient *client, - target_phys_addr_t phys_addr, ram_addr_t size) +static void kvm_log_stop(MemoryListener *listener, + MemoryRegionSection *section) { - return kvm_dirty_pages_log_change(phys_addr, size, false); + int r; + + r = kvm_dirty_pages_log_change(section->offset_within_address_space, + section->size, false); + if (r < 0) { + abort(); + } } static int kvm_set_migration_log(int enable) @@ -519,13 +532,15 @@ static int kvm_check_many_ioeventfds(void) return NULL; } -static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, - ram_addr_t phys_offset, bool log_dirty) +static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) { KVMState *s = kvm_state; - ram_addr_t flags = phys_offset & ~TARGET_PAGE_MASK; KVMSlot *mem, old; int err; + MemoryRegion *mr = section->mr; + bool log_dirty = memory_region_is_logging(mr); + target_phys_addr_t start_addr = section->offset_within_address_space; + ram_addr_t size = section->size; void *ram = NULL; /* kvm works in page size chunks, but the function may be called @@ -533,20 +548,19 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, size = TARGET_PAGE_ALIGN(size); start_addr = TARGET_PAGE_ALIGN(start_addr); - /* KVM does not support read-only slots */ - phys_offset &= ~IO_MEM_ROM; - - if ((phys_offset & ~TARGET_PAGE_MASK) == IO_MEM_RAM) { - ram = qemu_safe_ram_ptr(phys_offset); + if (!memory_region_is_ram(mr)) { + return; } + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; + while (1) { mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + size); if (!mem) { break; } - if (flags < IO_MEM_UNASSIGNED && start_addr >= mem->start_addr && + if (add && start_addr >= mem->start_addr && (start_addr + size <= mem->start_addr + mem->memory_size) && (ram - start_addr == mem->ram - mem->start_addr)) { /* The new slot fits into the existing one and comes with @@ -575,8 +589,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, * slot comes around later, we will fail (not seen in practice so far) * - and actually require a recent KVM version. */ if (s->broken_set_mem_region && - old.start_addr == start_addr && old.memory_size < size && - flags < IO_MEM_UNASSIGNED) { + old.start_addr == start_addr && old.memory_size < size && add) { mem = kvm_alloc_slot(s); mem->memory_size = old.memory_size; mem->start_addr = old.start_addr; @@ -591,7 +604,6 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, } start_addr += old.memory_size; - phys_offset += old.memory_size; ram += old.memory_size; size -= old.memory_size; continue; @@ -642,8 +654,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, if (!size) { return; } - /* KVM does not need to know about this memory */ - if (flags >= IO_MEM_UNASSIGNED) { + if (!add) { return; } mem = kvm_alloc_slot(s); @@ -660,33 +671,55 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, } } -static void kvm_client_set_memory(struct CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - ram_addr_t size, ram_addr_t phys_offset, - bool log_dirty) +static void kvm_region_add(MemoryListener *listener, + MemoryRegionSection *section) +{ + kvm_set_phys_mem(section, true); +} + +static void kvm_region_del(MemoryListener *listener, + MemoryRegionSection *section) { - kvm_set_phys_mem(start_addr, size, phys_offset, log_dirty); + kvm_set_phys_mem(section, false); +} + +static void kvm_log_sync(MemoryListener *listener, + MemoryRegionSection *section) +{ + target_phys_addr_t start = section->offset_within_address_space; + target_phys_addr_t end = start + section->size; + int r; + + r = kvm_physical_sync_dirty_bitmap(start, end); + if (r < 0) { + abort(); + } } -static int kvm_client_sync_dirty_bitmap(struct CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - target_phys_addr_t end_addr) +static void kvm_log_global_start(struct MemoryListener *listener) { - return kvm_physical_sync_dirty_bitmap(start_addr, end_addr); + int r; + + r = kvm_set_migration_log(1); + assert(r >= 0); } -static int kvm_client_migration_log(struct CPUPhysMemoryClient *client, - int enable) +static void kvm_log_global_stop(struct MemoryListener *listener) { - return kvm_set_migration_log(enable); + int r; + + r = kvm_set_migration_log(0); + assert(r >= 0); } -static CPUPhysMemoryClient kvm_cpu_phys_memory_client = { - .set_memory = kvm_client_set_memory, - .sync_dirty_bitmap = kvm_client_sync_dirty_bitmap, - .migration_log = kvm_client_migration_log, +static MemoryListener kvm_memory_listener = { + .region_add = kvm_region_add, + .region_del = kvm_region_del, .log_start = kvm_log_start, .log_stop = kvm_log_stop, + .log_sync = kvm_log_sync, + .log_global_start = kvm_log_global_start, + .log_global_stop = kvm_log_global_stop, }; static void kvm_handle_interrupt(CPUState *env, int mask) @@ -794,7 +827,7 @@ int kvm_init(void) } kvm_state = s; - cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client); + memory_listener_register(&kvm_memory_listener); s->many_ioeventfds = kvm_check_many_ioeventfds();
Drop the use of cpu_register_phys_memory_client() in favour of the new MemoryListener API. The new API simplifies the caller, since there is no need to deal with splitting and merging slots; however this is not exploited in this patch. Signed-off-by: Avi Kivity <avi@redhat.com> --- kvm-all.c | 107 ++++++++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 70 insertions(+), 37 deletions(-)