| Message ID | 4a671e00687b44ae527acf19ea21da07380fdf42.1367188423.git.julien.grall@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote: > Currently xen doesn't implement SYS MMU. When a device will talk with dom0 > with DMA request the domain will use GFN instead of MFN. > For instance on the arndale board, without this patch the network doesn't > work. Yes :-/ Pragmatically I think we are better off with this short term hack than without support for the Arndale, but it's not terribly satisfactory. If there were any other platforms available I'd say that we should pick a platform for which the IOMMU docs were more easily forthcoming than they have proven to be on this platform. I'm slightly worried that we may get stuck with this hack. Perhaps we should say, prominently, that this hack will go away in 4.4 and that platforms which have not been converted to an IOMMU will not be supported from 4.4 onwards and that, yes, this could include the arndale unless docs and/or a vendor written driver appear. Of course we may end up eating crow if all the other platforms have the same problem, since we clearly can't remove support for all platforms... > The 1:1 mapping is a workaround and MUST be remove as soon as a SYS MMU is > implemented in XEN. I wonder if we can make this conditional on a suitable board level DT compat node ("samsung,arndale" or "samsung,exynos5250" perhaps)? And print a big warning when enabling it of course. > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/domain_build.c | 51 ++++++++++++++++++++++++------------------- > xen/arch/arm/kernel.h | 1 - > 2 files changed, 28 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index ced73a7..11298e1 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -66,29 +66,36 @@ static int set_memory_reg(struct domain *d, struct kernel_info *kinfo, > int address_cells, int size_cells, u32 *new_cell) > { > int reg_size = (address_cells + size_cells) * sizeof(*cell); > - int l = 0; > - u64 start; > - u64 size; > + paddr_t start; > + paddr_t size; > + struct page_info *pg; > + unsigned int order = get_order_from_bytes(dom0_mem); > + int res; > + paddr_t spfn; > > - while ( kinfo->unassigned_mem > 0 && l + reg_size <= len > - && kinfo->mem.nr_banks < NR_MEM_BANKS ) > - { > - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); > - if ( size > kinfo->unassigned_mem ) > - size = kinfo->unassigned_mem; > - device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); > - > - printk("Populate P2M %#"PRIx64"->%#"PRIx64"\n", start, start + size); > - p2m_populate_ram(d, start, start + size); > - kinfo->mem.bank[kinfo->mem.nr_banks].start = start; > - kinfo->mem.bank[kinfo->mem.nr_banks].size = size; > - kinfo->mem.nr_banks++; > - kinfo->unassigned_mem -= size; > - > - l += reg_size; > - } > + pg = alloc_domheap_pages(d, order, 0); > + if ( !pg ) > + panic("Failed to allocate contiguous memory for dom0\n"); Interesting, so we don't actually need RAM to start at the same place as the real hardware would have it and the kernel copes? Slight surprising the kernel didn't whine, but OK! > + spfn = page_to_mfn(pg); > + start = spfn << PAGE_SHIFT; > + size = (1 << order) << PAGE_SHIFT; > + > + // 1:1 mapping > + printk("Populate P2M %#"PRIx64"->%#"PRIx64" (1:1 mapping for dom0)\n", > + start, start + size); > + res = guest_physmap_add_page(d, spfn, spfn, order); > > - return l; > + if ( res ) > + panic("Unable to add pages in DOM0: %d\n", res); > + > + device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); > + > + kinfo->mem.bank[0].start = start; > + kinfo->mem.bank[0].size = size; > + kinfo->mem.nr_banks = 1; > + > + return reg_size; > } > > static int write_properties(struct domain *d, struct kernel_info *kinfo, > @@ -434,8 +441,6 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) > int new_size; > int ret; > > - kinfo->unassigned_mem = dom0_mem; > - > fdt = device_tree_flattened; > > new_size = fdt_totalsize(fdt) + DOM0_FDT_EXTRA_SIZE; > diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h > index 1776a4d..687f6c4 100644 > --- a/xen/arch/arm/kernel.h > +++ b/xen/arch/arm/kernel.h > @@ -15,7 +15,6 @@ struct kernel_info { > #endif > > void *fdt; /* flat device tree */ > - paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */ > struct dt_mem_info mem; > > paddr_t dtb_paddr;
On 04/29/2013 05:13 PM, Ian Campbell wrote: > On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote: >> Currently xen doesn't implement SYS MMU. When a device will talk with dom0 >> with DMA request the domain will use GFN instead of MFN. >> For instance on the arndale board, without this patch the network doesn't >> work. > > Yes :-/ Pragmatically I think we are better off with this short term > hack than without support for the Arndale, but it's not terribly > satisfactory. If there were any other platforms available I'd say that > we should pick a platform for which the IOMMU docs were more easily > forthcoming than they have proven to be on this platform. > > I'm slightly worried that we may get stuck with this hack. Perhaps we > should say, prominently, that this hack will go away in 4.4 and that > platforms which have not been converted to an IOMMU will not be > supported from 4.4 onwards and that, yes, this could include the arndale > unless docs and/or a vendor written driver appear. > > Of course we may end up eating crow if all the other platforms have the > same problem, since we clearly can't remove support for all platforms... > >> The 1:1 mapping is a workaround and MUST be remove as soon as a SYS MMU is >> implemented in XEN. > > I wonder if we can make this conditional on a suitable board level DT > compat node ("samsung,arndale" or "samsung,exynos5250" perhaps)? And > print a big warning when enabling it of course. What do you think about adding a workaround field in platform structure? It will contains PLATFORM_WORKAROUND_DOM0_11_MAPPING and perhaps other workaround if we really need it... This could avoid to check DT compat node for each platform where Xen need to do a 1:1 mapping for dom0. >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> --- >> xen/arch/arm/domain_build.c | 51 ++++++++++++++++++++++++------------------- >> xen/arch/arm/kernel.h | 1 - >> 2 files changed, 28 insertions(+), 24 deletions(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index ced73a7..11298e1 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -66,29 +66,36 @@ static int set_memory_reg(struct domain *d, struct kernel_info *kinfo, >> int address_cells, int size_cells, u32 *new_cell) >> { >> int reg_size = (address_cells + size_cells) * sizeof(*cell); >> - int l = 0; >> - u64 start; >> - u64 size; >> + paddr_t start; >> + paddr_t size; >> + struct page_info *pg; >> + unsigned int order = get_order_from_bytes(dom0_mem); >> + int res; >> + paddr_t spfn; >> >> - while ( kinfo->unassigned_mem > 0 && l + reg_size <= len >> - && kinfo->mem.nr_banks < NR_MEM_BANKS ) >> - { >> - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); >> - if ( size > kinfo->unassigned_mem ) >> - size = kinfo->unassigned_mem; >> - device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); >> - >> - printk("Populate P2M %#"PRIx64"->%#"PRIx64"\n", start, start + size); >> - p2m_populate_ram(d, start, start + size); >> - kinfo->mem.bank[kinfo->mem.nr_banks].start = start; >> - kinfo->mem.bank[kinfo->mem.nr_banks].size = size; >> - kinfo->mem.nr_banks++; >> - kinfo->unassigned_mem -= size; >> - >> - l += reg_size; >> - } >> + pg = alloc_domheap_pages(d, order, 0); >> + if ( !pg ) >> + panic("Failed to allocate contiguous memory for dom0\n"); > > Interesting, so we don't actually need RAM to start at the same place as > the real hardware would have it and the kernel copes? Slight surprising > the kernel didn't whine, but OK! I didn't see specific warning from the kernel. Why the kernel should whine if the memory banks have changed?
On Mon, 2013-04-29 at 18:43 +0100, Julien Grall wrote: > On 04/29/2013 05:13 PM, Ian Campbell wrote: > > > On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote: > >> Currently xen doesn't implement SYS MMU. When a device will talk with dom0 > >> with DMA request the domain will use GFN instead of MFN. > >> For instance on the arndale board, without this patch the network doesn't > >> work. > > > > Yes :-/ Pragmatically I think we are better off with this short term > > hack than without support for the Arndale, but it's not terribly > > satisfactory. If there were any other platforms available I'd say that > > we should pick a platform for which the IOMMU docs were more easily > > forthcoming than they have proven to be on this platform. > > > > I'm slightly worried that we may get stuck with this hack. Perhaps we > > should say, prominently, that this hack will go away in 4.4 and that > > platforms which have not been converted to an IOMMU will not be > > supported from 4.4 onwards and that, yes, this could include the arndale > > unless docs and/or a vendor written driver appear. > > > > Of course we may end up eating crow if all the other platforms have the > > same problem, since we clearly can't remove support for all platforms... > > > >> The 1:1 mapping is a workaround and MUST be remove as soon as a SYS MMU is > >> implemented in XEN. > > > > I wonder if we can make this conditional on a suitable board level DT > > compat node ("samsung,arndale" or "samsung,exynos5250" perhaps)? And > > print a big warning when enabling it of course. > > > What do you think about adding a workaround field in platform structure? > It will contains PLATFORM_WORKAROUND_DOM0_11_MAPPING and perhaps other > workaround if we really need it... > > This could avoid to check DT compat node for each platform where Xen > need to do a 1:1 mapping for dom0. Sure, if that works then that is even better. People tend to call these things QUIRKS rather than WORKAROUNDS. > > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >> --- > >> xen/arch/arm/domain_build.c | 51 ++++++++++++++++++++++++------------------- > >> xen/arch/arm/kernel.h | 1 - > >> 2 files changed, 28 insertions(+), 24 deletions(-) > >> > >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > >> index ced73a7..11298e1 100644 > >> --- a/xen/arch/arm/domain_build.c > >> +++ b/xen/arch/arm/domain_build.c > >> @@ -66,29 +66,36 @@ static int set_memory_reg(struct domain *d, struct kernel_info *kinfo, > >> int address_cells, int size_cells, u32 *new_cell) > >> { > >> int reg_size = (address_cells + size_cells) * sizeof(*cell); > >> - int l = 0; > >> - u64 start; > >> - u64 size; > >> + paddr_t start; > >> + paddr_t size; > >> + struct page_info *pg; > >> + unsigned int order = get_order_from_bytes(dom0_mem); > >> + int res; > >> + paddr_t spfn; > >> > >> - while ( kinfo->unassigned_mem > 0 && l + reg_size <= len > >> - && kinfo->mem.nr_banks < NR_MEM_BANKS ) > >> - { > >> - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); > >> - if ( size > kinfo->unassigned_mem ) > >> - size = kinfo->unassigned_mem; > >> - device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); > >> - > >> - printk("Populate P2M %#"PRIx64"->%#"PRIx64"\n", start, start + size); > >> - p2m_populate_ram(d, start, start + size); > >> - kinfo->mem.bank[kinfo->mem.nr_banks].start = start; > >> - kinfo->mem.bank[kinfo->mem.nr_banks].size = size; > >> - kinfo->mem.nr_banks++; > >> - kinfo->unassigned_mem -= size; > >> - > >> - l += reg_size; > >> - } > >> + pg = alloc_domheap_pages(d, order, 0); > >> + if ( !pg ) > >> + panic("Failed to allocate contiguous memory for dom0\n"); > > > > Interesting, so we don't actually need RAM to start at the same place as > > the real hardware would have it and the kernel copes? Slight surprising > > the kernel didn't whine, but OK! > > I didn't see specific warning from the kernel. Why the kernel should > whine if the memory banks have changed? I just imagined that the kernel would assume that PHYS_OFFSET for a given platform was static, e.g. memory on the platform starts at 0x80000000 but we are giving it an arbitrary 128M aligned region, e.g. stating at 0x88000000 or something, I'm just surprised something didn't break! Ian.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index ced73a7..11298e1 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -66,29 +66,36 @@ static int set_memory_reg(struct domain *d, struct kernel_info *kinfo, int address_cells, int size_cells, u32 *new_cell) { int reg_size = (address_cells + size_cells) * sizeof(*cell); - int l = 0; - u64 start; - u64 size; + paddr_t start; + paddr_t size; + struct page_info *pg; + unsigned int order = get_order_from_bytes(dom0_mem); + int res; + paddr_t spfn; - while ( kinfo->unassigned_mem > 0 && l + reg_size <= len - && kinfo->mem.nr_banks < NR_MEM_BANKS ) - { - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - if ( size > kinfo->unassigned_mem ) - size = kinfo->unassigned_mem; - device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); - - printk("Populate P2M %#"PRIx64"->%#"PRIx64"\n", start, start + size); - p2m_populate_ram(d, start, start + size); - kinfo->mem.bank[kinfo->mem.nr_banks].start = start; - kinfo->mem.bank[kinfo->mem.nr_banks].size = size; - kinfo->mem.nr_banks++; - kinfo->unassigned_mem -= size; - - l += reg_size; - } + pg = alloc_domheap_pages(d, order, 0); + if ( !pg ) + panic("Failed to allocate contiguous memory for dom0\n"); + + spfn = page_to_mfn(pg); + start = spfn << PAGE_SHIFT; + size = (1 << order) << PAGE_SHIFT; + + // 1:1 mapping + printk("Populate P2M %#"PRIx64"->%#"PRIx64" (1:1 mapping for dom0)\n", + start, start + size); + res = guest_physmap_add_page(d, spfn, spfn, order); - return l; + if ( res ) + panic("Unable to add pages in DOM0: %d\n", res); + + device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); + + kinfo->mem.bank[0].start = start; + kinfo->mem.bank[0].size = size; + kinfo->mem.nr_banks = 1; + + return reg_size; } static int write_properties(struct domain *d, struct kernel_info *kinfo, @@ -434,8 +441,6 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) int new_size; int ret; - kinfo->unassigned_mem = dom0_mem; - fdt = device_tree_flattened; new_size = fdt_totalsize(fdt) + DOM0_FDT_EXTRA_SIZE; diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h index 1776a4d..687f6c4 100644 --- a/xen/arch/arm/kernel.h +++ b/xen/arch/arm/kernel.h @@ -15,7 +15,6 @@ struct kernel_info { #endif void *fdt; /* flat device tree */ - paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */ struct dt_mem_info mem; paddr_t dtb_paddr;
Currently xen doesn't implement SYS MMU. When a device will talk with dom0 with DMA request the domain will use GFN instead of MFN. For instance on the arndale board, without this patch the network doesn't work. The 1:1 mapping is a workaround and MUST be remove as soon as a SYS MMU is implemented in XEN. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/domain_build.c | 51 ++++++++++++++++++++++++------------------- xen/arch/arm/kernel.h | 1 - 2 files changed, 28 insertions(+), 24 deletions(-)