| Message ID | 1423058539-26403-28-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 5 February 2015 at 22:19, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Wed, 4 Feb 2015, parth.dixit@linaro.org wrote: >> From: Parth Dixit <parth.dixit@linaro.org> >> >> map mmio regions described in uefi tables to dom0 address space >> >> Signed-off-by: Parth Dixit <parth.dixit@linaro.org> >> --- >> xen/arch/arm/domain_build.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index d781c63..49eb52a 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -1235,6 +1235,56 @@ static int make_chosen_node(const struct domain *d, const struct kernel_info *ki >> return res; >> } >> >> +static int acpi_map_mmio(struct domain *d) >> +{ >> + int i,res; >> + u64 addr,size; >> + >> + for( i = 0; i < acpi_mmio.nr_banks; i++ ) >> + { >> + addr = acpi_mmio.bank[i].start; >> + size = acpi_mmio.bank[i].size; >> + >> + res = iomem_permit_access(d, paddr_to_pfn(addr & PAGE_MASK), >> + paddr_to_pfn(PAGE_ALIGN(addr + size - 1))); >> + if ( res ) >> + { >> + printk(XENLOG_ERR "Unable to permit to dom%d access to" >> + " 0x%"PRIx64" - 0x%"PRIx64"\n", >> + d->domain_id, >> + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1); >> + return res; >> + } >> + >> + res = map_mmio_regions(d, >> + paddr_to_pfn(addr & PAGE_MASK), >> + DIV_ROUND_UP(size, PAGE_SIZE), >> + paddr_to_pfn(addr & PAGE_MASK)); >> + if ( res ) >> + { >> + printk(XENLOG_ERR "Unable to map 0x%"PRIx64 >> + " - 0x%"PRIx64" in domain %d\n", >> + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1, >> + d->domain_id); >> + return res; >> + } >> + >> + } >> + >> + return 0; >> +} >> + >> +static int map_acpi_regions(struct domain *d) >> +{ >> + int res; >> + >> + res = acpi_map_mmio(d); >> + if ( res ) >> + return res; >> + >> + return 0; >> +} > > I don't think that splitting the code in two functions is useful. Just > implement the remapping here. ok, will take care in next patchset. > >> /* >> * Prepare a minimal DTB for DOM0 which contains >> * bootargs, memory information, >> @@ -1264,6 +1314,10 @@ static int prepare_dtb_acpi(struct domain *d, struct kernel_info *kinfo) >> if ( ret < 0 ) >> goto err; >> >> + ret = map_acpi_regions(d); >> + if ( ret < 0 ) >> + goto err; > > Do they also need to described in the mini dtb for dom0? > If not, how is dom0 going to retrieve the list of mmio regions? I guess > via the acpi tables? right , dom0 retrieves it via acpi tables. > >> ret = fdt_begin_node(kinfo->fdt, "/"); >> if ( ret < 0 ) >> goto err; >> -- >> 1.9.1 >>
On 06/02/2015 03:40, Parth Dixit wrote: >>> +static int map_acpi_regions(struct domain *d) >>> +{ >>> + int res; >>> + >>> + res = acpi_map_mmio(d); >>> + if ( res ) >>> + return res; >>> + >>> + return 0; >>> +} >> >> I don't think that splitting the code in two functions is useful. Just >> implement the remapping here. > ok, will take care in next patchset. I agree that the splitting doesn't make really sense right now (only one call is done). But the next following patches add few more calls (such has mapping ACPI blob...). So I would keep the splitting here. I would also rename the function to prepare_acpi to make clear that we setups ACPI for DOM0.
Hi Parth, On 04/02/2015 22:02, parth.dixit@linaro.org wrote: > From: Parth Dixit <parth.dixit@linaro.org> > > map mmio regions described in uefi tables to dom0 address space > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > --- > xen/arch/arm/domain_build.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d781c63..49eb52a 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1235,6 +1235,56 @@ static int make_chosen_node(const struct domain *d, const struct kernel_info *ki > return res; > } > > +static int acpi_map_mmio(struct domain *d) > +{ > + int i,res; > + u64 addr,size; Missing space after the comma. > + > + for( i = 0; i < acpi_mmio.nr_banks; i++ ) > + { > + addr = acpi_mmio.bank[i].start; > + size = acpi_mmio.bank[i].size; > + > + res = iomem_permit_access(d, paddr_to_pfn(addr & PAGE_MASK), There is multiple place within this loop which use paddr_to_pfn(addr & PAGE_MASK). I would create a temporary variable to store the value and use it everywhere. > + paddr_to_pfn(PAGE_ALIGN(addr + size - 1))); The indentation is wrong. > + if ( res ) > + { > + printk(XENLOG_ERR "Unable to permit to dom%d access to" > + " 0x%"PRIx64" - 0x%"PRIx64"\n", > + d->domain_id, > + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1); > + return res; > + } > + > + res = map_mmio_regions(d, > + paddr_to_pfn(addr & PAGE_MASK), > + DIV_ROUND_UP(size, PAGE_SIZE), > + paddr_to_pfn(addr & PAGE_MASK)); Ditto Regards,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d781c63..49eb52a 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1235,6 +1235,56 @@ static int make_chosen_node(const struct domain *d, const struct kernel_info *ki return res; } +static int acpi_map_mmio(struct domain *d) +{ + int i,res; + u64 addr,size; + + for( i = 0; i < acpi_mmio.nr_banks; i++ ) + { + addr = acpi_mmio.bank[i].start; + size = acpi_mmio.bank[i].size; + + res = iomem_permit_access(d, paddr_to_pfn(addr & PAGE_MASK), + paddr_to_pfn(PAGE_ALIGN(addr + size - 1))); + if ( res ) + { + printk(XENLOG_ERR "Unable to permit to dom%d access to" + " 0x%"PRIx64" - 0x%"PRIx64"\n", + d->domain_id, + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1); + return res; + } + + res = map_mmio_regions(d, + paddr_to_pfn(addr & PAGE_MASK), + DIV_ROUND_UP(size, PAGE_SIZE), + paddr_to_pfn(addr & PAGE_MASK)); + if ( res ) + { + printk(XENLOG_ERR "Unable to map 0x%"PRIx64 + " - 0x%"PRIx64" in domain %d\n", + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1, + d->domain_id); + return res; + } + + } + + return 0; +} + +static int map_acpi_regions(struct domain *d) +{ + int res; + + res = acpi_map_mmio(d); + if ( res ) + return res; + + return 0; +} + /* * Prepare a minimal DTB for DOM0 which contains * bootargs, memory information, @@ -1264,6 +1314,10 @@ static int prepare_dtb_acpi(struct domain *d, struct kernel_info *kinfo) if ( ret < 0 ) goto err; + ret = map_acpi_regions(d); + if ( ret < 0 ) + goto err; + ret = fdt_begin_node(kinfo->fdt, "/"); if ( ret < 0 ) goto err;