Kouya SHIMURA wrote:
> Hi Jes,
>
> I wrote the patch corresponding to discontig. You are wrong.
> alloc_dir_page() must return a physical address because the page table
> walker for frame_table runs under off-state of data-address-translation.
> (i.e. psr.dt=0)
>
> p*d_populate() functions never be called while handling TLB miss to
> frame_table. The page table walker for frame_table is written in
> assembler in xen/arch/ia64/xen/ivt.S. See the code between
> 'GLOBAL_ENTRY(frametable_miss)' and 'END(frametable_miss)'
Hi Kouya,
I am not talking about during the TLB miss but during the creation
of the tables. I changed the code in ivt.S to make sure what was
happening was explicit, ie. if we are trying to resolve things in the
VIRT_FRAME_TABLE_ADDR space, then the test I put into ivt.S should be
identical to your original code, but I cannot boot with your code, it
MCA's because frametable_miss is never called since we are comparing for
the wrong address.
Please take a look at this code:
arch/ia64/xen/xenmem.c:
static int
create_frametable_page_table (u64 start, u64 end, void *arg)
{
...
[snip]
...
if (pud_none(*pud))
pud_populate(NULL, pud, alloc_dir_page());
pmd = pmd_offset(pud, address);
if (pmd_none(*pmd))
pmd_populate_kernel(NULL, pmd,
alloc_dir_page());
pte = pte_offset_kernel(pmd, address);
Then in include/asm-ia64/linux-xen/asm/pgalloc.h you have this:
static inline void
pud_populate(struct mm_struct *mm, pud_t * pud_entry, pmd_t * pmd)
{
pud_val(*pud_entry) = __pa(pmd);
}
.... and....
static inline void
pmd_populate_kernel(struct mm_struct *mm, pmd_t * pmd_entry, pte_t * pte)
{
pmd_val(*pmd_entry) = __pa(pte);
}
In other words, if alloc_dir_page() returns a physical address as it
did before, you end up doing __pa() on the physical address which
just cannot be correct.
My guess is that we were in fact doing a __pa() on the physical address
and the compare in ivt.S somehow was made to look at this address
instead of what it was pretending to do.
I'd like to be proven wrong though, but without this patch Xen on SN2
only gets to an MCA at the moment it tries to initialize the page table.
> You'd better have a look at the thread below:
> http://lists.xensource.com/archives/html/xen-ia64-devel/2006-04/msg00014.html
I'll take a look.
Cheers,
Jes
> Thanks,
> Kouya
>
> Jes Sorensen writes:
> > Hi,
> >
> > I sent this patch to Alex last week, but it didn't make it onto the list
> > because it's wrongly configured, so here we go again.
> >
> > I know that this patch is causing problems on ZX1, but I have looked at
> > it over and over again and I feel pretty certain it is correct. In fact
> > I cannot understand that Xen could boot on any ia64 platform prior to
> > this at all.
> >
> > I would be very interested in hearing how this patch affects other
> > platforms such as DIG and Fujitsu's machines (if they are not DIG :)
> >
> > Any input or comments on this is most welcome - if you think I am wrong
> > about this patch, please tell me, I really want to understand why this
> > worked in the past.
> >
> > Thanks,
> > Jes
> >
> > alloc_dir_page() must return a virtual address so it can handle being
> > passed to the p*d_populate() functions which do a __pa() on the address
> > before sticking them into the page tables.
> >
> > To match this it is also necessary to correctly check the faulting
> > address for being in the virtual frame table range, otherwise page
> > faults for this space weren't being served at all.
> >
> > This could probably be done more efficiently, but for now I think it's
> > better to keep the code explicit.
> >
> > Signed-off-by: Jes Sorensen <jes@xxxxxxx>
> >
> > diff -r 3e4fa8b5b245 xen/arch/ia64/xen/ivt.S
> > --- a/xen/arch/ia64/xen/ivt.S Tue Sep 12 11:43:22 2006 -0600
> > +++ b/xen/arch/ia64/xen/ivt.S Wed Sep 20 14:56:37 2006 +0200
> > @@ -542,8 +542,16 @@ late_alt_dtlb_miss:
> > ;;
> > #ifdef CONFIG_VIRTUAL_FRAME_TABLE
> > shr r22=r16,56 // Test for the address of virtual frame_table
> > +#if 1
> > + mov r23=VIRT_FRAME_TABLE_ADDR>>56
> > + ;;
> > + xor r23=r22,r23
> > + ;;
> > + cmp.eq p8,p0=r23,r0
> > +#else
> > ;;
> > cmp.eq p8,p0=((VIRT_FRAME_TABLE_ADDR>>56)&0xff)-0x100,r22
> > +#endif
> > (p8) br.cond.sptk frametable_miss ;;
> > #endif
> > // If it is not a Xen address, handle it via page_fault.
> > diff -r 3e4fa8b5b245 xen/arch/ia64/xen/xenmem.c
> > --- a/xen/arch/ia64/xen/xenmem.c Tue Sep 12 11:43:22 2006 -0600
> > +++ b/xen/arch/ia64/xen/xenmem.c Wed Sep 20 17:14:01 2006 +0200
> > @@ -76,13 +76,13 @@ alloc_dir_page(void)
> > alloc_dir_page(void)
> > {
> > unsigned long mfn = alloc_boot_pages(1, 1);
> > - unsigned long dir;
> > + unsigned char *virtual;
> > if (!mfn)
> > panic("Not enough memory for virtual frame table!\n");
> > ++table_size;
> > - dir = mfn << PAGE_SHIFT;
> > - memset(__va(dir), 0, PAGE_SIZE);
> > - return (void *)dir;
> > + virtual = __va(mfn << PAGE_SHIFT);
> > + memset(virtual, 0, PAGE_SIZE);
> > + return virtual;
> > }
> >
> > static inline unsigned long
> > _______________________________________________
> > Xen-ia64-devel mailing list
> > Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-ia64-devel
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home