[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] cirrus: handle wraparound in cirrus_invalidate_region
From: |
Li Qiang |
Subject: |
Re: [PATCH] cirrus: handle wraparound in cirrus_invalidate_region |
Date: |
Tue, 1 Sep 2020 12:55:26 +0800 |
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
>
- Re: [PATCH] cirrus: handle wraparound in cirrus_invalidate_region,
Li Qiang <=