mm,x86: fix span coverage in e820_all_mapped()

In the following case, e820_all_mapped() will return 1.
A < start < B-1 and B < end < C, it means <start, end> spans two regions.
<start, end>: [start - end]
e820 addr: ...[A - B-1][B - C]...

Signed-off-by Xishi Qiu <[email protected]>

---
 arch/x86/kernel/e820.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

arch/x86/kernel/e820.c +0 -0

index 174da5f..31ecab2 100644
@@ -85,20 +85,11 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)
 
                if (type && ei->type != type)
                        continue;

arch/x86/kernel/e820.c +2 -5

-               /*
-                * if start is now at or beyond end, we're done, full
-                * coverage
-                */
-               if (start >= end)
 
 
+               /* is the region full coverage of <start, end> ? */
+               if (ei->addr <= start && ei->addr + ei->size >= end)
                        return 1;

        }
        return 0;

Quoted 4 lines · Xishi Qiu

In the following case, e820_all_mapped() will return 1.
A < start < B-1 and B < end < C, it means <start, end> spans two regions.
<start, end>: [start - end]
e820 addr: ...[A - B-1][B - C]...

should be [start, end) right?
and
[A, B),[B, C)

Quoted 6 lines · Yinghai Lu

Quoted 4 lines · Xishi Qiu

In the following case, e820_all_mapped() will return 1.
A < start < B-1 and B < end < C, it means <start, end> spans two regions.
<start, end>: [start - end]
e820 addr: ...[A - B-1][B - C]...

Quoted 4 lines

should be [start, end) right?
and
[A, B),[B, C)

What happens if it spans more than two regions?

Quoted 2 lines

What happens if it spans more than two regions?

[A, B), [B+1, C), [C+1, D) ?
start in [A, B), and end in [C+1, D).

old code:
first with [A, B), start set to B. then with [B+1, C), start still keep as B. then with [C+1, D), start still keep as B. at last still return 0...aka not_all_mapped.

old code is still right.

Quoted 12 lines · Yinghai Lu

Quoted 2 lines

What happens if it spans more than two regions?

[A, B), [B+1, C), [C+1, D) ?
start in [A, B), and end in [C+1, D).

old code:
first with [A, B), start set to B. then with [B+1, C), start still keep as B. then with [C+1, D), start still keep as B. at last still return 0...aka not_all_mapped.

Quoted 2 lines

old code is still right.

Why not_all_mapped?

Quoted 13 lines · H. Peter Anvin

Quoted 12 lines · Yinghai Lu

Quoted 2 lines

What happens if it spans more than two regions?

[A, B), [B+1, C), [C+1, D) ?
start in [A, B), and end in [C+1, D).

old code:
first with [A, B), start set to B. then with [B+1, C), start still keep as B. then with [C+1, D), start still keep as B. at last still return 0...aka not_all_mapped.

Quoted 4 lines

Quoted 2 lines

old code is still right.

Why not_all_mapped?

[B, B+1), and [C, C+1) are not there.

Ok, the issue I thought we were discussing was actually [A,B) [B,C) [C,D) ...

Quoted 20 lines · Yinghai Lu

Quoted 13 lines · H. Peter Anvin

Quoted 12 lines · Yinghai Lu

Quoted 2 lines

What happens if it spans more than two regions?

[A, B), [B+1, C), [C+1, D) ?
start in [A, B), and end in [C+1, D).

old code:
first with [A, B), start set to B. then with [B+1, C), start still keep as B. then with [C+1, D), start still keep as B. at last still return 0...aka not_all_mapped.

Quoted 4 lines

Quoted 2 lines

old code is still right.

Why not_all_mapped?

[B, B+1), and [C, C+1) are not there.

Quoted 4 lines · Xishi Qiu

In the following case, e820_all_mapped() will return 1.
A < start < B-1 and B < end < C, it means <start, end> spans two regions.
<start, end>: [start - end]
e820 addr: ...[A - B-1][B - C]...

should be [start, end) right?
and
[A, B),[B, C)

Quoted 14 lines

Signed-off-by Xishi Qiu <[email protected]>

---
 arch/x86/kernel/e820.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

arch/x86/kernel/e820.c +0 -0

index 174da5f..31ecab2 100644
@@ -85,20 +85,11 @@ int __init e820_all_mapped(u64 start, u64 end, unsigned type)
 
                if (type && ei->type != type)
                        continue;

arch/x86/kernel/e820.c int __init e820_all_mapped(u64 start, u64 end, unsigned type) · line 96

Quoted 10 lines

arch/x86/kernel/e820.c +1 -7

-               /* is the region (part) in overlap with the current region ?*/
+               /* is the region (part) in overlap with the current region ? */
                if (ei->addr >= end || ei->addr + ei->size <= start)
                        continue;
-
-               /* if the region is at the beginning of <start,end> we move
-                * start to the end of the region since it's ok until there
-                */
-               if (ei->addr <= start)
-                       start = ei->addr + ei->size;

so in your case new start will be B ?

next run will be C

Quoted 5 lines

arch/x86/kernel/e820.c +0 -5

-               /*
-                * if start is now at or beyond end, we're done, full
-                * coverage
-                */
-               if (start >= end)

arch/x86/kernel/e820.c int __init e820_all_mapped(u64 start, u64 end, unsigned type) · line 95

Quoted 5 lines

arch/x86/kernel/e820.c +2 -0

+               /* is the region full coverage of <start, end> ? */
+               if (ei->addr <= start && ei->addr + ei->size >= end)
                        return 1;
        }
        return 0;

also e820 should be sanitized already to have [A,C).

or you are talking about [A,B), [B+1, C)
first run start will be B, and next run with [B+1, ...), that will be skipped...
will not return 1.

so old code should be ok.

Thanks

Yinghai

.

In this case, old code is right, but I discuss in another one that you wrote above.

Quoted 4 lines · Xishi Qiu

In this case, old code is right, but I discuss in another one that you wrote above.

So is there a problem or not? I have lost track...

-hpa

I think there is a problem.
e.g.
[start, end)=[8, 12), and [A, B)=[0, 10), [B, C)=[10,20), then e820_all_mapped() will return 1, it spans two regions.

Quoted 6 lines · Xishi Qiu

I think there is a problem.
e.g.
[start, end)=[8, 12), and [A, B)=[0, 10), [B, C)=[10,20), then e820_all_mapped() will return 1, it spans two regions.

Why is that a problem?

-hpa

Quoted 10 lines · H. Peter Anvin

Quoted 6 lines · Xishi Qiu

I think there is a problem.
e.g.
[start, end)=[8, 12), and [A, B)=[0, 10), [B, C)=[10,20), then e820_all_mapped() will return 1, it spans two regions.

Why is that a problem?

[start, end) should be included in one region ?

Thanks,
Xishi Qiu

Quoted 5 lines

-hpa