Re: xen3pae on 5.0_stable/i386

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Fri, Jan 22, 2010 at 01:27:54PM +0100, Christoph Egger wrote:
> 
> > > With a xen-debug.gz kernel, does anything show up in
> > > 'xm dmesg' after the core dump ?
> > 
> > This appears at the end of dmesg:
> > 
> > 
> > 
> 
> [snip]
> 
> > (XEN) mm.c:1608:d0 Error pfn 128eb: rd=ffbd4100, od=00000000, 
> > caf=00000000, taf=00000000
> > (XEN) mm.c:649:d0 Error getting mfn 128eb (pfn 55555555) from L1 entry 
> > 00000000128eb067 for dom1
> > (XEN) mm.c:1608:d0 Error pfn d45a: rd=ffbd4100, od=00000000, 
> > caf=00000000, taf=00000000
> > (XEN) mm.c:649:d0 Error getting mfn d45a (pfn 55555555) from L1 entry 
> > 000000000d45a067 for dom1
> 
> 
> Manuell: In sys/arch/xen/xen/privcmd.c I found maddr
> and mfn variables storing machine physical addresses.
> They are of type u_long, int or paddr_t.
> IMO, they should always be of type paddr_t.

We can't really change the type of mfn as it's part of the ABI (or closely
related). A 32bit value should be enough here as it's not in bytes but
in pages, a 32bit value gives us 44 bytes physical addresses which should
be enough.

But it seems there's (paddr_t) cast missing when converting mfn to paddr,
and a 'ma' variable is u_long which should be a paddr_t.
Mark, can you try the attached patch ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: xen/privcmd.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/privcmd.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 privcmd.c
--- xen/privcmd.c       23 Oct 2009 02:32:34 -0000      1.39
+++ xen/privcmd.c       22 Jan 2010 14:26:40 -0000
@@ -336,7 +336,7 @@ privcmd_ioctl(void *v)
                privcmd_mmap_t *mcmd = ap->a_data;
                privcmd_mmap_entry_t mentry;
                vaddr_t va;
-               u_long ma;
+               paddr_t ma;
                struct vm_map *vmm = &curlwp->l_proc->p_vmspace->vm_map;
 
                for (i = 0; i < mcmd->num; i++) {
@@ -357,7 +357,7 @@ privcmd_ioctl(void *v)
                        if (maddr == NULL)
                                return ENOMEM;
                        va = mentry.va & ~PAGE_MASK;
-                       ma = mentry.mfn <<  PGSHIFT; /* XXX ??? */
+                       ma = ((paddr_t)mentry.mfn) <<  PGSHIFT; /* XXX ??? */
                        for (j = 0; j < mentry.npages; j++) {
                                maddr[j] = ma;
                                ma += PAGE_SIZE;
@@ -374,7 +374,8 @@ privcmd_ioctl(void *v)
                int i;
                privcmd_mmapbatch_t* pmb = ap->a_data;
                vaddr_t va0, va;
-               u_long mfn, ma;
+               u_long mfn;
+               paddr_t ma;
                struct vm_map *vmm;
                struct vm_map_entry *entry;
                vm_prot_t prot;
@@ -420,7 +421,7 @@ privcmd_ioctl(void *v)
                                    UVM_KMF_VAONLY);
                                return error;
                        }
-                       ma = mfn << PGSHIFT;
+                       ma = ((paddr_t)mfn) << PGSHIFT;
                        if (pmap_enter_ma(pmap_kernel(), trymap, ma, 0,
                            prot, PMAP_CANFAIL, pmb->dom)) {
                                mfn |= 0xF0000000;