Re: [Qemu-devel] vga optmization

[Blue Swirl]

On 11/3/08, Glauber Costa <address@hidden> wrote:
> On Mon, Nov 3, 2008 at 4:03 PM, Blue Swirl <address@hidden> wrote:
>  > On 11/3/08, Glauber Costa <address@hidden> wrote:
>  >> Hi guys,
>  >>
>  >>  this is a port of current kvm vga memory optimization to our new
>  >>  infrastructure proposed by anthony. It's goal is to use as few
>  >>  kvm specific hooks as possible. In fact, the only one I'm relying
>  >>  on is enabling/disabling of logging. The rest, is pretty much general.
>  >>
>  >>  We map the linear frame buffer area as RAM, and then use dirty tracking
>  >>  to decide whether or not to update it. To be consistent with qemu,
>  >>  this version, differently from upstream kvm, tracks memory based on its
>  >>  physical address, represented by vram_offset, instead of vram_ptr, or
>  >>  any other construct.
>  >>
>  >>  Let me know what you think
>  >
>  > The patch does not apply, because of the kvm files. What patches do I
>  > need to apply first? I'd like to see how does the optimization apply
>  > to TCX.
>
>
> Yes, you are missing kvm patches some time ago, which he plans to merge.
>
>
>  > This also may mean that some of my comments below can be invalid.
>  >
>  >>  +void set_vram_mapping(target_phys_addr_t begin, target_phys_addr_t end, 
> ram_addr_t target)
>  >
>  > No "static"?
>
> thanks, will update.
>
>
>  >
>  >>  +{
>  >>  +    /* align begin and end address */
>  >>  +    begin = begin & TARGET_PAGE_MASK;
>  >>  +    end = begin + VGA_RAM_SIZE;
>  >>  +    end = (end + TARGET_PAGE_SIZE -1 ) & TARGET_PAGE_MASK;
>  >
>  > This will fail if "end" is at the last page of the memory.
>
>
> How so? I don't think it's possible for the addr 0xsomething0001 to be
>  valid, but 0x(something+1)000 to be not.

0xfffff001 + 0xfff = 0 (wrapped), then masking still gives 0.

>  >
>  >>  +    uint32_t vga_io_memory;                                             
> \
>  >
>  > cpu_register_io_memory() returns an "int".
>
>
> can it possibly be negative?
>  that said, of course I can change it for consistency, but I'd like to
>  understand it more

In case of error it could be negative but I'm only thinking about consistency.