pfn_pdx_hole_setup is meant to skip the first MAX_ORDER bits, but
actually it only skips the first MAX_ORDER-1 bits. The issue was
probably introduced by bdb5439c3f, when changing to loop to start from
MAX_ORDER-1 an adjustment by 1 was needed in the call to find_next_bit()
but not done.

Fix the issue by passing j+1 and i+1 to find_next_zero_bit and
find_next_bit. Also add a check for i >= BITS_PER_LONG because
find_{,next_}zero_bit() on x86 assume their last argument to be less
than their middle one.

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Signed-off-by: Jan Beulich <JBeulich@suse.com>
CC: andrew.cooper3@citrix.com
CC: JBeulich@suse.com
CC: George.Dunlap@eu.citrix.com
CC: ian.jackson@eu.citrix.com
CC: konrad.wilk@oracle.com
CC: tim@xen.org
CC: wei.liu2@citrix.com

---
 xen/common/pdx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/pdx.c b/xen/common/pdx.c
index 50c21b6..bb7e437 100644
--- a/xen/common/pdx.c
+++ b/xen/common/pdx.c
@@ -83,8 +83,10 @@ void __init pfn_pdx_hole_setup(unsigned long mask)
      */
     for ( j = MAX_ORDER-1; ; )
     {
-        i = find_next_zero_bit(&mask, BITS_PER_LONG, j);
-        j = find_next_bit(&mask, BITS_PER_LONG, i);
+        i = find_next_zero_bit(&mask, BITS_PER_LONG, j + 1);
+        if ( i >= BITS_PER_LONG )
+            break;
+        j = find_next_bit(&mask, BITS_PER_LONG, i + 1);
         if ( j >= BITS_PER_LONG )
             break;
         if ( j - i > hole_shift )
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

>>> On 03.05.19 at 22:50, <sstabellini@kernel.org> wrote:
> Fix the issue by passing j+1 and i+1 to find_next_zero_bit and
> find_next_bit. Also add a check for i >= BITS_PER_LONG because
> find_{,next_}zero_bit() on x86 assume their last argument to be less
> than their middle one.

I had pointed out x86 since I knew it has this assumption. Now
that you mention it here, I would have expected you've checked
that Arm doesn't make similar assumptions. 32-bit Arm looks to
do, though (while 64-bit has a dedicated if() to deal with the
situation).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Mon, 6 May 2019, Jan Beulich wrote:
> >>> On 03.05.19 at 22:50, <sstabellini@kernel.org> wrote:
> > Fix the issue by passing j+1 and i+1 to find_next_zero_bit and
> > find_next_bit. Also add a check for i >= BITS_PER_LONG because
> > find_{,next_}zero_bit() on x86 assume their last argument to be less
> > than their middle one.
> 
> I had pointed out x86 since I knew it has this assumption. Now
> that you mention it here, I would have expected you've checked
> that Arm doesn't make similar assumptions. 32-bit Arm looks to
> do, though (while 64-bit has a dedicated if() to deal with the
> situation).

I think that either way we want to say that those functions are not
supposed to be called that way. I'll update the commit message.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel