diff mbox

[Xen-devel,RFC,27/35] arm: acpi map mmio regions to dom0

Message ID 1423058539-26403-28-git-send-email-parth.dixit@linaro.org
State New
Headers show

Commit Message

Parth Dixit Feb. 4, 2015, 2:02 p.m. UTC
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(+)

Comments

Parth Dixit Feb. 5, 2015, 7:40 p.m. UTC | #1
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
>>
Julien Grall Feb. 6, 2015, 12:44 a.m. UTC | #2
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.
Julien Grall Feb. 11, 2015, 9:26 a.m. UTC | #3
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 mbox

Patch

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;