Messages in this thread Patch in this message | | | Date | Mon, 08 Feb 2016 10:24:58 -0800 | From | David Daney <> | 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@imgtec.com> >>> Cc: David Daney <david.daney@cavium.com> >>> Cc: Paul Burton <paul.burton@imgtec.com> >>> Cc: James Hogan <james.hogan@imgtec.com> >>> Cc: linux-kernel@vger.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
| |