12:42 a.m.
-----Original Message-----
> > > > @@ -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;
I had thought it worked for all versions before commit 5383cc6efed1 (arm64: mm: Introduce
vabits_actual).
So !(machdep->flags & FLIPPED_VM) branch should use the formula.
But it turns out to be wrong. PLS see the comment followed.
> > > > + 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?
> >
[...]
>
> Yes, it's one of them, and this looks better, but...
>
> Hmm ok, I summarized these and my question why they're asymmetric emerged:
>
Thank you for the patient.
> 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 ?
>
It works for !FLIPPED_VM. But I did make a mistake on VTOP()
Flipped mm is introduced by 14c127c957c1 ("arm64: mm: Flip kernel VA space")
$ git show 14c127c957c1c6070647c171e72f06e0db275ebf:arch/arm64/include/asm/memory.h |
grep "#define
__phys_to_virt"
#define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
$ git show 14c127c957c1c6070647c171e72f06e0db275ebf~1:arch/arm64/include/asm/memory.h |
grep "#define
__phys_to_virt"
#define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
So the formula keeps unchange across filpped-mm. The same case for VTOP.
And what matters is physvirt_offset introduced by 5383cc6efed1 ("arm64: mm:
Introduce vabits_actual")
> 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
>
Similar,
$git show 14c127c957c1c6070647c171e72f06e0db275ebf:arch/arm64/include/asm/memory.h | grep
__lm_to_phys
#define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
__is_lm_address(__x) ? __lm_to_phys(__x) : \
$ git show 14c127c957c1c6070647c171e72f06e0db275ebf~1:arch/arm64/include/asm/memory.h |
grep __lm_to_phys
#define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
__is_lm_address(__x) ? __lm_to_phys(__x) : \
>
> 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?
>
So I need to drop the !(machdep->flags & FLIPPED_VM) in VTOP(), instead of adding
in PTOV().
Sorry for the confusion and hope I make it clear.
If you do so, you mean that you will change
ms->physvirt_offset = ms->phys_offset - ms->page_offset
to
ms->physvirt_offset = ms->phys_offset
if !FLIPPED_VM ?
For me, it's readable and understandable to match the formulas with kernel's
one. In this case, use the physvirt_offset only if physvirt_offset exists.
We can do it now because we are introducing a switchable PTOV/VTOP.
For example:
ms->phys_offset_nominal = read phys_offset;
if (ms->phys_offset_nominal < 0)
ms->phys_offset = ms->phys_offset_nominal + MEMSTART_ADDR_OFFSET;
else
ms->phys_offset = ms->phys_offset_nominal
PTOV:
if (machdep->flags & HAS_PHYSVIRT_OFFSET)
v = paddr - ms->physvirt_offset;
else
v = (paddr - ms->phys_offset_nominal) | PAGE_OFFSET
VTOP:
if (machdep->flags & HAS_PHYSVIRT_OFFSET)
p = vaddr + ms->physvirt_offset;
else
p = (vaddr & ~PAGE_OFFSET) + ms->phys_offset_nominal;
btw, just to clarify, where is the phys_offset used except for
PTOV and VTOP? I cannot find it..
while getting PFN offset in a dumpfile, phys_offset is
required.
Thanks,
Kazu