----- Original Message -----
> The following series implements :
>
> * An infrastructure for platform based vmalloc translation for PPC32
> * vmalloc translation support for PPC44x
>
> Changes since V2:
>
> * Rebased to crash-6.0.3
> * Maintains a list of probe functions, rather than platform
> definitions.
>
>
> Each platform can define their own probe_function which would get the name of the
> ppc platform (read from kernel) and the probe can check if the platform is one of its
> variant. The probe function can then update the 'platform' defintions for the virtual
> address translation.
>
> If none of the defined platforms match, falls back to using the default PPC32
> definitions.
>
> ---
>
> Suzuki K. Poulose (3):
> [ppc] virtual address translation bits for PPC44x
> [ppc] Support for platform based Virtual address translation
> [ppc] Non-linear address translation routine
Hi Suzuki,
I'll defer the technical ACK to Toshi, but I do have a couple of other suggestions.
This may have been a pre-existing issue, but for vmalloc addresses, the
page struct translation at the end of the display (under PAGE PHYSICAL MAPPING...)
is missing for vmalloc addresses. For user-space and unity-mapped
addresses the translation is done as intended:
Secondly, that "Using ppc440gp board definitions:" statement
stuck in the midst of the "vtop" output is not the place for it,
except perhaps if it's enclosed with a CRASHDEBUG() qualifier.
That being said, it certainly is a useful piece of information,
and should be made available somewhere.
What I suggest is that you create a PPC "machine_specific" data
structure like the other architectures have, and which the global
machdep_table structure points to. Then have it either contain,
or at least point to, your new base_platform structure, and
enhance ppc_dump_machdep_table() to dump its contents for the
"help -m" output.
Furthermore, the board's name string seems worthy of display in
ppc_cmd_mach() for the "mach" command, don't you?
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-14-2012, 01:45 AM
"Suzuki K. Poulose"
vmalloc translation support for PPC
On 02/13/2012 10:17 PM, Dave Anderson wrote:
----- Original Message -----
The following series implements :
* An infrastructure for platform based vmalloc translation for PPC32
* vmalloc translation support for PPC44x
Changes since V2:
* Rebased to crash-6.0.3
* Maintains a list of probe functions, rather than platform
definitions.
Each platform can define their own probe_function which would get the name of the
ppc platform (read from kernel) and the probe can check if the platform is one of its
variant. The probe function can then update the 'platform' defintions for the virtual
address translation.
If none of the defined platforms match, falls back to using the default PPC32
definitions.
---
Suzuki K. Poulose (3):
[ppc] virtual address translation bits for PPC44x
[ppc] Support for platform based Virtual address translation
[ppc] Non-linear address translation routine
Hi Suzuki,
I'll defer the technical ACK to Toshi, but I do have a couple of other suggestions.
This may have been a pre-existing issue, but for vmalloc addresses, the
page struct translation at the end of the display (under PAGE PHYSICAL MAPPING...)
is missing for vmalloc addresses. For user-space and unity-mapped
addresses the translation is done as intended:
[..]
That should be a trivial fix.
Ok. I will take a look at this.
Secondly, that "Using ppc440gp board definitions:" statement
stuck in the midst of the "vtop" output is not the place for it,
except perhaps if it's enclosed with a CRASHDEBUG() qualifier.
That being said, it certainly is a useful piece of information,
and should be made available somewhere.
What I suggest is that you create a PPC "machine_specific" data
structure like the other architectures have, and which the global
machdep_table structure points to. Then have it either contain,
or at least point to, your new base_platform structure, and
enhance ppc_dump_machdep_table() to dump its contents for the
"help -m" output.
Furthermore, the board's name string seems worthy of display in
ppc_cmd_mach() for the "mach" command, don't you?
Agreed ! Thanks a lot for the pointers . I will work it out.
Thanks
Suzuki
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-15-2012, 04:26 AM
"Suzuki K. Poulose"
vmalloc translation support for PPC
On 02/13/2012 10:17 PM, Dave Anderson wrote:
----- Original Message -----
The following series implements :
* An infrastructure for platform based vmalloc translation for PPC32
* vmalloc translation support for PPC44x
Changes since V2:
* Rebased to crash-6.0.3
* Maintains a list of probe functions, rather than platform
definitions.
Each platform can define their own probe_function which would get the name of the
ppc platform (read from kernel) and the probe can check if the platform is one of its
variant. The probe function can then update the 'platform' defintions for the virtual
address translation.
If none of the defined platforms match, falls back to using the default PPC32
definitions.
---
Suzuki K. Poulose (3):
[ppc] virtual address translation bits for PPC44x
[ppc] Support for platform based Virtual address translation
[ppc] Non-linear address translation routine
Hi Suzuki,
I'll defer the technical ACK to Toshi, but I do have a couple of other suggestions.
This may have been a pre-existing issue, but for vmalloc addresses, the
page struct translation at the end of the display (under PAGE PHYSICAL MAPPING...)
is missing for vmalloc addresses. For user-space and unity-mapped
addresses the translation is done as intended:
crash> x /i vmlist->caller
0xc042bf40 <setup_indirect_pci+84>: blr
Here, the total amount for RAM on the machine is 128M and looks like
the above address is memory mapped PCI bus, which lies above the 128M.
Also note that the number of pages is '0'. Since the page lies above the
128M and the number of pages is 0, the dump_mem_map fails to find the page struct
for the corresponding phsyical address.
If we go further in the vmlist to find the vmalloc address pages that have pages,
we get :
So, may be we could add a check in the vmalloc translation to see if there is really
a page allocated for the block and then do the translation of the pages.
Thoughts ?
Suzuki
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-15-2012, 02:19 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
> On 02/15/2012 10:56 AM, Suzuki K. Poulose wrote:
> > On 02/13/2012 10:17 PM, Dave Anderson wrote:
> >>
> >> ----- Original Message -----
> >>> The following series implements :
> >>>
> >>> * An infrastructure for platform based vmalloc translation for
> >>> PPC32
> >>> * vmalloc translation support for PPC44x
> >>>
> >>> Changes since V2:
> >>>
> >>> * Rebased to crash-6.0.3
> >>> * Maintains a list of probe functions, rather than platform
> >>> definitions.
> >>>
> >>>
> >>> Each platform can define their own probe_function which would get
> >>> the name of the
> >>> ppc platform (read from kernel) and the probe can check if the
> >>> platform is one of its
> >>> variant. The probe function can then update the 'platform'
> >>> defintions for the virtual
> >>> address translation.
> >>>
> >>> If none of the defined platforms match, falls back to using the
> >>> default PPC32
> >>> definitions.
> >>>
> >>> ---
> >>>
> >>> Suzuki K. Poulose (3):
> >>> [ppc] virtual address translation bits for PPC44x
> >>> [ppc] Support for platform based Virtual address translation
> >>> [ppc] Non-linear address translation routine
> >>
> >> Hi Suzuki,
> >>
> >> I'll defer the technical ACK to Toshi, but I do have a couple of
> >> other suggestions.
> >>
> >> Here's a sample vmalloc translation:
> >>
> >> crash> vtop d1180000
> >> VIRTUAL PHYSICAL
> >> d1180000 ff800000
> >>
> >> Using ppc440gp board definitions:
> >> PAGE DIRECTORY: c056f000
> >> PGD: c0570a20 => c784b000
> >> PMD: c784b000 => c784bc00
> >> PTE: c784bc00 => 1ff80051b
> >> PAGE: ff800000
> >>
> >> PTE PHYSICAL FLAGS
> >> ff80051b ff800000 (PRESENT|USER|GUARDED|COHERENT|ACCESSED)
> >>
> >> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
> >> crash>
> >>
> >> This may have been a pre-existing issue, but for vmalloc
> >> addresses, the
> >> page struct translation at the end of the display (under PAGE
> >> PHYSICAL MAPPING...)
> >> is missing for vmalloc addresses. For user-space and unity-mapped
> >> addresses the translation is done as intended:
> >>
> >> User-space:
> >>
> >> crash> vtop ff8f000
> >> VIRTUAL PHYSICAL
> >> ff8f000 6b90000
> >>
> >> Using ppc440gp board definitions:
> >> PAGE DIRECTORY: c7a3a000
> >> PGD: c7a3a1fc => c7bfc000
> >> PMD: c7bfc000 => c7bfcc78
> >> PTE: c7bfcc78 => 6b9005b
> >> PAGE: 6b90000
> >>
> >> PTE PHYSICAL FLAGS
> >> 6b9005b 6b90000 (PRESENT|USER|GUARDED|COHERENT|WRITETHRU)
> >>
> >> VMA START END FLAGS FILE
> >> c7b09898 ff8f000 ff92000 100073
> >>
> >> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
> >> c06b5200 6b90000 c7a9fc61 ff8f 1 80068
> >> crash>
> >>
> >> Kernel unity-mapped:
> >>
> >> crash> vtop c7b14000
> >> VIRTUAL PHYSICAL
> >> c7b14000 7b14000
> >>
> >> Using ppc440gp board definitions:
> >> PAGE DIRECTORY: c056f000
> >> PGD: c05708f4 => 0
> >>
> >> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
> >> c06d4280 7b14000 0 0 1 0
> >> crash>
> >>
> >> That should be a trivial fix.
> >
> > I took a look at the above issue of vtop report and here is what
> > I find :
> >
> > crash> p *vmlist
> > $17 = {
> > next = 0xc784e880,
> > addr = 0xd1002000,
> > size = 8192,
> > flags = 1,
> > pages = 0x0,
> > nr_pages = 0,
> > phys_addr = 8837398528,
> > caller = 0xc042bf40
> > }
> > crash> vtop 0xd1002000
> > VIRTUAL PHYSICAL
> > d1002000 ec00000
> >
> > PAGE DIRECTORY: c0578000
> > PGD: c0579a20 => c784b000
> > PMD: c784b000 => c784b010
> > PTE: c784b010 => 20ec0051b
> > PAGE: ec00000
> >
> > PTE PHYSICAL FLAGS
> > ec0051b ec00000 (PRESENT|USER|GUARDED|COHERENT|ACCESSED)
> >
> > PAGE PHYSICAL MAPPING INDEX CNT FLAGS
> >
> > crash> x /i vmlist->caller
> > 0xc042bf40 <setup_indirect_pci+84>: blr
> >
> > Here, the total amount for RAM on the machine is 128M and looks like
> > the above address is memory mapped PCI bus, which lies above the 128M.
> > Also note that the number of pages is '0'. Since the page lies above the
> > 128M and the number of pages is 0, the dump_mem_map fails to find the page struct
> > for the corresponding phsyical address.
> >
> > If we go further in the vmlist to find the vmalloc address pages that have pages,
> > we get :
> >
> > crash> p *(vmlist->next->next->next)
> > $16 = {
> > next = 0xc78e51c0,
> > addr = 0xd1008000,
> > size = 8192,
> > flags = 2,
> > pages = 0xc7891680,
> > nr_pages = 1,
> > phys_addr = 0,
> > caller = 0xc006a1d0
> > }
> > crash> vtop 0xd1008000
> > VIRTUAL PHYSICAL
> > d1008000 7896000
> >
> > PAGE DIRECTORY: c0578000
> > PGD: c0579a20 => c784b000
> > PMD: c784b000 => c784b040
> > PTE: c784b040 => 789601f
> > PAGE: 7896000
> >
> > PTE PHYSICAL FLAGS
> > 789601f 7896000 (PRESENT|USER|RW|GUARDED|COHERENT)
> >
> > PAGE PHYSICAL MAPPING INDEX CNT FLAGS
> > c06d72c0 7896000 0 0 1 0
> >
> > So, may be we could add a check in the vmalloc translation to see if there is really
> > a page allocated for the block and then do the translation of the pages.
>
> I have a patch which could do something like:
>
> crash> vtop d1002000
> VIRTUAL PHYSICAL
> d1002000 20ec00000
>
> PAGE DIRECTORY: c0578000
> PGD: c0579a20 => c784b000
> PMD: c784b000 => c784b010
> PTE: c784b010 => 20ec0051b
> PAGE: 20ec00000
>
> PTE PHYSICAL FLAGS
> 20ec0051b 20ec00000 (PRESENT|USER|GUARDED|COHERENT|ACCESSED)
>
> The memory 0x20ec00000 doesn't have a PFN associated with it.
> It could be an MMIO region above the RAM(128 MB).
> crash>
Ah yes, I'm sorry Suzuike -- I forgot that can be seen on any
architecture.
Taking x86_64 as an example, it's seen by just looking at the
first two entries on the vmlist:
So just do the same thing -- no verbose expanation is required.
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-16-2012, 03:22 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
...
> > So just do the same thing -- no verbose expanation is required.
>
> There are two ways to fix this :
>
> 1) Fix dump_mem_map*() to print the header only when there is
> information to dump.
>
> --- a/memory.c
> +++ b/memory.c
> @@ -4637,13 +4637,6 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> continue;
> }
>
> - if (print_hdr) {
> - if (!(pc->curcmd_flags & HEADER_PRINTED))
> - fprintf(fp, "%s", hdr);
> - print_hdr = FALSE;
> - pc->curcmd_flags |= HEADER_PRINTED;
> - }
> -
> pp = section_mem_map_addr(section);
> pp = sparse_decode_mem_map(pp, section_nr);
> phys = (physaddr_t) section_nr * PAGES_PER_SECTION()
> * PAGESIZE();
> @@ -4854,6 +4847,13 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> }
>
> if (bufferindex > buffersize) {
> + if (print_hdr) {
> + if (!(pc->curcmd_flags & HEADER_PRINTED))
> + fprintf(fp, "%s", hdr);
> + print_hdr = FALSE;
> + pc->curcmd_flags |= HEADER_PRINTED;
> + }
> +
> fprintf(fp, "%s", outputbuffer);
> bufferindex = 0;
> }
> @@ -4867,6 +4867,13 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> }
>
> if (bufferindex > 0) {
> + if (print_hdr) {
> + if (!(pc->curcmd_flags & HEADER_PRINTED))
> + fprintf(fp, "%s", hdr);
> + print_hdr = FALSE;
> + pc->curcmd_flags |= HEADER_PRINTED;
> + }
> +
> fprintf(fp, "%s", outputbuffer);
> }
>
> Similarly for the dump_mem_map().
>
> 2) Fix ppc_pgd_vtop() to return FALSE if the paddr > machdep->memsize
>
> --- a/ppc.c
> +++ b/ppc.c
> @@ -438,6 +438,10 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr, physaddr_t
> *paddr, int verbose)
>
> *paddr = PAGEBASE(pte) + PAGEOFFSET(vaddr);
>
> + if (*paddr > machdep->memsize)
> + /* We don't have pages above System RAM */
> + return FALSE;
> +
> return TRUE;
>
> no_page:
>
> I prefer the (1). What do you think ?
Hi Suzuki,
Hmmm -- with respect to (1), I suppose that would work, although
given that both x86 and x86_64 pass through dump_mem_map_SPARSEMEM()
without printing the header in a non-existent-page case, I don't
understand why ppc is different?
And I'm thinking that a more general solution might be to change
do_vtop() here, and not even bother calling the relevant dump_mem_map()
function if there's no page struct associated with it:
- if (page_exists) {
+ if (page_exists && phys_to_page(paddr, &pageptr)) {
if ((pc->flags & DEVMEM) && (paddr >= VTOP(vt->high_memory)))
return;
BZERO(&meminfo, sizeof(struct meminfo));
And w/respect to (2), wouldn't that just cause the generic kvtop()
to fail? And if so, it kind of re-defines the meaning of kvtop(),
even though its current callers pretty much expect to receive
a legitimate physical memory address. But if a virtual address
resolves to a PTE with any legitimate address in it, then kvtop()
should return whatever's there.
But I'm still wondering what makes ppc behave differently in
dump_mem_map_SPARSEMEM()?
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-16-2012, 06:13 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
> On 02/16/2012 09:52 PM, Dave Anderson wrote:
> >
> >
> > ----- Original Message -----
> > ...
> >>> So just do the same thing -- no verbose expanation is required.
> >>
> >> There are two ways to fix this :
> >>
> >> 1) Fix dump_mem_map*() to print the header only when there is
> >> information to dump.
> >>
> >> --- a/memory.c
> >> +++ b/memory.c
> >> @@ -4637,13 +4637,6 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> >> continue;
> >> }
> >>
> >> - if (print_hdr) {
> >> - if (!(pc->curcmd_flags& HEADER_PRINTED))
> >> - fprintf(fp, "%s", hdr);
> >> - print_hdr = FALSE;
> >> - pc->curcmd_flags |= HEADER_PRINTED;
> >> - }
> >> -
> >> pp = section_mem_map_addr(section);
> >> pp = sparse_decode_mem_map(pp, section_nr);
> >> phys = (physaddr_t) section_nr *
> >> PAGES_PER_SECTION()
> >> * PAGESIZE();
> >> @@ -4854,6 +4847,13 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> >> }
> >>
> >> if (bufferindex> buffersize) {
> >> + if (print_hdr) {
> >> + if (!(pc->curcmd_flags&
> >> HEADER_PRINTED))
> >> + fprintf(fp, "%s",
> >> hdr);
> >> + print_hdr = FALSE;
> >> + pc->curcmd_flags |=
> >> HEADER_PRINTED;
> >> + }
> >> +
> >> fprintf(fp, "%s", outputbuffer);
> >> bufferindex = 0;
> >> }
> >> @@ -4867,6 +4867,13 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
> >> }
> >>
> >> if (bufferindex> 0) {
> >> + if (print_hdr) {
> >> + if (!(pc->curcmd_flags& HEADER_PRINTED))
> >> + fprintf(fp, "%s", hdr);
> >> + print_hdr = FALSE;
> >> + pc->curcmd_flags |= HEADER_PRINTED;
> >> + }
> >> +
> >> fprintf(fp, "%s", outputbuffer);
> >> }
> >>
> >> Similarly for the dump_mem_map().
> >>
> >> 2) Fix ppc_pgd_vtop() to return FALSE if the paddr>
> >> machdep->memsize
> >>
> >> --- a/ppc.c
> >> +++ b/ppc.c
> >> @@ -438,6 +438,10 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr,
> >> physaddr_t
> >> *paddr, int verbose)
> >>
> >> *paddr = PAGEBASE(pte) + PAGEOFFSET(vaddr);
> >>
> >> + if (*paddr> machdep->memsize)
> >> + /* We don't have pages above System RAM */
> >> + return FALSE;
> >> +
> >> return TRUE;
> >>
> >> no_page:
> >>
> >> I prefer the (1). What do you think ?
> >
> > Hi Suzuki,
> >
> > Hmmm -- with respect to (1), I suppose that would work, although
> > given that both x86 and x86_64 pass through dump_mem_map_SPARSEMEM()
> > without printing the header in a non-existent-page case, I don't
> > understand why ppc is different?
> Yep, I digged into that a little, but not deep enough to debug it with
> a dump. Nothing was evident from the code .
Right -- I tried debugging it from the x86 and x86_64 perspective,
and couldn't see why ppc would be different! ;-)
Oh well...
> >
> > And I'm thinking that a more general solution might be to change
> > do_vtop() here, and not even bother calling the relevant
> > dump_mem_map()
> > function if there's no page struct associated with it:
> >
> > --- memory.c 10 Feb 2012 16:41:38 -0000 1.273
> > +++ memory.c 16 Feb 2012 14:18:03 -0000
> > @@ -2796,7 +2796,7 @@
> > do_vtop(ulong vaddr, struct task_context *tc, ulong vtop_flags)
> > {
> > physaddr_t paddr;
> > - ulong vma;
> > + ulong vma, pageptr;
> > int page_exists;
> > struct meminfo meminfo;
> > char buf1[BUFSIZE];
> > @@ -2930,7 +2930,7 @@
> >
> > fprintf(fp, "
");
> >
> > - if (page_exists) {
> > + if (page_exists&& phys_to_page(paddr,&pageptr)) {
> > if ((pc->flags& DEVMEM)&& (paddr>=
> > VTOP(vt->high_memory)))
> > return;
> > BZERO(&meminfo, sizeof(struct meminfo));
> >
> > And w/respect to (2), wouldn't that just cause the generic kvtop()
> > to fail? And if so, it kind of re-defines the meaning of kvtop(),
> > even though its current callers pretty much expect to receive
> > a legitimate physical memory address. But if a virtual address
> > resolves to a PTE with any legitimate address in it, then kvtop()
> > should return whatever's there.
>
> Yep, I agree.
>
> >
> > But I'm still wondering what makes ppc behave differently in
> > dump_mem_map_SPARSEMEM()?
> >
> Btw, we don't have SPARSMEM on ppc44x, and end up in dump_mem_map(). I was
> patching both the functions to cover all the platforms.
OK, so do you agree that just patching do_vtop() makes more sense?
>
> Also, I found out that we need to abstract away the definition of Page flags
> as well, since it differes for different platforms (except for the _PAGE_PRESENT).
> I will include the changes in the next version.
OK.
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-17-2012, 12:55 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
> On 02/17/2012 12:43 AM, Dave Anderson wrote:
> >
> >
> > ----- Original Message -----
> >> On 02/16/2012 09:52 PM, Dave Anderson wrote:
> >>>
> >>>
> >>> ----- Original Message -----
> >>> ...
> >>>>> So just do the same thing -- no verbose expanation is required.
> >>>>
> >>>> There are two ways to fix this :
> >>>>
> >>>> 1) Fix dump_mem_map*() to print the header only when there is
> >>>> information to dump.
> >>>>
> >>>> --- a/memory.c
> >>>> +++ b/memory.c
> >>>> @@ -4637,13 +4637,6 @@ dump_mem_map_SPARSEMEM(struct meminfo
> >>>> *mi)
> >>>> continue;
> >>>> }
> >>>>
> >>>> - if (print_hdr) {
> >>>> - if (!(pc->curcmd_flags&
> >>>> HEADER_PRINTED))
> >>>> - fprintf(fp, "%s", hdr);
> >>>> - print_hdr = FALSE;
> >>>> - pc->curcmd_flags |= HEADER_PRINTED;
> >>>> - }
> >>>> -
> >>>> pp = section_mem_map_addr(section);
> >>>> pp = sparse_decode_mem_map(pp, section_nr);
> >>>> phys = (physaddr_t) section_nr *
> >>>> PAGES_PER_SECTION()
> >>>> * PAGESIZE();
> >>>> @@ -4854,6 +4847,13 @@ dump_mem_map_SPARSEMEM(struct meminfo
> >>>> *mi)
> >>>> }
> >>>>
> >>>> if (bufferindex> buffersize) {
> >>>> + if (print_hdr) {
> >>>> + if (!(pc->curcmd_flags&
> >>>> HEADER_PRINTED))
> >>>> + fprintf(fp,
> >>>> "%s",
> >>>> hdr);
> >>>> + print_hdr = FALSE;
> >>>> + pc->curcmd_flags |=
> >>>> HEADER_PRINTED;
> >>>> + }
> >>>> +
> >>>> fprintf(fp, "%s",
> >>>> outputbuffer);
> >>>> bufferindex = 0;
> >>>> }
> >>>> @@ -4867,6 +4867,13 @@ dump_mem_map_SPARSEMEM(struct meminfo
> >>>> *mi)
> >>>> }
> >>>>
> >>>> if (bufferindex> 0) {
> >>>> + if (print_hdr) {
> >>>> + if (!(pc->curcmd_flags&
> >>>> HEADER_PRINTED))
> >>>> + fprintf(fp, "%s", hdr);
> >>>> + print_hdr = FALSE;
> >>>> + pc->curcmd_flags |= HEADER_PRINTED;
> >>>> + }
> >>>> +
> >>>> fprintf(fp, "%s", outputbuffer);
> >>>> }
> >>>>
> >>>> Similarly for the dump_mem_map().
> >>>>
> >>>> 2) Fix ppc_pgd_vtop() to return FALSE if the paddr>
> >>>> machdep->memsize
> >>>>
> >>>> --- a/ppc.c
> >>>> +++ b/ppc.c
> >>>> @@ -438,6 +438,10 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr,
> >>>> physaddr_t
> >>>> *paddr, int verbose)
> >>>>
> >>>> *paddr = PAGEBASE(pte) + PAGEOFFSET(vaddr);
> >>>>
> >>>> + if (*paddr> machdep->memsize)
> >>>> + /* We don't have pages above System RAM */
> >>>> + return FALSE;
> >>>> +
> >>>> return TRUE;
> >>>>
> >>>> no_page:
> >>>>
> >>>> I prefer the (1). What do you think ?
> >>>
> >>> Hi Suzuki,
> >>>
> >>> Hmmm -- with respect to (1), I suppose that would work, although
> >>> given that both x86 and x86_64 pass through
> >>> dump_mem_map_SPARSEMEM()
> >>> without printing the header in a non-existent-page case, I don't
> >>> understand why ppc is different?
> >> Yep, I digged into that a little, but not deep enough to debug it
> >> with
> >> a dump. Nothing was evident from the code .
> >
> > Right -- I tried debugging it from the x86 and x86_64 perspective,
> > and couldn't see why ppc would be different! ;-)
> Ah, well, I was talking about the x86_64 dump.
> I could explain the PPC side of affairs :-). We have page the
> following
> flags set for the page(with the actual PPC44x page flags support) :
>
> VIRTUAL PHYSICAL
> d1002000 20ec00000
>
> PAGE DIRECTORY: c0578000
> PGD: c0579a20 => c784b000
> PMD: c784b000 => c784b010
> PTE: c784b010 => 20ec0051b
> PAGE: 20ec00000
>
> PTE PHYSICAL FLAGS
> 20ec0051b 20ec00000 (PRESENT|RW|GUARDED|NO_CACHE|DIRTY|ACCESSED)
>
>
> So the page is 'present', but there is no page descriptor associated
> with it.
> Hence dump_mem_map() would still be called and hence the problem.
>
> Why doesn't it get called in x86_64 case even when the flags indicate
> page 'PRESENT' ?
I don't know -- that's what I was asking! But I'd like to prevent it
in all cases with the patch to do_vtop().
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-17-2012, 01:04 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
> On 02/17/2012 12:43 AM, Dave Anderson wrote:
> >
> >
> > ----- Original Message -----
> >> On 02/16/2012 09:52 PM, Dave Anderson wrote:
> >>>
> >>>
> >>> ----- Original Message -----
> >>> ...
> >>>>> So just do the same thing -- no verbose expanation is required.
> >>>>
> >>>> There are two ways to fix this :
> >>>>
> >>>> 1) Fix dump_mem_map*() to print the header only when there is
> >>>> information to dump.
> >>>>
> >>>> --- a/memory.c
> >>>> +++ b/memory.c
> >>>> @@ -4637,13 +4637,6 @@ dump_mem_map_SPARSEMEM(struct meminfo
> >>>> *mi)
> >>>> continue;
> >>>> }
> >>>>
> >>>> - if (print_hdr) {
> >>>> - if (!(pc->curcmd_flags&
> >>>> HEADER_PRINTED))
> >>>> - fprintf(fp, "%s", hdr);
> >>>> - print_hdr = FALSE;
> >>>> - pc->curcmd_flags |= HEADER_PRINTED;
> >>>> - }
> >>>> -
> >>>> pp = section_mem_map_addr(section);
> >>>> pp = sparse_decode_mem_map(pp, section_nr);
> >>>> phys = (physaddr_t) section_nr *
> >>>> PAGES_PER_SECTION()
> >>>> * PAGESIZE();
> >>>> @@ -4854,6 +4847,13 @@ dump_mem_map_SPARSEMEM(struct meminfo
> >>>> *mi)
> >>>> }
> >>>>
> >>>> if (bufferindex> buffersize) {
> >>>> + if (print_hdr) {
> >>>> + if (!(pc->curcmd_flags&
> >>>> HEADER_PRINTED))
> >>>> + fprintf(fp,
> >>>> "%s",
> >>>> hdr);
> >>>> + print_hdr = FALSE;
> >>>> + pc->curcmd_flags |=
> >>>> HEADER_PRINTED;
> >>>> + }
> >>>> +
> >>>> fprintf(fp, "%s",
> >>>> outputbuffer);
> >>>> bufferindex = 0;
> >>>> }
> >>>> @@ -4867,6 +4867,13 @@ dump_mem_map_SPARSEMEM(struct meminfo
> >>>> *mi)
> >>>> }
> >>>>
> >>>> if (bufferindex> 0) {
> >>>> + if (print_hdr) {
> >>>> + if (!(pc->curcmd_flags&
> >>>> HEADER_PRINTED))
> >>>> + fprintf(fp, "%s", hdr);
> >>>> + print_hdr = FALSE;
> >>>> + pc->curcmd_flags |= HEADER_PRINTED;
> >>>> + }
> >>>> +
> >>>> fprintf(fp, "%s", outputbuffer);
> >>>> }
> >>>>
> >>>> Similarly for the dump_mem_map().
> >>>>
> >>>> 2) Fix ppc_pgd_vtop() to return FALSE if the paddr>
> >>>> machdep->memsize
> >>>>
> >>>> --- a/ppc.c
> >>>> +++ b/ppc.c
> >>>> @@ -438,6 +438,10 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr,
> >>>> physaddr_t
> >>>> *paddr, int verbose)
> >>>>
> >>>> *paddr = PAGEBASE(pte) + PAGEOFFSET(vaddr);
> >>>>
> >>>> + if (*paddr> machdep->memsize)
> >>>> + /* We don't have pages above System RAM */
> >>>> + return FALSE;
> >>>> +
> >>>> return TRUE;
> >>>>
> >>>> no_page:
> >>>>
> >>>> I prefer the (1). What do you think ?
> >>>
> >>> Hi Suzuki,
> >>>
> >>> Hmmm -- with respect to (1), I suppose that would work, although
> >>> given that both x86 and x86_64 pass through
> >>> dump_mem_map_SPARSEMEM()
> >>> without printing the header in a non-existent-page case, I don't
> >>> understand why ppc is different?
> >> Yep, I digged into that a little, but not deep enough to debug it
> >> with
> >> a dump. Nothing was evident from the code .
> >
> > Right -- I tried debugging it from the x86 and x86_64 perspective,
> > and couldn't see why ppc would be different! ;-)
> >
> > Oh well...
> >
> >>>
> >>> And I'm thinking that a more general solution might be to change
> >>> do_vtop() here, and not even bother calling the relevant
> >>> dump_mem_map()
> >>> function if there's no page struct associated with it:
> >>>
> >>> --- memory.c 10 Feb 2012 16:41:38 -0000 1.273
> >>> +++ memory.c 16 Feb 2012 14:18:03 -0000
> >>> @@ -2796,7 +2796,7 @@
> >>> do_vtop(ulong vaddr, struct task_context *tc, ulong
> >>> vtop_flags)
> >>> {
> >>> physaddr_t paddr;
> >>> - ulong vma;
> >>> + ulong vma, pageptr;
> >>> int page_exists;
> >>> struct meminfo meminfo;
> >>> char buf1[BUFSIZE];
> >>> @@ -2930,7 +2930,7 @@
> >>>
> >>> fprintf(fp, "
");
> >>>
> >>> - if (page_exists) {
> >>> + if (page_exists&& phys_to_page(paddr,&pageptr)) {
> >>> if ((pc->flags& DEVMEM)&& (paddr>=
> >>> VTOP(vt->high_memory)))
> >>> return;
> >>> BZERO(&meminfo, sizeof(struct meminfo));
> >>>
> >>> And w/respect to (2), wouldn't that just cause the generic
> >>> kvtop()
> >>> to fail? And if so, it kind of re-defines the meaning of
> >>> kvtop(),
> >>> even though its current callers pretty much expect to receive
> >>> a legitimate physical memory address. But if a virtual address
> >>> resolves to a PTE with any legitimate address in it, then kvtop()
> >>> should return whatever's there.
> >>
> >> Yep, I agree.
> >>
> >>>
> >>> But I'm still wondering what makes ppc behave differently in
> >>> dump_mem_map_SPARSEMEM()?
> >>>
> >> Btw, we don't have SPARSMEM on ppc44x, and end up in
> >> dump_mem_map(). I was
> >> patching both the functions to cover all the platforms.
> >
> > OK, so do you agree that just patching do_vtop() makes more sense?
>
> Yep. One more question. Why don't we use phys_to_page() in the dump_mem_mapXX() to
> get the page struct and then do the rest of the work ?
It probably could. It's just that when the function is used to display
a single virtual or physical address is passed to the function, it's
pretty much riding on the back of the function's original design which
reads and caches huge chunks of the mem_map, and then walks through
them.
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-17-2012, 06:08 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
> On 02/17/2012 07:25 PM, Dave Anderson wrote:
...
> >> Why doesn't it get called in x86_64 case even when the flags indicate
> >> page 'PRESENT' ?
> >
> > I don't know -- that's what I was asking!
>
> The only suspect is :
>
> if (page_exists) {
> >>> if ((pc->flags & DEVMEM) && (paddr >= VTOP(vt->high_memory))) <<
> return;
I'm not sure whether you are testing ppc on a dumpfile or a live
system using /dev/mem, and I don't know what your paddr value and
vs your vt->high_memory are.
But I'm testing x86_64 on a live system with the crash memory
driver, so DEVMEM is not set. And as it turns out, it doesn't
print the header because it returns here:
if (!(section = valid_section_nr(section_nr))) {
#ifdef NOTDEF
break; /* On a real sparsemem system we need to check
* every section as gaps may exist. But this
* can be slow. If we know we don't have gaps
* just stop validating sections when we
* get to the end of the valid ones.
* In the future find a way to short circuit
* this loop.
*/
#endif
if (mi->flags & ADDRESS_SPECIFIED)
break;
continue;
}
if (print_hdr) {
...
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
02-21-2012, 04:14 PM
Dave Anderson
vmalloc translation support for PPC
----- Original Message -----
> The following series implements :
>
> * An infrastructure for platform based vmalloc translation for PPC32
> * vmalloc translation support for PPC44x
>
> Changes since V3:
>
> * Use platform specific 'page flags'
> * Avoid calling dump_mem_map() by checking if there is a page
> associated
> with the physical memory. (Suggested by: Dave)
>
> Changes since V2:
>
> * Rebased to crash-6.0.3
> * Maintains a list of probe functions, rather than platform
> definitions.
>
>
> Each platform can define their own probe_function which would get the name of the
> ppc platform (read from kernel) and the probe can check if it is one of its variant.
> The probe function can then update the 'platform' defintions for the virtual address
> translation.
>
> If none of the defined platforms match, we fall back to using the default PPC32
> definitions.
Hi Suzuki,
With the updated patch #2 addressing Toshi's concerns, the patch-set looks
and tests OK -- for me anyway.
One thing I will add is the translation of the PAE and CPU_BOOKE flag bits
in ppc_dump_machdep_table(), so that you'll see:
vmalloc translation support for PPC
. I will work it out.
.
