[Crash-utility] [External Mail]Re: [PATCH] Fix for the determination of the ARM64 SECTION_SIZE_BITS

Dave Anderson anderson at redhat.com
Thu Sep 19 19:40:30 UTC 2019



----- Original Message -----
> Hi Dave,
> I resend the email with the patch attached. It’s a bugfix for the
> determination of the ARM64 SECTION_SIZE_BITS.
> For some ARM64 arch platforms such as qcom 8150/8250 boards, they define
> their own section size bits in kernel config to support
> memory hotplug, which is not same as ARM64 default SECTION_SIZE_BITS value:
> 30. The SECTION_SIZE_BITS definition in qcom's linux-4.14 code as below:
> [arch/arm64/include/asm/sparsemem.h]
> #ifdef CONFIG_SPARSEMEM
> #define MAX_PHYSMEM_BITS        48
> #ifndef CONFIG_MEMORY_HOTPLUG
> #define SECTION_SIZE_BITS       30
> #else
> #define SECTION_SIZE_BITS       CONFIG_HOTPLUG_SIZE_BITS
> #endif
> #endif

Hi Qiwu,

I don't have any ARM64 kernel dumpfiles that were configured with CONFIG_IKCONFIG,
so when testing your patch, the call to get_kernel_config() generates the 
"WARNING: CONFIG_IKCONFIG is not set" message during session initialization, 
which clearly we don't want.  So I slightly reworked your get_section_size_bits()
function to check for (kt->ikconfig_flags & IKCONFIG_AVAIL) before making the call.

Queued for crash-7.2.7:
  
  https://github.com/crash-utility/crash/commit/a566cb75cc768f1fdfa2c9d13949ba7349782880

Thanks,
  Dave



