| Message ID | 1375100921-12990-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 29 July 2013 13:28, Paolo Bonzini <pbonzini@redhat.com> wrote: > The problem is introduced by commit 2332616 (exec: Support 64-bit > operations in address_space_rw, 2013-07-08). Before that commit, > memory_access_size would only return 1/2/4. > > Since alignment is already handled above, reduce l to the largest > power of two that is smaller than l. > > Reported-by: Oleksii Shevchuk <alxchk@gmail.com> > Tested-by: Oleksii Shevchuk <alxchk@gmail.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > exec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/exec.c b/exec.c > index c4f2894..e122c81 100644 > --- a/exec.c > +++ b/exec.c > @@ -1925,6 +1925,9 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > if (l > access_size_max) { > l = access_size_max; > } > + if (l & (l - 1)) { > + l = 1 << (qemu_fls(l) - 1); > + } Is this a hot enough code path that we care about going via the not-inline qemu_fls() rather than calling clz32() directly? (probably not, I guess.) Alternatively, we seem to have a pow2floor() function... -- PMM
Il 29/07/2013 14:41, Peter Maydell ha scritto: > On 29 July 2013 13:28, Paolo Bonzini <pbonzini@redhat.com> wrote: >> The problem is introduced by commit 2332616 (exec: Support 64-bit >> operations in address_space_rw, 2013-07-08). Before that commit, >> memory_access_size would only return 1/2/4. >> >> Since alignment is already handled above, reduce l to the largest >> power of two that is smaller than l. >> >> Reported-by: Oleksii Shevchuk <alxchk@gmail.com> >> Tested-by: Oleksii Shevchuk <alxchk@gmail.com> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> exec.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/exec.c b/exec.c >> index c4f2894..e122c81 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1925,6 +1925,9 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) >> if (l > access_size_max) { >> l = access_size_max; >> } >> + if (l & (l - 1)) { >> + l = 1 << (qemu_fls(l) - 1); >> + } > > Is this a hot enough code path that we care about going via > the not-inline qemu_fls() rather than calling clz32() directly? It is not that hot because of the "if". > (probably not, I guess.) Alternatively, we seem to have a > pow2floor() function... Yeah, pow2floor is also nice. There's still a lot of opportunity for unification... Paolo
Il 29/07/2013 14:28, Paolo Bonzini ha scritto: > The problem is introduced by commit 2332616 (exec: Support 64-bit > operations in address_space_rw, 2013-07-08). Before that commit, > memory_access_size would only return 1/2/4. > > Since alignment is already handled above, reduce l to the largest > power of two that is smaller than l. > > Reported-by: Oleksii Shevchuk <alxchk@gmail.com> > Tested-by: Oleksii Shevchuk <alxchk@gmail.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > exec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/exec.c b/exec.c > index c4f2894..e122c81 100644 > --- a/exec.c > +++ b/exec.c > @@ -1925,6 +1925,9 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > if (l > access_size_max) { > l = access_size_max; > } > + if (l & (l - 1)) { > + l = 1 << (qemu_fls(l) - 1); > + } > > return l; > } > Ping, and adding qemu-stable. Paolo
diff --git a/exec.c b/exec.c index c4f2894..e122c81 100644 --- a/exec.c +++ b/exec.c @@ -1925,6 +1925,9 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) if (l > access_size_max) { l = access_size_max; } + if (l & (l - 1)) { + l = 1 << (qemu_fls(l) - 1); + } return l; }