diff mbox

[Xen-devel,for-4.9] xen/arm: acpi: Map MMIO on fault in stage-2 page table for the hardware domain

Message ID 20170330124306.17357-1-julien.grall@arm.com
State Superseded
Headers show

Commit Message

Julien Grall March 30, 2017, 12:43 p.m. UTC
When booting using ACPI, not all MMIOs can be discovered by parsing the
static tables or the UEFI memory map. A lot of them will be described in
the DSDT. However, Xen does not have an AML parser which requires us to
find a different approach.

During the first discussions on supporting ACPI (see design doc [1]), it
was decided to rely on the hardware domain to make a request to the
hypervisor to map the MMIO region in stage-2 page table before accessing
it. This approach works fine if the OS has limited hooks to modify the
page tables.

In the case of Linux kernel, notifiers have been added to map
the MMIO regions when adding a new AMBA/platform device. Whilst this is
covering most of the MMIOs, some of them (e.g OpRegion, ECAM...) are not
related to a specific device or the driver is not using the
AMBA/platform API. So more hooks would need to be added in the code.

Various approaches have been discussed (see [2]), one of them was to
create stage-2 mappings seamlessly in Xen upon hardware memory faults.
This approach was first ruled out because it relies on the hardware
domain to probe the region before any use. So this would not work when
DMA'ing to another device's MMIO region when the device is protected by
an SMMU. It has been pointed out that this is a limited use case compare
to DMA'ing between MMIO and RAM.

This patch implements this approach. All MMIOs region will be mapped in
stage-2 using p2m_mmio_direct_c (i.e normal memory outer and inner
write-back cacheable). The stage-1 page table will be in control of the
memory attribute. This is fine because the hardware domain is a trusted
domain.

Note that MMIO will only be mapped on a data abort fault. It is assumed
that it will not be possible to execute code from MMIO
(p2m_mmio_direct_c will forbid that).

As mentioned above, this solution will cover most of the cases. If a
platform requires to do DMA'ing to another device's MMIO region without
any access performed by the OS. Then it will be expected to have
specific platform code in the hypervisor to map the MMIO at boot time or
the OS to use the existing hypercalls (i.e XENMEM_add_to_add_physmap{,_batch})
before any access.

[1] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html
[2] https://marc.info/?l=linux-arm-kernel&m=148469169210500&w=2

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    This patch is a candidate for Xen 4.9. Whilst the last posting date
    was few days ago, I think it would be good to have this patch in the
    next release. Although ACPI support for ARM64 is still a technical
    preview, it would help servers to test Xen upstream on their platform
    with ACPI and provide feedback on missing features.

    All the code is gated by !acpi_disabled and therefore will not be
    executed on when using Device Tree.

    RM hat on: I will leave the decision to Stefano.

    Shanker: You mentioned offline that you tested the patch. May I add
    your tested-by?

---
 xen/arch/arm/traps.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Shanker Donthineni March 30, 2017, 12:52 p.m. UTC | #1
Hi Julien,


