Re: [PATCHv5 1/3] crash-utility/arm64: store phy_offset and memstart_addr separately

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 31, 2021 at 4:16 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> 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


--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux