Re: [Xen-devel] [PATCH v2 1/4] x86/e820: introduce a function to remove ranges from e820

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.12.18 at 13:04, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -599,6 +599,63 @@ int __init e820_add_range(
>      return 1;
>  }
>  
> +uint64_t __init e820_remove_range(struct e820map *e820, uint64_t start,
> +                                  uint64_t end, uint32_t type, bool 
> check_type)
> +{
> +    unsigned int i;
> +    uint64_t real_removed_size = 0;
> +
> +    ASSERT(end > start);
> +
> +    for ( i = 0; i < e820->nr_map; i++ )
> +    {
> +        struct e820entry *entry = &e820->map[i];
> +        uint64_t final_start, final_end, entry_end;
> +
> +        if ( check_type && entry->type != type )
> +            continue;
> +
> +        entry_end = entry->addr + entry->size;
> +
> +        /* Completely covered? */
> +        if ( entry->addr >= start && entry_end <= end )
> +        {
> +            real_removed_size += entry->size;
> +            memset(entry, 0, sizeof(*entry));

This will break assumptions by other functions, e.g. the
neighboring e820_add_range().

> +            continue;
> +        }
> +
> +        /* Is the new range completely covered? */
> +        if ( entry->addr < start && entry_end > end )

Why < and > instead of <= and >= ? At the very least this is
not in line with the comment.

> +        {
> +            e820_add_range(e820, end, entry_end, entry->type);
> +            entry->size = start - entry->addr;
> +            real_removed_size += end - start;
> +            continue;
> +        }
> +
> +        /* Partially covered: */
> +        final_start = max(start, entry->addr);
> +        final_end = min(end, entry_end);
> +        if ( final_start >= final_end )
> +            continue;

Isn't this supposed to be unreachable?

Jan



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