Re: [Xen-devel] [PATCH v02 2/7] arm: omap: introduce iommu translation for IPU remoteproc

[Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>]

Hi Ian,

On Wed, Jul 16, 2014 at 6:36 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Thu, 2014-06-26 at 14:07 +0300, Andrii Tseglytskyi wrote:
>> The following patch introduced platform specific MMU data
>> definitions and pagetable translation function for OMAP5 IPU
>> remoteproc. This MMU is a bit specific - it typically performs
>> one level translation and map a big chunks of memory. 16 Mb
>> supersections and 1 Mb sections are mapped instead of 4 Kb pages.
>> Introduced algorithm performs internal remapping of big sections
>> to small 4 Kb pages.
>
> How does that work if the MMU only supports 1MB/16MB supersections? Or
> have I misunderstood?
>

In my case MMU supports all  - 4K, 1MB and 16MB. But for performance
reasons only 1MB and 16MB mappings are used during
IPU boot  - using 4K pages mapping increases boot time for about 40
seconds, which is not acceptable for Android.
Taking in account, that MMU supports 4K pages - I can overwrite
pagetables changing 1MB and 16 MB mappings to corresponding subsets of
4K pages mappings. Kernel code will not know anything about this
change, and MMU will continue working fine.

>> Change-Id: If20449f07e22f780e1fded67fed4f79cbe1fc156
>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>
>> ---
>>  xen/arch/arm/platforms/Makefile     |    1 +
>>  xen/arch/arm/platforms/omap_iommu.c |  247 
>> +++++++++++++++++++++++++++++++++++
>
> I don't think this is the right home for this.
>
> I think either xen/arch/arm/remoteproc/* or xen/drivers/remoteproc/*
> would be more appropriate.

OK.

>
>> +#define      OMAP_IPU_MMU_MEM_BASE   0x55082000
>> +
>> +static u32 mmu_ipu_translate_pagetable(struct mmu_info *mmu, struct 
>> mmu_pagetable *pgt);
>> +
>> +static u32 ipu_trap_offsets[] = {
>> +    MMU_IPU_TTB_OFFSET,
>
> How large is this region? I think the machinery needs to be told.
>

Not sure that I get your question correctly.
To make a trap - I unmap 1 page of MMU device iomem, where register
which contains pagetable address is stored.

> Also if this functionality is to be used by guests then it can't really
> use the h/w base address, you'd need to define a region of guest address
> map (!= host/dom0 address map) for it.
>

This is just a register offset and it is used to define is guest
accessing a proper iomem register. IOW if offset is the same as offset
where pagetable address is stored - than translation is performed. Do
I need to define this in a different way?

>> diff --git a/xen/arch/arm/remoteproc_iommu.c 
>> b/xen/arch/arm/remoteproc_iommu.c
>> index b4d22d9..8291f3f 100644
>> --- a/xen/arch/arm/remoteproc_iommu.c
>> +++ b/xen/arch/arm/remoteproc_iommu.c
>> @@ -33,6 +33,7 @@
>>  #include "io.h"
>>
>>  static struct mmu_info *mmu_list[] = {
>> +    &omap_ipu_mmu,
>
> This suggests there is exactly one such and it is exposed to every
> domain.
>
> Wouldn't this rather be dynamic and per domain?
>

Yes, this is what Julien already suggested. I will investigate how to
implement this in the right way.

> Ian.
>



-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel