Re: xen3pae on 5.0_stable/i386

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

On 23.01.10 02:53, Jean-Yves Migeon wrote:
> On 01/23/10 02:01, Christoph Egger wrote:
>> So we have address overflow in xennet and in xengnt.
>> Some more variables storing physical addresses must be of type paddr_t.
> [snip]

> 
> <side_note>
> On a more general perspective, MD code is messy with Xen and PAE,
> because there is an assumption that obtaining the page address is as
> easy as PAGE_SHIFTing the PFN, which is wrong. Xen handles PFNs as
> "unsigned long" and not "unsigned long long" in its API; if we are not
> careful enough, we may lose bits in the shift process. Bha.
> </side_note>

Thanks for the comments, Jean. Writing a patch at 1am is probably not
a good idea.

Attached patch should now work.

Mark: Please apply attached patch additionally to Manuel's patch,
recompile both dom0 and domu kernels and retry.

Jean, Manuel: We should introduce helper macros which hide the
needed casts completely. This makes new code less error prone.

Christoph

Index: sys/arch/xen/xen/xengnt.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/xengnt.c,v
retrieving revision 1.16
diff -u -p -r1.16 xengnt.c
--- sys/arch/xen/xen/xengnt.c   7 Nov 2009 07:27:49 -0000       1.16
+++ sys/arch/xen/xen/xengnt.c   23 Jan 2010 08:06:06 -0000
@@ -127,14 +127,14 @@ static int
 xengnt_more_entries(void)
 {
        gnttab_setup_table_t setup;
-       unsigned long *pages;
+       u_long *pages;
        int nframes_new = gnt_nr_grant_frames + 1;
        int i;
 
        if (gnt_nr_grant_frames == gnt_max_grant_frames)
                return ENOMEM;
 
-       pages = malloc(nframes_new * sizeof(long), M_DEVBUF, M_NOWAIT);
+       pages = malloc(nframes_new * sizeof(u_long), M_DEVBUF, M_NOWAIT);
        if (pages == NULL)
                return ENOMEM;
 
@@ -165,7 +165,8 @@ xengnt_more_entries(void)
         * the grant table frames
         */
        pmap_kenter_ma(((vaddr_t)grant_table) + gnt_nr_grant_frames * PAGE_SIZE,
-           pages[gnt_nr_grant_frames] << PAGE_SHIFT, VM_PROT_WRITE, 0);
+           ((paddr_t)pages[gnt_nr_grant_frames]) << PAGE_SHIFT,
+           VM_PROT_WRITE, 0);
 
        /*
         * add the grant entries associated to the last grant table frame
Index: sys/arch/xen/xen/xennetback_xenbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/xennetback_xenbus.c,v
retrieving revision 1.32
diff -u -p -r1.32 xennetback_xenbus.c
--- sys/arch/xen/xen/xennetback_xenbus.c        19 Jan 2010 22:06:23 -0000      
1.32
+++ sys/arch/xen/xen/xennetback_xenbus.c        23 Jan 2010 08:06:06 -0000
@@ -591,7 +591,7 @@ xennetback_get_mcl_page(paddr_t *map)
                 */
                return -1;
 
-       *map = mcl_pages[mcl_pages_alloc] << PAGE_SHIFT;
+       *map = ((paddr_t)mcl_pages[mcl_pages_alloc]) << PAGE_SHIFT;
        mcl_pages_alloc--;
        return 0;
        
Index: sys/arch/xen/x86/xen_bus_dma.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/x86/xen_bus_dma.c,v
retrieving revision 1.15
diff -u -p -r1.15 xen_bus_dma.c
--- sys/arch/xen/x86/xen_bus_dma.c      29 Jul 2009 12:02:08 -0000      1.15
+++ sys/arch/xen/x86/xen_bus_dma.c      23 Jan 2010 08:06:06 -0000
@@ -82,8 +82,8 @@ _xen_alloc_contig(bus_size_t size, bus_s
        KASSERT(npages >= npagesreq);
 
        /* get npages from UWM, and give them back to the hypervisor */
-       error = uvm_pglistalloc(npages << PAGE_SHIFT, 0, avail_end, 0, 0,
-           mlistp, npages, (flags & BUS_DMA_NOWAIT) == 0);
+       error = uvm_pglistalloc(((psize_t)npages) << PAGE_SHIFT,
+            0, avail_end, 0, 0, mlistp, npages, (flags & BUS_DMA_NOWAIT) == 0);
        if (error)
                return (error);
 
@@ -133,7 +133,7 @@ _xen_alloc_contig(bus_size_t size, bus_s
                pa = VM_PAGE_TO_PHYS(pg);
                xpmap_phys_to_machine_mapping[
                    (pa - XPMAP_OFFSET) >> PAGE_SHIFT] = mfn+i;
-               xpq_queue_machphys_update((mfn+i) << PAGE_SHIFT, pa);
+               xpq_queue_machphys_update(((paddr_t)(mfn+i)) << PAGE_SHIFT, pa);
                /* while here, give extra pages back to UVM */
                if (i >= npagesreq) {
                        TAILQ_REMOVE(mlistp, pg, pageq.queue);
@@ -179,7 +179,7 @@ failed:
                pa = VM_PAGE_TO_PHYS(pg);
                xpmap_phys_to_machine_mapping[
                    (pa - XPMAP_OFFSET) >> PAGE_SHIFT] = mfn;
-               xpq_queue_machphys_update((mfn) << PAGE_SHIFT, pa);
+               xpq_queue_machphys_update(((paddr_t)mfn) << PAGE_SHIFT, pa);
                TAILQ_REMOVE(mlistp, pg, pageq.queue);
                uvm_pagefree(pg);
        }