All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: David Hildenbrand <david@redhat.com>
Cc: Patrick Daly <pdaly@codeaurora.org>,
	linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH] arm64: mte: Use PAGE_KERNEL_TAGGED in arch_add_memory
Date: Fri, 5 Mar 2021 17:44:13 +0000	[thread overview]
Message-ID: <20210305174413.GF23855@arm.com> (raw)
In-Reply-To: <3dae4529-6c73-d4c0-3fa6-6c2bda6518c9@redhat.com>

On Fri, Mar 05, 2021 at 04:55:28PM +0100, David Hildenbrand wrote:
> On 05.03.21 16:43, Catalin Marinas wrote:
> > On Fri, Mar 05, 2021 at 11:19:08AM +0000, Catalin Marinas wrote:
> > > On Tue, Mar 02, 2021 at 08:21:03PM -0800, Patrick Daly wrote:
> > > > In a system which supports MTE, the linear kernel region must allow
> > > > reading/writing allocation tags. For memory present at boot this
> > > > is already being done in map_mem(). Add the same in arch_add_memory().
> > > > 
> > > > Signed-off-by: Patrick Daly <pdaly@codeaurora.org>
> > > > ---
> > > >   arch/arm64/mm/mmu.c | 12 +++++++++++-
> > > >   1 file changed, 11 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > > > index 9b25d60b..0fcfe90 100644
> > > > --- a/arch/arm64/mm/mmu.c
> > > > +++ b/arch/arm64/mm/mmu.c
> > > > @@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> > > >   		    struct mhp_params *params)
> > > >   {
> > > >   	int ret, flags = 0;
> > > > +	pgprot_t pgprot;
> > > >   	if (!inside_linear_region(start, size)) {
> > > >   		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);
> > > > @@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
> > > >   	    IS_ENABLED(CONFIG_KFENCE))
> > > >   		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
> > > > +	/*
> > > > +	 * The linear map must allow allocation tags reading/writing
> > > > +	 * if MTE is present. Otherwise, it has the same attributes as
> > > > +	 * PAGE_KERNEL.
> > > > +	 */
> > > > +	pgprot = params->pgprot;
> > > > +	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
> > > > +		pgprot = PAGE_KERNEL_TAGGED;
> > > > +
> > > >   	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
> > > > -			     size, params->pgprot, __pgd_pgtable_alloc,
> > > > +			     size, pgprot, __pgd_pgtable_alloc,
> > > >   			     flags);
[...]
> > ---------------8<-------------------------------
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > index e17b96d0e4b5..5c78b92d9ec5 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
> >   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
> >   #define pgprot_device(prot) \
> >   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
> > +#define pgprot_tagged(prot) \
> > +	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
> >   /*
> >    * DMA allocations for non-coherent devices use what the Arm architecture calls
> >    * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
> > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> > index cdfc4e9f253e..f5f5044db2ce 100644
> > --- a/include/linux/pgtable.h
> > +++ b/include/linux/pgtable.h
> > @@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
> >   #define pgprot_device pgprot_noncached
> >   #endif
> > 
> > +#ifndef pgprot_tagged
> > +#define pgprot_tagged(prot)	(prot)
> > +#endif
> > +
> >   #ifdef CONFIG_MMU
> >   #ifndef pgprot_modify
> >   #define pgprot_modify pgprot_modify
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 5ba51a8bdaeb..4253d80a59ba 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
> >    */
> >   int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> >   {
> > -	struct mhp_params params = { .pgprot = PAGE_KERNEL };
> > +	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };
> 
> This looks like we're pushing arch specific stuff ("tagged") in here. Can't
> we generalize this to something like
> 
> pgprot_mhp_default
> 
> (or a better name)
> 
> that defaults to PAGE_KERNEL on all architectures except arm64 which
> overwrites this somehow?

It works for me but I prefer the prot modification style similar to
pgprot_writecombine() etc. (i.e. takes a parameter like PAGE_KERNEL and
just changes bits of it).

Thanks.

-- 
Catalin


WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: David Hildenbrand <david@redhat.com>
Cc: Patrick Daly <pdaly@codeaurora.org>,
	linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH] arm64: mte: Use PAGE_KERNEL_TAGGED in arch_add_memory
Date: Fri, 5 Mar 2021 17:44:13 +0000	[thread overview]
Message-ID: <20210305174413.GF23855@arm.com> (raw)
In-Reply-To: <3dae4529-6c73-d4c0-3fa6-6c2bda6518c9@redhat.com>

