Re: [PATCH] cirrus: handle wraparound in cirrus_invalidate_region

[Li Qiang]

Gerd Hoffmann <kraxel@redhat.com> 于2020年9月1日周二 下午1:15写道：
>
>   Hi,
>
> > > > >          off_cur_end = ((off_cur + bytesperline - 1) & 
> > > > > s->cirrus_addr_mask) + 1;
> [ ... ]
> > > > > +            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.
>
> Exactly.  And memory_region_set_dirty wants the size.  So everything is
> fine, right?



+        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);

The s->cirrus_addr_mask can be reached. I mean we can do following:
s->vga.vram[s->cirrus_addr_mask].

If I understand correctly, the 'off_cur' and 's->cirrus_addr_mask' is both index
[off_cur, s->cirrus_addr_mask].

So the len is 's->cirrus_addr_mask->off_cur+1'.


+            memory_region_set_dirty(&s->vga.vram, 0, off_cur_end);


For the 'off_cur_end' here, why we add 1 at the first?:

"off_cur_end = ((off_cur + bytesperline - 1) & s->cirrus_addr_mask) + 1;"
This addition '1' is what I think should be substracted in wrapped cases.

Thanks,
Li Qiang

+        }
         off_begin += off_pitch;
     }
 }



>
> take care,
>   Gerd
>