On 01/29/2012 03:39 PM, Blue Swirl wrote:
> >> >> +++ b/exec-obsolete.h
> >> >> @@ -81,11 +81,10 @@ static inline void
> >> >> cpu_physical_memory_set_dirty_range(ram_addr_t start,
> >> >> int dirty_flags)
> >> >> {
> >> >> uint8_t *p;
> >> >> - ram_addr_t addr, end;
> >> >> + ram_addr_t cur;
> >> >>
> >> >> - end = start + length;
> >> >> p = ram_list.phys_dirty + (start >> TARGET_PAGE_BITS);
> >> >> - for (addr = start; addr <= end; addr += TARGET_PAGE_SIZE) {
> >> >> + for (cur = 0; cur < length; cur += TARGET_PAGE_SIZE) {
> >> >> *p++ |= dirty_flags;
> >> >> }
> >> >
> >> > I think this is still wrong - if length == 2 it will iterate once, but
> >> > we need two iterations if start == 0xfff.
> >>
> >> Yes, tricky. We could do something like
> >> for (cur = start & TARGET_PAGE_MASK; cur < length; cur +=
> >> TARGET_PAGE_SIZE) {
> >> but I'll send a new patch with just s/<=/</.
> >
> > That's broken too.
>
> Because length should be adjusted by -1?
0xfff/2 breaks.
More generally, you can't have a loop that just looks at length, since
0/2 wants one iteration, and 0xfff/2 wants two.
>
> > I have:
> >
> > uint8_t *p;
> > ram_addr_t addr, end;
> >
> > - end = start + length;
> > + end = (start + length - 1) | (TARGET_PAGE_SIZE - 1);
>
> Why | (TARGET_PAGE_SIZE - 1), for length == 1?
Yes. More generally, I wanted something that is easily understood -
start/end addresses without trickery, given all the broken patches for
fixing this.
> TARGET_PAGE_ALIGN()
> could be useful here.
True, I'll respin.
--
error compiling committee.c: too many arguments to function