On 03/30/2017 07:43 AM, Julien Grall wrote:
> When booting using ACPI, not all MMIOs can be discovered by parsing the
> static tables or the UEFI memory map. A lot of them will be described in
> the DSDT. However, Xen does not have an AML parser which requires us to
> find a different approach.
>
> During the first discussions on supporting ACPI (see design doc [1]), it
> was decided to rely on the hardware domain to make a request to the
> hypervisor to map the MMIO region in stage-2 page table before accessing
> it. This approach works fine if the OS has limited hooks to modify the
> page tables.
>
> In the case of Linux kernel, notifiers have been added to map
> the MMIO regions when adding a new AMBA/platform device. Whilst this is
> covering most of the MMIOs, some of them (e.g OpRegion, ECAM...) are not
> related to a specific device or the driver is not using the
> AMBA/platform API. So more hooks would need to be added in the code.
>
> Various approaches have been discussed (see [2]), one of them was to
> create stage-2 mappings seamlessly in Xen upon hardware memory faults.
> This approach was first ruled out because it relies on the hardware
> domain to probe the region before any use. So this would not work when
> DMA'ing to another device's MMIO region when the device is protected by
> an SMMU. It has been pointed out that this is a limited use case compare
> to DMA'ing between MMIO and RAM.
>
> This patch implements this approach. All MMIOs region will be mapped in
> stage-2 using p2m_mmio_direct_c (i.e normal memory outer and inner
> write-back cacheable). The stage-1 page table will be in control of the
> memory attribute. This is fine because the hardware domain is a trusted
> domain.
>
> Note that MMIO will only be mapped on a data abort fault. It is assumed
> that it will not be possible to execute code from MMIO
> (p2m_mmio_direct_c will forbid that).
>
> As mentioned above, this solution will cover most of the cases. If a
> platform requires to do DMA'ing to another device's MMIO region without
> any access performed by the OS. Then it will be expected to have
> specific platform code in the hypervisor to map the MMIO at boot time or
> the OS to use the existing hypercalls (i.e XENMEM_add_to_add_physmap{,_batch})
> before any access.
>
> [1] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html
> [2] https://marc.info/?l=linux-arm-kernel&m=148469169210500&w=2
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> ---
>     This patch is a candidate for Xen 4.9. Whilst the last posting date
>     was few days ago, I think it would be good to have this patch in the
>     next release. Although ACPI support for ARM64 is still a technical
>     preview, it would help servers to test Xen upstream on their platform
>     with ACPI and provide feedback on missing features.
>
>     All the code is gated by !acpi_disabled and therefore will not be
>     executed on when using Device Tree.
>
>     RM hat on: I will leave the decision to Stefano.
>
>     Shanker: You mentioned offline that you tested the patch. May I add
>     your tested-by?

Sure add my tested-by. If you want I can test one more time this mailing list patch.
Julien Grall March 30, 2017, 12:58 p.m. UTC | #2
On 30/03/17 13:52, Shanker Donthineni wrote:
> Hi Julien,

Hi Shanker,

> On 03/30/2017 07:43 AM, Julien Grall wrote:
>>     Shanker: You mentioned offline that you tested the patch. May I add
>>     your tested-by?
> 
> Sure add my tested-by. If you want I can test one more time this mailing list patch.

The code is the same, but it would not hurt to have another go at it :).

FWIW, I did some testing on Juno with a small patch to turn the mapping
hypercall into a nop and adding a debug message when mapping:



