Use the %vaddr type for virtual addresses.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/s390x/s390x-internal.h | 5 +++--
target/s390x/helper.c | 2 +-
target/s390x/mmu_helper.c | 6 +++---
target/s390x/tcg/excp_helper.c | 3 ++-
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index a4b54dc441c..f0f7bb5cb11 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -10,6 +10,7 @@
#ifndef S390X_INTERNAL_H
#define S390X_INTERNAL_H
+#include "exec/vaddr.h"
#include "cpu.h"
#include "fpu/softfloat.h"
@@ -376,9 +377,9 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
bool mmu_absolute_addr_valid(target_ulong addr, bool is_write);
/* Special access mode only valid for mmu_translate() */
#define MMU_S390_LRA -1
-int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
+int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc,
target_ulong *raddr, int *flags, uint64_t *tec);
-int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
+int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw,
target_ulong *addr, int *flags, uint64_t *tec);
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index d0c36d4a537..5002eebd4c8 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -71,7 +71,7 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
hwaddr s390_cpu_get_phys_addr_debug(CPUState *cs, vaddr v_addr)
{
hwaddr phys_addr;
- target_ulong page;
+ vaddr page;
page = v_addr & TARGET_PAGE_MASK;
phys_addr = cpu_get_phys_page_debug(cs, page);
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 30f09ec3de4..3313487b41d 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -122,7 +122,7 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
return ret == MEMTX_OK;
}
-static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
+static int mmu_translate_asce(CPUS390XState *env, vaddr vaddr,
uint64_t asc, uint64_t asce, target_ulong *raddr,
int *flags)
{
@@ -381,7 +381,7 @@ static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
* there is an exception to raise
* @return 0 = success, != 0, the exception to raise
*/
-int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
+int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc,
target_ulong *raddr, int *flags, uint64_t *tec)
{
uint64_t asce;
@@ -584,7 +584,7 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
* @param flags the PAGE_READ/WRITE/EXEC flags are stored to this pointer
* @return 0 = success, != 0, the exception to raise
*/
-int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
+int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw,
target_ulong *addr, int *flags, uint64_t *tec)
{
const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
index 019eb4fba1f..292d130d03e 100644
--- a/target/s390x/tcg/excp_helper.c
+++ b/target/s390x/tcg/excp_helper.c
@@ -147,7 +147,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
bool probe, uintptr_t retaddr)
{
CPUS390XState *env = cpu_env(cs);
- target_ulong vaddr, raddr;
+ vaddr vaddr;
+ target_ulong raddr;
uint64_t asc, tec;
int prot, excp;
--
2.52.0
On Wed, 2026-02-04 at 19:27 +0100, Philippe Mathieu-Daudé wrote:
> Use the %vaddr type for virtual addresses.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> target/s390x/s390x-internal.h | 5 +++--
> target/s390x/helper.c | 2 +-
> target/s390x/mmu_helper.c | 6 +++---
> target/s390x/tcg/excp_helper.c | 3 ++-
> 4 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index a4b54dc441c..f0f7bb5cb11 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -10,6 +10,7 @@
> #ifndef S390X_INTERNAL_H
> #define S390X_INTERNAL_H
>
> +#include "exec/vaddr.h"
> #include "cpu.h"
> #include "fpu/softfloat.h"
>
> @@ -376,9 +377,9 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
> bool mmu_absolute_addr_valid(target_ulong addr, bool is_write);
> /* Special access mode only valid for mmu_translate() */
> #define MMU_S390_LRA -1
> -int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
> +int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc,
> target_ulong *raddr, int *flags, uint64_t *tec);
> -int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
> +int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw,
I think this one should remain target_ulong, and become hwaddr in the next patch, no?
> target_ulong *addr, int *flags, uint64_t *tec);
>
>
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index d0c36d4a537..5002eebd4c8 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -71,7 +71,7 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
> hwaddr s390_cpu_get_phys_addr_debug(CPUState *cs, vaddr v_addr)
> {
> hwaddr phys_addr;
> - target_ulong page;
> + vaddr page;
>
> page = v_addr & TARGET_PAGE_MASK;
> phys_addr = cpu_get_phys_page_debug(cs, page);
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 30f09ec3de4..3313487b41d 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -122,7 +122,7 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
> return ret == MEMTX_OK;
> }
>
> -static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
> +static int mmu_translate_asce(CPUS390XState *env, vaddr vaddr,
> uint64_t asc, uint64_t asce, target_ulong *raddr,
> int *flags)
> {
> @@ -381,7 +381,7 @@ static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
> * there is an exception to raise
> * @return 0 = success, != 0, the exception to raise
> */
> -int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
> +int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc,
> target_ulong *raddr, int *flags, uint64_t *tec)
> {
> uint64_t asce;
> @@ -584,7 +584,7 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
> * @param flags the PAGE_READ/WRITE/EXEC flags are stored to this pointer
> * @return 0 = success, != 0, the exception to raise
> */
> -int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
> +int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw,
> target_ulong *addr, int *flags, uint64_t *tec)
> {
> const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
> diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
> index 019eb4fba1f..292d130d03e 100644
> --- a/target/s390x/tcg/excp_helper.c
> +++ b/target/s390x/tcg/excp_helper.c
> @@ -147,7 +147,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> bool probe, uintptr_t retaddr)
> {
> CPUS390XState *env = cpu_env(cs);
> - target_ulong vaddr, raddr;
> + vaddr vaddr;
> + target_ulong raddr;
> uint64_t asc, tec;
> int prot, excp;
>
Hi Eric, On 5/2/26 22:23, Eric Farman wrote: > On Wed, 2026-02-04 at 19:27 +0100, Philippe Mathieu-Daudé wrote: >> Use the %vaddr type for virtual addresses. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> target/s390x/s390x-internal.h | 5 +++-- >> target/s390x/helper.c | 2 +- >> target/s390x/mmu_helper.c | 6 +++--- >> target/s390x/tcg/excp_helper.c | 3 ++- >> 4 files changed, 9 insertions(+), 7 deletions(-) >> @@ -376,9 +377,9 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len, >> bool mmu_absolute_addr_valid(target_ulong addr, bool is_write); >> /* Special access mode only valid for mmu_translate() */ >> #define MMU_S390_LRA -1 >> -int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, >> +int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc, >> target_ulong *raddr, int *flags, uint64_t *tec); >> -int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, >> +int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw, > > I think this one should remain target_ulong, and become hwaddr in the next patch, no? Without s390x background, I don't understand well what real addresses are. IFAICT the single caller s390_cpu_tlb_fill() provides a virtual address. But then indeed mmu_real2abs() consumes a hwaddr as real addr. > >> target_ulong *addr, int *flags, uint64_t *tec); >> >>
On Thu, 2026-02-05 at 22:37 +0100, Philippe Mathieu-Daudé wrote: > Hi Eric, > > On 5/2/26 22:23, Eric Farman wrote: > > On Wed, 2026-02-04 at 19:27 +0100, Philippe Mathieu-Daudé wrote: > > > Use the %vaddr type for virtual addresses. > > > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > --- > > > target/s390x/s390x-internal.h | 5 +++-- > > > target/s390x/helper.c | 2 +- > > > target/s390x/mmu_helper.c | 6 +++--- > > > target/s390x/tcg/excp_helper.c | 3 ++- > > > 4 files changed, 9 insertions(+), 7 deletions(-) > > > > > @@ -376,9 +377,9 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len, > > > bool mmu_absolute_addr_valid(target_ulong addr, bool is_write); > > > /* Special access mode only valid for mmu_translate() */ > > > #define MMU_S390_LRA -1 > > > -int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, > > > +int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc, > > > target_ulong *raddr, int *flags, uint64_t *tec); > > > -int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, > > > +int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw, > > > > I think this one should remain target_ulong, and become hwaddr in the next patch, no? > > Without s390x background, I don't understand well what real addresses > are. IFAICT the single caller s390_cpu_tlb_fill() provides a virtual > address. But then indeed mmu_real2abs() consumes a hwaddr as real addr. Hi Philippe, Ah, I didn't go the other direction (my bad). The usage of "real" here is kind of halfway between virtual and -really- real. I suspect the caller is mistaken here, but I think that's a research project for tomorrow. > > > > > > target_ulong *addr, int *flags, uint64_t *tec); > > > > > >
On 05/02/2026 22.49, Eric Farman wrote: > On Thu, 2026-02-05 at 22:37 +0100, Philippe Mathieu-Daudé wrote: >> Hi Eric, >> >> On 5/2/26 22:23, Eric Farman wrote: >>> On Wed, 2026-02-04 at 19:27 +0100, Philippe Mathieu-Daudé wrote: >>>> Use the %vaddr type for virtual addresses. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> --- >>>> target/s390x/s390x-internal.h | 5 +++-- >>>> target/s390x/helper.c | 2 +- >>>> target/s390x/mmu_helper.c | 6 +++--- >>>> target/s390x/tcg/excp_helper.c | 3 ++- >>>> 4 files changed, 9 insertions(+), 7 deletions(-) >> >> >>>> @@ -376,9 +377,9 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len, >>>> bool mmu_absolute_addr_valid(target_ulong addr, bool is_write); >>>> /* Special access mode only valid for mmu_translate() */ >>>> #define MMU_S390_LRA -1 >>>> -int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, >>>> +int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, uint64_t asc, >>>> target_ulong *raddr, int *flags, uint64_t *tec); >>>> -int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, >>>> +int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw, >>> >>> I think this one should remain target_ulong, and become hwaddr in the next patch, no? >> >> Without s390x background, I don't understand well what real addresses >> are. IFAICT the single caller s390_cpu_tlb_fill() provides a virtual >> address. But then indeed mmu_real2abs() consumes a hwaddr as real addr. > > Hi Philippe, > > Ah, I didn't go the other direction (my bad). The usage of "real" here is kind of halfway between > virtual and -really- real. Yes, real addresses are like physical addresses in s390 land - except for the two so-called low-core pages that might be swapped with two other pages in memory. > I suspect the caller is mistaken here, but I think that's a research > project for tomorrow. The caller s390_cpu_tlb_fill() seems to deal with both, real a virtual addresses, and uses the same variable for both, so I think that's fine there. But I agree with Eric mmu_translate_real() should use "hwaddr" for the raddr parameter instead. Philippe, could you please adjust the patch? Thanks, Thomas
On 6/2/26 14:09, Thomas Huth wrote: > On 05/02/2026 22.49, Eric Farman wrote: >> On Thu, 2026-02-05 at 22:37 +0100, Philippe Mathieu-Daudé wrote: >>> Hi Eric, >>> >>> On 5/2/26 22:23, Eric Farman wrote: >>>> On Wed, 2026-02-04 at 19:27 +0100, Philippe Mathieu-Daudé wrote: >>>>> Use the %vaddr type for virtual addresses. >>>>> >>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>>> --- >>>>> target/s390x/s390x-internal.h | 5 +++-- >>>>> target/s390x/helper.c | 2 +- >>>>> target/s390x/mmu_helper.c | 6 +++--- >>>>> target/s390x/tcg/excp_helper.c | 3 ++- >>>>> 4 files changed, 9 insertions(+), 7 deletions(-) >>> >>> >>>>> @@ -376,9 +377,9 @@ void probe_write_access(CPUS390XState *env, >>>>> uint64_t addr, uint64_t len, >>>>> bool mmu_absolute_addr_valid(target_ulong addr, bool is_write); >>>>> /* Special access mode only valid for mmu_translate() */ >>>>> #define MMU_S390_LRA -1 >>>>> -int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, >>>>> uint64_t asc, >>>>> +int mmu_translate(CPUS390XState *env, vaddr vaddr, int rw, >>>>> uint64_t asc, >>>>> target_ulong *raddr, int *flags, uint64_t *tec); >>>>> -int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int >>>>> rw, >>>>> +int mmu_translate_real(CPUS390XState *env, vaddr raddr, int rw, >>>> >>>> I think this one should remain target_ulong, and become hwaddr in >>>> the next patch, no? >>> >>> Without s390x background, I don't understand well what real addresses >>> are. IFAICT the single caller s390_cpu_tlb_fill() provides a virtual >>> address. But then indeed mmu_real2abs() consumes a hwaddr as real addr. >> >> Hi Philippe, >> >> Ah, I didn't go the other direction (my bad). The usage of "real" here >> is kind of halfway between >> virtual and -really- real. > > Yes, real addresses are like physical addresses in s390 land - except > for the two so-called low-core pages that might be swapped with two > other pages in memory. > >> I suspect the caller is mistaken here, but I think that's a research >> project for tomorrow. > > The caller s390_cpu_tlb_fill() seems to deal with both, real a virtual > addresses, and uses the same variable for both, so I think that's fine > there. > But I agree with Eric mmu_translate_real() should use "hwaddr" for the > raddr parameter instead. Philippe, could you please adjust the patch? Sure!
On 04/02/2026 19.27, Philippe Mathieu-Daudé wrote: > Use the %vaddr type for virtual addresses. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > target/s390x/s390x-internal.h | 5 +++-- > target/s390x/helper.c | 2 +- > target/s390x/mmu_helper.c | 6 +++--- > target/s390x/tcg/excp_helper.c | 3 ++- > 4 files changed, 9 insertions(+), 7 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com>
© 2016 - 2026 Red Hat, Inc.