| Message ID | 20210201190634.22942-3-catalin.marinas@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Further fixes to the linear address checking | expand |
On Mon, 1 Feb 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote: > > Because of the tagged addresses, the __is_lm_address() and > __lm_to_phys() macros grew to some harder to understand bitwise > operations using PAGE_OFFSET. Since these macros only accept untagged > addresses, use a simple subtract operation. > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > --- > arch/arm64/include/asm/memory.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > index 3c1aaa522cbd..ff4732785c32 100644 > --- a/arch/arm64/include/asm/memory.h > +++ b/arch/arm64/include/asm/memory.h > @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag) > * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the > * kernel's TTBR1 address range. > */ > -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) > +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) > Why still subtract PAGE_OFFSET on both sides of the comparison here? > -#define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET) > +#define __lm_to_phys(addr) (((addr) - PAGE_OFFSET) + PHYS_OFFSET) > #define __kimg_to_phys(addr) ((addr) - kimage_voffset) > > #define __virt_to_phys_nodebug(x) ({ \
On Mon, Feb 1, 2021 at 3:05 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > Why still subtract PAGE_OFFSET on both sides of the comparison here? Because otherwise you'd have to use 'addr' twice in a macro, and write it something like #define __is_lm_address(addr) \ (((u64)(addr) >= PAGE_OFFSET) && < ((u64)(addr) < PAGE_END)) which then would require you to do a local variable in a statement expression to avoid the double evaluation.. Linus
On Tue, Feb 02, 2021 at 12:05:10AM +0100, Ard Biesheuvel wrote: > On Mon, 1 Feb 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote: > > Because of the tagged addresses, the __is_lm_address() and > > __lm_to_phys() macros grew to some harder to understand bitwise > > operations using PAGE_OFFSET. Since these macros only accept untagged > > addresses, use a simple subtract operation. > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Cc: Ard Biesheuvel <ardb@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > --- > > arch/arm64/include/asm/memory.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > > index 3c1aaa522cbd..ff4732785c32 100644 > > --- a/arch/arm64/include/asm/memory.h > > +++ b/arch/arm64/include/asm/memory.h > > @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag) > > * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the > > * kernel's TTBR1 address range. > > */ > > -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) > > +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) > > Why still subtract PAGE_OFFSET on both sides of the comparison here? Not sure what you mean (Linus replied on the local variable aspect). With signed arithmetics, the above would be equivalent to addr < PAGE_END but it wouldn't cover the addr < PAGE_OFFSET case. So we rely on the unsigned wrap-around to detect this.
On Tue, 2 Feb 2021 at 12:36, Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Feb 02, 2021 at 12:05:10AM +0100, Ard Biesheuvel wrote: > > On Mon, 1 Feb 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote: > > > Because of the tagged addresses, the __is_lm_address() and > > > __lm_to_phys() macros grew to some harder to understand bitwise > > > operations using PAGE_OFFSET. Since these macros only accept untagged > > > addresses, use a simple subtract operation. > > > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > > Cc: Will Deacon <will@kernel.org> > > > Cc: Ard Biesheuvel <ardb@kernel.org> > > > Cc: Mark Rutland <mark.rutland@arm.com> > > > --- > > > arch/arm64/include/asm/memory.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > > > index 3c1aaa522cbd..ff4732785c32 100644 > > > --- a/arch/arm64/include/asm/memory.h > > > +++ b/arch/arm64/include/asm/memory.h > > > @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag) > > > * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the > > > * kernel's TTBR1 address range. > > > */ > > > -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) > > > +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) > > > > Why still subtract PAGE_OFFSET on both sides of the comparison here? > > Not sure what you mean (Linus replied on the local variable aspect). > > With signed arithmetics, the above would be equivalent to addr < PAGE_END > but it wouldn't cover the addr < PAGE_OFFSET case. So we rely on the > unsigned wrap-around to detect this. > Never mind, I just got confused. These changes look fine. For the series, Acled-by: Ard Biesheuvel <ardb@kernel.org>
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h index 3c1aaa522cbd..ff4732785c32 100644 --- a/arch/arm64/include/asm/memory.h +++ b/arch/arm64/include/asm/memory.h @@ -251,9 +251,9 @@ static inline const void *__tag_set(const void *addr, u8 tag) * lives in the [PAGE_OFFSET, PAGE_END) interval at the bottom of the * kernel's TTBR1 address range. */ -#define __is_lm_address(addr) (((u64)(addr) ^ PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) +#define __is_lm_address(addr) (((u64)(addr) - PAGE_OFFSET) < (PAGE_END - PAGE_OFFSET)) -#define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET) +#define __lm_to_phys(addr) (((addr) - PAGE_OFFSET) + PHYS_OFFSET) #define __kimg_to_phys(addr) ((addr) - kimage_voffset) #define __virt_to_phys_nodebug(x) ({ \
Because of the tagged addresses, the __is_lm_address() and __lm_to_phys() macros grew to some harder to understand bitwise operations using PAGE_OFFSET. Since these macros only accept untagged addresses, use a simple subtract operation. Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Ard Biesheuvel <ardb@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> --- arch/arm64/include/asm/memory.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)