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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to