6:26 a.m.
- Update s390x_vr_IS_VMALLOC_ADDR() to include a non-zero vmalloc_start
check, making it consistent with s390x_generic_IS_VMALLOC_ADDR().
- Remove redundant vmalloc_start check from s390x_kvtop().
Signed-off-by: Mikhail Zaslonko <zaslonko(a)linux.ibm.com>
---
s390x.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/s390x.c b/s390x.c
index 0e8e2b3..25dca5e 100644
--- a/s390x.c
+++ b/s390x.c
@@ -724,13 +724,13 @@ static ulong s390x_vr_PTOV(ulong paddr)
{
return paddr + machdep->identity_map_base;
}
static int s390x_vr_IS_VMALLOC_ADDR(ulong vaddr)
{
- return (vaddr >= vt->vmalloc_start && vaddr < machdep->kvbase);
+ return (vt->vmalloc_start && vaddr >= vt->vmalloc_start &&
vaddr < machdep->kvbase);
}
ulong s390x_VTOP(ulong vaddr)
{
return machdep->machspec->virt_to_phys(vaddr);
}
@@ -943,17 +943,12 @@ s390x_kvtop(struct task_context *tc, ulong vaddr, physaddr_t *paddr,
int verbose
if (!IS_KVADDR(vaddr)){
*paddr = 0;
return FALSE;
}
- if (!vt->vmalloc_start) {
- *paddr = VTOP(vaddr);
- return TRUE;
- }
-
if (!IS_VMALLOC_ADDR(vaddr)) {
*paddr = VTOP(vaddr);
return TRUE;
}
pgd_base = (unsigned long)vt->kernel_pgd[0];
--
2.49.0
6:26 a.m.
New subject: [PATCH v2 2/2] s390x: Always perform kernel address translation in vtop verbose mode
In s390x_kvtop() early return may take place despite the verbose
flag. Thus we can miss page-table walk information in the vtop output
for kernel virtual addresses.
Make sure that s390x_vtop() is always called for kernel virtual addresses
when the verbose flag is passed to s390x_kvtop() by do_vtop().
Suggested-by: Heiko Carstens <hca(a)linux.ibm.com>
Signed-off-by: Mikhail Zaslonko <zaslonko(a)linux.ibm.com>
---
s390x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/s390x.c b/s390x.c
index 25dca5e..77d9082 100644
--- a/s390x.c
+++ b/s390x.c
@@ -943,15 +943,15 @@ s390x_kvtop(struct task_context *tc, ulong vaddr, physaddr_t *paddr,
int verbose
if (!IS_KVADDR(vaddr)){
*paddr = 0;
return FALSE;
}
- if (!IS_VMALLOC_ADDR(vaddr)) {
- *paddr = VTOP(vaddr);
- return TRUE;
+ if (!verbose && !IS_VMALLOC_ADDR(vaddr)) {
+ *paddr = VTOP(vaddr);
+ return TRUE;
}
pgd_base = (unsigned long)vt->kernel_pgd[0];
return s390x_vtop(pgd_base, vaddr, paddr, verbose);
}
--
2.49.0
7:27 a.m.
New subject: [PATCH v2 2/2] s390x: Always perform kernel address translation in vtop verbose mode
On Fri, Oct 17, 2025 at 01:26:41PM +0200, Mikhail Zaslonko wrote:
In s390x_kvtop() early return may take place despite the verbose
flag. Thus we can miss page-table walk information in the vtop output
for kernel virtual addresses.
Make sure that s390x_vtop() is always called for kernel virtual addresses
when the verbose flag is passed to s390x_kvtop() by do_vtop().
Suggested-by: Heiko Carstens <hca(a)linux.ibm.com>
Signed-off-by: Mikhail Zaslonko <zaslonko(a)linux.ibm.com>
---
s390x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/s390x.c b/s390x.c
index 25dca5e..77d9082 100644
--- a/s390x.c
+++ b/s390x.c
@@ -943,15 +943,15 @@ s390x_kvtop(struct task_context *tc, ulong vaddr, physaddr_t
*paddr, int verbose
if (!IS_KVADDR(vaddr)){
*paddr = 0;
return FALSE;
}
- if (!IS_VMALLOC_ADDR(vaddr)) {
- *paddr = VTOP(vaddr);
- return TRUE;
+ if (!verbose && !IS_VMALLOC_ADDR(vaddr)) {
+ *paddr = VTOP(vaddr);
+ return TRUE;
Few other architectures do it like this:
if (!vt->vmalloc_start) {
*paddr = VTOP(kvaddr);
return TRUE;
}
if (!IS_VMALLOC_ADDR(kvaddr)) {
*paddr = VTOP(kvaddr);
if (!verbose)
return TRUE;
}
Would it work for s390x?
}
pgd_base = (unsigned long)vt->kernel_pgd[0];
return s390x_vtop(pgd_base, vaddr, paddr, verbose);
}
Thanks!
11:01 a.m.
New subject: [PATCH v2 2/2] s390x: Always perform kernel address translation in vtop verbose mode
On 10/20/2025 2:27 PM, Alexander Gordeev wrote:
On Fri, Oct 17, 2025 at 01:26:41PM +0200, Mikhail Zaslonko wrote:
> In s390x_kvtop() early return may take place despite the verbose
> flag. Thus we can miss page-table walk information in the vtop output
> for kernel virtual addresses.
> Make sure that s390x_vtop() is always called for kernel virtual addresses
> when the verbose flag is passed to s390x_kvtop() by do_vtop().
>
> Suggested-by: Heiko Carstens <hca(a)linux.ibm.com>
> Signed-off-by: Mikhail Zaslonko <zaslonko(a)linux.ibm.com>
> ---
> s390x.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/s390x.c b/s390x.c
> index 25dca5e..77d9082 100644
> --- a/s390x.c
> +++ b/s390x.c
> @@ -943,15 +943,15 @@ s390x_kvtop(struct task_context *tc, ulong vaddr, physaddr_t
*paddr, int verbose
>
> if (!IS_KVADDR(vaddr)){
> *paddr = 0;
> return FALSE;
> }
>
> - if (!IS_VMALLOC_ADDR(vaddr)) {
> - *paddr = VTOP(vaddr);
> - return TRUE;
> + if (!verbose && !IS_VMALLOC_ADDR(vaddr)) {
> + *paddr = VTOP(vaddr);
> + return TRUE;
Few other architectures do it like this:
if (!vt->vmalloc_start) {
*paddr = VTOP(kvaddr);
return TRUE;
}
if (!IS_VMALLOC_ADDR(kvaddr)) {
*paddr = VTOP(kvaddr);
if (!verbose)
return TRUE;
}
Would it work for s390x?
It would, but does it make sense updating *paddr value for non-verbose case prior to
calling s390x_vtop() where we set it back to zero at the very start (see below).
Do you think we should just try keeping the code similar to other architectures?
/* lookup virtual address in page tables */
int s390x_vtop(ulong table, ulong vaddr, physaddr_t *phys_addr, int verbose)
{
ulong entry, paddr;
int level, len;
if (verbose)
fprintf(fp, "PAGE DIRECTORY: %016lx\n", table);
*phys_addr = 0;
> }
>
> pgd_base = (unsigned long)vt->kernel_pgd[0];
> return s390x_vtop(pgd_base, vaddr, paddr, verbose);
> }
Thanks!
12:55 a.m.
New subject: [PATCH v2 2/2] s390x: Always perform kernel address translation in vtop verbose mode
On Mon, Oct 20, 2025 at 06:01:00PM +0200, Mikhail Zaslonko wrote:
>> @@ -943,15 +943,15 @@ s390x_kvtop(struct task_context *tc,
ulong vaddr, physaddr_t *paddr, int verbose
>>
>> if (!IS_KVADDR(vaddr)){
>> *paddr = 0;
>> return FALSE;
>> }
>>
>> - if (!IS_VMALLOC_ADDR(vaddr)) {
>> - *paddr = VTOP(vaddr);
>> - return TRUE;
>> + if (!verbose && !IS_VMALLOC_ADDR(vaddr)) {
>> + *paddr = VTOP(vaddr);
>> + return TRUE;
>
> Few other architectures do it like this:
>
> if (!vt->vmalloc_start) {
> *paddr = VTOP(kvaddr);
> return TRUE;
> }
> if (!IS_VMALLOC_ADDR(kvaddr)) {
> *paddr = VTOP(kvaddr);
> if (!verbose)
> return TRUE;
> }
>
> Would it work for s390x?
It would, but does it make sense updating *paddr value for non-verbose case prior to
calling s390x_vtop() where we set it back to zero at the very start (see below).
It is getting ugly this way, indeed.
Do you think we should just try keeping the code similar to other
architectures?
No, I am just afraid of missing the reason why some other archs do it (not all).
So it looks like you approach is fine.
/* lookup virtual address in page tables */
int s390x_vtop(ulong table, ulong vaddr, physaddr_t *phys_addr, int verbose)
{
ulong entry, paddr;
int level, len;
if (verbose)
fprintf(fp, "PAGE DIRECTORY: %016lx\n", table);
*phys_addr = 0;
Thanks!
7:25 a.m.
New subject: [PATCH v2 1/2] s390x: Add non-zero vmalloc_start check to s390x_vr_IS_VMALLOC_ADDR()
On Fri, Oct 17, 2025 at 01:26:40PM +0200, Mikhail Zaslonko wrote:
Hi Mikhail,
Could you please clarify why these are not two separate commits?
- Update s390x_vr_IS_VMALLOC_ADDR() to include a non-zero
vmalloc_start
check, making it consistent with s390x_generic_IS_VMALLOC_ADDR().
This one looks as a bug fix to me.
- Remove redundant vmalloc_start check from s390x_kvtop().
And this one is a follow-up optimization, unless there was
a reason for this check before commit d0164e7 ("s390x: uncouple
physical and virtual memory spaces"). Do you have any ideas?
Signed-off-by: Mikhail Zaslonko <zaslonko(a)linux.ibm.com>
---
s390x.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
Thanks!
11 a.m.
New subject: [PATCH v2 1/2] s390x: Add non-zero vmalloc_start check to s390x_vr_IS_VMALLOC_ADDR()
On 10/20/2025 2:25 PM, Alexander Gordeev wrote:
On Fri, Oct 17, 2025 at 01:26:40PM +0200, Mikhail Zaslonko wrote:
Hi Mikhail,
Could you please clarify why these are not two separate commits?
You might be right. It makes sense splitting this commit in two.
> - Update s390x_vr_IS_VMALLOC_ADDR() to include a non-zero vmalloc_start
> check, making it consistent with s390x_generic_IS_VMALLOC_ADDR().
This one looks as a bug fix to me.
> - Remove redundant vmalloc_start check from s390x_kvtop().
And this one is a follow-up optimization, unless there was
a reason for this check before commit d0164e7 ("s390x: uncouple
physical and virtual memory spaces"). Do you have any ideas?
I don't see a reason for this check as soon as IS_VMALLOC_ADDR() comprises
this check for s390x. I think it could come from kvtop implementation for other
architectures indeed.
> Signed-off-by: Mikhail Zaslonko <zaslonko(a)linux.ibm.com>
> ---
> s390x.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
Thanks!
Thanks for taking a look!
165
days inactive
169
days old
devel@lists.crash-utility.osci.io
6 comments
2 participants
participants (2)
-
Alexander Gordeev -
Mikhail Zaslonko