Re: [PATCHv7 1/4] crash-utility/arm64: introduce a dedicated field to record the mem layout changes

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

 



On Mon, Jun 7, 2021 at 2:27 PM lijiang <lijiang@xxxxxxxxxx> wrote:
>
> Hi, Pingfan
> Thank you for the update.
>
> On Fri, Jun 4, 2021 at 10:39 AM Pingfan Liu <piliu@xxxxxxxxxx> wrote:
> >
> > At present, we have the following important changes for arm64 memory
> > layout:
> >
> > -1. redesigned ARM64 kernel virtual memory layout and associated KASLR
> >     support that was introduced in Linux 4.6. And NEW_VMEMMAP is used to
> >     flag it.
> > -2. memory layout flipped just right before introducing 52-bits kernel.
> > -3. introducing of vabits_actual and phyvirt_offset in kernel
> > -4. removing phyvirt_offset.
> >
> > These changes have effects on PTOV()/VTOP() formula. So introducing a
> > dedicate field mmlayout_flags to record it.
> >
> > Among above, 2 and 3 are introduced closely, and are not distinguished
> > in current implement. And this patch also keep this practice and use
> > vabits_actual as a hint to flag mem flipped.
> >
> > Signed-off-by: Pingfan Liu <piliu@xxxxxxxxxx>
> > Cc: HAGIO KAZUHITO <k-hagio-ab@xxxxxxx>
> > Cc: Lianbo Jiang <lijiang@xxxxxxxxxx>
> > Cc: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx>
> > To: crash-utility@xxxxxxxxxx
> > ---
> >  arm64.c | 11 +++++++++++
> >  defs.h  |  1 +
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/arm64.c b/arm64.c
> > index 8934961..eb88ced 100644
> > --- a/arm64.c
> > +++ b/arm64.c
> > @@ -88,6 +88,10 @@ static int arm64_is_uvaddr(ulong, struct task_context *);
> >  static void arm64_calc_KERNELPACMASK(void);
> >
> >
>
> I tested this patchset with the old vmcore and the latest vmcore, and
> it passed. But I still have

Thanks for the quick response.

> some questions:
>
> > +/* arm64 kernel layout experiences changes, using these flags to distinguish them */
> > +#define MMLAYOUT_FLAGS_FLIP    0x1
> [1] For this micro, is it possible to check the "vabits_actual" and
> "TCR_EL1_T1SZ" in order to
> achieve a similar purpose?
>
> if (kernel_symbol_exists("vabits_actual") || ...
> read_vmcoreinfo("NUMBER(TCR_EL1_T1SZ)"))
>
> > +#define MMLAYOUT_FLAGS_HAS_PHYSVIRT_OFFSET     0x2
> [2] For this one, can it be replaced with the following ways instead
> of adding a new micro definition?
>
> if (kernel_symbol_exists("physvirt_offset"))
>
Just like your understanding, these two macros derive from the
judgement of these conditions.

But there are two benefits of the usage of macro
-1. it only includes a bit-test, so quick.
-2. a concentrated place to organize all memory changes history.  It
can serve as a hint for future code maintenance.

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