On Thu, Oct 17, 2019 at 10:20:11AM +0100, Julien Grall wrote:
> Hi,
>
> Sorry for the late answer.
>
> On 11/10/2019 20:07, Brian Woods wrote:
> >Which is why I wanted to put it where it was in the patch. Where the
> >user would see the warning after the information about the memory
> >modules were printed (and fair early).
>
> I had a think about it, dumping the modules informations before is useful if
> you know that you have one module max per kind. So you avoid to print the
> modules address/size in the warning.
>
> However, it is possible to have multiple kernel module (as long as they
> don't have the same start address), you could end up with the following
> message:
>
> "WARNING: modules Kernel and Kernel overlap"
>
> To make the message more meaningful, we would need to print the modules
> address/size. Therefore, I don't view that it is important to check
> overlapping in early_print_info(). In this case I would favor any code that
> don't add a double for loop.
Well, adding that information would be easy enough and cheap. It would
make it multiline prinktk though:
WARNING: memory modules over lap:
start_addr-end_addr: modulename
start_addr-end_addr: modulename
If we're not doing that though, would it make sense to have a initdata
bool that checks it in add_boot_module() and then prints a simple
warning that there's a memory module overlap in early_print_info()?
That way there's no nested for loop and it gets printed where all the
addresses get printed (so you can actually figure out where the overlap
is).
> While thinking about this case, it made me realize that we only check the
> start address to consider a match. This means if the size is different, then
> it will be ignored. I think we ought to throw at least warning for this case
> as well.
>
> Would you mind to have a look?
When you say starting address, do you mean like in the orginal patch?
If so, there's no functional change in checking the starts of n on m and
m on n then checking the start and end of n on m.
> >
> >Either way, take your pick of location and if it's only debug or not and
> >I can write it up and test it.
>
> I would still prefer in add_boot_module(). See why above.
I wrote I suggested above and tested it so that'll be sent out soon.
Brian
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
