[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e17189c7-54e0-f0a1-2b16-36f486111c4c@redhat.com>
Date: Thu, 29 Sep 2016 17:31:09 -0700
From: Laura Abbott <labbott@...hat.com>
To: Mark Rutland <mark.rutland@....com>
Cc: AKASHI Takahiro <takahiro.akashi@...aro.org>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
David Brown <david.brown@...aro.org>, Will Deacon <will.deacon@....com>,
Catalin Marinas <catalin.marinas@....com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Kees Cook <keescook@...omium.org>, kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH 1/3] arm64: dump: Make ptdump debugfs a separate option
On 09/29/2016 05:13 PM, Mark Rutland wrote:
> Hi,
>
> On Thu, Sep 29, 2016 at 02:32:55PM -0700, Laura Abbott wrote:
>> ptdump_register currently initializes a set of page table information and
>> registers debugfs. There are uses for the ptdump option without wanting the
>> debugfs options. Split this out to make it a separate option.
>>
>> Signed-off-by: Laura Abbott <labbott@...hat.com>
>> ---
>> arch/arm64/Kconfig.debug | 6 +++++-
>> arch/arm64/include/asm/ptdump.h | 15 +++++++++++++--
>> arch/arm64/mm/Makefile | 3 ++-
>> arch/arm64/mm/dump.c | 30 +++++++++---------------------
>> arch/arm64/mm/ptdump_debugfs.c | 33 +++++++++++++++++++++++++++++++++
>> 5 files changed, 62 insertions(+), 25 deletions(-)
>> create mode 100644 arch/arm64/mm/ptdump_debugfs.c
>
> As a heads-up, Ard has new ARM64_PTUMP user under drivers/firmware/efi queued
> up in the EFI tree, which will also need fixing up. See commit d80448ac92b72051
> ("efi/arm64: Add debugfs node to dump UEFI runtime page tables") [1].
>
> [...]
>
I'll take a look at that, thanks for the pointer!
>> +#include <linux/seq_file.h>
>> #include <linux/mm_types.h>
>
> Nit: please keep headers in alphabetical order.
>
>> -static void walk_pgd(struct pg_state *st, struct mm_struct *mm,
>> +static void __walk_pgd(struct pg_state *st, struct mm_struct *mm,
>
> Can we leave this name as-is? We didn't change walk_{pud,pmd,pte}, so this is
> inconsistent, and we haven't reused the name.
>
Yes, I think this is a relic of earlier refactoring attempts.
> [...]
>
>> +int ptdump_register(struct ptdump_info *info, const char *name)
>> +{
>> + ptdump_initialize(info);
>> + return ptdump_debugfs_create(info, name);
>> }
>
> It feels like a layering violation to have the core ptdump code call the
> debugfs ptdump code. Is there some reason this has to live here?
>
Which 'this' are you referring to here? Are you suggesting moving
the ptdump_register elsewhere or moving the debugfs create elsewhere?
> Other than the above points, this looks good to me.
>
> Thanks,
> Mark.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=next&id=9d80448ac92b720512c415265597d349d8b5c3e8
>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