> 
> Without the patch, after load ramdump by crash tool, the warning message
> maybe displayed:
> crash> kmem -P 0x80000000
> kmem: WARNING: cannot find mem_map page for address: fffffffe80000000
> kmem: cannot determine page for 80000000
> 80000000: physical address not found in mem map
> 
> So I introduce this patch to fix it, we can determine SECTION_SIZE_BITS
> either by reading VMCOREINFO or the kernel config,
> otherwise borrow the 64-bit ARM default definiton.
> diff --git a/arm64.c b/arm64.c
> index 6b34b5f..31c03d4 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -32,6 +32,7 @@ static int verify_kimage_voffset(void);
> static void arm64_calc_kimage_voffset(void);
> static void arm64_calc_phys_offset(void);
> static void arm64_calc_virtual_memory_ranges(void);
> +static void arm64_get_section_size_bits(void);
> static int arm64_kdump_phys_base(ulong *);
> static ulong arm64_processor_speed(void);
> static void arm64_init_kernel_pgd(void);
> @@ -375,7 +376,11 @@ arm64_init(int when)
> 
>         case POST_GDB:
>                 arm64_calc_virtual_memory_ranges();
> -               machdep->section_size_bits = _SECTION_SIZE_BITS;
> +               arm64_get_section_size_bits();
> +               if (CRASHDEBUG(1)) {
> +                       fprintf(fp, "SECTION_SIZE_BITS: %ld\n",
> machdep->section_size_bits);
> +               }
> +
>                 if (!machdep->max_physmem_bits) {
>                         if ((string =
>                         pc->read_vmcoreinfo("NUMBER(MAX_PHYSMEM_BITS)"))) {
>                                 machdep->max_physmem_bits = atol(string);
> @@ -1055,6 +1060,32 @@ arm64_calc_phys_offset(void)
>                 fprintf(fp, "using %lx as phys_offset\n", ms->phys_offset);
> }
> 
> +/*
> + *  Determine SECTION_SIZE_BITS either by reading VMCOREINFO or the kernel
> + *  config, otherwise borrow the 64-bit ARM default definiton.
> + */
> +static void
> +arm64_get_section_size_bits(void)
> +{
> +       int ret;
> +       char *string;
> +
> +       if ((string = pc->read_vmcoreinfo("NUMBER(SECTION_SIZE_BITS)"))) {
> +               machdep->section_size_bits = atol(string);
> +               free(string);
> +               return;
> +       }
> +
> +       if ((ret = get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL)) ==
> IKCONFIG_Y) {
> +               if ((ret = get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS",
> &string)) == IKCONFIG_STR) {
> +                       machdep->section_size_bits = atol(string);
> +                       free(string);
> +                       return;
> +               }
> +       }
> +
> +       machdep->section_size_bits = _SECTION_SIZE_BITS;
> +}
> 
> Thanks for your review. I’m looking forward to your favourable reply!
> 
> Best regards,
> Qiwu
> 
> 
> 
> -----Original Message-----
> From: Dave Anderson <anderson at redhat.com>
> Sent: Wednesday, September 18, 2019 9:32 PM
> To: 陈启武 <chenqiwu at xiaomi.com>
> Cc: crash-utility at redhat.com
> Subject: Re: [External Mail]Re: [PATCH] Fix a segfault in setup_ikconfig.
> 
> 
> 
> ----- Original Message -----
> > Hi Dave,
> > 1. I'm requesting the arm64.c change as well. Because some chip makers
> > would like to define their own section size bits in kernel config to
> > support memory hotplug, which is not same as ARM64 default
> > SECTION_SIZE_BITS value:
> > 30, such as QCOM SDM8150 or 8250 borads. Although linux kernel
> > maintain line still define default SECTION_SIZE_BITS value: 30 as
> > section_size_bits for sparsemem. The patch has detailed descript in
> > the email "[PATCH] Fix for the determination of the ARM64
> > SECTION_SIZE_BITS".
> 
> I never received an email with that header.  Can you please resend the email
> with the patch attached (i.e., not inlined)?
> 
> Thanks,
>   Dave
> 
> > 2. The output of "sys config" for my dumpfile has been uploaded, which
> > shows the last line is empty. it occurs a segment fault when add the
> > last empty line into ikconfig entry. I agree that the use of STRNEQ()
> > is better than strncmp().
> >
> > Best regards,
> > Qiwu
> >
> >
> >
> > -----Original Message-----
> > From: Dave Anderson <anderson at redhat.com>
> > Sent: Wednesday, September 18, 2019 3:22 AM
> > To: 陈启武 <chenqiwu at xiaomi.com>
> > Cc: crash-utility at redhat.com
> > Subject: [External Mail]Re: [PATCH] Fix a segfault in setup_ikconfig.
> >
> >
> >
> > ----- Original Message -----
> > >
> > > Hi Anderson,
> > > I want to introduce a patch to your crash tool project. It’s a
> > > bugfix for a segfault in setup_ikconfig.
> > > We add an ikconfig entry only if ent[0] != '#', it is not an
> > > advisable condition because there is a potential segfault risk if ent is
> > > gibberish.
> >
> > Using your dumpfile that has the bogus "ent" string, can you show me
> > what the output of "sys config" looks like?
> >
> > Dave
> >
> > > I explain the reproducing steps about this segfault case:
> > > I try to apply the following patch to crash 7.2.6++ code for a test.
> > > --- a/arm64.c
> > > +++ b/arm64.c
> > > @@ -32,6 +32,7 @@ static int verify_kimage_voffset(void); static
> > > void arm64_calc_kimage_voffset(void); static void
> > > arm64_calc_phys_offset(void); static void
> > > arm64_calc_virtual_memory_ranges(void);
> > > +static void arm64_get_section_size_bits(void);
> > > static int arm64_kdump_phys_base(ulong *); static ulong
> > > arm64_processor_speed(void); static void
> > > arm64_init_kernel_pgd(void); @@ -375,7 +376,11 @@ arm64_init(int
> > > when)
> > >
> > >         case POST_GDB:
> > >                 arm64_calc_virtual_memory_ranges();
> > > -               machdep->section_size_bits = _SECTION_SIZE_BITS;
> > > +               arm64_get_section_size_bits();
> > > +               if (CRASHDEBUG(1)) {
> > > +                       fprintf(fp, "SECTION_SIZE_BITS: %ld\n",
> > > machdep->section_size_bits);
> > > +               }
> > > +
> > >                 if (!machdep->max_physmem_bits) {
> > >                         if ((string =
> > >                         pc->read_vmcoreinfo("NUMBER(MAX_PHYSMEM_BITS)")))
> > >                         {
> > >                                 machdep->max_physmem_bits =
> > > atol(string); @@ -1055,6 +1060,32 @@ arm64_calc_phys_offset(void)
> > >                 fprintf(fp, "using %lx as phys_offset\n",
> > > ms->phys_offset); }
> > >
> > > +/*
> > > + *  Determine SECTION_SIZE_BITS either by reading VMCOREINFO or the
> > > +kernel
> > > + *  config, otherwise borrow the 64-bit ARM default definiton.
> > > + */
> > > +static void
> > > +arm64_get_section_size_bits(void)
> > > +{
> > > +       int ret;
> > > +       char *string;
> > > +
> > > +       if ((string = pc->read_vmcoreinfo("NUMBER(SECTION_SIZE_BITS)")))
> > > {
> > > +               machdep->section_size_bits = atol(string);
> > > +               free(string);
> > > +               return;
> > > +       }
> > > +
> > > +       if ((ret = get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL))
> > > + ==
> > > IKCONFIG_Y) {
> > > +               if ((ret =
> > > + get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS",
> > > &string)) == IKCONFIG_STR) {
> > > +                       machdep->section_size_bits = atol(string);
> > > +                       free(string);
> > > +                       return;
> > > +               }
> > > +       } else {
> > > +               machdep->section_size_bits = _SECTION_SIZE_BITS;
> > > +       }
> > > +}
> > >
> > > Then I make and load the dumpfiles by crash, it occurs a segment
> > > fault as
> > > below:
> > > crash[31000]: segfault at 0 ip 00007f0fb24d98d1 sp 00007fff1703f7e8
> > > error 4 in libc-2.26.so[7f0fb235b000+1d6000]
> > >
> > > So I add debug to find out the segfault reason, It occurred in
> > > setup_ikconfig-> add_ikconfig_entry.
> > > add_ikconfig_entry: ▒▒▒U                //The last ent is a gibberish,
> > > lead
> > > to segfault
> > >
> > > I think the most advisable judgement is if an ikconfig entry start
> > > with "CONFIG_". I debug by the following patch and never reproduce
> > > segfault again.
> > > diff --git a/kernel.c b/kernel.c
> > > index 7804aef..d023c87 100644
> > > --- a/kernel.c
> > > +++ b/kernel.c
> > > @@ -10144,7 +10144,7 @@ static int setup_ikconfig(char *config)
> > >                 while (whitespace(*ent))
> > >                         ent++;
> > >
> > > -               if (ent[0] != '#') {
> > > +               if (!strncmp(ent, "CONFIG_", strlen("CONFIG_"))) {
> > >                         add_ikconfig_entry(ent,
> > >                                          &ikconfig_all[kt->ikconfig_ents++]);
> > >                         if (kt->ikconfig_ents == IKCONFIG_MAX) {
> > >
> > > Thanks for your review. I’m looking forward to your favourable reply!
> > >
> > > Best regards,
> > > Qiwu
> > >
> > >
> > >
> > >
> > > #/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于
> > > 全部
> > > 或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件!
> > > This e-mail and its attachments contain confidential information
> > > from XIAOMI, which is intended only for the person or entity whose
> > > address is listed above. Any use of the information contained herein
> > > in any way (including, but not limited to, total or partial
> > > disclosure, reproduction, or dissemination) by persons other than
> > > the intended
> > > recipient(s) is prohibited. If you receive this e-mail in error,
> > > please notify the sender by phone or email immediately and delete
> > > it!******/#
> > >
> > #/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部
> > 或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件!
> > This e-mail and its attachments contain confidential information from
> > XIAOMI, which is intended only for the person or entity whose address
> > is listed above. Any use of the information contained herein in any
> > way (including, but not limited to, total or partial disclosure,
> > reproduction, or dissemination) by persons other than the intended
> > recipient(s) is prohibited. If you receive this e-mail in error,
> > please notify the sender by phone or email immediately and delete
> > it!******/#
> >
> #/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件!
> This e-mail and its attachments contain confidential information from
> XIAOMI, which is intended only for the person or entity whose address is
> listed above. Any use of the information contained herein in any way
> (including, but not limited to, total or partial disclosure, reproduction,
> or dissemination) by persons other than the intended recipient(s) is
> prohibited. If you receive this e-mail in error, please notify the sender by
> phone or email immediately and delete it!******/#
> 




More information about the Crash-utility mailing list