cpu_get_physical_page_desc() exposes the internals of the memory core to callers; as such it prevents the refactoring planned there. This patchset converts all callers memory API equivalents and removes the function. The conversion leaves a lot of potential for further cleanups; the MemoryListener API (which replaces CPUPhysMemoryClient) guarantees matched range_add and range_del calls, so the need to handle splitting is removed. This is left for later. Please review and test, especially the vhost and Xen parts, which I only build tested. Also available from: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/page_desc Avi Kivity (23): memory: introduce memory_region_find() sysbus: add sysbus_address_space() memory: add memory_region_is_ram() framebuffer: drop use of cpu_get_physical_page_desc() memory: add memory_region_is_rom() loader: remove calls to cpu_get_physical_page_desc() framebuffer: drop use of cpu_physical_sync_dirty_bitmap() memory: replace cpu_physical_sync_dirty_bitmap() with a memory API memory: add API for observing updates to the physical memory map memory: add memory_region_is_logging() kvm: switch kvm slots to use host virtual address instead of ram_addr_t fixup: listener fixes kvm: convert to MemoryListener API vhost: convert to MemoryListener API xen, vga: add API for registering the framebuffer memory: temporarily add memory_region_get_ram_addr() xen: convert to MemoryListener API memory: remove CPUPhysMemoryClient kvm: avoid cpu_get_physical_page_desc() vhost: avoid cpu_get_physical_page_desc() virtio-balloon: avoid cpu_get_physical_page_desc() sparc: avoid cpu_get_physical_page_desc() Remove cpu_get_physical_page_desc() arch_init.c | 6 +- cpu-all.h | 9 -- cpu-common.h | 24 ------ exec.c | 175 +--------------------------------------- hw/framebuffer.c | 32 +++---- hw/framebuffer.h | 3 + hw/loader.c | 9 +- hw/milkymist-vgafb.c | 2 +- hw/omap_lcdc.c | 4 +- hw/pl110.c | 2 +- hw/pxa2xx_lcd.c | 10 ++- hw/sysbus.c | 5 + hw/sysbus.h | 1 + hw/vga.c | 2 + hw/vhost.c | 167 ++++++++++++++++++++++++++++++--------- hw/vhost.h | 5 +- hw/virtio-balloon.c | 14 +++- hw/xen.h | 3 + kvm-all.c | 151 +++++++++++++++++++++-------------- kvm.h | 4 +- memory.c | 193 ++++++++++++++++++++++++++++++++++++++++++--- memory.h | 127 +++++++++++++++++++++++++++++ target-i386/kvm.c | 7 +- target-sparc/mmu_helper.c | 5 +- trace-events | 2 +- xen-all.c | 143 ++++++++++++++++++++------------- xen-stub.c | 4 + 27 files changed, 695 insertions(+), 414 deletions(-) -- 1.7.7.1 -- 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
Given an address space (represented by the top-level memory region), returns the memory region that maps a given range. Useful for implementing DMA. The implementation is a simplistic binary search. Once we have a tree representation this can be optimized. Signed-off-by: Avi Kivity <avi@redhat.com> --- memory.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ memory.h | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index cbd6988..d8b7c27 100644 --- a/memory.c +++ b/memory.c @@ -514,6 +514,20 @@ static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd) .ops = &address_space_ops_io, }; +static AddressSpace *memory_region_to_address_space(MemoryRegion *mr) +{ + while (mr->parent) { + mr = mr->parent; + } + if (mr == address_space_memory.root) { + return &address_space_memory; + } + if (mr == address_space_io.root) { + return &address_space_io; + } + abort(); +} + /* Render a memory region into the global view. Ranges in @view obscure * ranges in @mr. */ @@ -1309,6 +1323,54 @@ void memory_region_del_subregion(MemoryRegion *mr, memory_region_update_topology(); } +static int cmp_flatrange_addr(const void *_addr, const void *_fr) +{ + const AddrRange *addr = _addr; + const FlatRange *fr = _fr; + + if (int128_le(addrrange_end(*addr), fr->addr.start)) { + return -1; + } else if (int128_ge(addr->start, addrrange_end(fr->addr))) { + return 1; + } + return 0; +} + +static FlatRange *address_space_lookup(AddressSpace *as, AddrRange addr) +{ + return bsearch(&addr, as->current_map.ranges, as->current_map.nr, + sizeof(FlatRange), cmp_flatrange_addr); +} + +MemoryRegionSection memory_region_find(MemoryRegion *address_space, + target_phys_addr_t addr, uint64_t size) +{ + AddressSpace *as = memory_region_to_address_space(address_space); + AddrRange range = addrrange_make(int128_make64(addr), + int128_make64(size)); + FlatRange *fr = address_space_lookup(as, range); + MemoryRegionSection ret = { .mr = NULL }; + + if (!fr) { + return ret; + } + + while (fr > as->current_map.ranges + && addrrange_intersects(fr[-1].addr, range)) { + --fr; + } + + ret.mr = fr->mr; + range = addrrange_intersection(range, fr->addr); + ret.offset_within_region = fr->offset_in_region; + ret.offset_within_region += int128_get64(int128_sub(range.start, + fr->addr.start)); + ret.size = int128_get64(range.size); + ret.offset_within_address_space = int128_get64(range.start); + return ret; +} + + void set_system_memory_map(MemoryRegion *mr) { address_space_memory.root = mr; diff --git a/memory.h b/memory.h index beae127..1d5c414 100644 --- a/memory.h +++ b/memory.h @@ -146,6 +146,24 @@ struct MemoryRegionPortio { #define PORTIO_END_OF_LIST() { } +typedef struct MemoryRegionSection MemoryRegionSection; + +/** + * MemoryRegionSection: describes a fragment of a #MemoryRegion + * + * @mr: the region, or %NULL if empty + * @offset_within_region: the beginning of the section, relative to @mr''s start + * @size: the size of the section; will not exceed @mr''s boundaries + * @offset_within_address_space: the address of the first byte of the section + * relative to the region''s address space + */ +struct MemoryRegionSection { + MemoryRegion *mr; + target_phys_addr_t offset_within_region; + uint64_t size; + target_phys_addr_t offset_within_address_space; +}; + /** * memory_region_init: Initialize a memory region * @@ -502,6 +520,20 @@ void memory_region_del_subregion(MemoryRegion *mr, MemoryRegion *subregion); /** + * memory_region_find: locate a MemoryRegion in an address space + * + * Locates the first #MemoryRegion within an address space given by + * @address_space that overlaps the range given by @addr and @size. + * + * @address_space: a top-level (i.e. parentless) region that contains + * the region to be found + * @addr: start of the area within @address_space to be searched + * @size: size of the area to be searched + */ +MemoryRegionSection memory_region_find(MemoryRegion *address_space, + target_phys_addr_t addr, uint64_t size); + +/** * memory_region_transaction_begin: Start a transaction. * * During a transaction, changes will be accumulated and made visible -- 1.7.7.1 -- 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
Given a bus device, retrieves the memory address space for its bus. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/sysbus.c | 5 +++++ hw/sysbus.h | 1 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/hw/sysbus.c b/hw/sysbus.c index b315a8c..492a7c6 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -277,3 +277,8 @@ void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem) { memory_region_del_subregion(get_system_io(), mem); } + +MemoryRegion *sysbus_address_space(SysBusDevice *dev) +{ + return get_system_memory(); +} diff --git a/hw/sysbus.h b/hw/sysbus.h index 9bac582..7cecf98 100644 --- a/hw/sysbus.h +++ b/hw/sysbus.h @@ -62,6 +62,7 @@ void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem); void sysbus_add_io(SysBusDevice *dev, target_phys_addr_t addr, MemoryRegion *mem); void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem); +MemoryRegion *sysbus_address_space(SysBusDevice *dev); /* Legacy helper function for creating devices. */ DeviceState *sysbus_create_varargs(const char *name, -- 1.7.7.1 -- 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
Signed-off-by: Avi Kivity <avi@redhat.com> --- memory.c | 8 ++++++++ memory.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index d8b7c27..6fd31d4 100644 --- a/memory.c +++ b/memory.c @@ -831,6 +831,7 @@ void memory_region_init(MemoryRegion *mr, mr->addr = 0; mr->offset = 0; mr->terminates = false; + mr->ram = false; mr->readable = true; mr->readonly = false; mr->destructor = memory_region_destructor_none; @@ -997,6 +998,7 @@ void memory_region_init_ram(MemoryRegion *mr, uint64_t size) { memory_region_init(mr, name, size); + mr->ram = true; mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->ram_addr = qemu_ram_alloc(dev, name, size, mr); @@ -1010,6 +1012,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, void *ptr) { memory_region_init(mr, name, size); + mr->ram = true; mr->terminates = true; mr->destructor = memory_region_destructor_ram_from_ptr; mr->ram_addr = qemu_ram_alloc_from_ptr(dev, name, size, ptr, mr); @@ -1065,6 +1068,11 @@ uint64_t memory_region_size(MemoryRegion *mr) return int128_get64(mr->size); } +bool memory_region_is_ram(MemoryRegion *mr) +{ + return mr->ram; +} + void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset) { mr->offset = offset; diff --git a/memory.h b/memory.h index 1d5c414..13f88c0 100644 --- a/memory.h +++ b/memory.h @@ -122,6 +122,7 @@ struct MemoryRegion { IORange iorange; bool terminates; bool readable; + bool ram; bool readonly; /* For RAM regions */ MemoryRegion *alias; target_phys_addr_t alias_offset; @@ -284,6 +285,15 @@ void memory_region_destroy(MemoryRegion *mr); uint64_t memory_region_size(MemoryRegion *mr); /** + * memory_region_is_ram: check whether a memory region is random access + * + * Returns %true is a memory region is random access. + * + * @mr: the memory region being queried + */ +bool memory_region_is_ram(MemoryRegion *mr); + +/** * memory_region_get_ram_ptr: Get a pointer into a RAM memory region. * * Returns a host pointer to a RAM memory region (created with -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 04/23] framebuffer: drop use of cpu_get_physical_page_desc()
cpu_get_physical_page_desc() is tied into the memory core''s innards, replace it with uses of the API. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/framebuffer.c | 30 +++++++++++++----------------- hw/framebuffer.h | 3 +++ hw/milkymist-vgafb.c | 2 +- hw/omap_lcdc.c | 4 +++- hw/pl110.c | 2 +- hw/pxa2xx_lcd.c | 10 ++++++---- 6 files changed, 27 insertions(+), 24 deletions(-) diff --git a/hw/framebuffer.c b/hw/framebuffer.c index 56cf16e..3f4e773 100644 --- a/hw/framebuffer.c +++ b/hw/framebuffer.c @@ -22,6 +22,7 @@ void framebuffer_update_display( DisplayState *ds, + MemoryRegion *address_space, target_phys_addr_t base, int cols, /* Width in pixels. */ int rows, /* Leight in pixels. */ @@ -42,27 +43,21 @@ void framebuffer_update_display( int dirty; int i; ram_addr_t addr; - ram_addr_t pd; - ram_addr_t pd2; + MemoryRegionSection mem_section; + MemoryRegion *mem; i = *first_row; *first_row = -1; src_len = src_width * rows; cpu_physical_sync_dirty_bitmap(base, base + src_len); - pd = cpu_get_physical_page_desc(base); - pd2 = cpu_get_physical_page_desc(base + src_len - 1); - /* We should reall check that this is a continuous ram region. - Instead we just check that the first and last pages are - both ram, and the right distance apart. */ - if ((pd & ~TARGET_PAGE_MASK) > IO_MEM_ROM - || (pd2 & ~TARGET_PAGE_MASK) > IO_MEM_ROM) { - return; - } - pd = (pd & TARGET_PAGE_MASK) + (base & ~TARGET_PAGE_MASK); - if (((pd + src_len - 1) & TARGET_PAGE_MASK) != (pd2 & TARGET_PAGE_MASK)) { + mem_section = memory_region_find(address_space, base, src_len); + if (mem_section.size != src_len || !memory_region_is_ram(mem_section.mr)) { return; } + mem = mem_section.mr; + assert(mem); + assert(mem_section.offset_within_address_space == base); src_base = cpu_physical_memory_map(base, &src_len, 0); /* If we can''t map the framebuffer then bail. We could try harder, @@ -82,7 +77,7 @@ void framebuffer_update_display( dest -= dest_row_pitch * (rows - 1); } first = -1; - addr = pd; + addr = mem_section.offset_within_region; addr += i * src_width; src += i * src_width; @@ -93,8 +88,8 @@ void framebuffer_update_display( dirty = 0; dirty_offset = 0; while (addr + dirty_offset < TARGET_PAGE_ALIGN(addr + src_width)) { - dirty |= cpu_physical_memory_get_dirty(addr + dirty_offset, - VGA_DIRTY_FLAG); + dirty |= memory_region_get_dirty(mem, addr + dirty_offset, + DIRTY_MEMORY_VGA); dirty_offset += TARGET_PAGE_SIZE; } @@ -112,7 +107,8 @@ void framebuffer_update_display( if (first < 0) { return; } - cpu_physical_memory_reset_dirty(pd, pd + src_len, VGA_DIRTY_FLAG); + memory_region_reset_dirty(mem, mem_section.offset_within_region, src_len, + DIRTY_MEMORY_VGA); *first_row = first; *last_row = last; return; diff --git a/hw/framebuffer.h b/hw/framebuffer.h index a3a2146..527a6b8 100644 --- a/hw/framebuffer.h +++ b/hw/framebuffer.h @@ -1,12 +1,15 @@ #ifndef QEMU_FRAMEBUFFER_H #define QEMU_FRAMEBUFFER_H +#include "memory.h" + /* Framebuffer device helper routines. */ typedef void (*drawfn)(void *, uint8_t *, const uint8_t *, int, int); void framebuffer_update_display( DisplayState *ds, + MemoryRegion *address_space, target_phys_addr_t base, int cols, int rows, diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c index 01cd309..108115e 100644 --- a/hw/milkymist-vgafb.c +++ b/hw/milkymist-vgafb.c @@ -120,7 +120,7 @@ static void vgafb_update_display(void *opaque) break; } - framebuffer_update_display(s->ds, + framebuffer_update_display(s->ds, sysbus_address_space(&s->busdev), s->regs[R_BASEADDRESS] + s->fb_offset, s->regs[R_HRES], s->regs[R_VRES], diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c index 8484f70..f265306 100644 --- a/hw/omap_lcdc.c +++ b/hw/omap_lcdc.c @@ -22,6 +22,7 @@ #include "framebuffer.h" struct omap_lcd_panel_s { + MemoryRegion *sysmem; MemoryRegion iomem; qemu_irq irq; DisplayState *state; @@ -211,7 +212,7 @@ static void omap_update_display(void *opaque) step = width * bpp >> 3; linesize = ds_get_linesize(omap_lcd->state); - framebuffer_update_display(omap_lcd->state, + framebuffer_update_display(omap_lcd->state, omap_lcd->sysmem, frame_base, width, height, step, linesize, 0, omap_lcd->invalidate, @@ -440,6 +441,7 @@ struct omap_lcd_panel_s *omap_lcdc_init(MemoryRegion *sysmem, s->irq = irq; s->dma = dma; + s->sysmem = sysmem; omap_lcdc_reset(s); memory_region_init_io(&s->iomem, &omap_lcdc_ops, s, "omap.lcdc", 0x100); diff --git a/hw/pl110.c b/hw/pl110.c index cc1eb6d..303a9bc 100644 --- a/hw/pl110.c +++ b/hw/pl110.c @@ -229,7 +229,7 @@ static void pl110_update_display(void *opaque) } dest_width *= s->cols; first = 0; - framebuffer_update_display(s->ds, + framebuffer_update_display(s->ds, sysbus_address_space(&s->busdev), s->upbase, s->cols, s->rows, src_width, dest_width, 0, s->invalidate, diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c index fd23d63..5dd4ef0 100644 --- a/hw/pxa2xx_lcd.c +++ b/hw/pxa2xx_lcd.c @@ -30,6 +30,7 @@ struct DMAChannel { }; struct PXA2xxLCDState { + MemoryRegion *sysmem; MemoryRegion iomem; qemu_irq irq; int irqlevel; @@ -681,7 +682,7 @@ static void pxa2xx_lcdc_dma0_redraw_rot0(PXA2xxLCDState *s, dest_width = s->xres * s->dest_width; *miny = 0; - framebuffer_update_display(s->ds, + framebuffer_update_display(s->ds, s->sysmem, addr, s->xres, s->yres, src_width, dest_width, s->dest_width, s->invalidated, @@ -708,7 +709,7 @@ static void pxa2xx_lcdc_dma0_redraw_rot90(PXA2xxLCDState *s, dest_width = s->yres * s->dest_width; *miny = 0; - framebuffer_update_display(s->ds, + framebuffer_update_display(s->ds, s->sysmem, addr, s->xres, s->yres, src_width, s->dest_width, -dest_width, s->invalidated, @@ -739,7 +740,7 @@ static void pxa2xx_lcdc_dma0_redraw_rot180(PXA2xxLCDState *s, dest_width = s->xres * s->dest_width; *miny = 0; - framebuffer_update_display(s->ds, + framebuffer_update_display(s->ds, s->sysmem, addr, s->xres, s->yres, src_width, -dest_width, -s->dest_width, s->invalidated, @@ -769,7 +770,7 @@ static void pxa2xx_lcdc_dma0_redraw_rot270(PXA2xxLCDState *s, dest_width = s->yres * s->dest_width; *miny = 0; - framebuffer_update_display(s->ds, + framebuffer_update_display(s->ds, s->sysmem, addr, s->xres, s->yres, src_width, -s->dest_width, dest_width, s->invalidated, @@ -985,6 +986,7 @@ static int pxa2xx_lcdc_post_load(void *opaque, int version_id) s = (PXA2xxLCDState *) g_malloc0(sizeof(PXA2xxLCDState)); s->invalidated = 1; s->irq = irq; + s->sysmem = sysmem; pxa2xx_lcdc_orientation(s, graphic_rotate); -- 1.7.7.1 -- 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
Signed-off-by: Avi Kivity <avi@redhat.com> --- memory.c | 5 +++++ memory.h | 9 +++++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index 6fd31d4..c57b7de 100644 --- a/memory.c +++ b/memory.c @@ -1073,6 +1073,11 @@ bool memory_region_is_ram(MemoryRegion *mr) return mr->ram; } +bool memory_region_is_rom(MemoryRegion *mr) +{ + return mr->ram && mr->readonly; +} + void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset) { mr->offset = offset; diff --git a/memory.h b/memory.h index 13f88c0..42546c4 100644 --- a/memory.h +++ b/memory.h @@ -294,6 +294,15 @@ uint64_t memory_region_size(MemoryRegion *mr); bool memory_region_is_ram(MemoryRegion *mr); /** + * memory_region_is_rom: check whether a memory region is ROM + * + * Returns %true is a memory region is read-only memory. + * + * @mr: the memory region being queried + */ +bool memory_region_is_rom(MemoryRegion *mr); + +/** * memory_region_get_ram_ptr: Get a pointer into a RAM memory region. * * Returns a host pointer to a RAM memory region (created with -- 1.7.7.1
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 06/23] loader: remove calls to cpu_get_physical_page_desc()
cpu_get_physical_page_desc() is tied into the memory core''s innards, replace it with uses of the API. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/loader.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/loader.c b/hw/loader.c index 9bbcddd..7af5411 100644 --- a/hw/loader.c +++ b/hw/loader.c @@ -49,6 +49,8 @@ #include "uboot_image.h" #include "loader.h" #include "fw_cfg.h" +#include "memory.h" +#include "exec-memory.h" #include <zlib.h> @@ -674,7 +676,7 @@ static void rom_reset(void *unused) int rom_load_all(void) { target_phys_addr_t addr = 0; - int memtype; + MemoryRegionSection section; Rom *rom; QTAILQ_FOREACH(rom, &roms, next) { @@ -690,9 +692,8 @@ int rom_load_all(void) } addr = rom->addr; addr += rom->romsize; - memtype = cpu_get_physical_page_desc(rom->addr) & (3 << IO_MEM_SHIFT); - if (memtype == IO_MEM_ROM) - rom->isrom = 1; + section = memory_region_find(get_system_memory(), rom->addr, 1); + rom->isrom = section.mr && memory_region_is_rom(section.mr); } qemu_register_reset(rom_reset, NULL); roms_loaded = 1; -- 1.7.7.1
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 07/23] framebuffer: drop use of cpu_physical_sync_dirty_bitmap()
Replace with memory API equivalent. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/framebuffer.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/framebuffer.c b/hw/framebuffer.c index 3f4e773..b43bcdf 100644 --- a/hw/framebuffer.c +++ b/hw/framebuffer.c @@ -50,7 +50,6 @@ void framebuffer_update_display( *first_row = -1; src_len = src_width * rows; - cpu_physical_sync_dirty_bitmap(base, base + src_len); mem_section = memory_region_find(address_space, base, src_len); if (mem_section.size != src_len || !memory_region_is_ram(mem_section.mr)) { return; @@ -59,6 +58,7 @@ void framebuffer_update_display( assert(mem); assert(mem_section.offset_within_address_space == base); + memory_region_sync_dirty_bitmap(mem); src_base = cpu_physical_memory_map(base, &src_len, 0); /* If we can''t map the framebuffer then bail. We could try harder, but it''s not really worth it as dirty flag tracking will probably -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 08/23] memory: replace cpu_physical_sync_dirty_bitmap() with a memory API
The function is still used as the implementation. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch_init.c | 6 ++---- cpu-all.h | 3 --- exec-obsolete.h | 3 +++ memory.c | 4 ++++ memory.h | 10 ++++++++++ 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/arch_init.c b/arch_init.c index a411fdf..ceef26e 100644 --- a/arch_init.c +++ b/arch_init.c @@ -41,6 +41,7 @@ #include "net.h" #include "gdbstub.h" #include "hw/smbios.h" +#include "exec-memory.h" #ifdef TARGET_SPARC int graphic_width = 1024; @@ -263,10 +264,7 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque) return 0; } - if (cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX) != 0) { - qemu_file_set_error(f, -EINVAL); - return -EINVAL; - } + memory_global_sync_dirty_bitmap(get_system_memory()); if (stage == 1) { RAMBlock *block; diff --git a/cpu-all.h b/cpu-all.h index 9d78715..f2c5382 100644 --- a/cpu-all.h +++ b/cpu-all.h @@ -569,9 +569,6 @@ int cpu_physical_memory_set_dirty_tracking(int enable); int cpu_physical_memory_get_dirty_tracking(void); -int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, - target_phys_addr_t end_addr); - int cpu_physical_log_start(target_phys_addr_t start_addr, ram_addr_t size); diff --git a/exec-obsolete.h b/exec-obsolete.h index 1d0b610..aa0a04d 100644 --- a/exec-obsolete.h +++ b/exec-obsolete.h @@ -64,6 +64,9 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr, void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size); void qemu_unregister_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size); +int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, + target_phys_addr_t end_addr); + #endif #endif diff --git a/memory.c b/memory.c index c57b7de..639f3b1 100644 --- a/memory.c +++ b/memory.c @@ -1383,6 +1383,10 @@ MemoryRegionSection memory_region_find(MemoryRegion *address_space, return ret; } +void memory_global_sync_dirty_bitmap(MemoryRegion *address_space) +{ + cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX); +} void set_system_memory_map(MemoryRegion *mr) { diff --git a/memory.h b/memory.h index 42546c4..3e27593 100644 --- a/memory.h +++ b/memory.h @@ -552,6 +552,16 @@ void memory_region_del_subregion(MemoryRegion *mr, MemoryRegionSection memory_region_find(MemoryRegion *address_space, target_phys_addr_t addr, uint64_t size); + +/** + * memory_global_sync_dirty_bitmap: synchronize the dirty log for all memory + * + * Synchronizes the dirty page log for an entire address space. + * @address_space: a top-level (i.e. parentless) region that contains the + * memory being synchronized + */ +void memory_global_sync_dirty_bitmap(MemoryRegion *address_space); + /** * memory_region_transaction_begin: Start a transaction. * -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 09/23] memory: add API for observing updates to the physical memory map
Add an API that allows a client to observe changes in the global memory map: - region added (possibly with logging enabled) - region removed (possibly with logging enabled) - logging started on a region - logging stopped on a region - global logging started - global logging removed This API will eventually replace cpu_register_physical_memory_client(). Signed-off-by: Avi Kivity <avi@redhat.com> --- exec.c | 5 +++ memory.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ memory.h | 47 +++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 0 deletions(-) diff --git a/exec.c b/exec.c index 32782b4..36b61c9 100644 --- a/exec.c +++ b/exec.c @@ -1762,6 +1762,11 @@ static int cpu_notify_sync_dirty_bitmap(target_phys_addr_t start, static int cpu_notify_migration_log(int enable) { CPUPhysMemoryClient *client; + if (enable) { + memory_global_dirty_log_start(); + } else { + memory_global_dirty_log_stop(); + } QLIST_FOREACH(client, &memory_client_list, list) { int r = client->migration_log(client, enable); if (r < 0) diff --git a/memory.c b/memory.c index 639f3b1..ee9053a 100644 --- a/memory.c +++ b/memory.c @@ -22,6 +22,10 @@ #include "exec-obsolete.h" unsigned memory_region_transaction_depth = 0; +static bool global_dirty_log = false; + +static QLIST_HEAD(, MemoryListener) memory_listeners + = QLIST_HEAD_INITIALIZER(memory_listeners); typedef struct AddrRange AddrRange; @@ -692,6 +696,32 @@ static void address_space_update_ioeventfds(AddressSpace *as) as->ioeventfd_nb = ioeventfd_nb; } +typedef void ListenerCallback(MemoryListener *listener, + MemoryRegionSection *mrs); + +/* Want "void (&MemoryListener::*callback)(const MemoryRegionSection& s)" */ +static void memory_listener_update_region(FlatRange *fr, AddressSpace *as, + size_t callback_offset) +{ + MemoryRegionSection section = { + .mr = fr->mr, + .address_space = as->root, + .offset_within_region = fr->offset_in_region, + .size = int128_get64(fr->addr.size), + .offset_within_region = int128_get64(fr->addr.start), + }; + MemoryListener *listener; + + QLIST_FOREACH(listener, &memory_listeners, link) { + ListenerCallback *callback + = *(ListenerCallback *)((void *)listener + callback_offset); + callback(listener, §ion); + } +} + +#define MEMORY_LISTENER_UPDATE_REGION(fr, as, callback) \ + memory_listener_update_region(fr, as, offsetof(MemoryListener, callback)) + static void address_space_update_topology_pass(AddressSpace *as, FlatView old_view, FlatView new_view, @@ -724,6 +754,7 @@ static void address_space_update_topology_pass(AddressSpace *as, /* In old, but (not in new, or in new but attributes changed). */ if (!adding) { + MEMORY_LISTENER_UPDATE_REGION(frold, as, region_del); as->ops->range_del(as, frold); } @@ -733,9 +764,11 @@ static void address_space_update_topology_pass(AddressSpace *as, if (adding) { if (frold->dirty_log_mask && !frnew->dirty_log_mask) { + MEMORY_LISTENER_UPDATE_REGION(frold, as, log_stop); as->ops->log_stop(as, frnew); } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) { as->ops->log_start(as, frnew); + MEMORY_LISTENER_UPDATE_REGION(frold, as, log_start); } } @@ -746,6 +779,7 @@ static void address_space_update_topology_pass(AddressSpace *as, if (adding) { as->ops->range_add(as, frnew); + MEMORY_LISTENER_UPDATE_REGION(frold, as, region_add); } ++inew; @@ -1385,7 +1419,65 @@ MemoryRegionSection memory_region_find(MemoryRegion *address_space, void memory_global_sync_dirty_bitmap(MemoryRegion *address_space) { + AddressSpace *as = memory_region_to_address_space(address_space); + FlatRange *fr; + cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX); + FOR_EACH_FLAT_RANGE(fr, &as->current_map) { + MEMORY_LISTENER_UPDATE_REGION(fr, as, log_sync); + } +} + +void memory_global_dirty_log_start(void) +{ + MemoryListener *listener; + + global_dirty_log = true; + QLIST_FOREACH(listener, &memory_listeners, link) { + listener->log_global_start(listener); + } +} + +void memory_global_dirty_log_stop(void) +{ + MemoryListener *listener; + + global_dirty_log = false; + QLIST_FOREACH(listener, &memory_listeners, link) { + listener->log_global_stop(listener); + } +} + +static void listener_add_address_space(MemoryListener *listener, + AddressSpace *as) +{ + FlatRange *fr; + + if (global_dirty_log) { + listener->log_global_start(listener); + } + FOR_EACH_FLAT_RANGE(fr, &as->current_map) { + MemoryRegionSection section = { + .mr = fr->mr, + .address_space = as->root, + .offset_within_region = fr->offset_in_region, + .size = int128_get64(fr->addr.size), + .offset_within_region = int128_get64(fr->addr.start), + }; + listener->region_add(listener, §ion); + } +} + +void memory_listener_register(MemoryListener *listener) +{ + QLIST_INSERT_HEAD(&memory_listeners, listener, link); + listener_add_address_space(listener, &address_space_memory); + listener_add_address_space(listener, &address_space_io); +} + +void memory_listener_unregister(MemoryListener *listener) +{ + QLIST_REMOVE(listener, link); } void set_system_memory_map(MemoryRegion *mr) diff --git a/memory.h b/memory.h index 3e27593..6b50f6b 100644 --- a/memory.h +++ b/memory.h @@ -153,6 +153,7 @@ typedef struct MemoryRegionSection MemoryRegionSection; * MemoryRegionSection: describes a fragment of a #MemoryRegion * * @mr: the region, or %NULL if empty + * @address_space: the address space the region is mapped in * @offset_within_region: the beginning of the section, relative to @mr''s start * @size: the size of the section; will not exceed @mr''s boundaries * @offset_within_address_space: the address of the first byte of the section @@ -160,11 +161,31 @@ typedef struct MemoryRegionSection MemoryRegionSection; */ struct MemoryRegionSection { MemoryRegion *mr; + MemoryRegion *address_space; target_phys_addr_t offset_within_region; uint64_t size; target_phys_addr_t offset_within_address_space; }; +typedef struct MemoryListener MemoryListener; + +/** + * MemoryListener: callbacks structure for updates to the physical memory map + * + * Allows a component to adjust to changes in the guest-visible memory map. + * Use with memory_listener_register() and memory_listener_unregister(). + */ +struct MemoryListener { + void (*region_add)(MemoryListener *listener, MemoryRegionSection *section); + void (*region_del)(MemoryListener *listener, MemoryRegionSection *section); + void (*log_start)(MemoryListener *listener, MemoryRegionSection *section); + void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section); + void (*log_sync)(MemoryListener *listener, MemoryRegionSection *section); + void (*log_global_start)(MemoryListener *listener); + void (*log_global_stop)(MemoryListener *listener); + QLIST_ENTRY(MemoryListener) link; +}; + /** * memory_region_init: Initialize a memory region * @@ -576,6 +597,32 @@ void memory_region_transaction_begin(void); */ void memory_region_transaction_commit(void); +/** + * memory_listener_register: register callbacks to be called when memory + * sections are mapped or unmapped into an address + * space + * + * @listener: an object containing the callbacks to be called + */ +void memory_listener_register(MemoryListener *listener); + +/** + * memory_listener_unregister: undo the effect of memory_listener_register() + * + * @listener: an object containing the callbacks to be removed + */ +void memory_listener_unregister(MemoryListener *listener); + +/** + * memory_global_dirty_log_start: begin dirty logging for all regions + */ +void memory_global_dirty_log_start(void); + +/** + * memory_global_dirty_log_stop: begin dirty logging for all regions + */ +void memory_global_dirty_log_stop(void); + void mtree_info(fprintf_function mon_printf, void *f); #endif -- 1.7.7.1 -- 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
Signed-off-by: Avi Kivity <avi@redhat.com> --- memory.c | 5 +++++ memory.h | 9 +++++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index ee9053a..2e5ff43 100644 --- a/memory.c +++ b/memory.c @@ -1107,6 +1107,11 @@ bool memory_region_is_ram(MemoryRegion *mr) return mr->ram; } +bool memory_region_is_logging(MemoryRegion *mr) +{ + return mr->dirty_log_mask; +} + bool memory_region_is_rom(MemoryRegion *mr) { return mr->ram && mr->readonly; diff --git a/memory.h b/memory.h index 6b50f6b..b7d39ed 100644 --- a/memory.h +++ b/memory.h @@ -315,6 +315,15 @@ uint64_t memory_region_size(MemoryRegion *mr); bool memory_region_is_ram(MemoryRegion *mr); /** + * memory_region_is_logging: return whether a memory region is logging writes + * + * Returns %true if the memory region is logging writes + * + * @mr: the memory region being queried + */ +bool memory_region_is_logging(MemoryRegion *mr); + +/** * memory_region_is_rom: check whether a memory region is ROM * * Returns %true is a memory region is read-only memory. -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 11/23] kvm: switch kvm slots to use host virtual address instead of ram_addr_t
This simplifies a later switch to the memory API in slot management. Signed-off-by: Avi Kivity <avi@redhat.com> --- kvm-all.c | 29 +++++++++++++++++------------ kvm.h | 4 ++-- memory.c | 6 +++--- target-i386/kvm.c | 7 +++---- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 4c466d6..4f58ae8 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -50,7 +50,7 @@ { target_phys_addr_t start_addr; ram_addr_t memory_size; - ram_addr_t phys_offset; + void *ram; int slot; int flags; } KVMSlot; @@ -146,17 +146,16 @@ struct KVMState return found; } -int kvm_physical_memory_addr_from_ram(KVMState *s, ram_addr_t ram_addr, - target_phys_addr_t *phys_addr) +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram, + target_phys_addr_t *phys_addr) { int i; for (i = 0; i < ARRAY_SIZE(s->slots); i++) { KVMSlot *mem = &s->slots[i]; - if (ram_addr >= mem->phys_offset && - ram_addr < mem->phys_offset + mem->memory_size) { - *phys_addr = mem->start_addr + (ram_addr - mem->phys_offset); + if (ram >= mem->ram && ram < mem->ram + mem->memory_size) { + *phys_addr = mem->start_addr + (ram - mem->ram); return 1; } } @@ -171,7 +170,7 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot) mem.slot = slot->slot; mem.guest_phys_addr = slot->start_addr; mem.memory_size = slot->memory_size; - mem.userspace_addr = (unsigned long)qemu_safe_ram_ptr(slot->phys_offset); + mem.userspace_addr = (unsigned long)slot->ram; mem.flags = slot->flags; if (s->migration_log) { mem.flags |= KVM_MEM_LOG_DIRTY_PAGES; @@ -527,6 +526,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, ram_addr_t flags = phys_offset & ~TARGET_PAGE_MASK; KVMSlot *mem, old; int err; + void *ram = NULL; /* kvm works in page size chunks, but the function may be called with sub-page size and unaligned start address. */ @@ -536,6 +536,10 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, /* 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); + } + while (1) { mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + size); if (!mem) { @@ -544,7 +548,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, if (flags < IO_MEM_UNASSIGNED && start_addr >= mem->start_addr && (start_addr + size <= mem->start_addr + mem->memory_size) && - (phys_offset - start_addr == mem->phys_offset - mem->start_addr)) { + (ram - start_addr == mem->ram - mem->start_addr)) { /* The new slot fits into the existing one and comes with * identical parameters - update flags and done. */ kvm_slot_dirty_pages_log_change(mem, log_dirty); @@ -576,7 +580,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, mem = kvm_alloc_slot(s); mem->memory_size = old.memory_size; mem->start_addr = old.start_addr; - mem->phys_offset = old.phys_offset; + mem->ram = old.ram; mem->flags = kvm_mem_flags(s, log_dirty); err = kvm_set_user_memory_region(s, mem); @@ -588,6 +592,7 @@ 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; } @@ -597,7 +602,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, mem = kvm_alloc_slot(s); mem->memory_size = start_addr - old.start_addr; mem->start_addr = old.start_addr; - mem->phys_offset = old.phys_offset; + mem->ram = old.ram; mem->flags = kvm_mem_flags(s, log_dirty); err = kvm_set_user_memory_region(s, mem); @@ -621,7 +626,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, mem->start_addr = start_addr + size; size_delta = mem->start_addr - old.start_addr; mem->memory_size = old.memory_size - size_delta; - mem->phys_offset = old.phys_offset + size_delta; + mem->ram = old.ram + size_delta; mem->flags = kvm_mem_flags(s, log_dirty); err = kvm_set_user_memory_region(s, mem); @@ -644,7 +649,7 @@ static void kvm_set_phys_mem(target_phys_addr_t start_addr, ram_addr_t size, mem = kvm_alloc_slot(s); mem->memory_size = size; mem->start_addr = start_addr; - mem->phys_offset = phys_offset; + mem->ram = ram; mem->flags = kvm_mem_flags(s, log_dirty); err = kvm_set_user_memory_region(s, mem); diff --git a/kvm.h b/kvm.h index 243b063..c1de81a 100644 --- a/kvm.h +++ b/kvm.h @@ -188,8 +188,8 @@ static inline void cpu_synchronize_post_init(CPUState *env) #if !defined(CONFIG_USER_ONLY) -int kvm_physical_memory_addr_from_ram(KVMState *s, ram_addr_t ram_addr, - target_phys_addr_t *phys_addr); +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, + target_phys_addr_t *phys_addr); #endif #endif diff --git a/memory.c b/memory.c index 2e5ff43..c08186d 100644 --- a/memory.c +++ b/memory.c @@ -764,11 +764,11 @@ static void address_space_update_topology_pass(AddressSpace *as, if (adding) { if (frold->dirty_log_mask && !frnew->dirty_log_mask) { - MEMORY_LISTENER_UPDATE_REGION(frold, as, log_stop); + MEMORY_LISTENER_UPDATE_REGION(frnew, as, log_stop); as->ops->log_stop(as, frnew); } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) { as->ops->log_start(as, frnew); - MEMORY_LISTENER_UPDATE_REGION(frold, as, log_start); + MEMORY_LISTENER_UPDATE_REGION(frnew, as, log_start); } } @@ -779,7 +779,7 @@ static void address_space_update_topology_pass(AddressSpace *as, if (adding) { as->ops->range_add(as, frnew); - MEMORY_LISTENER_UPDATE_REGION(frold, as, region_add); + MEMORY_LISTENER_UPDATE_REGION(frnew, as, region_add); } ++inew; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 5bfc21f..74d81ef 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -253,8 +253,7 @@ int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr) if ((env->mcg_cap & MCG_SER_P) && addr && (code == BUS_MCEERR_AR || code == BUS_MCEERR_AO)) { if (qemu_ram_addr_from_host(addr, &ram_addr) || - !kvm_physical_memory_addr_from_ram(env->kvm_state, ram_addr, - &paddr)) { + !kvm_physical_memory_addr_from_host(env->kvm_state, addr, &paddr)) { fprintf(stderr, "Hardware memory error for memory used by " "QEMU itself instead of guest system!\n"); /* Hope we are lucky for AO MCE */ @@ -286,8 +285,8 @@ int kvm_arch_on_sigbus(int code, void *addr) /* Hope we are lucky for AO MCE */ if (qemu_ram_addr_from_host(addr, &ram_addr) || - !kvm_physical_memory_addr_from_ram(first_cpu->kvm_state, ram_addr, - &paddr)) { + !kvm_physical_memory_addr_from_host(first_cpu->kvm_state, addr, + &paddr)) { fprintf(stderr, "Hardware memory error for memory used by " "QEMU itself instead of guest system!: %p\n", addr); return 0; -- 1.7.7.1 -- 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
--- memory.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/memory.c b/memory.c index c08186d..2dcbf80 100644 --- a/memory.c +++ b/memory.c @@ -708,13 +708,13 @@ static void memory_listener_update_region(FlatRange *fr, AddressSpace *as, .address_space = as->root, .offset_within_region = fr->offset_in_region, .size = int128_get64(fr->addr.size), - .offset_within_region = int128_get64(fr->addr.start), + .offset_within_address_space = int128_get64(fr->addr.start), }; MemoryListener *listener; QLIST_FOREACH(listener, &memory_listeners, link) { ListenerCallback *callback - = *(ListenerCallback *)((void *)listener + callback_offset); + = *(ListenerCallback **)((void *)listener + callback_offset); callback(listener, §ion); } } @@ -1149,6 +1149,7 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr) FOR_EACH_FLAT_RANGE(fr, &address_space_memory.current_map) { if (fr->mr == mr) { + MEMORY_LISTENER_UPDATE_REGION(fr, &address_space_memory, log_sync); cpu_physical_sync_dirty_bitmap(int128_get64(fr->addr.start), int128_get64(addrrange_end(fr->addr))); } @@ -1467,7 +1468,7 @@ static void listener_add_address_space(MemoryListener *listener, .address_space = as->root, .offset_within_region = fr->offset_in_region, .size = int128_get64(fr->addr.size), - .offset_within_region = int128_get64(fr->addr.start), + .offset_within_address_space = int128_get64(fr->addr.start), }; listener->region_add(listener, §ion); } -- 1.7.7.1 -- 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
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(-) 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(); -- 1.7.7.1 -- 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
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> --- hw/vhost.c | 126 ++++++++++++++++++++++++++++++++++++++++++++--------------- hw/vhost.h | 3 +- 2 files changed, 96 insertions(+), 33 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index 0870cb7..a1c5e4c 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -57,12 +57,12 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, } } -static int vhost_client_sync_dirty_bitmap(CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - target_phys_addr_t end_addr) +static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, + target_phys_addr_t start_addr, + target_phys_addr_t end_addr) { - struct vhost_dev *dev = container_of(client, struct vhost_dev, client); int i; + if (!dev->log_enabled || !dev->started) { return 0; } @@ -81,6 +81,17 @@ static int vhost_client_sync_dirty_bitmap(CPUPhysMemoryClient *client, return 0; } +static void vhost_log_sync(MemoryListener *listener, + MemoryRegionSection *section) +{ + struct vhost_dev *dev = container_of(listener, struct vhost_dev, + memory_listener); + target_phys_addr_t start_addr = section->offset_within_address_space; + target_phys_addr_t end_addr = start_addr + section->size; + + vhost_sync_dirty_bitmap(dev, start_addr, end_addr); +} + /* Assign/unassign. Keep an unsorted array of non-overlapping * memory regions in dev->mem. */ static void vhost_dev_unassign_memory(struct vhost_dev *dev, @@ -259,8 +270,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size) log_base = (uint64_t)(unsigned long)log; r = ioctl(dev->control, VHOST_SET_LOG_BASE, &log_base); assert(r >= 0); - vhost_client_sync_dirty_bitmap(&dev->client, 0, - (target_phys_addr_t)~0x0ull); + vhost_sync_dirty_bitmap(dev, 0, (target_phys_addr_t)~0x0ull); if (dev->log) { g_free(dev->log); } @@ -335,31 +345,37 @@ static bool vhost_dev_cmp_memory(struct vhost_dev *dev, return uaddr != reg->userspace_addr + start_addr - reg->guest_phys_addr; } -static void vhost_client_set_memory(CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - ram_addr_t size, - ram_addr_t phys_offset, - bool log_dirty) +static void vhost_set_memory(MemoryListener *listener, + MemoryRegionSection *section, + bool add) { - struct vhost_dev *dev = container_of(client, struct vhost_dev, client); - ram_addr_t flags = phys_offset & ~TARGET_PAGE_MASK; + struct vhost_dev *dev = container_of(listener, struct vhost_dev, + memory_listener); + target_phys_addr_t start_addr = section->offset_within_address_space; + ram_addr_t size = section->size; + bool log_dirty = memory_region_is_logging(section->mr); int s = offsetof(struct vhost_memory, regions) + (dev->mem->nregions + 1) * sizeof dev->mem->regions[0]; uint64_t log_size; int r; + void *ram; + + if (!memory_region_is_ram(section->mr)) { + return; + } dev->mem = g_realloc(dev->mem, s); if (log_dirty) { - flags = IO_MEM_UNASSIGNED; + add = false; } assert(size); /* Optimize no-change case. At least cirrus_vga does this a lot at this time. */ - if (flags == IO_MEM_RAM) { - if (!vhost_dev_cmp_memory(dev, start_addr, size, - (uintptr_t)qemu_get_ram_ptr(phys_offset))) { + ram = memory_region_get_ram_ptr(section->mr); + if (add) { + if (!vhost_dev_cmp_memory(dev, start_addr, size, (uintptr_t)ram)) { /* Region exists with same address. Nothing to do. */ return; } @@ -371,10 +387,9 @@ static void vhost_client_set_memory(CPUPhysMemoryClient *client, } vhost_dev_unassign_memory(dev, start_addr, size); - if (flags == IO_MEM_RAM) { + if (add) { /* Add given mapping, merging adjacent regions if any */ - vhost_dev_assign_memory(dev, start_addr, size, - (uintptr_t)qemu_get_ram_ptr(phys_offset)); + vhost_dev_assign_memory(dev, start_addr, size, (uintptr_t)ram); } else { /* Remove old mapping for this memory, if any. */ vhost_dev_unassign_memory(dev, start_addr, size); @@ -410,6 +425,18 @@ static void vhost_client_set_memory(CPUPhysMemoryClient *client, } } +static void vhost_region_add(MemoryListener *listener, + MemoryRegionSection *section) +{ + vhost_set_memory(listener, section, true); +} + +static void vhost_region_del(MemoryListener *listener, + MemoryRegionSection *section) +{ + vhost_set_memory(listener, section, false); +} + static int vhost_virtqueue_set_addr(struct vhost_dev *dev, struct vhost_virtqueue *vq, unsigned idx, bool enable_log) @@ -467,10 +494,10 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log) return r; } -static int vhost_client_migration_log(CPUPhysMemoryClient *client, - int enable) +static int vhost_migration_log(MemoryListener *listener, int enable) { - struct vhost_dev *dev = container_of(client, struct vhost_dev, client); + struct vhost_dev *dev = container_of(listener, struct vhost_dev, + memory_listener); int r; if (!!enable == dev->log_enabled) { return 0; @@ -500,6 +527,38 @@ static int vhost_client_migration_log(CPUPhysMemoryClient *client, return 0; } +static void vhost_log_global_start(MemoryListener *listener) +{ + int r; + + r = vhost_migration_log(listener, true); + if (r < 0) { + abort(); + } +} + +static void vhost_log_global_stop(MemoryListener *listener) +{ + int r; + + r = vhost_migration_log(listener, false); + if (r < 0) { + abort(); + } +} + +static void vhost_log_start(MemoryListener *listener, + MemoryRegionSection *section) +{ + /* FIXME: implement */ +} + +static void vhost_log_stop(MemoryListener *listener, + MemoryRegionSection *section) +{ + /* FIXME: implement */ +} + static int vhost_virtqueue_init(struct vhost_dev *dev, struct VirtIODevice *vdev, struct vhost_virtqueue *vq, @@ -645,17 +704,21 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force) } hdev->features = features; - hdev->client.set_memory = vhost_client_set_memory; - hdev->client.sync_dirty_bitmap = vhost_client_sync_dirty_bitmap; - hdev->client.migration_log = vhost_client_migration_log; - hdev->client.log_start = NULL; - hdev->client.log_stop = NULL; + hdev->memory_listener = (MemoryListener) { + .region_add = vhost_region_add, + .region_del = vhost_region_del, + .log_start = vhost_log_start, + .log_stop = vhost_log_stop, + .log_sync = vhost_log_sync, + .log_global_start = vhost_log_global_start, + .log_global_stop = vhost_log_global_stop, + }; hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions)); hdev->log = NULL; hdev->log_size = 0; hdev->log_enabled = false; hdev->started = false; - cpu_register_phys_memory_client(&hdev->client); + memory_listener_register(&hdev->memory_listener); hdev->force = force; return 0; fail: @@ -666,7 +729,7 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force) void vhost_dev_cleanup(struct vhost_dev *hdev) { - cpu_unregister_phys_memory_client(&hdev->client); + memory_listener_unregister(&hdev->memory_listener); g_free(hdev->mem); close(hdev->control); } @@ -808,8 +871,7 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) hdev->vqs + i, i); } - vhost_client_sync_dirty_bitmap(&hdev->client, 0, - (target_phys_addr_t)~0x0ull); + vhost_sync_dirty_bitmap(hdev, 0, (target_phys_addr_t)~0x0ull); r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, false); if (r < 0) { fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r); diff --git a/hw/vhost.h b/hw/vhost.h index c9452f0..d1824ec 100644 --- a/hw/vhost.h +++ b/hw/vhost.h @@ -3,6 +3,7 @@ #include "hw/hw.h" #include "hw/virtio.h" +#include "memory.h" /* Generic structures common for any vhost based device. */ struct vhost_virtqueue { @@ -26,7 +27,7 @@ typedef unsigned long vhost_log_chunk_t; struct vhost_memory; struct vhost_dev { - CPUPhysMemoryClient client; + MemoryListener memory_listener; int control; struct vhost_memory *mem; struct vhost_virtqueue *vqs; -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 15/23] xen, vga: add API for registering the framebuffer
Xen currently uses the name of a memory region to determine whether it is the framebuffer. Replace with an explicit API. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/vga.c | 2 ++ hw/xen.h | 3 +++ xen-all.c | 6 ++++++ xen-stub.c | 4 ++++ 4 files changed, 15 insertions(+), 0 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index ca79aa1..7e1dd5a 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -28,6 +28,7 @@ #include "vga_int.h" #include "pixel_ops.h" #include "qemu-timer.h" +#include "xen.h" //#define DEBUG_VGA //#define DEBUG_VGA_MEM @@ -2222,6 +2223,7 @@ void vga_common_init(VGACommonState *s, int vga_ram_size) s->is_vbe_vmstate = 0; #endif memory_region_init_ram(&s->vram, NULL, "vga.vram", vga_ram_size); + xen_register_framebuffer(&s->vram); s->vram_ptr = memory_region_get_ram_ptr(&s->vram); s->vram_size = vga_ram_size; s->get_bpp = vga_get_bpp; diff --git a/hw/xen.h b/hw/xen.h index f9f66e8..b46879c 100644 --- a/hw/xen.h +++ b/hw/xen.h @@ -49,6 +49,9 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, struct MemoryRegion *mr); #endif +struct MemoryRegion; +void xen_register_framebuffer(struct MemoryRegion *mr); + #if defined(CONFIG_XEN) && CONFIG_XEN_CTRL_INTERFACE_VERSION < 400 # define HVM_MAX_VCPUS 32 #endif diff --git a/xen-all.c b/xen-all.c index bd89889..51315ce 100644 --- a/xen-all.c +++ b/xen-all.c @@ -33,6 +33,7 @@ #endif static MemoryRegion ram_memory, ram_640k, ram_lo, ram_hi; +static MemoryRegion *framebuffer; /* Compatibility with older version */ #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a @@ -982,3 +983,8 @@ void destroy_hvm_domain(void) xc_interface_close(xc_handle); } } + +void xen_register_framebuffer(MemoryRegion *mr) +{ + framebuffer = mr; +} diff --git a/xen-stub.c b/xen-stub.c index 5fa400f..d403d86 100644 --- a/xen-stub.c +++ b/xen-stub.c @@ -44,3 +44,7 @@ int xen_init(void) { return -ENOSYS; } + +void xen_register_framebuffer(MemoryRegion *mr) +{ +} -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 16/23] memory: temporarily add memory_region_get_ram_addr()
This is a layering violation, but needed while the code contains naked calls to qemu_get_ram_ptr() and the like. Signed-off-by: Avi Kivity <avi@redhat.com> --- memory.c | 6 ++++++ memory.h | 10 ++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index 2dcbf80..41a106a 100644 --- a/memory.c +++ b/memory.c @@ -1376,6 +1376,12 @@ void memory_region_del_subregion(MemoryRegion *mr, memory_region_update_topology(); } +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr) +{ + assert(mr->backend_registered); + return mr->ram_addr; +} + static int cmp_flatrange_addr(const void *_addr, const void *_fr) { const AddrRange *addr = _addr; diff --git a/memory.h b/memory.h index b7d39ed..1e44d86 100644 --- a/memory.h +++ b/memory.h @@ -557,6 +557,16 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, target_phys_addr_t offset, MemoryRegion *subregion, unsigned priority); + +/** + * memory_region_get_ram_addr: Get the ram address associated with a memory + * region + * + * DO NOT USE THIS FUCNTION. This is a temporary workaround while the Xen + * code is being reworked. + */ +ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr); + /** * memory_region_del_subregion: Remove a subregion. * -- 1.7.7.1 -- 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
Signed-off-by: Avi Kivity <avi@redhat.com> --- trace-events | 2 +- xen-all.c | 137 ++++++++++++++++++++++++++++++++++------------------------ 2 files changed, 81 insertions(+), 58 deletions(-) diff --git a/trace-events b/trace-events index bf1cf57..728df97 100644 --- a/trace-events +++ b/trace-events @@ -464,7 +464,7 @@ mipsnet_irq(uint32_t isr, uint32_t intctl) "set irq to %d (%02x)" # xen-all.c xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: %#lx, size %#lx" -xen_client_set_memory(uint64_t start_addr, unsigned long size, unsigned long phys_offset, bool log_dirty) "%#"PRIx64" size %#lx, offset %#lx, log_dirty %i" +xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "%#"PRIx64" size %#lx, log_dirty %i" # xen-mapcache.c xen_map_cache(uint64_t phys_addr) "want %#"PRIx64 diff --git a/xen-all.c b/xen-all.c index 51315ce..e662dc5 100644 --- a/xen-all.c +++ b/xen-all.c @@ -63,6 +63,7 @@ static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i) typedef struct XenPhysmap { target_phys_addr_t start_addr; ram_addr_t size; + MemoryRegion *mr; target_phys_addr_t phys_offset; QLIST_ENTRY(XenPhysmap) list; @@ -80,8 +81,9 @@ static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i) int send_vcpu; struct xs_handle *xenstore; - CPUPhysMemoryClient client; + MemoryListener memory_listener; QLIST_HEAD(, XenPhysmap) physmap; + target_phys_addr_t free_phys_offset; const XenPhysmap *log_for_dirtybit; Notifier exit; @@ -226,13 +228,14 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) static int xen_add_to_physmap(XenIOState *state, target_phys_addr_t start_addr, ram_addr_t size, - target_phys_addr_t phys_offset) + MemoryRegion *mr, + target_phys_addr_t offset_within_region) { unsigned long i = 0; int rc = 0; XenPhysmap *physmap = NULL; target_phys_addr_t pfn, start_gpfn; - RAMBlock *block; + target_phys_addr_t phys_offset = memory_region_get_ram_addr(mr); if (get_physmapping(state, start_addr, size)) { return 0; @@ -245,17 +248,13 @@ static int xen_add_to_physmap(XenIOState *state, * the linear framebuffer to be that region. * Avoid tracking any regions that is not videoram and avoid tracking * the legacy vga region. */ - QLIST_FOREACH(block, &ram_list.blocks, next) { - if (!strcmp(block->idstr, "vga.vram") && block->offset == phys_offset - && start_addr > 0xbffff) { - goto go_physmap; - } + if (mr == framebuffer && start_addr > 0xbffff) { + goto go_physmap; } return -1; go_physmap: - DPRINTF("mapping vram to %llx - %llx, from %llx\n", - start_addr, start_addr + size, phys_offset); + DPRINTF("mapping vram to %llx - %llx\n", start_addr, start_addr + size); pfn = phys_offset >> TARGET_PAGE_BITS; start_gpfn = start_addr >> TARGET_PAGE_BITS; @@ -347,49 +346,62 @@ static int xen_remove_from_physmap(XenIOState *state, } #endif -static void xen_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 xen_set_memory(struct MemoryListener *listener, + MemoryRegionSection *section, + bool add) { - XenIOState *state = container_of(client, XenIOState, client); - ram_addr_t flags = phys_offset & ~TARGET_PAGE_MASK; + XenIOState *state = container_of(listener, XenIOState, memory_listener); + target_phys_addr_t start_addr = section->offset_within_address_space; + ram_addr_t size = section->size; + bool log_dirty = memory_region_is_logging(section->mr); hvmmem_type_t mem_type; - if (!(start_addr != phys_offset - && ( (log_dirty && flags < IO_MEM_UNASSIGNED) - || (!log_dirty && flags == IO_MEM_UNASSIGNED)))) { + if (!memory_region_is_ram(section->mr)) { + return; + } + + if (!(section->mr != &ram_memory + && ( (log_dirty && add) || (!log_dirty && !add)))) { return; } - trace_xen_client_set_memory(start_addr, size, phys_offset, log_dirty); + trace_xen_client_set_memory(start_addr, size, log_dirty); start_addr &= TARGET_PAGE_MASK; size = TARGET_PAGE_ALIGN(size); - phys_offset &= TARGET_PAGE_MASK; - - switch (flags) { - case IO_MEM_RAM: - xen_add_to_physmap(state, start_addr, size, phys_offset); - break; - case IO_MEM_ROM: - mem_type = HVMMEM_ram_ro; - if (xc_hvm_set_mem_type(xen_xc, xen_domid, mem_type, - start_addr >> TARGET_PAGE_BITS, - size >> TARGET_PAGE_BITS)) { - DPRINTF("xc_hvm_set_mem_type error, addr: "TARGET_FMT_plx"\n", - start_addr); + + if (add) { + if (!memory_region_is_rom(section->mr)) { + xen_add_to_physmap(state, start_addr, size, + section->mr, section->offset_within_region); + } else { + mem_type = HVMMEM_ram_ro; + if (xc_hvm_set_mem_type(xen_xc, xen_domid, mem_type, + start_addr >> TARGET_PAGE_BITS, + size >> TARGET_PAGE_BITS)) { + DPRINTF("xc_hvm_set_mem_type error, addr: "TARGET_FMT_plx"\n", + start_addr); + } } - break; - case IO_MEM_UNASSIGNED: + } else { if (xen_remove_from_physmap(state, start_addr, size) < 0) { DPRINTF("physmapping does not exist at "TARGET_FMT_plx"\n", start_addr); } - break; } } +static void xen_region_add(MemoryListener *listener, + MemoryRegionSection *section) +{ + xen_set_memory(listener, section, true); +} + +static void xen_region_del(MemoryListener *listener, + MemoryRegionSection *section) +{ + xen_set_memory(listener, section, false); +} + static int xen_sync_dirty_bitmap(XenIOState *state, target_phys_addr_t start_addr, ram_addr_t size) @@ -433,43 +445,54 @@ static int xen_sync_dirty_bitmap(XenIOState *state, return 0; } -static int xen_log_start(CPUPhysMemoryClient *client, target_phys_addr_t phys_addr, ram_addr_t size) +static void xen_log_start(MemoryListener *listener, + MemoryRegionSection *section) { - XenIOState *state = container_of(client, XenIOState, client); + XenIOState *state = container_of(listener, XenIOState, memory_listener); + int r; - return xen_sync_dirty_bitmap(state, phys_addr, size); + r = xen_sync_dirty_bitmap(state, section->offset_within_address_space, + section->size); + assert(r >= 0); } -static int xen_log_stop(CPUPhysMemoryClient *client, target_phys_addr_t phys_addr, ram_addr_t size) +static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section) { - XenIOState *state = container_of(client, XenIOState, client); + XenIOState *state = container_of(listener, XenIOState, memory_listener); + int r; state->log_for_dirtybit = NULL; /* Disable dirty bit tracking */ - return xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL); + r = xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL); + assert(r >= 0); } -static int xen_client_sync_dirty_bitmap(struct CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - target_phys_addr_t end_addr) +static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section) { - XenIOState *state = container_of(client, XenIOState, client); + XenIOState *state = container_of(listener, XenIOState, memory_listener); + int r; - return xen_sync_dirty_bitmap(state, start_addr, end_addr - start_addr); + r = xen_sync_dirty_bitmap(state, section->offset_within_address_space, + section->size); + assert(r >= 0); } -static int xen_client_migration_log(struct CPUPhysMemoryClient *client, - int enable) +static void xen_log_global_start(MemoryListener *listener) +{ +} + +static void xen_log_global_stop(MemoryListener *listener) { - return 0; } -static CPUPhysMemoryClient xen_cpu_phys_memory_client = { - .set_memory = xen_client_set_memory, - .sync_dirty_bitmap = xen_client_sync_dirty_bitmap, - .migration_log = xen_client_migration_log, +static MemoryListener xen_memory_listener = { + .region_add = xen_region_add, + .region_del = xen_region_del, .log_start = xen_log_start, .log_stop = xen_log_stop, + .log_sync = xen_log_sync, + .log_global_start = xen_log_global_start, + .log_global_stop = xen_log_global_stop, }; /* VCPU Operations, MMIO, IO ring ... */ @@ -947,9 +970,9 @@ int xen_hvm_init(void) qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state); - state->client = xen_cpu_phys_memory_client; + state->memory_listener = xen_memory_listener; QLIST_INIT(&state->physmap); - cpu_register_phys_memory_client(&state->client); + memory_listener_register(&state->memory_listener); state->log_for_dirtybit = NULL; /* Initialize backend core & drivers */ -- 1.7.7.1 -- 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
No longer used. Signed-off-by: Avi Kivity <avi@redhat.com> --- cpu-all.h | 6 -- cpu-common.h | 23 -------- exec-obsolete.h | 3 - exec.c | 169 ++----------------------------------------------------- memory.c | 12 ---- 5 files changed, 5 insertions(+), 208 deletions(-) diff --git a/cpu-all.h b/cpu-all.h index f2c5382..734833a 100644 --- a/cpu-all.h +++ b/cpu-all.h @@ -569,12 +569,6 @@ int cpu_physical_memory_set_dirty_tracking(int enable); int cpu_physical_memory_get_dirty_tracking(void); -int cpu_physical_log_start(target_phys_addr_t start_addr, - ram_addr_t size); - -int cpu_physical_log_stop(target_phys_addr_t start_addr, - ram_addr_t size); - void dump_exec_info(FILE *f, fprintf_function cpu_fprintf); #endif /* !CONFIG_USER_ONLY */ diff --git a/cpu-common.h b/cpu-common.h index 8295e4f..eee2faf 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -71,29 +71,6 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len, void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque)); void cpu_unregister_map_client(void *cookie); -struct CPUPhysMemoryClient; -typedef struct CPUPhysMemoryClient CPUPhysMemoryClient; -struct CPUPhysMemoryClient { - void (*set_memory)(struct CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - ram_addr_t size, - ram_addr_t phys_offset, - bool log_dirty); - int (*sync_dirty_bitmap)(struct CPUPhysMemoryClient *client, - target_phys_addr_t start_addr, - target_phys_addr_t end_addr); - int (*migration_log)(struct CPUPhysMemoryClient *client, - int enable); - int (*log_start)(struct CPUPhysMemoryClient *client, - target_phys_addr_t phys_addr, ram_addr_t size); - int (*log_stop)(struct CPUPhysMemoryClient *client, - target_phys_addr_t phys_addr, ram_addr_t size); - QLIST_ENTRY(CPUPhysMemoryClient) list; -}; - -void cpu_register_phys_memory_client(CPUPhysMemoryClient *); -void cpu_unregister_phys_memory_client(CPUPhysMemoryClient *); - /* Coalesced MMIO regions are areas where write operations can be reordered. * This usually implies that write operations are side-effect free. This allows * batching which can make a major impact on performance when using diff --git a/exec-obsolete.h b/exec-obsolete.h index aa0a04d..1d0b610 100644 --- a/exec-obsolete.h +++ b/exec-obsolete.h @@ -64,9 +64,6 @@ static inline void cpu_register_physical_memory(target_phys_addr_t start_addr, void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size); void qemu_unregister_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size); -int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, - target_phys_addr_t end_addr); - #endif #endif diff --git a/exec.c b/exec.c index 36b61c9..e1f7462 100644 --- a/exec.c +++ b/exec.c @@ -1732,129 +1732,6 @@ void cpu_exit(CPUState *env) { 0, NULL, NULL }, }; -#ifndef CONFIG_USER_ONLY -static QLIST_HEAD(memory_client_list, CPUPhysMemoryClient) memory_client_list - = QLIST_HEAD_INITIALIZER(memory_client_list); - -static void cpu_notify_set_memory(target_phys_addr_t start_addr, - ram_addr_t size, - ram_addr_t phys_offset, - bool log_dirty) -{ - CPUPhysMemoryClient *client; - QLIST_FOREACH(client, &memory_client_list, list) { - client->set_memory(client, start_addr, size, phys_offset, log_dirty); - } -} - -static int cpu_notify_sync_dirty_bitmap(target_phys_addr_t start, - target_phys_addr_t end) -{ - CPUPhysMemoryClient *client; - QLIST_FOREACH(client, &memory_client_list, list) { - int r = client->sync_dirty_bitmap(client, start, end); - if (r < 0) - return r; - } - return 0; -} - -static int cpu_notify_migration_log(int enable) -{ - CPUPhysMemoryClient *client; - if (enable) { - memory_global_dirty_log_start(); - } else { - memory_global_dirty_log_stop(); - } - QLIST_FOREACH(client, &memory_client_list, list) { - int r = client->migration_log(client, enable); - if (r < 0) - return r; - } - return 0; -} - -struct last_map { - target_phys_addr_t start_addr; - ram_addr_t size; - ram_addr_t phys_offset; -}; - -/* The l1_phys_map provides the upper P_L1_BITs of the guest physical - * address. Each intermediate table provides the next L2_BITs of guest - * physical address space. The number of levels vary based on host and - * guest configuration, making it efficient to build the final guest - * physical address by seeding the L1 offset and shifting and adding in - * each L2 offset as we recurse through them. */ -static void phys_page_for_each_1(CPUPhysMemoryClient *client, int level, - void **lp, target_phys_addr_t addr, - struct last_map *map) -{ - int i; - - if (*lp == NULL) { - return; - } - if (level == 0) { - PhysPageDesc *pd = *lp; - addr <<= L2_BITS + TARGET_PAGE_BITS; - for (i = 0; i < L2_SIZE; ++i) { - if (pd[i].phys_offset != IO_MEM_UNASSIGNED) { - target_phys_addr_t start_addr = addr | i << TARGET_PAGE_BITS; - - if (map->size && - start_addr == map->start_addr + map->size && - pd[i].phys_offset == map->phys_offset + map->size) { - - map->size += TARGET_PAGE_SIZE; - continue; - } else if (map->size) { - client->set_memory(client, map->start_addr, - map->size, map->phys_offset, false); - } - - map->start_addr = start_addr; - map->size = TARGET_PAGE_SIZE; - map->phys_offset = pd[i].phys_offset; - } - } - } else { - void **pp = *lp; - for (i = 0; i < L2_SIZE; ++i) { - phys_page_for_each_1(client, level - 1, pp + i, - (addr << L2_BITS) | i, map); - } - } -} - -static void phys_page_for_each(CPUPhysMemoryClient *client) -{ - int i; - struct last_map map = { }; - - for (i = 0; i < P_L1_SIZE; ++i) { - phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1, - l1_phys_map + i, i, &map); - } - if (map.size) { - client->set_memory(client, map.start_addr, map.size, map.phys_offset, - false); - } -} - -void cpu_register_phys_memory_client(CPUPhysMemoryClient *client) -{ - QLIST_INSERT_HEAD(&memory_client_list, client, list); - phys_page_for_each(client); -} - -void cpu_unregister_phys_memory_client(CPUPhysMemoryClient *client) -{ - QLIST_REMOVE(client, list); -} -#endif - static int cmp1(const char *s1, int n, const char *s2) { if (strlen(s2) != n) @@ -2131,7 +2008,11 @@ int cpu_physical_memory_set_dirty_tracking(int enable) { int ret = 0; in_migration = enable; - ret = cpu_notify_migration_log(!!enable); + if (enable) { + memory_global_dirty_log_start(); + } else { + memory_global_dirty_log_stop(); + } return ret; } @@ -2140,45 +2021,6 @@ int cpu_physical_memory_get_dirty_tracking(void) return in_migration; } -int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, - target_phys_addr_t end_addr) -{ - int ret; - - ret = cpu_notify_sync_dirty_bitmap(start_addr, end_addr); - return ret; -} - -int cpu_physical_log_start(target_phys_addr_t start_addr, - ram_addr_t size) -{ - CPUPhysMemoryClient *client; - QLIST_FOREACH(client, &memory_client_list, list) { - if (client->log_start) { - int r = client->log_start(client, start_addr, size); - if (r < 0) { - return r; - } - } - } - return 0; -} - -int cpu_physical_log_stop(target_phys_addr_t start_addr, - ram_addr_t size) -{ - CPUPhysMemoryClient *client; - QLIST_FOREACH(client, &memory_client_list, list) { - if (client->log_stop) { - int r = client->log_stop(client, start_addr, size); - if (r < 0) { - return r; - } - } - } - return 0; -} - static inline void tlb_update_dirty(CPUTLBEntry *tlb_entry) { ram_addr_t ram_addr; @@ -2681,7 +2523,6 @@ void cpu_register_physical_memory_log(target_phys_addr_t start_addr, subpage_t *subpage; assert(size); - cpu_notify_set_memory(start_addr, size, phys_offset, log_dirty); if (phys_offset == IO_MEM_UNASSIGNED) { region_offset = start_addr; diff --git a/memory.c b/memory.c index 41a106a..520e37b 100644 --- a/memory.c +++ b/memory.c @@ -337,11 +337,6 @@ static void as_memory_range_add(AddressSpace *as, FlatRange *fr) static void as_memory_range_del(AddressSpace *as, FlatRange *fr) { - if (fr->dirty_log_mask) { - Int128 end = addrrange_end(fr->addr); - cpu_physical_sync_dirty_bitmap(int128_get64(fr->addr.start), - int128_get64(end)); - } cpu_register_physical_memory(int128_get64(fr->addr.start), int128_get64(fr->addr.size), IO_MEM_UNASSIGNED); @@ -349,14 +344,10 @@ static void as_memory_range_del(AddressSpace *as, FlatRange *fr) static void as_memory_log_start(AddressSpace *as, FlatRange *fr) { - cpu_physical_log_start(int128_get64(fr->addr.start), - int128_get64(fr->addr.size)); } static void as_memory_log_stop(AddressSpace *as, FlatRange *fr) { - cpu_physical_log_stop(int128_get64(fr->addr.start), - int128_get64(fr->addr.size)); } static void as_memory_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd) @@ -1150,8 +1141,6 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr) FOR_EACH_FLAT_RANGE(fr, &address_space_memory.current_map) { if (fr->mr == mr) { MEMORY_LISTENER_UPDATE_REGION(fr, &address_space_memory, log_sync); - cpu_physical_sync_dirty_bitmap(int128_get64(fr->addr.start), - int128_get64(addrrange_end(fr->addr))); } } } @@ -1434,7 +1423,6 @@ void memory_global_sync_dirty_bitmap(MemoryRegion *address_space) AddressSpace *as = memory_region_to_address_space(address_space); FlatRange *fr; - cpu_physical_sync_dirty_bitmap(0, TARGET_PHYS_ADDR_MAX); FOR_EACH_FLAT_RANGE(fr, &as->current_map) { MEMORY_LISTENER_UPDATE_REGION(fr, as, log_sync); } -- 1.7.7.1 -- 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
This reaches into the innards of the memory core, which are being changed. Switch to a memory API version. Signed-off-by: Avi Kivity <avi@redhat.com> --- kvm-all.c | 27 ++++++++++----------------- 1 files changed, 10 insertions(+), 17 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 138e0a2..d94710c 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -340,16 +340,12 @@ static int kvm_set_migration_log(int enable) } /* get kvm''s dirty pages bitmap and update qemu''s */ -static int kvm_get_dirty_pages_log_range(unsigned long start_addr, - unsigned long *bitmap, - unsigned long offset, - unsigned long mem_size) +static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section, + unsigned long *bitmap) { unsigned int i, j; unsigned long page_number, addr, addr1, c; - ram_addr_t ram_addr; - unsigned int len = ((mem_size / TARGET_PAGE_SIZE) + HOST_LONG_BITS - 1) / - HOST_LONG_BITS; + unsigned int len = ((section->size / TARGET_PAGE_SIZE) + HOST_LONG_BITS - 1) / HOST_LONG_BITS; /* * bitmap-traveling is faster than memory-traveling (for addr...) @@ -363,9 +359,8 @@ static int kvm_get_dirty_pages_log_range(unsigned long start_addr, c &= ~(1ul << j); page_number = i * HOST_LONG_BITS + j; addr1 = page_number * TARGET_PAGE_SIZE; - addr = offset + addr1; - ram_addr = cpu_get_physical_page_desc(addr); - cpu_physical_memory_set_dirty(ram_addr); + addr = section->offset_within_region + addr1; + memory_region_set_dirty(section->mr, addr); } while (c != 0); } } @@ -382,14 +377,15 @@ static int kvm_get_dirty_pages_log_range(unsigned long start_addr, * @start_add: start of logged region. * @end_addr: end of logged region. */ -static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, - target_phys_addr_t end_addr) +static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section) { KVMState *s = kvm_state; unsigned long size, allocated_size = 0; KVMDirtyLog d; KVMSlot *mem; int ret = 0; + target_phys_addr_t start_addr = section->offset_within_address_space; + target_phys_addr_t end_addr = start_addr + section->size; d.dirty_bitmap = NULL; while (start_addr < end_addr) { @@ -428,8 +424,7 @@ static int kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, break; } - kvm_get_dirty_pages_log_range(mem->start_addr, d.dirty_bitmap, - mem->start_addr, mem->memory_size); + kvm_get_dirty_pages_log_range(section, d.dirty_bitmap); start_addr = mem->start_addr + mem->memory_size; } g_free(d.dirty_bitmap); @@ -686,11 +681,9 @@ static void kvm_region_del(MemoryListener *listener, 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); + r = kvm_physical_sync_dirty_bitmap(section); if (r < 0) { abort(); } -- 1.7.7.1 -- 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
This reaches into the innards of the memory core, which are being changed. Switch to a memory API version. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/vhost.c | 47 +++++++++++++++++++++++++++++++++++++++-------- hw/vhost.h | 2 ++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index a1c5e4c..cd56e75 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -17,6 +17,7 @@ #include <linux/vhost.h> static void vhost_dev_sync_region(struct vhost_dev *dev, + MemoryRegionSection *section, uint64_t mfirst, uint64_t mlast, uint64_t rfirst, uint64_t rlast) { @@ -49,8 +50,8 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, ffsll(log) : ffs(log))) { ram_addr_t ram_addr; bit -= 1; - ram_addr = cpu_get_physical_page_desc(addr + bit * VHOST_LOG_PAGE); - cpu_physical_memory_set_dirty(ram_addr); + ram_addr = section->offset_within_region + bit * VHOST_LOG_PAGE; + memory_region_set_dirty(section->mr, ram_addr); log &= ~(0x1ull << bit); } addr += VHOST_LOG_CHUNK; @@ -58,6 +59,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, } static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, + MemoryRegionSection *section, target_phys_addr_t start_addr, target_phys_addr_t end_addr) { @@ -68,14 +70,14 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev, } for (i = 0; i < dev->mem->nregions; ++i) { struct vhost_memory_region *reg = dev->mem->regions + i; - vhost_dev_sync_region(dev, start_addr, end_addr, + vhost_dev_sync_region(dev, section, start_addr, end_addr, reg->guest_phys_addr, range_get_last(reg->guest_phys_addr, reg->memory_size)); } for (i = 0; i < dev->nvqs; ++i) { struct vhost_virtqueue *vq = dev->vqs + i; - vhost_dev_sync_region(dev, start_addr, end_addr, vq->used_phys, + vhost_dev_sync_region(dev, section, start_addr, end_addr, vq->used_phys, range_get_last(vq->used_phys, vq->used_size)); } return 0; @@ -89,7 +91,7 @@ static void vhost_log_sync(MemoryListener *listener, target_phys_addr_t start_addr = section->offset_within_address_space; target_phys_addr_t end_addr = start_addr + section->size; - vhost_sync_dirty_bitmap(dev, start_addr, end_addr); + vhost_sync_dirty_bitmap(dev, section, start_addr, end_addr); } /* Assign/unassign. Keep an unsorted array of non-overlapping @@ -261,7 +263,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size) { vhost_log_chunk_t *log; uint64_t log_base; - int r; + int r, i; if (size) { log = g_malloc0(size * sizeof *log); } else { @@ -270,7 +272,10 @@ static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size) log_base = (uint64_t)(unsigned long)log; r = ioctl(dev->control, VHOST_SET_LOG_BASE, &log_base); assert(r >= 0); - vhost_sync_dirty_bitmap(dev, 0, (target_phys_addr_t)~0x0ull); + for (i = 0; i < dev->n_mem_sections; ++i) { + vhost_sync_dirty_bitmap(dev, &dev->mem_sections[i], + 0, (target_phys_addr_t)~0x0ull); + } if (dev->log) { g_free(dev->log); } @@ -428,13 +433,33 @@ static void vhost_set_memory(MemoryListener *listener, static void vhost_region_add(MemoryListener *listener, MemoryRegionSection *section) { + struct vhost_dev *dev = container_of(listener, struct vhost_dev, + memory_listener); + + ++dev->n_mem_sections; + dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections, + dev->n_mem_sections); + dev->mem_sections[dev->n_mem_sections - 1] = *section; vhost_set_memory(listener, section, true); } static void vhost_region_del(MemoryListener *listener, MemoryRegionSection *section) { + struct vhost_dev *dev = container_of(listener, struct vhost_dev, + memory_listener); + int i; + vhost_set_memory(listener, section, false); + for (i = 0; i < dev->n_mem_sections; ++i) { + if (dev->mem_sections[i].offset_within_address_space + == section->offset_within_address_space) { + --dev->n_mem_sections; + memmove(&dev->mem_sections[i], &dev->mem_sections[i+1], + dev->n_mem_sections - i); + break; + } + } } static int vhost_virtqueue_set_addr(struct vhost_dev *dev, @@ -714,6 +739,8 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force) .log_global_stop = vhost_log_global_stop, }; hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions)); + hdev->n_mem_sections = 0; + hdev->mem_sections = NULL; hdev->log = NULL; hdev->log_size = 0; hdev->log_enabled = false; @@ -731,6 +758,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) { memory_listener_unregister(&hdev->memory_listener); g_free(hdev->mem); + g_free(hdev->mem_sections); close(hdev->control); } @@ -871,7 +899,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) hdev->vqs + i, i); } - vhost_sync_dirty_bitmap(hdev, 0, (target_phys_addr_t)~0x0ull); + for (i = 0; i < hdev->n_mem_sections; ++i) { + vhost_sync_dirty_bitmap(hdev, &hdev->mem_sections[i], + 0, (target_phys_addr_t)~0x0ull); + } r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, false); if (r < 0) { fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r); diff --git a/hw/vhost.h b/hw/vhost.h index d1824ec..80e64df 100644 --- a/hw/vhost.h +++ b/hw/vhost.h @@ -30,6 +30,8 @@ struct vhost_dev { MemoryListener memory_listener; int control; struct vhost_memory *mem; + int n_mem_sections; + MemoryRegionSection *mem_sections; struct vhost_virtqueue *vqs; int nvqs; unsigned long long features; -- 1.7.7.1 -- 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
Avi Kivity
2011-Dec-19 14:13 UTC
[PATCH 21/23] virtio-balloon: avoid cpu_get_physical_page_desc()
This reaches into the innards of the memory core, which are being changed. Switch to a memory API version. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/virtio-balloon.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index e24a2bf..bb4a8dd 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -21,6 +21,7 @@ #include "balloon.h" #include "virtio-balloon.h" #include "kvm.h" +#include "exec-memory.h" #if defined(__linux__) #include <sys/mman.h> @@ -70,6 +71,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) { VirtIOBalloon *s = to_virtio_balloon(vdev); VirtQueueElement elem; + MemoryRegionSection section; while (virtqueue_pop(vq, &elem)) { size_t offset = 0; @@ -82,13 +84,17 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) pa = (ram_addr_t)ldl_p(&pfn) << VIRTIO_BALLOON_PFN_SHIFT; offset += 4; - addr = cpu_get_physical_page_desc(pa); - if ((addr & ~TARGET_PAGE_MASK) != IO_MEM_RAM) + /* FIXME: remove get_system_memory(), but how? */ + section = memory_region_find(get_system_memory(), pa, 1); + if (!section.mr || !memory_region_is_ram(section.mr)) continue; - /* Using qemu_get_ram_ptr is bending the rules a bit, but + /* Using memory_region_get_ram_ptr is bending the rules a bit, but should be OK because we only want a single page. */ - balloon_page(qemu_get_ram_ptr(addr), !!(vq == s->dvq)); + addr -= section.offset_within_address_space; + addr += section.offset_within_region; + balloon_page(memory_region_get_ram_ptr(section.mr) + addr, + !!(vq == s->dvq)); } virtqueue_push(vq, &elem, offset); -- 1.7.7.1 -- 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
This reaches into the innards of the memory core, which are being changed. Switch to a memory API version. Signed-off-by: Avi Kivity <avi@redhat.com> --- target-sparc/mmu_helper.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c index 8cdc224..eec8947 100644 --- a/target-sparc/mmu_helper.c +++ b/target-sparc/mmu_helper.c @@ -19,6 +19,7 @@ #include "cpu.h" #include "trace.h" +#include "exec-memory.h" /* Sparc MMU emulation */ @@ -839,13 +840,15 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr) { target_phys_addr_t phys_addr; int mmu_idx = cpu_mmu_index(env); + MemoryRegionSection section; if (cpu_sparc_get_phys_page(env, &phys_addr, addr, 2, mmu_idx) != 0) { if (cpu_sparc_get_phys_page(env, &phys_addr, addr, 0, mmu_idx) != 0) { return -1; } } - if (cpu_get_physical_page_desc(phys_addr) == IO_MEM_UNASSIGNED) { + section = memory_region_find(get_system_memory(), phys_addr, 1); + if (!section.mr) { return -1; } return phys_addr; -- 1.7.7.1 -- 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
No longer used. Signed-off-by: Avi Kivity <avi@redhat.com> --- cpu-common.h | 1 - exec.c | 11 ----------- 2 files changed, 0 insertions(+), 12 deletions(-) diff --git a/cpu-common.h b/cpu-common.h index eee2faf..3fe44d2 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -38,7 +38,6 @@ typedef unsigned long ram_addr_t; typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value); typedef uint32_t CPUReadMemoryFunc(void *opaque, target_phys_addr_t addr); -ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr); void qemu_ram_remap(ram_addr_t addr, ram_addr_t length); /* This should only be used for ram local to a device. */ void *qemu_get_ram_ptr(ram_addr_t addr); diff --git a/exec.c b/exec.c index e1f7462..b02199b 100644 --- a/exec.c +++ b/exec.c @@ -2595,17 +2595,6 @@ void cpu_register_physical_memory_log(target_phys_addr_t start_addr, } } -/* XXX: temporary until new memory mapping API */ -ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr) -{ - PhysPageDesc *p; - - 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) { if (kvm_enabled()) -- 1.7.7.1 -- 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
On 19 December 2011 14:13, Avi Kivity <avi@redhat.com> wrote:> ---This is a rather uninformative commit message -- was this patch intended to be in this series? -- PMM> memory.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/memory.c b/memory.c > index c08186d..2dcbf80 100644 > --- a/memory.c > +++ b/memory.c > @@ -708,13 +708,13 @@ static void memory_listener_update_region(FlatRange *fr, AddressSpace *as, > .address_space = as->root, > .offset_within_region = fr->offset_in_region, > .size = int128_get64(fr->addr.size), > - .offset_within_region = int128_get64(fr->addr.start), > + .offset_within_address_space = int128_get64(fr->addr.start), > }; > MemoryListener *listener; > > QLIST_FOREACH(listener, &memory_listeners, link) { > ListenerCallback *callback > - = *(ListenerCallback *)((void *)listener + callback_offset); > + = *(ListenerCallback **)((void *)listener + callback_offset); > callback(listener, §ion); > } > } > @@ -1149,6 +1149,7 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr) > > FOR_EACH_FLAT_RANGE(fr, &address_space_memory.current_map) { > if (fr->mr == mr) { > + MEMORY_LISTENER_UPDATE_REGION(fr, &address_space_memory, log_sync); > cpu_physical_sync_dirty_bitmap(int128_get64(fr->addr.start), > int128_get64(addrrange_end(fr->addr))); > } > @@ -1467,7 +1468,7 @@ static void listener_add_address_space(MemoryListener *listener, > .address_space = as->root, > .offset_within_region = fr->offset_in_region, > .size = int128_get64(fr->addr.size), > - .offset_within_region = int128_get64(fr->addr.start), > + .offset_within_address_space = int128_get64(fr->addr.start), > }; > listener->region_add(listener, §ion); > } > -- > 1.7.7.1 > >-- 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
On 12/19/2011 04:32 PM, Peter Maydell wrote:> On 19 December 2011 14:13, Avi Kivity <avi@redhat.com> wrote: > > --- > > This is a rather uninformative commit message -- was this patch > intended to be in this series? >It was intended to be merged into patch 9. Will fix for the next round. -- error compiling committee.c: too many arguments to function -- 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
Anthony Liguori
2011-Dec-19 14:50 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 08:13 AM, Avi Kivity wrote:> Given an address space (represented by the top-level memory region), > returns the memory region that maps a given range. Useful for implementing > DMA. > > The implementation is a simplistic binary search. Once we have a tree > representation this can be optimized. > > Signed-off-by: Avi Kivity<avi@redhat.com> > --- > memory.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > memory.h | 32 ++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+), 0 deletions(-) > > diff --git a/memory.c b/memory.c > index cbd6988..d8b7c27 100644 > --- a/memory.c > +++ b/memory.c > @@ -514,6 +514,20 @@ static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd) > .ops =&address_space_ops_io, > }; > > +static AddressSpace *memory_region_to_address_space(MemoryRegion *mr) > +{ > + while (mr->parent) { > + mr = mr->parent; > + } > + if (mr == address_space_memory.root) { > + return&address_space_memory; > + } > + if (mr == address_space_io.root) { > + return&address_space_io; > + } > + abort(); > +} > + > /* Render a memory region into the global view. Ranges in @view obscure > * ranges in @mr. > */ > @@ -1309,6 +1323,54 @@ void memory_region_del_subregion(MemoryRegion *mr, > memory_region_update_topology(); > } > > +static int cmp_flatrange_addr(const void *_addr, const void *_fr) > +{ > + const AddrRange *addr = _addr; > + const FlatRange *fr = _fr;Please don''t prefix with an underscore.> + > + if (int128_le(addrrange_end(*addr), fr->addr.start)) { > + return -1; > + } else if (int128_ge(addr->start, addrrange_end(fr->addr))) { > + return 1; > + } > + return 0; > +} > + > +static FlatRange *address_space_lookup(AddressSpace *as, AddrRange addr) > +{ > + return bsearch(&addr, as->current_map.ranges, as->current_map.nr, > + sizeof(FlatRange), cmp_flatrange_addr); > +} > + > +MemoryRegionSection memory_region_find(MemoryRegion *address_space, > + target_phys_addr_t addr, uint64_t size) > +{ > + AddressSpace *as = memory_region_to_address_space(address_space); > + AddrRange range = addrrange_make(int128_make64(addr), > + int128_make64(size)); > + FlatRange *fr = address_space_lookup(as, range); > + MemoryRegionSection ret = { .mr = NULL }; > + > + if (!fr) { > + return ret; > + } > + > + while (fr> as->current_map.ranges > +&& addrrange_intersects(fr[-1].addr, range)) { > + --fr; > + } > + > + ret.mr = fr->mr; > + range = addrrange_intersection(range, fr->addr); > + ret.offset_within_region = fr->offset_in_region; > + ret.offset_within_region += int128_get64(int128_sub(range.start, > + fr->addr.start)); > + ret.size = int128_get64(range.size); > + ret.offset_within_address_space = int128_get64(range.start); > + return ret; > +} > + > + > void set_system_memory_map(MemoryRegion *mr) > { > address_space_memory.root = mr; > diff --git a/memory.h b/memory.h > index beae127..1d5c414 100644 > --- a/memory.h > +++ b/memory.h > @@ -146,6 +146,24 @@ struct MemoryRegionPortio { > > #define PORTIO_END_OF_LIST() { } > > +typedef struct MemoryRegionSection MemoryRegionSection; > + > +/** > + * MemoryRegionSection: describes a fragment of a #MemoryRegion > + * > + * @mr: the region, or %NULL if empty > + * @offset_within_region: the beginning of the section, relative to @mr''s start > + * @size: the size of the section; will not exceed @mr''s boundaries > + * @offset_within_address_space: the address of the first byte of the section > + * relative to the region''s address space > + */ > +struct MemoryRegionSection { > + MemoryRegion *mr; > + target_phys_addr_t offset_within_region; > + uint64_t size; > + target_phys_addr_t offset_within_address_space; > +}; > + > /** > * memory_region_init: Initialize a memory region > * > @@ -502,6 +520,20 @@ void memory_region_del_subregion(MemoryRegion *mr, > MemoryRegion *subregion); > > /** > + * memory_region_find: locate a MemoryRegion in an address space > + * > + * Locates the first #MemoryRegion within an address space given by > + * @address_space that overlaps the range given by @addr and @size. > + * > + * @address_space: a top-level (i.e. parentless) region that contains > + * the region to be found > + * @addr: start of the area within @address_space to be searched > + * @size: size of the area to be searched > + */ > +MemoryRegionSection memory_region_find(MemoryRegion *address_space, > + target_phys_addr_t addr, uint64_t size);Returning structs by value is a bit unexpected. Otherwise, the patch looks good. Regards, Anthony Liguori> + > +/** > * memory_region_transaction_begin: Start a transaction. > * > * During a transaction, changes will be accumulated and made visible-- 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
Anthony Liguori
2011-Dec-19 14:54 UTC
Re: [Qemu-devel] [PATCH 00/23] Remove cpu_get_physical_page_desc()
On 12/19/2011 08:13 AM, Avi Kivity wrote:> cpu_get_physical_page_desc() exposes the internals of the memory core to > callers; as such it prevents the refactoring planned there. This patchset > converts all callers memory API equivalents and removes the function. > > The conversion leaves a lot of potential for further cleanups; the > MemoryListener API (which replaces CPUPhysMemoryClient) guarantees matched > range_add and range_del calls, so the need to handle splitting is removed. > This is left for later. > > Please review and test, especially the vhost and Xen parts, which I only > build tested.Other than the few style comments, the whole series looks reasonable to me. Regards, Anthony Liguori> > Also available from: > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/page_desc > > Avi Kivity (23): > memory: introduce memory_region_find() > sysbus: add sysbus_address_space() > memory: add memory_region_is_ram() > framebuffer: drop use of cpu_get_physical_page_desc() > memory: add memory_region_is_rom() > loader: remove calls to cpu_get_physical_page_desc() > framebuffer: drop use of cpu_physical_sync_dirty_bitmap() > memory: replace cpu_physical_sync_dirty_bitmap() with a memory API > memory: add API for observing updates to the physical memory map > memory: add memory_region_is_logging() > kvm: switch kvm slots to use host virtual address instead of > ram_addr_t > fixup: listener fixes > kvm: convert to MemoryListener API > vhost: convert to MemoryListener API > xen, vga: add API for registering the framebuffer > memory: temporarily add memory_region_get_ram_addr() > xen: convert to MemoryListener API > memory: remove CPUPhysMemoryClient > kvm: avoid cpu_get_physical_page_desc() > vhost: avoid cpu_get_physical_page_desc() > virtio-balloon: avoid cpu_get_physical_page_desc() > sparc: avoid cpu_get_physical_page_desc() > Remove cpu_get_physical_page_desc() > > arch_init.c | 6 +- > cpu-all.h | 9 -- > cpu-common.h | 24 ------ > exec.c | 175 +--------------------------------------- > hw/framebuffer.c | 32 +++---- > hw/framebuffer.h | 3 + > hw/loader.c | 9 +- > hw/milkymist-vgafb.c | 2 +- > hw/omap_lcdc.c | 4 +- > hw/pl110.c | 2 +- > hw/pxa2xx_lcd.c | 10 ++- > hw/sysbus.c | 5 + > hw/sysbus.h | 1 + > hw/vga.c | 2 + > hw/vhost.c | 167 ++++++++++++++++++++++++++++++--------- > hw/vhost.h | 5 +- > hw/virtio-balloon.c | 14 +++- > hw/xen.h | 3 + > kvm-all.c | 151 +++++++++++++++++++++-------------- > kvm.h | 4 +- > memory.c | 193 ++++++++++++++++++++++++++++++++++++++++++--- > memory.h | 127 +++++++++++++++++++++++++++++ > target-i386/kvm.c | 7 +- > target-sparc/mmu_helper.c | 5 +- > trace-events | 2 +- > xen-all.c | 143 ++++++++++++++++++++------------- > xen-stub.c | 4 + > 27 files changed, 695 insertions(+), 414 deletions(-) >-- 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
Avi Kivity
2011-Dec-19 15:04 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 04:50 PM, Anthony Liguori wrote:>> +static int cmp_flatrange_addr(const void *_addr, const void *_fr) >> +{ >> + const AddrRange *addr = _addr; >> + const FlatRange *fr = _fr; > > > Please don''t prefix with an underscore.Why not? It''s legal according to the standards, if that''s your concern (only _[_A-Z]+ are reserved).>> @@ -502,6 +520,20 @@ void memory_region_del_subregion(MemoryRegion *mr, >> MemoryRegion *subregion); >> >> /** >> + * memory_region_find: locate a MemoryRegion in an address space >> + * >> + * Locates the first #MemoryRegion within an address space given by >> + * @address_space that overlaps the range given by @addr and @size. >> + * >> + * @address_space: a top-level (i.e. parentless) region that contains >> + * the region to be found >> + * @addr: start of the area within @address_space to be searched >> + * @size: size of the area to be searched >> + */ >> +MemoryRegionSection memory_region_find(MemoryRegion *address_space, >> + target_phys_addr_t addr, >> uint64_t size); > > > Returning structs by value is a bit unexpected.It''s just prejudice, here''s the call sequence: 127a63: 48 89 c6 mov %rax,%rsi 127a66: 48 8d 45 b0 lea -0x50(%rbp),%rax 127a6a: b9 01 00 00 00 mov $0x1,%ecx 127a6f: 48 89 da mov %rbx,%rdx 127a72: 48 89 c7 mov %rax,%rdi 127a75: e8 89 46 15 00 callq 27c103 <memory_region_find> The return value is passed via %rax, so no performance penalty. On the other hand, it''s clear that it is a return value, unlike a pointer parameter. -- error compiling committee.c: too many arguments to function -- 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
Anthony Liguori
2011-Dec-19 15:10 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 09:04 AM, Avi Kivity wrote:> On 12/19/2011 04:50 PM, Anthony Liguori wrote: >>> +static int cmp_flatrange_addr(const void *_addr, const void *_fr) >>> +{ >>> + const AddrRange *addr = _addr; >>> + const FlatRange *fr = _fr; >> >> >> Please don''t prefix with an underscore. > > Why not? It''s legal according to the standards, if that''s your concern > (only _[_A-Z]+ are reserved).http://www.gnu.org/s/hello/manual/libc/Reserved-Names.html "In addition to the names documented in this manual, reserved names include all external identifiers (global functions and variables) that begin with an underscore (‘_’)" Now that might just mean that you''re shadowing a global name, so maybe that''s okay, but I think it''s easier to just enforce a consistent rule of, "don''t start identifiers with an underscore".> >>> @@ -502,6 +520,20 @@ void memory_region_del_subregion(MemoryRegion *mr, >>> MemoryRegion *subregion); >>> >>> /** >>> + * memory_region_find: locate a MemoryRegion in an address space >>> + * >>> + * Locates the first #MemoryRegion within an address space given by >>> + * @address_space that overlaps the range given by @addr and @size. >>> + * >>> + * @address_space: a top-level (i.e. parentless) region that contains >>> + * the region to be found >>> + * @addr: start of the area within @address_space to be searched >>> + * @size: size of the area to be searched >>> + */ >>> +MemoryRegionSection memory_region_find(MemoryRegion *address_space, >>> + target_phys_addr_t addr, >>> uint64_t size); >> >> >> Returning structs by value is a bit unexpected. > > It''s just prejudice, here''s the call sequence:It''s not about whether it''s fast or slow. It''s just unexpected. Instead of returning a NULL pointer on error, you set .mr to NULL if an error occurs (which isn''t documented by the comment btw). Returning a pointer, or taking a pointer and returning a 0/-1 return value makes for a more consistent semantic with the rest of the code base. Regards, Anthony Liguori -- 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
Avi Kivity
2011-Dec-19 15:17 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 05:10 PM, Anthony Liguori wrote:> On 12/19/2011 09:04 AM, Avi Kivity wrote: >> On 12/19/2011 04:50 PM, Anthony Liguori wrote: >>>> +static int cmp_flatrange_addr(const void *_addr, const void *_fr) >>>> +{ >>>> + const AddrRange *addr = _addr; >>>> + const FlatRange *fr = _fr; >>> >>> >>> Please don''t prefix with an underscore. >> >> Why not? It''s legal according to the standards, if that''s your concern >> (only _[_A-Z]+ are reserved). > > http://www.gnu.org/s/hello/manual/libc/Reserved-Names.html > > "In addition to the names documented in this manual, reserved names > include all external identifiers (global functions and variables) that > begin with an underscore (‘_’)" > > Now that might just mean that you''re shadowing a global name, so maybe > that''s okay, but I think it''s easier to just enforce a consistent rule > of, "don''t start identifiers with an underscore".I rather like the pattern void callback(void *_foo) { Foo *foo = _foo; ... } If the consensus is that we don''t want it, I''ll change it, but I do think it''s useful.>>>> +MemoryRegionSection memory_region_find(MemoryRegion *address_space, >>>> + target_phys_addr_t addr, >>>> uint64_t size); >>> >>> >>> Returning structs by value is a bit unexpected. >> >> It''s just prejudice, here''s the call sequence: > > It''s not about whether it''s fast or slow. It''s just unexpected.It''s plain C, I don''t see why it''s so unexpected.> > Instead of returning a NULL pointer on error, you set .mr to NULL if > an error occurs (which isn''t documented by the comment btw). > Returning a pointer, or taking a pointer and returning a 0/-1 return > value makes for a more consistent semantic with the rest of the code > base. >How can I return a pointer? If I allocate it, someone has to free it. If I pass it as a parameter, we end up with a silly looking calling convention. If I return an error code, the caller has to set up an additional variable. The natural check is section.size which is always meaningful - memory_region_find() returns a subrange of the input, which may be zero. You''re right that I should document it (and I should use it consistently). -- error compiling committee.c: too many arguments to function -- 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
Anthony Liguori
2011-Dec-19 15:25 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 09:17 AM, Avi Kivity wrote:> On 12/19/2011 05:10 PM, Anthony Liguori wrote: >> On 12/19/2011 09:04 AM, Avi Kivity wrote: >>> On 12/19/2011 04:50 PM, Anthony Liguori wrote: >>>>> +static int cmp_flatrange_addr(const void *_addr, const void *_fr) >>>>> +{ >>>>> + const AddrRange *addr = _addr; >>>>> + const FlatRange *fr = _fr; >>>> >>>> >>>> Please don''t prefix with an underscore. >>> >>> Why not? It''s legal according to the standards, if that''s your concern >>> (only _[_A-Z]+ are reserved). >> >> http://www.gnu.org/s/hello/manual/libc/Reserved-Names.html >> >> "In addition to the names documented in this manual, reserved names >> include all external identifiers (global functions and variables) that >> begin with an underscore (‘_’)" >> >> Now that might just mean that you''re shadowing a global name, so maybe >> that''s okay, but I think it''s easier to just enforce a consistent rule >> of, "don''t start identifiers with an underscore". > > I rather like the pattern > > void callback(void *_foo) > { > Foo *foo = _foo; > > ... > } > > If the consensus is that we don''t want it, I''ll change it, but I do > think it''s useful.I dislike enforcing coding style but it''s a necessary evil. I greater prefer simple rules to subtle ones as it creates less confusion so I''d prefer you change this.> >>>>> +MemoryRegionSection memory_region_find(MemoryRegion *address_space, >>>>> + target_phys_addr_t addr, >>>>> uint64_t size); >>>> >>>> >>>> Returning structs by value is a bit unexpected. >>> >>> It''s just prejudice, here''s the call sequence: >> >> It''s not about whether it''s fast or slow. It''s just unexpected. > > It''s plain C, I don''t see why it''s so unexpected. > >> >> Instead of returning a NULL pointer on error, you set .mr to NULL if >> an error occurs (which isn''t documented by the comment btw). >> Returning a pointer, or taking a pointer and returning a 0/-1 return >> value makes for a more consistent semantic with the rest of the code >> base. >> > > How can I return a pointer? If I allocate it, someone has to free it. > If I pass it as a parameter, we end up with a silly looking calling > convention. If I return an error code, the caller has to set up an > additional variable. > > The natural check is section.size which is always meaningful - > memory_region_find() returns a subrange of the input, which may be > zero. You''re right that I should document it (and I should use it > consistently).I''m not going to make a fuss over something like this so if you really prefer this style, I''m not going to object strongly. But it should be at least be documented and used consistently. Regards, Anthony Liguori -- 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
Avi Kivity
2011-Dec-19 15:28 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 05:25 PM, Anthony Liguori wrote:>> I rather like the pattern >> >> void callback(void *_foo) >> { >> Foo *foo = _foo; >> >> ... >> } >> >> If the consensus is that we don''t want it, I''ll change it, but I do >> think it''s useful. > > > I dislike enforcing coding style but it''s a necessary evil. I greater > prefer simple rules to subtle ones as it creates less confusion so I''d > prefer you change this.Okay.>> How can I return a pointer? If I allocate it, someone has to free it. >> If I pass it as a parameter, we end up with a silly looking calling >> convention. If I return an error code, the caller has to set up an >> additional variable. >> >> The natural check is section.size which is always meaningful - >> memory_region_find() returns a subrange of the input, which may be >> zero. You''re right that I should document it (and I should use it >> consistently). > > I''m not going to make a fuss over something like this so if you really > prefer this style, I''m not going to object strongly. > > But it should be at least be documented and used consistently.Will update both. -- error compiling committee.c: too many arguments to function -- 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
Avi Kivity
2011-Dec-19 15:52 UTC
Re: [Qemu-devel] [PATCH 01/23] memory: introduce memory_region_find()
On 12/19/2011 05:28 PM, Avi Kivity wrote:> > > > But it should be at least be documented and used consistently. > > Will update both.Updated patchset in qemu-kvm.git page_desc. Will refrain from reposting until some time has passed for review. -- error compiling committee.c: too many arguments to function -- 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
Michael S. Tsirkin
2011-Dec-22 12:48 UTC
Re: [PATCH 20/23] vhost: avoid cpu_get_physical_page_desc()
On Mon, Dec 19, 2011 at 04:13:41PM +0200, Avi Kivity wrote:> @@ -871,7 +899,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) > hdev->vqs + i, > i); > } > - vhost_sync_dirty_bitmap(hdev, 0, (target_phys_addr_t)~0x0ull); > + for (i = 0; i < hdev->n_mem_sections; ++i) { > + vhost_sync_dirty_bitmap(hdev, &hdev->mem_sections[i], > + 0, (target_phys_addr_t)~0x0ull); > + } > r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, false); > if (r < 0) { > fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r); > diff --git a/hw/vhost.h b/hw/vhost.h > index d1824ec..80e64df 100644 > --- a/hw/vhost.h > +++ b/hw/vhost.h > @@ -30,6 +30,8 @@ struct vhost_dev { > MemoryListener memory_listener; > int control; > struct vhost_memory *mem; > + int n_mem_sections; > + MemoryRegionSection *mem_sections; > struct vhost_virtqueue *vqs; > int nvqs; > unsigned long long features;This adds need to track all sections which is unfortunate. Couldn''t the memory API get an extension e.g. to scan them all?> -- > 1.7.7.1-- 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
Avi Kivity
2011-Dec-22 12:49 UTC
Re: [PATCH 20/23] vhost: avoid cpu_get_physical_page_desc()
On 12/22/2011 02:48 PM, Michael S. Tsirkin wrote:> On Mon, Dec 19, 2011 at 04:13:41PM +0200, Avi Kivity wrote: > > @@ -871,7 +899,10 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) > > hdev->vqs + i, > > i); > > } > > - vhost_sync_dirty_bitmap(hdev, 0, (target_phys_addr_t)~0x0ull); > > + for (i = 0; i < hdev->n_mem_sections; ++i) { > > + vhost_sync_dirty_bitmap(hdev, &hdev->mem_sections[i], > > + 0, (target_phys_addr_t)~0x0ull); > > + } > > r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, false); > > if (r < 0) { > > fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r); > > diff --git a/hw/vhost.h b/hw/vhost.h > > index d1824ec..80e64df 100644 > > --- a/hw/vhost.h > > +++ b/hw/vhost.h > > @@ -30,6 +30,8 @@ struct vhost_dev { > > MemoryListener memory_listener; > > int control; > > struct vhost_memory *mem; > > + int n_mem_sections; > > + MemoryRegionSection *mem_sections; > > struct vhost_virtqueue *vqs; > > int nvqs; > > unsigned long long features; > > This adds need to track all sections which is unfortunate. > Couldn''t the memory API get an extension e.g. to scan them all?I thought about it, it makes sense. We even have memory_region_find() which can be used to implement it, just need a FOR_EACH wrapper. -- error compiling committee.c: too many arguments to function -- 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
Michael S. Tsirkin
2011-Dec-22 12:50 UTC
Re: [PATCH 14/23] vhost: convert to MemoryListener API
On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote:> +static void vhost_log_start(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + /* FIXME: implement */ > +} > + > +static void vhost_log_stop(MemoryListener *listener, > + MemoryRegionSection *section) > +{ > + /* FIXME: implement */ > +} > +What exactly do we need to fix here? -- 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
On 12/22/2011 02:50 PM, Michael S. Tsirkin wrote:> On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote: > > +static void vhost_log_start(MemoryListener *listener, > > + MemoryRegionSection *section) > > +{ > > + /* FIXME: implement */ > > +} > > + > > +static void vhost_log_stop(MemoryListener *listener, > > + MemoryRegionSection *section) > > +{ > > + /* FIXME: implement */ > > +} > > + > > What exactly do we need to fix here?Tell vhost to start tracking those regions? I guess you don''t often read packets into the framebuffer, or we''d have a lot of bug reports. -- error compiling committee.c: too many arguments to function -- 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
Michael S. Tsirkin
2011-Dec-22 12:57 UTC
Re: [PATCH 14/23] vhost: convert to MemoryListener API
On Thu, Dec 22, 2011 at 02:50:27PM +0200, Avi Kivity wrote:> On 12/22/2011 02:50 PM, Michael S. Tsirkin wrote: > > On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote: > > > +static void vhost_log_start(MemoryListener *listener, > > > + MemoryRegionSection *section) > > > +{ > > > + /* FIXME: implement */ > > > +} > > > + > > > +static void vhost_log_stop(MemoryListener *listener, > > > + MemoryRegionSection *section) > > > +{ > > > + /* FIXME: implement */ > > > +} > > > + > > > > What exactly do we need to fix here? > > Tell vhost to start tracking those regions? > > I guess you don''t often read packets into the framebuffer, or we''d have > a lot of bug reports.Yes, we currently simply don''t pass these regions to vhost. It currently signals an error if such is observed, so we could handle framebuffer in userspace if we wanted to.> -- > error compiling committee.c: too many arguments to function-- 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
On 12/22/2011 02:57 PM, Michael S. Tsirkin wrote:> On Thu, Dec 22, 2011 at 02:50:27PM +0200, Avi Kivity wrote: > > On 12/22/2011 02:50 PM, Michael S. Tsirkin wrote: > > > On Mon, Dec 19, 2011 at 04:13:35PM +0200, Avi Kivity wrote: > > > > +static void vhost_log_start(MemoryListener *listener, > > > > + MemoryRegionSection *section) > > > > +{ > > > > + /* FIXME: implement */ > > > > +} > > > > + > > > > +static void vhost_log_stop(MemoryListener *listener, > > > > + MemoryRegionSection *section) > > > > +{ > > > > + /* FIXME: implement */ > > > > +} > > > > + > > > > > > What exactly do we need to fix here? > > > > Tell vhost to start tracking those regions? > > > > I guess you don''t often read packets into the framebuffer, or we''d have > > a lot of bug reports. > > Yes, we currently simply don''t pass these regions to vhost. > It currently signals an error if such is > observed, so we could handle framebuffer in userspace > if we wanted to.I see, thanks. -- error compiling committee.c: too many arguments to function
Stefano Stabellini
2012-Jan-04 18:06 UTC
Re: [PATCH 17/23] xen: convert to MemoryListener API
On Mon, 19 Dec 2011, Avi Kivity wrote:> -static int xen_log_start(CPUPhysMemoryClient *client, target_phys_addr_t phys_addr, ram_addr_t size) > +static void xen_log_start(MemoryListener *listener, > + MemoryRegionSection *section) > { > - XenIOState *state = container_of(client, XenIOState, client); > + XenIOState *state = container_of(listener, XenIOState, memory_listener); > + int r; > > - return xen_sync_dirty_bitmap(state, phys_addr, size); > + r = xen_sync_dirty_bitmap(state, section->offset_within_address_space, > + section->size); > + assert(r >= 0); > }I really feel I should thank you for your work because you did a very good job porting xen to the new api. In fact apart from the dirty bitmap (Anthony is about to send a patch to fix the issue: xen_sync_dirty_bitmap can actually fail sometimes), everything else is done right and works correctly. However I would have appreciated if you could have given us more time to review the four patches you wrote: considering the time of the year both Anthony and I were on vacation and didn''t have a chance to read them until today.
On 01/04/2012 08:06 PM, Stefano Stabellini wrote:> On Mon, 19 Dec 2011, Avi Kivity wrote: > > -static int xen_log_start(CPUPhysMemoryClient *client, target_phys_addr_t phys_addr, ram_addr_t size) > > +static void xen_log_start(MemoryListener *listener, > > + MemoryRegionSection *section) > > { > > - XenIOState *state = container_of(client, XenIOState, client); > > + XenIOState *state = container_of(listener, XenIOState, memory_listener); > > + int r; > > > > - return xen_sync_dirty_bitmap(state, phys_addr, size); > > + r = xen_sync_dirty_bitmap(state, section->offset_within_address_space, > > + section->size); > > + assert(r >= 0); > > } > > I really feel I should thank you for your work because you did a very > good job porting xen to the new api. In fact apart from the dirty bitmap > (Anthony is about to send a patch to fix the issue: > xen_sync_dirty_bitmap can actually fail sometimes), everything else > is done right and works correctly.Thanks.> However I would have appreciated if you could have given us more time to > review the four patches you wrote: considering the time of the year both > Anthony and I were on vacation and didn''t have a chance to read them > until today.I realize that I bypassed the normal protocol here, but I had to choose one of several bad choices: - continue developing without merging, and risk large rebases in case the patches (or something else in qemu) had to be changed - stop developing until you returned from your (undoubtedly well deserved) vacations - merge and look away while whistling innocently I chose the third, since I still have quite a lot of work with the memory API. Of course I will help with fixing the fallout if needed, and since you''re back online, we can go back to the normal way of reviewing and testing patches before merging. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- 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
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. -- error compiling committee.c: too many arguments to function -- 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
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. -- error compiling committee.c: too many arguments to function -- 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
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? -- error compiling committee.c: too many arguments to function -- 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
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