2:07 a.m.
-----Original Message-----
> > @@ -1033,7 +1050,11 @@ arm64_calc_phys_offset(void)
> > ms->kimage_voffset && (sp =
kernel_symbol_search("memstart_addr"))) {
> > if (pc->flags & PROC_KCORE) {
> > if ((string = pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
> > - ms->phys_offset = htol(string, QUIET, NULL);
> > + ms->phys_offset_nominal = htol(string, QUIET, NULL);
> > + if (ms->phys_offset_nominal < 0)
> > + ms->phys_offset = ms->phys_offset_nominal +
> > MEMSTART_ADDR_OFFSET;
> > + else
> > + ms->phys_offset = ms->phys_offset_nominal;
> > free(string);
> > return;
> > }
>
> If it's /dev/mem (not /proc/kcore), the memstart_addr value is still negative?
> It's no problem?
I have no big picture about /dev/mem.
But for the value, memstart_addr's calculation is done by
arch/arm64/mm/init.c:349: memstart_addr -= _PAGE_OFFSET(48) -
_PAGE_OFFSET(52);
in arm64_memblock_init().
Does it raise issue for /dev/mem ? Could you enlighten me about your idea?
Recently /proc/kcore has had ELF note vmcoreinfo, so crash can read it, but
in case of /dev/mem, it cannot be used and crash will read the memstart_addr
value with the following READMEM(). In this case, I thought that the same
offset adjustment would be needed in theory.
if (ACTIVE()) {
...
if ((machdep->flags & NEW_VMEMMAP) &&
ms->kimage_voffset && (sp =
kernel_symbol_search("memstart_addr"))) {
if (pc->flags & PROC_KCORE) {
if ((string =
pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
ms->phys_offset_nominal = htol(string, QUIET,
NULL);
if (ms->phys_offset_nominal < 0)
ms->phys_offset =
ms->phys_offset_nominal + MEMSTART_ADDR_OFFSET;
else
ms->phys_offset =
ms->phys_offset_nominal;
free(string);
return;
}
vaddr =
symbol_value_from_proc_kallsyms("memstart_addr");
if (vaddr == BADVAL)
vaddr = sp->value;
paddr = KCORE_USE_VADDR;
} else {
vaddr = sp->value;
paddr = sp->value -
machdep->machspec->kimage_voffset;
}
if (READMEM(pc->mfd, &phys_offset, sizeof(phys_offset),
vaddr, paddr) > 0) {
ms->phys_offset = phys_offset; <<-- read from
memstart_addr
return;
}
>
> > @@ -1085,7 +1106,18 @@ arm64_calc_phys_offset(void)
> > } else if (DISKDUMP_DUMPFILE() &&
diskdump_phys_base(&phys_offset)) {
> > ms->phys_offset = phys_offset;
>
> makedumpfile also set kdump_sub_header.phys_base to NUMBER(PHYS_OFFSET) ?
I miss this diskdump. It should also have the same logic.
diff --git a/arm64.c b/arm64.c
index 6346753..cd84a74 100644
--- a/arm64.c
+++ b/arm64.c
@@ -1108,9 +1108,8 @@ arm64_calc_phys_offset(void)
return;
ms->phys_offset = phys_offset;
- } else if (DISKDUMP_DUMPFILE() && diskdump_phys_base(&phys_offset)) {
- ms->phys_offset = phys_offset;
- } else if (KDUMP_DUMPFILE() && arm64_kdump_phys_base(&phys_offset)) {
+ } else if ((DISKDUMP_DUMPFILE() && diskdump_phys_base(&phys_offset)) ||
+ (KDUMP_DUMPFILE() && arm64_kdump_phys_base(&phys_offset))) {
Oh, this looks nicely done.
/*
* When running a 52bits kernel on 48bits hardware. Kernel plays a trick:
* if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52) && (vabits_actual != 52))
--
2.29.2
>
> > } else if (KDUMP_DUMPFILE() &&
arm64_kdump_phys_base(&phys_offset)) {
> > - ms->phys_offset = phys_offset;
> > + /*
> > + * When running a 52bits kernel on 48bits hardware. Kernel plays a trick:
> > + * if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52) && (vabits_actual !=
52))
> > + * memstart_addr -= _PAGE_OFFSET(48) - _PAGE_OFFSET(52);
> > + *
> > + * In crash, this should be detected to get a real physical start address.
> > + */
> > + ms->phys_offset_nominal = phys_offset;
> > + if ((long)phys_offset < 0)
> > + ms->phys_offset = phys_offset + MEMSTART_ADDR_OFFSET;
> > + else
> > + ms->phys_offset = phys_offset;
> > } else {
> > error(WARNING,
> > "phys_offset cannot be determined from the dumpfile.\n");
> > @@ -1175,6 +1207,23 @@ arm64_init_kernel_pgd(void)
> > vt->kernel_pgd[i] = value;
> > }
> >
> > +ulong arm64_PTOV(ulong paddr)
> > +{
> > + ulong v;
> > + struct machine_specific *ms = machdep->machspec;
> > +
> > + /*
> > + * Either older kernel before kernel has 'physvirt_offset' or newer
kernel which
> > + * removes 'physvirt_offset' has the same formula
> > + */
> > + if (!(machdep->flags & HAS_PHYSVIRT_OFFSET))
> > + v = (paddr - ms->physvirt_offset) | PAGE_OFFSET;
> > + else
> > + v = paddr - ms->physvirt_offset;
> > +
> > + return v;
> > +}
> > +
> > ulong
> > arm64_VTOP(ulong addr)
> > {
> > @@ -1185,8 +1234,20 @@ arm64_VTOP(ulong addr)
> > return addr - machdep->machspec->kimage_voffset;
> > }
> >
> > - if (addr >= machdep->machspec->page_offset)
> > - return addr + machdep->machspec->physvirt_offset;
> > + if (addr >= machdep->machspec->page_offset) {
> > + ulong paddr;
> > +
> > + if (!(machdep->flags & FLIPPED_VM) || (machdep->flags &
HAS_PHYSVIRT_OFFSET)) {
> > + paddr = addr;
> > + } else {
> > + /*
> > + * #define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
> > + */
> > + paddr = addr & ~
_PAGE_OFFSET(machdep->machspec->CONFIG_ARM64_VA_BITS);
> > + }
> > + paddr += machdep->machspec->physvirt_offset;
> > + return paddr;
>
> Hmm, complex. It should be symmetric to PTOV, why differences?
Not sure that I catch your meaning. Replacing
_PAGE_OFFSET(machdep->machspec->CONFIG_ARM64_VA_BITS) with
PAGE_OFFSET ?
Something like this ?
diff --git a/arm64.c b/arm64.c
index 9a77628..0917613 100644
--- a/arm64.c
+++ b/arm64.c
@@ -1241,15 +1241,14 @@ arm64_VTOP(ulong addr)
ulong paddr;
if (!(machdep->flags & FLIPPED_VM) || (machdep->flags &
HAS_PHYSVIRT_OFFSET)) {
- paddr = addr;
+ return addr + machdep->machspec->physvirt_offset;
} else {
/*
* #define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
*/
- paddr = addr & ~ _PAGE_OFFSET(machdep->machspec->CONFIG_ARM64_VA_BITS);
+ paddr = addr & ~ PAGE_OFFSET;
+ return paddr + machdep->machspec->physvirt_offset;
}
- paddr += machdep->machspec->physvirt_offset;
- return paddr;
Yes, it's one of them, and this looks better, but...
Hmm ok, I summarized these and smy question why they're asymmetric emerged:
if physvirt_offset exists // HAS_PHYSVIRT_OFFSET
ms->physvirt_offset = read physvirt_offset;
else if (machdep->flags & FLIPPED_VM)
ms->physvirt_offset = ms->phys_offset_nominal;
else // !FLIPPED_VM
ms->physvirt_offset = ms->phys_offset - ms->page_offset;
PTOV:
if (machdep->flags & HAS_PHYSVIRT_OFFSET)
v = paddr - ms->physvirt_offset; // looks ok
else
v = (paddr - ms->physvirt_offset) | PAGE_OFFSET; // Is this ok when !FLIPPED_VM ?
VTOP:
if (machdep->flags & HAS_PHYSVIRT_OFFSET || !(machdep->flags & FLIPPED_VM))
p = vaddr + ms->physvirt_offset; // looks ok
else
p = (vaddr & ~PAGE_OFFSET) + ms->physvirt_offset; // looks ok
When !FLIPPED_VM, PTOV calculates:
v = (paddr - ms->physvirt_offset) | PAGE_OFFSET
= (paddr - ms->physoffset + ms->page_offset) | PAGE_OFFSET
This might be not wrong in the result value because of the or operation,
but looks wrong formula. So PTOV also needs the !(machdep->flags & FLIPPED_VM)
condition? or I'm missing something?
Thanks,
Kazu
}
else if (machdep->machspec->kimage_voffset)
return addr - machdep->machspec->kimage_voffset;
Again, appreciate your review.
Thanks,
Pingfan