Message ID | 20130816155430.16354.31104.stgit@bling.home |
---|---|
State | New |
Headers | show |
On 08/16/13 18:00, Alex Williamson wrote: > Since commit 23326164 we align access sizes to match the alignment of > the address, but we don't align the access size itself. This means we > let illegal access sizes (ex. 3) slip through if the address is > sufficiently aligned (ex. 4). This results in an abort which would be > easy for a guest to trigger. Account for aligning the access size. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > Cc: qemu-stable@nongnu.org > --- > > v3: Highest power of 2, not lowest > v2: Remove unnecessary loop condition > > exec.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/exec.c b/exec.c > index 3ca9381..8c90cef 100644 > --- a/exec.c > +++ b/exec.c > @@ -1924,6 +1924,13 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > } > } > > + /* Size must be a power of 2 */ > + if (l & (l - 1)) { > + while (!(l & access_size_max) && l & (access_size_max - 1)) { > + access_size_max >>= 1; > + } > + } > + > /* Don't attempt accesses larger than the maximum. */ > if (l > access_size_max) { > l = access_size_max; > Apologies, but I'm now totally confused. Suppose that the new code is reached with (access_size_max == 4). Now, l==9 and l==3 will enter the loop just the same, both shifting "access_size_max" right at least once, even though 9 is greater than 4, and 3 is less than 4. Is that OK? What's the goal here? Sorry for being dense... Thanks, Laszlo
On Fri, 2013-08-16 at 23:00 +0200, Laszlo Ersek wrote: > On 08/16/13 18:00, Alex Williamson wrote: > > Since commit 23326164 we align access sizes to match the alignment of > > the address, but we don't align the access size itself. This means we > > let illegal access sizes (ex. 3) slip through if the address is > > sufficiently aligned (ex. 4). This results in an abort which would be > > easy for a guest to trigger. Account for aligning the access size. > > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > Cc: qemu-stable@nongnu.org > > --- > > > > v3: Highest power of 2, not lowest > > v2: Remove unnecessary loop condition > > > > exec.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/exec.c b/exec.c > > index 3ca9381..8c90cef 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -1924,6 +1924,13 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) > > } > > } > > > > + /* Size must be a power of 2 */ > > + if (l & (l - 1)) { > > + while (!(l & access_size_max) && l & (access_size_max - 1)) { > > + access_size_max >>= 1; > > + } > > + } > > + > > /* Don't attempt accesses larger than the maximum. */ > > if (l > access_size_max) { > > l = access_size_max; > > > > Apologies, but I'm now totally confused. > > Suppose that the new code is reached with (access_size_max == 4). > > Now, l==9 and l==3 will enter the loop just the same, both shifting > "access_size_max" right at least once, even though 9 is greater than 4, > and 3 is less than 4. *sigh*, just as I was getting ready to point out that the above is faster than pow2floor, you have to go and point out that the result is wrong ;) I don't think clz or pow2floor is the answer though, the problem size is too small. Rather than trying to solve this with an algorithm, I think we just need a simple: if (size >= 8) size = 8; else if (size >=4) size = 4; ... We only have 4 cases to deal with and it comes out a couple times faster than pow2floor. v4... Thanks, Alex
diff --git a/exec.c b/exec.c index 3ca9381..8c90cef 100644 --- a/exec.c +++ b/exec.c @@ -1924,6 +1924,13 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) } } + /* Size must be a power of 2 */ + if (l & (l - 1)) { + while (!(l & access_size_max) && l & (access_size_max - 1)) { + access_size_max >>= 1; + } + } + /* Don't attempt accesses larger than the maximum. */ if (l > access_size_max) { l = access_size_max;
Since commit 23326164 we align access sizes to match the alignment of the address, but we don't align the access size itself. This means we let illegal access sizes (ex. 3) slip through if the address is sufficiently aligned (ex. 4). This results in an abort which would be easy for a guest to trigger. Account for aligning the access size. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Cc: qemu-stable@nongnu.org --- v3: Highest power of 2, not lowest v2: Remove unnecessary loop condition exec.c | 7 +++++++ 1 file changed, 7 insertions(+)