On Fri, Mar 05, 2021 at 04:55:28PM +0100, David Hildenbrand wrote:
> On 05.03.21 16:43, Catalin Marinas wrote:
> > On Fri, Mar 05, 2021 at 11:19:08AM +0000, Catalin Marinas wrote:
> > > On Tue, Mar 02, 2021 at 08:21:03PM -0800, Patrick Daly wrote:
> > > > In a system which supports MTE, the linear kernel region must allow
> > > > reading/writing allocation tags. For memory present at boot this
> > > > is already being done in map_mem(). Add the same in arch_add_memory().
> > > > 
> > > > Signed-off-by: Patrick Daly <pdaly@codeaurora.org>
> > > > ---
> > > >   arch/arm64/mm/mmu.c | 12 +++++++++++-
> > > >   1 file changed, 11 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > > > index 9b25d60b..0fcfe90 100644
> > > > --- a/arch/arm64/mm/mmu.c
> > > > +++ b/arch/arm64/mm/mmu.c
> > > > @@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> > > >   		    struct mhp_params *params)
> > > >   {
> > > >   	int ret, flags = 0;
> > > > +	pgprot_t pgprot;
> > > >   	if (!inside_linear_region(start, size)) {
> > > >   		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);
> > > > @@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
> > > >   	    IS_ENABLED(CONFIG_KFENCE))
> > > >   		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
> > > > +	/*
> > > > +	 * The linear map must allow allocation tags reading/writing
> > > > +	 * if MTE is present. Otherwise, it has the same attributes as
> > > > +	 * PAGE_KERNEL.
> > > > +	 */
> > > > +	pgprot = params->pgprot;
> > > > +	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
> > > > +		pgprot = PAGE_KERNEL_TAGGED;
> > > > +
> > > >   	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
> > > > -			     size, params->pgprot, __pgd_pgtable_alloc,
> > > > +			     size, pgprot, __pgd_pgtable_alloc,
> > > >   			     flags);
[...]
> > ---------------8<-------------------------------
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > index e17b96d0e4b5..5c78b92d9ec5 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
> >   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
> >   #define pgprot_device(prot) \
> >   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
> > +#define pgprot_tagged(prot) \
> > +	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
> >   /*
> >    * DMA allocations for non-coherent devices use what the Arm architecture calls
> >    * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
> > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> > index cdfc4e9f253e..f5f5044db2ce 100644
> > --- a/include/linux/pgtable.h
> > +++ b/include/linux/pgtable.h
> > @@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
> >   #define pgprot_device pgprot_noncached
> >   #endif
> > 
> > +#ifndef pgprot_tagged
> > +#define pgprot_tagged(prot)	(prot)
> > +#endif
> > +
> >   #ifdef CONFIG_MMU
> >   #ifndef pgprot_modify
> >   #define pgprot_modify pgprot_modify
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 5ba51a8bdaeb..4253d80a59ba 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
> >    */
> >   int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> >   {
> > -	struct mhp_params params = { .pgprot = PAGE_KERNEL };
> > +	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };
> 
> This looks like we're pushing arch specific stuff ("tagged") in here. Can't
> we generalize this to something like
> 
> pgprot_mhp_default
> 
> (or a better name)
> 
> that defaults to PAGE_KERNEL on all architectures except arm64 which
> overwrites this somehow?

It works for me but I prefer the prot modification style similar to
pgprot_writecombine() etc. (i.e. takes a parameter like PAGE_KERNEL and
just changes bits of it).

Thanks.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-03-05 17:44 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-03  4:21 [PATCH] arm64: mte: Use PAGE_KERNEL_TAGGED in arch_add_memory Patrick Daly
2021-03-05 11:19 ` Catalin Marinas
2021-03-05 15:43   ` Catalin Marinas
2021-03-05 15:43     ` Catalin Marinas
2021-03-05 15:55     ` David Hildenbrand
2021-03-05 15:55       ` David Hildenbrand
2021-03-05 17:44       ` Catalin Marinas [this message]
2021-03-05 17:44         ` Catalin Marinas
2021-03-08 11:00         ` David Hildenbrand
2021-03-08 11:00           ` David Hildenbrand
2021-03-08 11:18           ` Catalin Marinas
2021-03-08 11:18             ` Catalin Marinas
2021-03-08 23:44             ` Patrick Daly

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=20210305174413.GF23855@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=anshuman.khandual@arm.com \
    --cc=david@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mm@kvack.org \
    --cc=pdaly@codeaurora.org \
    --cc=vincenzo.frascino@arm.com \
    --cc=will@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.