* [PATCH] target/arm: Avoid target_ulong for physical address lookups
@ 2024-09-23 9:22 Ard Biesheuvel
2024-09-23 10:03 ` Richard Henderson
2024-09-23 11:37 ` Arnd Bergmann
0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2024-09-23 9:22 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, richard.henderson, Ard Biesheuvel, Arnd Bergmann
From: Ard Biesheuvel <ardb@kernel.org>
target_ulong is typedef'ed as a 32-bit integer when building the
qemu-system-arm target, and this is smaller than the size of an
intermediate physical address when LPAE is being used.
Given that Linux may place leaf level user page tables in high memory
when built for LPAE, the kernel will crash with an external abort as
soon as it enters user space when running with more than ~3 GiB of
system RAM.
So replace target_ulong with hwaddr in places where it may carry an
address value that is not representable in 32 bits.
Fixes: f3639a64f602ea ("target/arm: Use softmmu tlbs for page table walking")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
target/arm/internals.h | 4 ++--
target/arm/ptw.c | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c5d7b0b492..31c82430d8 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1449,7 +1449,7 @@ typedef struct GetPhysAddrResult {
* * for PSMAv5 based systems we don't bother to return a full FSR format
* value.
*/
-bool get_phys_addr(CPUARMState *env, target_ulong address,
+bool get_phys_addr(CPUARMState *env, hwaddr address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
__attribute__((nonnull));
@@ -1468,7 +1468,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
* Similar to get_phys_addr, but use the given security space and don't perform
* a Granule Protection Check on the resulting address.
*/
-bool get_phys_addr_with_space_nogpc(CPUARMState *env, target_ulong address,
+bool get_phys_addr_with_space_nogpc(CPUARMState *env, hwaddr address,
MMUAccessType access_type,
ARMMMUIdx mmu_idx, ARMSecuritySpace space,
GetPhysAddrResult *result,
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index defd6b84de..8ec4f7f368 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -74,13 +74,13 @@ typedef struct S1Translate {
} S1Translate;
static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw,
- target_ulong address,
+ hwaddr address,
MMUAccessType access_type,
GetPhysAddrResult *result,
ARMMMUFaultInfo *fi);
static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw,
- target_ulong address,
+ hwaddr address,
MMUAccessType access_type,
GetPhysAddrResult *result,
ARMMMUFaultInfo *fi);
@@ -3217,7 +3217,7 @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr,
*/
static bool get_phys_addr_disabled(CPUARMState *env,
S1Translate *ptw,
- target_ulong address,
+ hwaddr address,
MMUAccessType access_type,
GetPhysAddrResult *result,
ARMMMUFaultInfo *fi)
@@ -3300,7 +3300,7 @@ static bool get_phys_addr_disabled(CPUARMState *env,
}
static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
- target_ulong address,
+ hwaddr address,
MMUAccessType access_type,
GetPhysAddrResult *result,
ARMMMUFaultInfo *fi)
@@ -3405,7 +3405,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
}
static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw,
- target_ulong address,
+ hwaddr address,
MMUAccessType access_type,
GetPhysAddrResult *result,
ARMMMUFaultInfo *fi)
@@ -3542,7 +3542,7 @@ static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw,
}
static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw,
- target_ulong address,
+ hwaddr address,
MMUAccessType access_type,
GetPhysAddrResult *result,
ARMMMUFaultInfo *fi)
@@ -3558,7 +3558,7 @@ static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw,
return false;
}
-bool get_phys_addr_with_space_nogpc(CPUARMState *env, target_ulong address,
+bool get_phys_addr_with_space_nogpc(CPUARMState *env, hwaddr address,
MMUAccessType access_type,
ARMMMUIdx mmu_idx, ARMSecuritySpace space,
GetPhysAddrResult *result,
@@ -3571,7 +3571,7 @@ bool get_phys_addr_with_space_nogpc(CPUARMState *env, target_ulong address,
return get_phys_addr_nogpc(env, &ptw, address, access_type, result, fi);
}
-bool get_phys_addr(CPUARMState *env, target_ulong address,
+bool get_phys_addr(CPUARMState *env, hwaddr address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
{
--
2.46.0.792.g87dc391469-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] target/arm: Avoid target_ulong for physical address lookups
2024-09-23 9:22 [PATCH] target/arm: Avoid target_ulong for physical address lookups Ard Biesheuvel
@ 2024-09-23 10:03 ` Richard Henderson
2024-09-23 11:37 ` Arnd Bergmann
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2024-09-23 10:03 UTC (permalink / raw)
To: Ard Biesheuvel, qemu-devel; +Cc: qemu-arm, Ard Biesheuvel, Arnd Bergmann
On 9/23/24 11:22, Ard Biesheuvel wrote:
> From: Ard Biesheuvel<ardb@kernel.org>
>
> target_ulong is typedef'ed as a 32-bit integer when building the
> qemu-system-arm target, and this is smaller than the size of an
> intermediate physical address when LPAE is being used.
>
> Given that Linux may place leaf level user page tables in high memory
> when built for LPAE, the kernel will crash with an external abort as
> soon as it enters user space when running with more than ~3 GiB of
> system RAM.
>
> So replace target_ulong with hwaddr in places where it may carry an
> address value that is not representable in 32 bits.
>
> Fixes: f3639a64f602ea ("target/arm: Use softmmu tlbs for page table walking")
> Reported-by: Arnd Bergmann<arnd@arndb.de>
> Signed-off-by: Ard Biesheuvel<ardb@kernel.org>
> ---
> target/arm/internals.h | 4 ++--
> target/arm/ptw.c | 16 ++++++++--------
> 2 files changed, 10 insertions(+), 10 deletions(-)
Ouch, my bad. Thanks for catching.
Nit: The type "vaddr" is probably more descriptive than "hwaddr" as input to
get_phys_addr. Both are typedefs of uint64_t, so there's no functional difference between
them.
Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] target/arm: Avoid target_ulong for physical address lookups
2024-09-23 9:22 [PATCH] target/arm: Avoid target_ulong for physical address lookups Ard Biesheuvel
2024-09-23 10:03 ` Richard Henderson
@ 2024-09-23 11:37 ` Arnd Bergmann
1 sibling, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2024-09-23 11:37 UTC (permalink / raw)
To: Ard Biesheuvel, qemu-devel
Cc: open list:ARM TCG CPUs, Richard Henderson, Ard Biesheuvel
On Mon, Sep 23, 2024, at 09:22, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> target_ulong is typedef'ed as a 32-bit integer when building the
> qemu-system-arm target, and this is smaller than the size of an
> intermediate physical address when LPAE is being used.
>
> Given that Linux may place leaf level user page tables in high memory
> when built for LPAE, the kernel will crash with an external abort as
> soon as it enters user space when running with more than ~3 GiB of
> system RAM.
>
> So replace target_ulong with hwaddr in places where it may carry an
> address value that is not representable in 32 bits.
>
> Fixes: f3639a64f602ea ("target/arm: Use softmmu tlbs for page table walking")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Thanks for the fix, I now confirmed that this addresses the problem.
I had looked at this code before and got confused thinking that these
addresses were ok as 32-bit wide integers.
Arnd
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-23 11:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-23 9:22 [PATCH] target/arm: Avoid target_ulong for physical address lookups Ard Biesheuvel
2024-09-23 10:03 ` Richard Henderson
2024-09-23 11:37 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).