>The fourth patch modifies pci-gart-xen.c and
>aperture-xen.c to work with Xen. It adds a
>hypervisor call to clear the aperture address
>range from the hypervisor's memory tables,
>which is necessary to avoid a cache coherency
>issue, and ups the range of contiguous memory
>that Xen provides, which is necessary since
>the aperture must be at least 32 MB.
>--- a/linux-2.6-xen-sparse/arch/i386/mm/hypervisor.c Thu Mar 01 15:04:47
>2007 -0600
>+++ b/linux-2.6-xen-sparse/arch/i386/mm/hypervisor.c Mon Mar 05 16:01:11
>2007 -0600
>@@ -252,7 +252,7 @@ static void contiguous_bitmap_clear(
> }
>
> /* Protected by balloon_lock. */
>-#define MAX_CONTIG_ORDER 9 /* 2MB */
>+#define MAX_CONTIG_ORDER 16 /* 256MB */
> static unsigned long discontig_frames[1<<MAX_CONTIG_ORDER];
> static multicall_entry_t cr_mcl[1<<MAX_CONTIG_ORDER];
Keir had asked you to not statically bump MAX_CONTIG_ORDER here as a
minimal acceptance criteria.
>--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/aperture-xen.c Thu Mar 01
>15:04:47 2007 -0600
>+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/aperture-xen.c Mon Mar 05
>16:01:11 2007 -0600
>@@ -53,7 +53,7 @@ static u32 __init allocate_aperture(void
> * IOMMU useless.
> */
> p = __alloc_bootmem_node(nd0, aper_size, aper_size, 0);
>- if (!p || __pa(p)+aper_size > 0xffffffff) {
>+ if (!p || (xen_create_contiguous_region((unsigned long) p,
>get_order(aper_size), 31) != 0) ||
virt_to_bus(p)+aper_size >
>0xffffffff) {
> printk("Cannot allocate aperture memory hole (%p,%uK)\n",
> p, aper_size>>10);
> if (p)
Why 31 when native code uses a check for 32 bits?
Also, I don't think the range check is necessary after the call to
xen_create_contiguous_region() - if at all you should BUG() if the
call was successful but the resulting address isn't fitting.
>--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/e820-xen.c Thu Mar 01
>15:04:47 2007 -0600
>+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/e820-xen.c Mon Mar 05
>16:01:11 2007 -0600
>@@ -97,7 +97,6 @@ static inline int bad_addr(unsigned long
> return 0;
> }
>
>-#ifndef CONFIG_XEN
> /*
> * This function checks if any part of the range <start,end> is mapped
> * with type.
>@@ -116,7 +115,6 @@ e820_any_mapped(unsigned long start, uns
> }
> return 0;
> }
>-#endif
>
> /*
> * This function checks if the entire range <start,end> is mapped with type.
Simply enabling this function is insufficient: It must check against
machine_e820
for Xen (see e820_all_mapped), which is populated only under
is_initial_xen_domain() (though that's not a problem, as domU-s shouldn't find
any AGP bridge in the first place).
>--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-gart-xen.c Thu Mar 01
>15:04:47 2007 -0600
>+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-gart-xen.c Mon Mar 05
>16:01:11 2007 -0600
>...
>@@ -523,6 +523,9 @@ static __init int init_k8_gatt(struct ag
> gatt = (void *)__get_free_pages(GFP_KERNEL, get_order(gatt_size));
> if (!gatt)
> panic("Cannot allocate GATT table");
>+ if (xen_create_contiguous_region((unsigned long) gatt,
>get_order(gatt_size), 31))
>+ panic("Cannot create a contiguous memory region below 4GB for
>the GATT table");
>+
> memset(gatt, 0, gatt_size);
> agp_gatt_table = gatt;
>
Why 31, and more importantly, why a restriction at all? Native uses GFP_KERNEL
(not GFP_DMA32), so why is there a restriction here under Xen?
>@@ -531,7 +534,7 @@ static __init int init_k8_gatt(struct ag
> u32 gatt_reg;
>
> dev = k8_northbridges[i];
>- gatt_reg = __pa(gatt) >> 12;
>+ gatt_reg = phys_to_machine(__pa(gatt)) >> 12;
> gatt_reg <<= 4;
> pci_write_config_dword(dev, 0x98, gatt_reg);
> pci_read_config_dword(dev, 0x90, &ctl);
Wouldn't virt_to_bus() be more appropriate/consistent here?
>--- a/linux-2.6-xen-sparse/arch/x86_64/mm/init-xen.c Thu Mar 01 15:04:47
>2007 -0600
>+++ b/linux-2.6-xen-sparse/arch/x86_64/mm/init-xen.c Mon Mar 05 16:01:11
>2007 -0600
>...
>@@ -855,13 +856,10 @@ void __init clear_kernel_mapping(unsigne
> pmd = pmd_offset(pud, address);
> if (!pmd || pmd_none(*pmd))
> continue;
>- if (0 == (pmd_val(*pmd) & _PAGE_PSE)) {
>- /* Could handle this, but it should not happen
>currently. */
>- printk(KERN_ERR
>- "clear_kernel_mapping: mapping has been split. will leak
>memory\n");
>- pmd_ERROR(*pmd);
>- }
>- set_pmd(pmd, __pmd(0));
>+ pte = pte_offset_map(pmd, address);
>+ if (!pte || pte_none(*pte))
>+ continue;
>+ pte_clear(&init_mm,address,pte);
I'd prefer keeping the original code by means of a #ifndef CONFIG_XEN
construct.
>--- a/xen/arch/x86/mm.c Thu Mar 01 15:04:47 2007 -0600
>+++ b/xen/arch/x86/mm.c Mon Mar 05 16:01:11 2007 -0600
>@@ -2098,6 +2098,13 @@ int do_mmuext_op(
> }
> }
> break;
>+
>+ case MMUEXT_UNMAP_REGION:
>+ map_pages_to_xen(
>+ PAGE_OFFSET + (op.arg1.mfn << PAGE_SHIFT),
>+ op.arg1.mfn, op.arg2.nr_ents, PAGE_HYPERVISOR &
>+ ~_PAGE_PRESENT);
>+ break;
> #endif
>
> case MMUEXT_TLB_FLUSH_LOCAL:
This must be refused for non-privileged domains. I would also think that it
should be verified that the pages are not in use elsewhere.
Finally, hypervisor sources use 4-space indentation, not tabs.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home