[Xen-devel] [PATCH] x86/MCFG: fix off-by-one in E820 check

[Xen-devel] [PATCH] x86/MCFG: fix off-by-one in E820 check

[Jan Beulich]

Also adjust the comment ahead of e820_all_mapped() to clarify that the
range is not inclusive at its end.

Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -38,7 +38,7 @@ struct e820map e820;
 struct e820map __initdata e820_raw;
 
 /*
- * This function checks if the entire range <start,end> is mapped with type.
+ * This function checks if the entire range [start,end) is mapped with type.
  *
  * Note: this function only works correct if the e820 table is sorted and
  * not-overlapping, which is the case
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -337,7 +337,7 @@ static int __init is_mmconf_reserved(
     u64 old_size = size;
     int valid = 0;
 
-    while (!e820_all_mapped(addr, addr + size - 1, E820_RESERVED)) {
+    while (!e820_all_mapped(addr, addr + size, E820_RESERVED)) {
         size >>= 1;
         if (size < (16UL<<20))
             break;

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

Re: [Xen-devel] [PATCH] x86/MCFG: fix off-by-one in E820 check

[Wei Liu]

On Fri, Feb 07, 2020 at 02:25:17PM +0100, Jan Beulich wrote:
> Also adjust the comment ahead of e820_all_mapped() to clarify that the
> range is not inclusive at its end.
> 
> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wl@xen.org>

> 
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -38,7 +38,7 @@ struct e820map e820;
>  struct e820map __initdata e820_raw;
>  
>  /*
> - * This function checks if the entire range <start,end> is mapped with type.
> + * This function checks if the entire range [start,end) is mapped with type.

There is another instance of <> inside e820_all_mapped. It would be good
if that's fixed too.

>   *
>   * Note: this function only works correct if the e820 table is sorted and
>   * not-overlapping, which is the case
> --- a/xen/arch/x86/x86_64/mmconfig-shared.c
> +++ b/xen/arch/x86/x86_64/mmconfig-shared.c
> @@ -337,7 +337,7 @@ static int __init is_mmconf_reserved(
>      u64 old_size = size;
>      int valid = 0;
>  
> -    while (!e820_all_mapped(addr, addr + size - 1, E820_RESERVED)) {
> +    while (!e820_all_mapped(addr, addr + size, E820_RESERVED)) {
>          size >>= 1;
>          if (size < (16UL<<20))
>              break;

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

Re: [Xen-devel] [PATCH] x86/MCFG: fix off-by-one in E820 check

[Jan Beulich]

On 07.02.2020 15:25, Wei Liu wrote:
> On Fri, Feb 07, 2020 at 02:25:17PM +0100, Jan Beulich wrote:
>> Also adjust the comment ahead of e820_all_mapped() to clarify that the
>> range is not inclusive at its end.
>>
>> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Wei Liu <wl@xen.org>

Thanks.

>> --- a/xen/arch/x86/e820.c
>> +++ b/xen/arch/x86/e820.c
>> @@ -38,7 +38,7 @@ struct e820map e820;
>>  struct e820map __initdata e820_raw;
>>  
>>  /*
>> - * This function checks if the entire range <start,end> is mapped with type.
>> + * This function checks if the entire range [start,end) is mapped with type.
> 
> There is another instance of <> inside e820_all_mapped. It would be good
> if that's fixed too.

That one isn't misleading (talking about just the start of the
region), so I left it alone. But since you've asked for it -
fixed.

Jan

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

Re: [Xen-devel] [PATCH] x86/MCFG: fix off-by-one in E820 check

[Roger Pau Monné]

On Fri, Feb 07, 2020 at 02:25:17PM +0100, Jan Beulich wrote:
> Also adjust the comment ahead of e820_all_mapped() to clarify that the
> range is not inclusive at its end.
> 
> Reported-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -38,7 +38,7 @@ struct e820map e820;
>  struct e820map __initdata e820_raw;
>  
>  /*
> - * This function checks if the entire range <start,end> is mapped with type.
> + * This function checks if the entire range [start,end) is mapped with type.
>   *
>   * Note: this function only works correct if the e820 table is sorted and

Wouldn't mind if you took the opportunity to also s/correct/correctly/
in the sentence above.

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

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