Re: [Crash-utility] [PATCHv5 1/3] crash-utility/arm64: store phy_offset and memstart_addr separately
11:10 p.m.
Hi, Pingfan
Thank you for the update.
This bug connects with kernel commit 7bc1a0f9e176 ("arm64: mm:
use> single quantity to represent the PA to VA translation"), memstart_addr>
can be negative, which makes it different from real phys_offset. If> using
memstart_addr to calculate the real paddr, the unreasonable paddr> will be got.
Furthermore, in crash utility, PTOV() needs memstart_addr to calculate> VA from PA,
while getting PFN offset in a dumpfile, phys_offset is> required.
As you mentioned above, the calculation formula has been changed, how to
deal with the backward compatibility issue? Should we use kernel version
to determine which code branch it should execute? Please correct me if I
was wrong.
Thanks.
Lianbo
To serve the different purpose, using phys_offset_nominal and>
phys_offset to store them.
3:16 a.m.
New subject: [PATCHv5 1/3] crash-utility/arm64: store phy_offset and memstart_addr separately
Hi Pingfan, Lianbo,
-----Original Message-----
Hi, Pingfan
Thank you for the update.
> This bug connects with kernel commit 7bc1a0f9e176 ("arm64: mm: use
> single quantity to represent the PA to VA translation"), memstart_addr
> can be negative, which makes it different from real phys_offset. If
> using memstart_addr to calculate the real paddr, the unreasonable paddr
> will be got.
> Furthermore, in crash utility, PTOV() needs memstart_addr to calculate
> VA from PA, while getting PFN offset in a dumpfile, phys_offset is
> required.
As you mentioned above, the calculation formula has been changed, how to
deal with the backward compatibility issue? Should we use kernel version
to determine which code branch it should execute? Please correct me if I
was wrong.
I'm not sure whether this is the same as Lianbo's concern, but my concern
is that if a kernel has the "physvirt_offset" variable, PTOV() looks to be
changed needlessly by the patch. Does it work with such an old kernel?
static void
arm64_calc_physvirt_offset(void)
{
...
ms->physvirt_offset = ms->phys_offset_nominal;
if ((sp = kernel_symbol_search("physvirt_offset")) &&
machdep->machspec->kimage_voffset) {
if (READMEM(pc->mfd, &physvirt_offset, sizeof(physvirt_offset),
sp->value, sp->value -
machdep->machspec->kimage_voffset) > 0) {
ms->physvirt_offset = physvirt_offset; <<-- this
case
}
}
}
-#define PTOV(X) \
- ((unsigned long)(X) - (machdep->machspec->physvirt_offset))
+#define PTOV(X) \
+ (((unsigned long)(X) - (machdep->machspec->physvirt_offset)) | PAGE_OFFSET)
Maybe we can set ms->physvirt_offset to work with the current PTOV()
when no symbol?
Sorry that I cannot try the patch or ideas because of no machine.
Thanks,
Kazu
4:34 a.m.
New subject: [PATCHv5 1/3] crash-utility/arm64: store phy_offset and memstart_addr separately
On Mon, May 31, 2021 at 4:16 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
Hi Pingfan, Lianbo,
-----Original Message-----
> Hi, Pingfan
>
> Thank you for the update.
>
> > This bug connects with kernel commit 7bc1a0f9e176 ("arm64: mm: use
> > single quantity to represent the PA to VA translation"), memstart_addr
> > can be negative, which makes it different from real phys_offset. If
> > using memstart_addr to calculate the real paddr, the unreasonable paddr
> > will be got.
>
> > Furthermore, in crash utility, PTOV() needs memstart_addr to calculate
> > VA from PA, while getting PFN offset in a dumpfile, phys_offset is
> > required.
>
> As you mentioned above, the calculation formula has been changed, how to
> deal with the backward compatibility issue? Should we use kernel version
> to determine which code branch it should execute? Please correct me if I
> was wrong.
I'm not sure whether this is the same as Lianbo's concern, but my concern
is that if a kernel has the "physvirt_offset" variable, PTOV() looks to be
changed needlessly by the patch. Does it work with such an old kernel?
Yes, you share the same concern with Lianbo.
And I have discussed with Lianbo, and think about how to cope with the
old kernel.
static void
arm64_calc_physvirt_offset(void)
{
...
ms->physvirt_offset = ms->phys_offset_nominal;
if ((sp = kernel_symbol_search("physvirt_offset")) &&
machdep->machspec->kimage_voffset) {
if (READMEM(pc->mfd, &physvirt_offset, sizeof(physvirt_offset),
sp->value, sp->value -
machdep->machspec->kimage_voffset) > 0) {
ms->physvirt_offset = physvirt_offset; <<--
this case
}
}
}
-#define PTOV(X) \
- ((unsigned long)(X) - (machdep->machspec->physvirt_offset))
+#define PTOV(X) \
+ (((unsigned long)(X) - (machdep->machspec->physvirt_offset)) |
PAGE_OFFSET)
Maybe we can set ms->physvirt_offset to work with the current PTOV()
when no symbol?
Yes, you are right.
At present, my idea is to provide two sets of PTOV()/VTOP()
Thanks,
Pingfan
1126
days inactive
1132
days old
devel@lists.crash-utility.osci.io
2 comments
3 participants
participants (3)
-
HAGIO KAZUHITO(萩尾 一仁) -
lijiang -
Pingfan Liu