[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap


  • To: Brian Woods <brian.woods@xxxxxxxxxx>
  • From: Julien Grall <Julien.Grall@xxxxxxx>
  • Date: Thu, 17 Oct 2019 20:23:21 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=zkqfuJIRFtOc/WuBj9XC36tyiRcssujp/v5blAyZ9ZI=; b=nbdb2A+ToNnT0m93dHHstlZOc5tERp3uiZbXEQYmrLQMhlmVcSHretpsjEpGwUr5SK2P5DiIJ2sGwxyrkDWtW8IMqceyNpwWBmK8x2JIDQAcmrkJTLkQAgaKRzGg6eaiJUlA1naC2NE8aHKUzQBRYM2F+0l2U2WW/iQeQuBvH7FhE/q/tvDxKBfoD89pnUKKgQBCX//eq+l+t8FM/I8XYFX0TMhRz0pQg8a8RBuFIjGVU3SgNMrcnI+fP4hLdfhCGm5E4eoOpFiTt6rTJ6o53nLB7YHgscSPjVNesh/aVkq1AYcRDt/c3zBpmP8yaf9cn12UvRQwuiqKf/Wb15RnCg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=C/s79G3TQ1m6+X2CIajiygDPLci/ma5V0HripDrKj+s35fCXgHHx4awHM0tZpByJxqiiyYIdt0cEye/qj+t3xv4BYhyx9dCxExjL6NFXsUMqNfs8siThOiFA8nKSW5isb3A6BOuaNKkDKbeMBv6N0mUfYzz6uL6rZvkeOti/D55r0B0gi1AZDVnJ+rNzi+RVxzl386mTGSuI7sJqeyroFUv6LHAPQ9EjrjRwvS8GXgKmSQwiNr1djrkzi0iWyKema5ekdSnHREf4wFjPDi16GraAxVvkbDJDzO6Sk3KXa3BP6+bZgTskiDcEAe3urZXpVkmlGn6bj7b/czsVS/ASOQ==
  • Authentication-results: spf=temperror (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; lists.xenproject.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;lists.xenproject.org; dmarc=none action=none header.from=arm.com;
  • Authentication-results-original: spf=none (sender IP is ) smtp.mailfrom=Julien.Grall@xxxxxxx;
  • Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, nd <nd@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • Delivery-date: Thu, 17 Oct 2019 20:23:51 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: True
  • Original-authentication-results: spf=none (sender IP is ) smtp.mailfrom=Julien.Grall@xxxxxxx;
  • Thread-index: AQHVftqpGyCF495J3kWN1K0obwmWaKdUFMKAgAGTigCAABT/gIAAAiKAgAAT6YD///1PgIAI2o+AgACe5YCAAAmfgA==
  • Thread-topic: [Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap

Hi,

On 17/10/2019 20:48, Brian Woods wrote:
> 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

Why do you need a multiline? A single 80-charaters should really be 
sufficient.

> 
> 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).
Please no. There are no need to add a bool just for the sake of getting 
all the print together.

The more that if you print all the information as I suggested above, you 
don't need to have it printed by early_print_info().

To be honest, I really don't think this is Xen job to check that you 
specify your modules correctly. There are other way to screw up your 
device-tree anyway (like overlap in memory banks or reserved region...).

The modules overlap can really only happen if you try to have your DT 
pre-generated and don't bother to use the bootloader (U-boot/Grub) 
script to generate your DT/modules.

> 
>> 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.

No. I meant that you could have a device-tree describing two modules 
starting at the same address, but with a different size.

See the check in add_boot_module() to see if a module already exist of 
the same kind.

Cheers,

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.