Re: [RFC PATCH 2/5] arm-common: Introduce iommu functionality

Sat, 23 Mar 2019 09:19:42 -0700 


On 22/03/19 10:53 PM, Jan Kiszka wrote:
> On 22.03.19 10:44, Lokesh wrote:
>> From: Nikhil Devshatwar <[email protected]>
>>
>> Add the iommu hooks for ARM and ARM64 architectures with
>> dummy implementations.
>> Update the Kbuild to include new iommu files
>>
>> Introduce following hooks:
>> iommu_map_memory - Setup iommu mapping for the memory region
>> iommu_unmap_memory - Unmap the iommu mapping for the mem region
>> iommu_config_commit - Commit all the changes to the mem mapping
>>
>> Call the map/unmap iommu functions in addition to CPU map/unmap and
>> config_commit.
>> Also add iommu_units in the platform data for ARM cells.
>>
>> Signed-off-by: Nikhil Devshatwar <[email protected]>
>> Signed-off-by: Lokesh Vutla <[email protected]>
>> ---
>>   hypervisor/arch/arm-common/Kbuild             |  2 +-
>>   hypervisor/arch/arm-common/control.c          |  6 ++++
>>   .../arch/arm-common/include/asm/iommu.h       | 26 +++++++++++++++
>>   hypervisor/arch/arm-common/iommu.c            | 32 +++++++++++++++++++
>>   hypervisor/arch/arm-common/mmu_cell.c         | 12 +++++++
>>   include/jailhouse/cell-config.h               |  2 ++
>>   6 files changed, 79 insertions(+), 1 deletion(-)
>>   create mode 100644 hypervisor/arch/arm-common/include/asm/iommu.h
>>   create mode 100644 hypervisor/arch/arm-common/iommu.c
>>
>> diff --git a/hypervisor/arch/arm-common/Kbuild
>> b/hypervisor/arch/arm-common/Kbuild
>> index 78b9e512..6f57ef02 100644
>> --- a/hypervisor/arch/arm-common/Kbuild
>> +++ b/hypervisor/arch/arm-common/Kbuild
>> @@ -17,6 +17,6 @@ ccflags-$(CONFIG_JAILHOUSE_GCOV) += -fprofile-arcs
>> -ftest-coverage
>>   objs-y += dbg-write.o lib.o psci.o control.o paging.o mmu_cell.o setup.o
>>   objs-y += irqchip.o pci.o ivshmem.o uart-pl011.o uart-xuartps.o 
>> uart-mvebu.o
>>   objs-y += uart-hscif.o uart-scifa.o uart-imx.o
>> -objs-y += gic-v2.o gic-v3.o smccc.o
>> +objs-y += gic-v2.o gic-v3.o smccc.o iommu.o
>>     common-objs-y = $(addprefix ../arm-common/,$(objs-y))
>> diff --git a/hypervisor/arch/arm-common/control.c
>> b/hypervisor/arch/arm-common/control.c
>> index b59c05d6..d712b049 100644
>> --- a/hypervisor/arch/arm-common/control.c
>> +++ b/hypervisor/arch/arm-common/control.c
>> @@ -16,6 +16,7 @@
>>   #include <jailhouse/printk.h>
>>   #include <asm/control.h>
>>   #include <asm/psci.h>
>> +#include <asm/iommu.h>
>>     static void enter_cpu_off(struct public_per_cpu *cpu_public)
>>   {
>> @@ -208,7 +209,12 @@ void arch_flush_cell_vcpu_caches(struct cell *cell)
>>     void arch_config_commit(struct cell *cell_added_removed)
>>   {
>> +    u8 err;
>> +
>>       irqchip_config_commit(cell_added_removed);
>> +    err = iommu_config_commit(cell_added_removed);
>> +    if (err)
>> +        printk("WARNING: iommu_config_commit failed\n");
> 
> What could make iommu_config_commit fail on ARM? We do not have this case (a
> non-void return type) on x86.

I guess this was added just to capture any failures for future iommus. But right
now for smmu there is no failure case. I can drop it.

> 
>>   }
>>     void __attribute__((noreturn)) arch_panic_stop(void)
>> diff --git a/hypervisor/arch/arm-common/include/asm/iommu.h
>> b/hypervisor/arch/arm-common/include/asm/iommu.h
>> new file mode 100644
>> index 00000000..d33ec17c
>> --- /dev/null
>> +++ b/hypervisor/arch/arm-common/include/asm/iommu.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Jailhouse, a Linux-based partitioning hypervisor
>> + *
>> + * Copyright (c) 2018 Texas Instruments Incorporated - http://www.ti.com
>> + *
>> + * Authors:
>> + *  Nikhil Devshatwar <[email protected]>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef _JAILHOUSE_ASM_IOMMU_H
>> +#define _JAILHOUSE_ASM_IOMMU_H
>> +
>> +#include <jailhouse/types.h>
>> +#include <jailhouse/utils.h>
>> +#include <jailhouse/config.h>
>> +#include <jailhouse/cell-config.h>
>> +
>> +int iommu_map_memory_region(struct cell *cell,
>> +                const struct jailhouse_memory *mem);
>> +int iommu_unmap_memory_region(struct cell *cell,
>> +                  const struct jailhouse_memory *mem);
>> +int iommu_config_commit(struct cell *cell);
>> +#endif
>> diff --git a/hypervisor/arch/arm-common/iommu.c
>> b/hypervisor/arch/arm-common/iommu.c
>> new file mode 100644
>> index 00000000..5bc9e6a9
>> --- /dev/null
>> +++ b/hypervisor/arch/arm-common/iommu.c
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Jailhouse, a Linux-based partitioning hypervisor
>> + *
>> + * Copyright (c) 2018 Texas Instruments Incorporated - http://www.ti.com
>> + *
>> + * Authors:
>> + *  Nikhil Devshatwar <[email protected]>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.  See
>> + * the COPYING file in the top-level directory.
>> + */
>> +
>> +#include <jailhouse/control.h>
>> +#include <jailhouse/config.h>
>> +#include <asm/iommu.h>
>> +
>> +int iommu_map_memory_region(struct cell *cell,
>> +                const struct jailhouse_memory *mem)
>> +{
>> +    return 0;
>> +}
>> +
>> +int iommu_unmap_memory_region(struct cell *cell,
>> +                  const struct jailhouse_memory *mem)
>> +{
>> +    return 0;
>> +}
>> +
>> +int iommu_config_commit(struct cell *cell)
>> +{
>> +    return 0;
>> +}
>> diff --git a/hypervisor/arch/arm-common/mmu_cell.c
>> b/hypervisor/arch/arm-common/mmu_cell.c
>> index 36a3016f..6461e9e9 100644
>> --- a/hypervisor/arch/arm-common/mmu_cell.c
>> +++ b/hypervisor/arch/arm-common/mmu_cell.c
>> @@ -15,12 +15,14 @@
>>   #include <jailhouse/printk.h>
>>   #include <asm/sysregs.h>
>>   #include <asm/control.h>
>> +#include <asm/iommu.h>
>>     int arch_map_memory_region(struct cell *cell,
>>                  const struct jailhouse_memory *mem)
>>   {
>>       u64 phys_start = mem->phys_start;
>>       u32 flags = PTE_FLAG_VALID | PTE_ACCESS_FLAG;
>> +    int err = 0;
>>         if (mem->flags & JAILHOUSE_MEM_READ)
>>           flags |= S2_PTE_ACCESS_RO;
>> @@ -37,6 +39,10 @@ int arch_map_memory_region(struct cell *cell,
>>           flags |= S2_PAGE_ACCESS_XN;
>>       */
>>   +    err = iommu_map_memory_region(cell, mem);
>> +    if (err)
>> +        return err;
>> +
>>       return paging_create(&cell->arch.mm, phys_start, mem->size,
>>                    mem->virt_start, flags, PAGING_COHERENT);
>>   }
>> @@ -44,6 +50,12 @@ int arch_map_memory_region(struct cell *cell,
>>   int arch_unmap_memory_region(struct cell *cell,
>>                    const struct jailhouse_memory *mem)
>>   {
>> +    int err = 0;
>> +
>> +    err = iommu_unmap_memory_region(cell, mem);
>> +    if (err)
>> +        return err;
>> +
>>       return paging_destroy(&cell->arch.mm, mem->virt_start, mem->size,
>>                     PAGING_COHERENT);
>>   }
>> diff --git a/include/jailhouse/cell-config.h 
>> b/include/jailhouse/cell-config.h
>> index 2d043bf9..519afefc 100644
>> --- a/include/jailhouse/cell-config.h
>> +++ b/include/jailhouse/cell-config.h
>> @@ -257,6 +257,8 @@ struct jailhouse_system {
>>                   u64 gich_base;
>>                   u64 gicv_base;
>>                   u64 gicr_base;
>> +                struct jailhouse_iommu
>> +                    iommu_units[JAILHOUSE_MAX_IOMMU_UNITS];
> 
> Will we be fine with such a static number on ARM? I thought there is one unit
> per device it rules, thus typically more than on x86.

Not necessarily that one unit should protect one device. One unit can be used
for protecting multiple devices. That is what stream id is used for.

Thanks and regards,
Lokesh

> 
>>               } __attribute__((packed)) arm;
>>           } __attribute__((packed));
>>       } __attribute__((packed)) platform_info;
>>
> 
> Jan
> 

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.






















					Reply via email to