From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51602) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YtIcI-0002H6-WA for qemu-devel@nongnu.org; Fri, 15 May 2015 12:37:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YtIcH-00017h-78 for qemu-devel@nongnu.org; Fri, 15 May 2015 12:37:42 -0400 Received: from mail-wg0-x230.google.com ([2a00:1450:400c:c00::230]:32951) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YtIcG-00017R-Tp for qemu-devel@nongnu.org; Fri, 15 May 2015 12:37:41 -0400 Received: by wgin8 with SMTP id n8so119224782wgi.0 for ; Fri, 15 May 2015 09:37:40 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Fri, 15 May 2015 18:37:01 +0200 Message-Id: <1431707823-51230-6-git-send-email-pbonzini@redhat.com> In-Reply-To: <1431707823-51230-1-git-send-email-pbonzini@redhat.com> References: <1431707823-51230-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH 5/7] memory: add kvm_mem_flags to MemoryRegion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, lersek@redhat.com, avi.kivity@gmail.com, kraxel@redhat.com This patch is ugly; it adds a KVM-specific flag to memory regions that is used to distinguish SMRAM regions from others. This of course is a layering violation, but I have no other good ideas about how to avoid it. If you let KVM use address_space_memory as it did until now, and add separate calls to KVM_SET_USER_MEMORY_REGION for SMRAM regions, it's much harder to cover the case when a RAM or ROM BAR is placed to cover parts of SMRAM. Hence, this series uses instead one address space only, including all of system memory, PCI BARs and SMRAM. This way, hiding the BARs is done automatically by the memory core. This, however, has an important issue which requires this kind of layering violation. The issue is that when you close SMRAM there is no change in the memory map as far as the memory API is concerned. The only change is in the flags we pass to KVM, but these are not visible to the memory API. So KVM needs to convince the memory API to see a region_del/region_add sequence from the listener; it needs to convince flatrange_equal to return false for the "open SMRAM" and "closed SMRAM" cases. This is what the kvm_mem_flags do. Besides the layering violation, there is also the question of how to handle flags from subregions. The current code just ORs the parent region's flags into all subregions. This being ugly, and hopefully temporary until someone shows me the light, I haven't yet added nice accessors for the field. The final patch accesses the field blindly from target-i386/kvm.c. One alternative would be to point two different RAM regions to the same guest memory, using memory_region_init_ram_ptr. That works, but it's even more gross. Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 4 ++++ kvm-all.c | 13 +++++++------ memory.c | 17 +++++++++++++---- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index b61c84f..cccba59 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -192,6 +192,8 @@ struct MemoryRegion { unsigned ioeventfd_nb; MemoryRegionIoeventfd *ioeventfds; NotifierList iommu_notify; + + int kvm_mem_flags; }; /** @@ -264,6 +266,8 @@ struct MemoryRegionSection { Int128 size; hwaddr offset_within_address_space; bool readonly; + + int kvm_mem_flags; }; /** diff --git a/kvm-all.c b/kvm-all.c index c70436e..ccb7dd3 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -237,10 +237,11 @@ err: * dirty pages logging control */ -static int kvm_mem_flags(MemoryRegion *mr) +static int kvm_mem_flags(MemoryRegionSection *section) { + MemoryRegion *mr = section->mr; bool readonly = mr->readonly || memory_region_is_romd(mr); - int flags = 0; + int flags = section->kvm_mem_flags; if (memory_region_is_logging(mr)) { flags |= KVM_MEM_LOG_DIRTY_PAGES; @@ -689,7 +690,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) mem->memory_size = old.memory_size; mem->start_addr = old.start_addr; mem->ram = old.ram; - mem->flags = kvm_mem_flags(mr); + mem->flags = kvm_mem_flags(section); err = kvm_set_user_memory_region(s, mem); if (err) { @@ -710,7 +711,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) mem->memory_size = start_addr - old.start_addr; mem->start_addr = old.start_addr; mem->ram = old.ram; - mem->flags = kvm_mem_flags(mr); + mem->flags = kvm_mem_flags(section); err = kvm_set_user_memory_region(s, mem); if (err) { @@ -734,7 +735,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) size_delta = mem->start_addr - old.start_addr; mem->memory_size = old.memory_size - size_delta; mem->ram = old.ram + size_delta; - mem->flags = kvm_mem_flags(mr); + mem->flags = kvm_mem_flags(section); err = kvm_set_user_memory_region(s, mem); if (err) { @@ -756,7 +757,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) mem->memory_size = size; mem->start_addr = start_addr; mem->ram = ram; - mem->flags = kvm_mem_flags(mr); + mem->flags = kvm_mem_flags(section); err = kvm_set_user_memory_region(s, mem); if (err) { diff --git a/memory.c b/memory.c index 03c536b..cf810f1 100644 --- a/memory.c +++ b/memory.c @@ -160,6 +160,7 @@ static bool memory_listener_match(MemoryListener *listener, .size = (fr)->addr.size, \ .offset_within_address_space = int128_get64((fr)->addr.start), \ .readonly = (fr)->readonly, \ + .kvm_mem_flags = (fr)->kvm_mem_flags, \ })) struct CoalescedMemoryRange { @@ -219,6 +220,7 @@ struct FlatRange { MemoryRegion *mr; hwaddr offset_in_region; AddrRange addr; + int kvm_mem_flags; uint8_t dirty_log_mask; bool romd_mode; bool readonly; @@ -246,6 +248,7 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b) && addrrange_equal(a->addr, b->addr) && a->offset_in_region == b->offset_in_region && a->romd_mode == b->romd_mode + && a->kvm_mem_flags == b->kvm_mem_flags && a->readonly == b->readonly; } @@ -306,7 +309,8 @@ static bool can_merge(FlatRange *r1, FlatRange *r2) int128_make64(r2->offset_in_region)) && r1->dirty_log_mask == r2->dirty_log_mask && r1->romd_mode == r2->romd_mode - && r1->readonly == r2->readonly; + && r1->readonly == r2->readonly + && r1->kvm_mem_flags == r2->kvm_mem_flags; } /* Attempt to simplify a view by merging adjacent ranges */ @@ -542,6 +546,7 @@ static void render_memory_region(FlatView *view, MemoryRegion *mr, Int128 base, AddrRange clip, + int kvm_mem_flags, bool readonly) { MemoryRegion *subregion; @@ -556,6 +561,7 @@ static void render_memory_region(FlatView *view, return; } + kvm_mem_flags |= mr->kvm_mem_flags; int128_addto(&base, int128_make64(mr->addr)); readonly |= mr->readonly; @@ -570,13 +576,15 @@ static void render_memory_region(FlatView *view, if (mr->alias) { int128_subfrom(&base, int128_make64(mr->alias->addr)); int128_subfrom(&base, int128_make64(mr->alias_offset)); - render_memory_region(view, mr->alias, base, clip, readonly); + render_memory_region(view, mr->alias, base, clip, + kvm_mem_flags, readonly); return; } /* Render subregions in priority order. */ QTAILQ_FOREACH(subregion, &mr->subregions, subregions_link) { - render_memory_region(view, subregion, base, clip, readonly); + render_memory_region(view, subregion, base, clip, + kvm_mem_flags, readonly); } if (!mr->terminates) { @@ -591,6 +599,7 @@ static void render_memory_region(FlatView *view, fr.dirty_log_mask = mr->dirty_log_mask; fr.romd_mode = mr->romd_mode; fr.readonly = readonly; + fr.kvm_mem_flags = kvm_mem_flags; /* Render the region itself into any gaps left by the current view. */ for (i = 0; i < view->nr && int128_nz(remain); ++i) { @@ -632,7 +641,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr) if (mr) { render_memory_region(view, mr, int128_zero(), - addrrange_make(int128_zero(), int128_2_64()), false); + addrrange_make(int128_zero(), int128_2_64()), 0, false); } flatview_simplify(view); -- 1.8.3.1