On Tue, Nov 13, 2012 at 04:37:38AM +0000, Sasha Levin wrote: > On 11/12/2012 06:57 AM, Will Deacon wrote: > > +struct kvm_mem_bank { > > + struct list_head list; > > + unsigned long guest_phys_addr; > > + void *host_addr; > > + unsigned long size; > > +}; > > Can we just reuse struct kvm_userspace_memory_region here? We're also using different > data types for guest_phys_addr and size than whats in kvm_userspace_memory_region - that > can't be good. I looked briefly at doing that when I wrote the multi-bank stuff, but I hit a couple of issues: - kvmtool itself tends to use void * for host addresses, rather than the __u64 userspace_addr in kvm_userspace_memory_region - kvm_userspace_memory_region is a superset of what we need (not the end of the world I guess) so you end up casting address types a fair amount. Still, I'll revisit it and see if I can come up with something cleaner. > > struct kvm { > > struct kvm_arch arch; > > struct kvm_config cfg; > > @@ -49,6 +56,7 @@ struct kvm { > > u64 ram_size; > > void *ram_start; > > u64 ram_pagesize; > > + struct list_head mem_banks; > > These memory banks actually look like a perfect example to use our augmented interval rb-tree, > can we switch them to use it, or is it a list on purpose? Well, the usual case is one memory bank but that doesn't swing the argument either way. I'll update to use the fancy new tree. Thanks for the comments, Will -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html