Cheers,diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 626376090d..dd1b0bd2b2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1116,6 +1116,8 @@ int map_dev_mmio_region(struct domain *d,
 {
     int res;
 
+    return 0;
+
     if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
         return 0;
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ebf915bb3b..9e6d2dafc8 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2643,7 +2643,11 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
             return;
 
         if ( try_map_mmio(_gfn(paddr_to_pfn(info.gpa))) )
+        {
+            gdprintk(XENLOG_DEBUG, "Map %#"PRIpaddr" - %#"PRIpaddr"\n",
+                     info.gpa, info.gpa + PAGE_SIZE - 1);
             return;
+        }
 
         break;
     default:

Shanker Donthineni March 30, 2017, 1:15 p.m. UTC | #3
Hi Julien,


On 03/30/2017 07:58 AM, Julien Grall wrote:
>
> On 30/03/17 13:52, Shanker Donthineni wrote:
>> Hi Julien,
> Hi Shanker,
>
>> On 03/30/2017 07:43 AM, Julien Grall wrote:
>>>     Shanker: You mentioned offline that you tested the patch. May I add
>>>     your tested-by?
>> Sure add my tested-by. If you want I can test one more time this mailing list patch.
> The code is the same, but it would not hurt to have another go at it :).
>
> FWIW, I did some testing on Juno with a small patch to turn the mapping
> hypercall into a nop and adding a debug message when mapping:
>

Tested-by: Shanker Donthineni <shankerd@codeaurora.org>

This patch has been tested on Qualcomm Datacenter Technologies QDF2400 server platform with ACPI based XEN/DOM0.

> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 626376090d..dd1b0bd2b2 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1116,6 +1116,8 @@ int map_dev_mmio_region(struct domain *d,
>  {
>      int res;
>  
> +    return 0;
> +
>      if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
>          return 0;
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index ebf915bb3b..9e6d2dafc8 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2643,7 +2643,11 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>              return;
>  
>          if ( try_map_mmio(_gfn(paddr_to_pfn(info.gpa))) )
> +        {
> +            gdprintk(XENLOG_DEBUG, "Map %#"PRIpaddr" - %#"PRIpaddr"\n",
> +                     info.gpa, info.gpa + PAGE_SIZE - 1);
>              return;
> +        }
>  
>          break;
>      default:
>
> Cheers,
>
Stefano Stabellini March 30, 2017, 6:37 p.m. UTC | #4
On Thu, 30 Mar 2017, Julien Grall wrote:
> When booting using ACPI, not all MMIOs can be discovered by parsing the
> static tables or the UEFI memory map. A lot of them will be described in
> the DSDT. However, Xen does not have an AML parser which requires us to
> find a different approach.
> 
> During the first discussions on supporting ACPI (see design doc [1]), it
> was decided to rely on the hardware domain to make a request to the
> hypervisor to map the MMIO region in stage-2 page table before accessing
> it. This approach works fine if the OS has limited hooks to modify the
> page tables.
> 
> In the case of Linux kernel, notifiers have been added to map
> the MMIO regions when adding a new AMBA/platform device. Whilst this is
> covering most of the MMIOs, some of them (e.g OpRegion, ECAM...) are not
> related to a specific device or the driver is not using the
> AMBA/platform API. So more hooks would need to be added in the code.
> 
> Various approaches have been discussed (see [2]), one of them was to
> create stage-2 mappings seamlessly in Xen upon hardware memory faults.
> This approach was first ruled out because it relies on the hardware
> domain to probe the region before any use. So this would not work when
> DMA'ing to another device's MMIO region when the device is protected by
> an SMMU. It has been pointed out that this is a limited use case compare
> to DMA'ing between MMIO and RAM.
> 
> This patch implements this approach. All MMIOs region will be mapped in
> stage-2 using p2m_mmio_direct_c (i.e normal memory outer and inner
> write-back cacheable). The stage-1 page table will be in control of the
> memory attribute. This is fine because the hardware domain is a trusted
> domain.
> 
> Note that MMIO will only be mapped on a data abort fault. It is assumed
> that it will not be possible to execute code from MMIO
> (p2m_mmio_direct_c will forbid that).
> 
> As mentioned above, this solution will cover most of the cases. If a
> platform requires to do DMA'ing to another device's MMIO region without
> any access performed by the OS. Then it will be expected to have
> specific platform code in the hypervisor to map the MMIO at boot time or
> the OS to use the existing hypercalls (i.e XENMEM_add_to_add_physmap{,_batch})
> before any access.
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html
> [2] https://marc.info/?l=linux-arm-kernel&m=148469169210500&w=2
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     This patch is a candidate for Xen 4.9. Whilst the last posting date
>     was few days ago, I think it would be good to have this patch in the
>     next release. Although ACPI support for ARM64 is still a technical
>     preview, it would help servers to test Xen upstream on their platform
>     with ACPI and provide feedback on missing features.
> 
>     All the code is gated by !acpi_disabled and therefore will not be
>     executed on when using Device Tree.
> 
>     RM hat on: I will leave the decision to Stefano.
> 
>     Shanker: You mentioned offline that you tested the patch. May I add
>     your tested-by?
> 
> ---
>  xen/arch/arm/traps.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 614501f761..ebf915bb3b 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -32,6 +32,7 @@
>  #include <xen/perfc.h>
>  #include <xen/virtual_region.h>
>  #include <xen/mem_access.h>
> +#include <xen/iocap.h>
>  #include <public/sched.h>
>  #include <public/xen.h>
>  #include <asm/debugger.h>
> @@ -49,6 +50,7 @@
>  #include <asm/gic.h>
>  #include <asm/vgic.h>
>  #include <asm/cpuerrata.h>
> +#include <asm/acpi.h>
>  
>  /* The base of the stack must always be double-word aligned, which means
>   * that both the kernel half of struct cpu_user_regs (which is pushed in
> @@ -2534,6 +2536,35 @@ static bool try_handle_mmio(struct cpu_user_regs *regs,
>      return !!handle_mmio(info);
>  }
>  
> +/*
> + * When using ACPI, most of the MMIO regions will be mapped on-demand
> + * in stage-2 page tables for the hardware domain because Xen is not
> + * able to know from the EFI memory map the MMIO regions.
> + */
> +static bool try_map_mmio(gfn_t gfn)
> +{
> +    struct domain *d = current->domain;
> +
> +    /* For the hardware domain, all MMIOs are mapped with GFN == MFN */
> +    mfn_t mfn = _mfn(gfn_x(gfn));
> +
> +    /*
> +     * Device-Tree should already have everything mapped when building
> +     * the hardware domain.
> +     */
> +    if ( acpi_disabled )
> +        return false;
> +
> +    if ( d != hardware_domain )

is_hardware_domain(d)


> +        return false;
> +
> +    /* The hardware domain can only map permitted MMIO regions */
> +    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
> +        return false;

Because of the potential of causing problems with DMA to device MMIO,
I would add a warning here, something like:

   dprintk(XENLOG_WARNING, "Mapping gfn=mfn=%"PRI_gfn" to Dom%d\n", d->domain_id);

I hope that it will help us debug issues when/if we'll actually
encounter such a case.


> +    return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c);
> +}
> +
>  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>                                       const union hsr hsr)
>  {
> @@ -2611,6 +2642,9 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>          if ( !mfn_eq(mfn, INVALID_MFN) )
>              return;
>  
> +        if ( try_map_mmio(_gfn(paddr_to_pfn(info.gpa))) )
> +            return;
> +
>          break;
>      default:
>          gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",
> -- 
> 2.11.0
>
Julien Grall March 30, 2017, 6:45 p.m. UTC | #5
Hi Stefano,

On 30/03/17 19:37, Stefano Stabellini wrote:
> On Thu, 30 Mar 2017, Julien Grall wrote:
>> +/*
>> + * When using ACPI, most of the MMIO regions will be mapped on-demand
>> + * in stage-2 page tables for the hardware domain because Xen is not
>> + * able to know from the EFI memory map the MMIO regions.
>> + */
>> +static bool try_map_mmio(gfn_t gfn)
>> +{
>> +    struct domain *d = current->domain;
>> +
>> +    /* For the hardware domain, all MMIOs are mapped with GFN == MFN */
>> +    mfn_t mfn = _mfn(gfn_x(gfn));
>> +
>> +    /*
>> +     * Device-Tree should already have everything mapped when building
>> +     * the hardware domain.
>> +     */
>> +    if ( acpi_disabled )
>> +        return false;
>> +
>> +    if ( d != hardware_domain )
>
> is_hardware_domain(d)

Will fix it.

>
>> +        return false;
>> +
>> +    /* The hardware domain can only map permitted MMIO regions */
>> +    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
>> +        return false;
>
> Because of the potential of causing problems with DMA to device MMIO,
> I would add a warning here, something like:
>
>    dprintk(XENLOG_WARNING, "Mapping gfn=mfn=%"PRI_gfn" to Dom%d\n", d->domain_id);
>
> I hope that it will help us debug issues when/if we'll actually
> encounter such a case.

I don't think this will help us and it will flood the console in 
debug-build (there are a lot of MMIO to map).

When SMMUs are not used, DMA to device MMIO will work regardless the 
mapping in stage-2.

When SMMUs are in use, you will get a SMMU fault that tells you the 
address used. With that you could know the region was not mapped.

Cheers,
Stefano Stabellini March 30, 2017, 6:55 p.m. UTC | #6
On Thu, 30 Mar 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 30/03/17 19:37, Stefano Stabellini wrote:
> > On Thu, 30 Mar 2017, Julien Grall wrote:
> > > +/*
> > > + * When using ACPI, most of the MMIO regions will be mapped on-demand
> > > + * in stage-2 page tables for the hardware domain because Xen is not
> > > + * able to know from the EFI memory map the MMIO regions.
> > > + */
> > > +static bool try_map_mmio(gfn_t gfn)
> > > +{
> > > +    struct domain *d = current->domain;
> > > +
> > > +    /* For the hardware domain, all MMIOs are mapped with GFN == MFN */
> > > +    mfn_t mfn = _mfn(gfn_x(gfn));
> > > +
> > > +    /*
> > > +     * Device-Tree should already have everything mapped when building
> > > +     * the hardware domain.
> > > +     */
> > > +    if ( acpi_disabled )
> > > +        return false;
> > > +
> > > +    if ( d != hardware_domain )
> > 
> > is_hardware_domain(d)
> 
> Will fix it.
> 
> > 
> > > +        return false;
> > > +
> > > +    /* The hardware domain can only map permitted MMIO regions */
> > > +    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
> > > +        return false;
> > 
> > Because of the potential of causing problems with DMA to device MMIO,
> > I would add a warning here, something like:
> > 
> >    dprintk(XENLOG_WARNING, "Mapping gfn=mfn=%"PRI_gfn" to Dom%d\n",
> > d->domain_id);
> > 
> > I hope that it will help us debug issues when/if we'll actually
> > encounter such a case.
> 
> I don't think this will help us and it will flood the console in debug-build
> (there are a lot of MMIO to map).
> 
> When SMMUs are not used, DMA to device MMIO will work regardless the mapping
> in stage-2.
> 
> When SMMUs are in use, you will get a SMMU fault that tells you the address
> used. With that you could know the region was not mapped.
 
Fair enough. Do you think there is any value in adding only one debug
warning, to say that we are mapping MMIOs on trap on this platform? But
today, it would basically be the same as saying "ACPI is enabled".
Julien Grall March 30, 2017, 7:54 p.m. UTC | #7
Hi Stefano,

On 30/03/2017 19:55, Stefano Stabellini wrote:
> On Thu, 30 Mar 2017, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 30/03/17 19:37, Stefano Stabellini wrote:
>>> On Thu, 30 Mar 2017, Julien Grall wrote:
>>>> +/*
>>>> + * When using ACPI, most of the MMIO regions will be mapped on-demand
>>>> + * in stage-2 page tables for the hardware domain because Xen is not
>>>> + * able to know from the EFI memory map the MMIO regions.
>>>> + */
>>>> +static bool try_map_mmio(gfn_t gfn)
>>>> +{
>>>> +    struct domain *d = current->domain;
>>>> +
>>>> +    /* For the hardware domain, all MMIOs are mapped with GFN == MFN */
>>>> +    mfn_t mfn = _mfn(gfn_x(gfn));
>>>> +
>>>> +    /*
>>>> +     * Device-Tree should already have everything mapped when building
>>>> +     * the hardware domain.
>>>> +     */
>>>> +    if ( acpi_disabled )
>>>> +        return false;
>>>> +
>>>> +    if ( d != hardware_domain )
>>>
>>> is_hardware_domain(d)
>>
>> Will fix it.
>>
>>>
>>>> +        return false;
>>>> +
>>>> +    /* The hardware domain can only map permitted MMIO regions */
>>>> +    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
>>>> +        return false;
>>>
>>> Because of the potential of causing problems with DMA to device MMIO,
>>> I would add a warning here, something like:
>>>
>>>    dprintk(XENLOG_WARNING, "Mapping gfn=mfn=%"PRI_gfn" to Dom%d\n",
>>> d->domain_id);
>>>
>>> I hope that it will help us debug issues when/if we'll actually
>>> encounter such a case.
>>
>> I don't think this will help us and it will flood the console in debug-build
>> (there are a lot of MMIO to map).
>>
>> When SMMUs are not used, DMA to device MMIO will work regardless the mapping
>> in stage-2.
>>
>> When SMMUs are in use, you will get a SMMU fault that tells you the address
>> used. With that you could know the region was not mapped.
>
> Fair enough. Do you think there is any value in adding only one debug
> warning, to say that we are mapping MMIOs on trap on this platform? But
> today, it would basically be the same as saying "ACPI is enabled".

I think we clearly need some documentation in tree to explain the design 
of Xen with ACPI and Device Tree. But this is lacking so far :/.

I can have a go when I find some times.

Cheers,
Julien Grall March 31, 2017, 2:50 p.m. UTC | #8
On 30/03/17 20:54, Julien Grall wrote:
> On 30/03/2017 19:55, Stefano Stabellini wrote:
>> On Thu, 30 Mar 2017, Julien Grall wrote:
>>> On 30/03/17 19:37, Stefano Stabellini wrote:
>>>> On Thu, 30 Mar 2017, Julien Grall wrote:
>> Fair enough. Do you think there is any value in adding only one debug
>> warning, to say that we are mapping MMIOs on trap on this platform? But
>> today, it would basically be the same as saying "ACPI is enabled".
>
> I think we clearly need some documentation in tree to explain the design
> of Xen with ACPI and Device Tree. But this is lacking so far :/.
>
> I can have a go when I find some times.

Just to make clear, I am planning to send that separately. Stefano, let 
me know if it is fine for you.

Cheers,
Stefano Stabellini March 31, 2017, 6:01 p.m. UTC | #9
On Fri, 31 Mar 2017, Julien Grall wrote:
> On 30/03/17 20:54, Julien Grall wrote:
> > On 30/03/2017 19:55, Stefano Stabellini wrote:
> > > On Thu, 30 Mar 2017, Julien Grall wrote:
> > > > On 30/03/17 19:37, Stefano Stabellini wrote:
> > > > > On Thu, 30 Mar 2017, Julien Grall wrote:
> > > Fair enough. Do you think there is any value in adding only one debug
> > > warning, to say that we are mapping MMIOs on trap on this platform? But
> > > today, it would basically be the same as saying "ACPI is enabled".
> > 
> > I think we clearly need some documentation in tree to explain the design
> > of Xen with ACPI and Device Tree. But this is lacking so far :/.
> > 
> > I can have a go when I find some times.
> 
> Just to make clear, I am planning to send that separately. Stefano, let me
> know if it is fine for you.

Yes, of course! But we still need the tiny is_hardware_domain fix.
Julien Grall March 31, 2017, 6:47 p.m. UTC | #10
Hi Stefano,

On 03/31/2017 07:01 PM, Stefano Stabellini wrote:
> On Fri, 31 Mar 2017, Julien Grall wrote:
>> On 30/03/17 20:54, Julien Grall wrote:
>>> On 30/03/2017 19:55, Stefano Stabellini wrote:
>>>> On Thu, 30 Mar 2017, Julien Grall wrote:
>>>>> On 30/03/17 19:37, Stefano Stabellini wrote:
>>>>>> On Thu, 30 Mar 2017, Julien Grall wrote:
>>>> Fair enough. Do you think there is any value in adding only one debug
>>>> warning, to say that we are mapping MMIOs on trap on this platform? But
>>>> today, it would basically be the same as saying "ACPI is enabled".
>>>
>>> I think we clearly need some documentation in tree to explain the design
>>> of Xen with ACPI and Device Tree. But this is lacking so far :/.
>>>
>>> I can have a go when I find some times.
>>
>> Just to make clear, I am planning to send that separately. Stefano, let me
>> know if it is fine for you.
>
> Yes, of course! But we still need the tiny is_hardware_domain fix.

I was hoping you to fix the is_hardware_domain :). Anyway, I will resend 
a new version on Monday with this change.

Cheers,
Stefano Stabellini March 31, 2017, 6:50 p.m. UTC | #11
On Fri, 31 Mar 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 03/31/2017 07:01 PM, Stefano Stabellini wrote:
> > On Fri, 31 Mar 2017, Julien Grall wrote:
> > > On 30/03/17 20:54, Julien Grall wrote:
> > > > On 30/03/2017 19:55, Stefano Stabellini wrote:
> > > > > On Thu, 30 Mar 2017, Julien Grall wrote:
> > > > > > On 30/03/17 19:37, Stefano Stabellini wrote:
> > > > > > > On Thu, 30 Mar 2017, Julien Grall wrote:
> > > > > Fair enough. Do you think there is any value in adding only one debug
> > > > > warning, to say that we are mapping MMIOs on trap on this platform?
> > > > > But
> > > > > today, it would basically be the same as saying "ACPI is enabled".
> > > > 
> > > > I think we clearly need some documentation in tree to explain the design
> > > > of Xen with ACPI and Device Tree. But this is lacking so far :/.
> > > > 
> > > > I can have a go when I find some times.
> > > 
> > > Just to make clear, I am planning to send that separately. Stefano, let me
> > > know if it is fine for you.
> > 
> > Yes, of course! But we still need the tiny is_hardware_domain fix.
> 
> I was hoping you to fix the is_hardware_domain :). Anyway, I will resend a new
> version on Monday with this change.

And you can add my reviewed-by to it
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 614501f761..ebf915bb3b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -32,6 +32,7 @@ 
 #include <xen/perfc.h>
 #include <xen/virtual_region.h>
 #include <xen/mem_access.h>
+#include <xen/iocap.h>
 #include <public/sched.h>
 #include <public/xen.h>
 #include <asm/debugger.h>
@@ -49,6 +50,7 @@ 
 #include <asm/gic.h>
 #include <asm/vgic.h>
 #include <asm/cpuerrata.h>
+#include <asm/acpi.h>
 
 /* The base of the stack must always be double-word aligned, which means
  * that both the kernel half of struct cpu_user_regs (which is pushed in
@@ -2534,6 +2536,35 @@  static bool try_handle_mmio(struct cpu_user_regs *regs,
     return !!handle_mmio(info);
 }
 
+/*
+ * When using ACPI, most of the MMIO regions will be mapped on-demand
+ * in stage-2 page tables for the hardware domain because Xen is not
+ * able to know from the EFI memory map the MMIO regions.
+ */
+static bool try_map_mmio(gfn_t gfn)
+{
+    struct domain *d = current->domain;
+
+    /* For the hardware domain, all MMIOs are mapped with GFN == MFN */
+    mfn_t mfn = _mfn(gfn_x(gfn));
+
+    /*
+     * Device-Tree should already have everything mapped when building
+     * the hardware domain.
+     */
+    if ( acpi_disabled )
+        return false;
+
+    if ( d != hardware_domain )
+        return false;
+
+    /* The hardware domain can only map permitted MMIO regions */
+    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + 1) )
+        return false;
+
+    return !map_regions_p2mt(d, gfn, 1, mfn, p2m_mmio_direct_c);
+}
+
 static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
                                      const union hsr hsr)
 {
@@ -2611,6 +2642,9 @@  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
         if ( !mfn_eq(mfn, INVALID_MFN) )
             return;
 
+        if ( try_map_mmio(_gfn(paddr_to_pfn(info.gpa))) )
+            return;
+
         break;
     default:
         gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",