Re: [PATCH] cirrus: handle wraparound in cirrus_invalidate_region

[Li Qiang]

Gerd Hoffmann <kraxel@redhat.com> 于2020年8月31日周一 下午7:23写道：
>
> > >      for (y = 0; y < lines; y++) {
> > > -        off_cur = off_begin;
> > > +        off_cur = off_begin & s->cirrus_addr_mask;
> > >          off_cur_end = ((off_cur + bytesperline - 1) & 
> > > s->cirrus_addr_mask) + 1;
> > > -        assert(off_cur_end >= off_cur);
> > > -        memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - 
> > > off_cur);
> > > +        if (off_cur_end >= off_cur) {
> > > +            memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - 
> > > off_cur);
> > > +        } else {
> > > +            /* wraparound */
> > > +            memory_region_set_dirty(&s->vga.vram, off_cur, 
> > > s->cirrus_addr_mask - off_cur);
> >
> > Should here be 's->cirrus_addr_mask + 1 - off_cur'
>
> Yes (mask != size).


Say if we have a range 0~0x2000 then the mask is '0x1fff' and the
off_cur is 0x1000.

0.    0x1000.  0x2000
        off_cur

Then the wrap occurs.

In the first set.
We just sets 0x1fff-0x1000= 0xfff bytes.
In fact we need to set 0x1000 bytes.




>
> > > +            memory_region_set_dirty(&s->vga.vram, 0, off_cur_end);
> >
> > And here be 'off_cur_end -1'
>
> --verbose please.  I think this one is correct.

Here the 'off_cur_end' is size.
In this second set we actually sets 'off_cur_end+1' size bytes.

In a word, I think the first lost a byte and the second added a more byte .

Thank,s
Li Qiang

>
> take care,
>   Gerd
>