xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: Peng Fan <van.freenix@gmail.com>
Cc: Peng Fan <peng.fan@nxp.com>,
	sstabellini@kernel.org, xen-devel@lists.xen.org
Subject: Re: [PATCH V1] xen/arm: domain_build: introduce dom0_lowmem bootargs
Date: Wed, 14 Sep 2016 13:34:10 +0100	[thread overview]
Message-ID: <3fa945fc-fa71-f16a-022f-e46becc8b964@arm.com> (raw)
In-Reply-To: <20160914121807.GB29758@linux-7smt.suse>



On 14/09/16 13:18, Peng Fan wrote:
> Hello Julien,
>
> On Wed, Sep 14, 2016 at 01:06:01PM +0100, Julien Grall wrote:
>>
>>
>> On 14/09/16 13:03, Peng Fan wrote:
>>> Hello Julien,
>>
>> Hello Peng,
>>
>>> On Wed, Sep 14, 2016 at 11:47:10AM +0100, Julien Grall wrote:
>>>> Hello,
>>>>
>>>> On 14/09/16 08:41, Peng Fan wrote:
>>>>> On Wed, Sep 14, 2016 at 08:23:24AM +0100, Julien Grall wrote:
>>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>>> index 35ab08d..cc71e6f 100644
>>>>> --- a/xen/arch/arm/domain_build.c
>>>>> +++ b/xen/arch/arm/domain_build.c
>>>>> @@ -28,6 +28,8 @@
>>>>>
>>>>> static unsigned int __initdata opt_dom0_max_vcpus;
>>>>> integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
>>>>> +static bool_t __initdata opt_dom0_use_lowmem;
>>>>> +boolean_param("dom0_use_lowmem", opt_dom0_use_lowmem);
>>>>>
>>>>> int dom0_11_mapping = 1;
>>>>>
>>>>> @@ -244,7 +246,7 @@ static void allocate_memory(struct domain *d, struct
>>>>> kernel_info *kinfo)
>>>>>    unsigned int order = get_11_allocation_size(kinfo->unassigned_mem);
>>>>>    int i;
>>>>> 	
>>>>> -    bool_t lowmem = is_32bit_domain(d);
>>>>> +    bool_t lowmem = is_32bit_domain(d) || opt_dom0_use_lowmem;
>>>>>    unsigned int bits;
>>>>>
>>>>>
>>>>> Pass "dom0_use_lowmem=1" to xen to allocate lowmem as much as possible.
>>>>
>>>> Again, what is the benefit to have a command line option for that?
>>>
>>> Then you prefer directly change "bool_t lowmem = is_32bit_domain(d);" to "bool_t lowmem = true" ?
>>> I just want to give user a choice.
>>
>> We don't add new command line parameter just because they look cool to have.
>> So far, you did not explain why it would be good to let the choice to the
>> user and how it could be used.
>
> I have not try, if there is no lowmem.
>
> I have not look into alloc_domheap_pages.
> I am not sure whether there is such a platform or not,
> just thinking if there is soc that dram memory starts from 4GB, and there is no dram
> below 4GB. If we still can get memory when lowmem is true, I am ok to change directly assign
> lowmem with value true. Anyway I have not look into the internals of domheap and
> not sure whether there is such a platform that no lowmem (:-

We cannot exclude this possibility. However, the only reason that Xen is 
requiring to allocate a bank below 4GB for 32-bit domain is to handle 
non-LPAE kernel.

I personally don't think this is a hard requirement and instead of 
panicking we should print a warning to say "no bank has been allocated 
below 4GB" or "a bank of N MB has been allocated below 4GB".

So my suggestion is to allocate as much as possible lowmem (i.e below 
4GB) and if it does not work print a warning then allocate the first 
bank above 4GB.

I much prefer to let Xen decide itself what to do when possible rather 
than asking the user to specify himself whether lowmem is required.

This is making life easier for distribution to support multiple 
platforms with Xen without having to modify the command line.

Regards,

-- 
Julien Grall

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

  reply	other threads:[~2016-09-14 12:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-14  5:12 [PATCH V1] xen/arm: domain_build: introduce dom0_lowmem bootargs Peng Fan
2016-09-14  7:23 ` Julien Grall
2016-09-14  7:41   ` Peng Fan
2016-09-14 10:47     ` Julien Grall
2016-09-14 12:03       ` Peng Fan
2016-09-14 12:06         ` Julien Grall
2016-09-14 12:18           ` Peng Fan
2016-09-14 12:34             ` Julien Grall [this message]
2016-09-14 12:40               ` Peng Fan
2016-09-14 14:16                 ` Edgar E. Iglesias
2016-09-15  0:20                   ` Peng Fan
2016-09-15  8:26                     ` Edgar E. Iglesias
2016-09-15  8:50                       ` Julien Grall
2016-09-15 11:12                       ` Peng Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3fa945fc-fa71-f16a-022f-e46becc8b964@arm.com \
    --to=julien.grall@arm.com \
    --cc=peng.fan@nxp.com \
    --cc=sstabellini@kernel.org \
    --cc=van.freenix@gmail.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).