Re: [Qemu-devel] [PATCH 6/6] kvm: vga optimization.

[Anthony Liguori]

Glauber Costa wrote:
> > > +    if (!s->map_addr)
> > > +        return;
> > > +
> > > +    if (cirrus_lfb_is_mapped(s)) {
> > > +        cpu_register_physical_memory(isa_mem_base + 0xa0000, 0x8000,
> > > +                                    (s->vram_offset +
> > > s->cirrus_bank_base[0]) | IO_MEM_RAM);
> > > +        cpu_register_physical_memory(isa_mem_base + 0xa8000, 0x8000,
> > > +                                    (s->vram_offset +
> > > s->cirrus_bank_base[1]) | IO_MEM_RAM);
> > >
> > >       
> > Isn't necessary to reregister 0xa0000 too?
> >     
>
> ENOFOLLOW. This is exactly what I'm doing.
>   

I was confused about isa_mem_base.  Just ignore me :-).

Regards,

Anthony Liguori

> > > +        if (kvm_enabled()) {
> > > +            kvm_log_start(0xa0000, 0x8000);
> > > +            kvm_log_start(0xa8000, 0x8000);
> > > +        }
> > >
> > >       
> > Why would you enable logging on a different region from what you've
> > registered?  Shouldn't you enable logging on both regions?  If we're going
> > to enable logging based on target_phys_addr_t instead of ram_addr_t (and I
> > think we should), then we should enable it on all target_phys_addr_ts.
> >     
>
> Again, I don't follow. We map 0xa0000 and 0xa8000 to some ram_addr_t,
> and then enable logging in the very 0xa0000 and 0xa8000. What's the problem
> with that? One late nitpick, it is that for consistency, I registered
> 0xa0000 + isa_mem_base,
> (usually 0), and should use it in kvm_log_start.
>
>   
> > > +    }
> > > +    else {
> > >
> > >       
> > This is formatted incorrectly.
> >
> >     
> > >  +
> > >  /*
> > >  * graphic modes
> > >  */
> > >
> > >       
> > More extra whitespace.
> >
> > Regards,
> >
> > Anthony Liguori
> >
> >
> >
> >     
>
>
>
>