Re: [PATCH] remove KVM_CAP_USER_MEMORY reference from qemu-kvm.c

[Glauber Costa <glommer@redhat.com>]

On Wed, Sep 10, 2008 at 09:23:18PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > From: Glauber Costa <glommer@redhat.com>
> > 
> > kvm_cpu_register_physical_memory() is its only user. Remove it.
> > 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  qemu/qemu-kvm.c |   52 +++++++++++++++++++++-------------------------------
> >  1 files changed, 21 insertions(+), 31 deletions(-)
> > 
> > diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
> > index 8d366e5..f0ef21e 100644
> > --- a/qemu/qemu-kvm.c
> > +++ b/qemu/qemu-kvm.c
> > @@ -775,42 +775,32 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
> >                                        unsigned long size,
> >                                        unsigned long phys_offset)
> >  {
> > -#ifdef KVM_CAP_USER_MEMORY
> >      int r = 0;
> > -
> > -    r = kvm_check_extension(kvm_context, KVM_CAP_USER_MEMORY);
> > -    if (r) {
> > -        if (!(phys_offset & ~TARGET_PAGE_MASK)) {
> > -                r = kvm_is_allocated_mem(kvm_context, start_addr, size);
> > -            if (r)
> > -                return;
> > -            r = kvm_is_intersecting_mem(kvm_context, start_addr);
> > -            if (r)
> > -                kvm_create_mem_hole(kvm_context, start_addr, size);
> > -            r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
> > -                                                phys_ram_base + phys_offset,
> > -                                                size, 0);
> > -        }
> > -        if (phys_offset & IO_MEM_ROM) {
> > -            phys_offset &= ~IO_MEM_ROM;
> > -            r = kvm_is_intersecting_mem(kvm_context, start_addr);
> > -            if (r)
> > -                kvm_create_mem_hole(kvm_context, start_addr, size);
> > -            r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
> > -                                                phys_ram_base + phys_offset,
> > -                                                size, 0);
> > -        }
> > -        if (r < 0) {
> > -            printf("kvm_cpu_register_physical_memory: failed\n");
> > -            exit(1);
> > -        }
> > -        return;
> > +    if (!(phys_offset & ~TARGET_PAGE_MASK)) {
> > +        r = kvm_is_allocated_mem(kvm_context, start_addr, size);
> > +        if (r)
> > +            return;
> > +        r = kvm_is_intersecting_mem(kvm_context, start_addr);
> > +        if (r)
> > +            kvm_create_mem_hole(kvm_context, start_addr, size);
> > +        r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
> > +                                            phys_ram_base + phys_offset,
> > +                                            size, 0);
> >      }
> > -#endif
> >      if (phys_offset & IO_MEM_ROM) {
> 
> At this chance: Shouldn't this become 'else if'?

fyi: just sent out a new series, this time with numbers and no tabs in qemu ;-)

As a coding style practice, yes. But they are mutually exclusive anyway, so it shouldn't
matter. We could definitely try to make it better, but they way to go is to have only one case
instead of two: look at how close they are to each other! I have a later series that do that,
so I'm just trying to set up the ground here.

> 
> >          phys_offset &= ~IO_MEM_ROM;
> > -        memcpy(phys_ram_base + start_addr, phys_ram_base + phys_offset, size);
> > +        r = kvm_is_intersecting_mem(kvm_context, start_addr);
> > +        if (r)
> > +            kvm_create_mem_hole(kvm_context, start_addr, size);
> > +        r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
> > +                                            phys_ram_base + phys_offset,
> > +                                            size, 0);
> > +    }
> > +    if (r < 0) {
> > +        printf("kvm_cpu_register_physical_memory: failed\n");
> > +        exit(1);
> >      }
> > +    return;
> >  }
> >  
> >  int kvm_setup_guest_memory(void *area, unsigned long size)
> 
> Jan
>