[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56B8DD7A.1010303@gmail.com>
Date: Mon, 08 Feb 2016 10:24:58 -0800
From: David Daney <ddaney.cavm@...il.com>
To: Harvey Hunt <harvey.hunt@...tec.com>
CC: David Daney <ddaney@...iumnetworks.com>, linux-mips@...ux-mips.org,
ralf@...ux-mips.org, David Daney <david.daney@...ium.com>,
Paul Burton <paul.burton@...tec.com>,
James Hogan <james.hogan@...tec.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] MIPS: Always page align TASK_SIZE
On 02/08/2016 10:15 AM, Harvey Hunt wrote:
> Hi David,
>
> On 02/08/2016 10:11 AM, David Daney wrote:
>> On 02/08/2016 10:05 AM, Harvey Hunt wrote:
>>> STACK_TOP_MAX is aligned on a 32k boundary. When __bprm_mm_init()
>>> creates an
>>> initial stack for a process, it does so using STACK_TOP_MAX as the end
>>> of the
>>> vma. A process's arguments and environment information are placed on
>>> the stack
>>> and then the stack is relocated and aligned on a page boundary. When
>>> using a 32
>>> bit kernel with 64k pages, the relocated stack has the process's args
>>> erroneously stored in the middle of the stack. This means that processes
>>> receive no arguments or environment variables, preventing them from
>>> running
>>> correctly.
>>>
>>> Fix this by aligning TASK_SIZE on a page boundary.
>>>
>>> Signed-off-by: Harvey Hunt <harvey.hunt@...tec.com>
>>> Cc: David Daney <david.daney@...ium.com>
>>> Cc: Paul Burton <paul.burton@...tec.com>
>>> Cc: James Hogan <james.hogan@...tec.com>
>>> Cc: linux-kernel@...r.kernel.org
>>> ---
>>> arch/mips/include/asm/processor.h | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/mips/include/asm/processor.h
>>> b/arch/mips/include/asm/processor.h
>>> index 3f832c3..b618b40 100644
>>> --- a/arch/mips/include/asm/processor.h
>>> +++ b/arch/mips/include/asm/processor.h
>>> @@ -39,13 +39,13 @@ extern unsigned int vced_count, vcei_count;
>>> #ifdef CONFIG_32BIT
>>> #ifdef CONFIG_KVM_GUEST
>>> /* User space process size is limited to 1GB in KVM Guest Mode */
>>> -#define TASK_SIZE 0x3fff8000UL
>>> +#define TASK_SIZE (0x40000000UL - PAGE_SIZE)
>>> #else
>>> /*
>>> * User space process size: 2GB. This is hardcoded into a few places,
>>> * so don't change it unless you know what you are doing.
>>> */
>>> -#define TASK_SIZE 0x7fff8000UL
>>> +#define TASK_SIZE (0x7fff8000UL & PAGE_SIZE)
>>
>> Can you check your math here. This doesn't seem correct.
>
> Thanks for spotting that - it should have been:
>
> (0x7fff8000UL & PAGE_MASK)
This brings up an interesting point. How was this tested? Please note
that in the change log.
Also look at the definition of PAGE_MASK in page.h
Is that correct? Most of the other related symbols have an "_AC(1,UL)"
in them. Why is this not also appropriate for PAGE_MASK?
It may also be a good idea to prepare and test a patch that defines
PAGE_MASK much in the same way HPAGE_MASK is defined.
David Daney
>
> I'll do a v2 now.
>
>>
>>> #endif
>>>
>>> #define STACK_TOP_MAX TASK_SIZE
>>> @@ -62,7 +62,7 @@ extern unsigned int vced_count, vcei_count;
>>> * support 16TB; the architectural reserve for future expansion is
>>> * 8192EB ...
>>> */
>>> -#define TASK_SIZE32 0x7fff8000UL
>>> +#define TASK_SIZE32 (0x7fff8000UL & PAGE_SIZE)
>>
>> Same here.
>
> As above.
>
>>
>>> #define TASK_SIZE64 0x10000000000UL
>>> #define TASK_SIZE (test_thread_flag(TIF_32BIT_ADDR) ? TASK_SIZE32 :
>>> TASK_SIZE64)
>>> #define STACK_TOP_MAX TASK_SIZE64
>>>
>>
>
> Thanks,
>
> Harvey
Powered by blists - more mailing lists