Re: [PATCH] staging: tidspbridge - configure full L1 MMU range

[Guzman Lugo, Fernando]

On Wed, Jan 5, 2011 at 12:46 PM, Ramirez Luna, Omar <omar.ramirez@xxxxxx> wrote:
> Hi,
>
> On Wed, Jan 5, 2011 at 12:20 AM, Fernando Guzman Lugo
> <fernando.lugo@xxxxxx> wrote:
>> Otherwise a virtual address beyond of the L1 size is used,
>> the MMU hardware will look into a memory that does not belong to
>> L1 translation tables. IOW; the MMU would allow to access any
>> memory, configured or not.
>>
>
> How about:
>
> IVA MMU can manage up to 4GB of address space through its page tables,
> given that it's L1 is divided into 1MB sections it requires at least
> 16KB for its table which represents 4096 entries of 32 bits each.
>
> Previously, only 1GB was being handled by setting the page table size
> to 4KB, any virtual address beyond of the L1 size used, would fall
> into memory that does not belong to L1 translation tables, leading to
> unpredictable results.
>
> So, set the L1 table size to cover the entire MMU range (4GB) whether
> is meant to be used or not.

looks good to me.

>
>> Reported-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
>> Signed-off-by: Fernando Guzman Lugo <fernando.lugo@xxxxxx>
>> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
>> ---
>>  drivers/staging/tidspbridge/core/tiomap3430.c |    6 ++----
>>  1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c b/drivers/staging/tidspbridge/core/tiomap3430.c
>> index 1be081f..ec96d1e 100644
>> --- a/drivers/staging/tidspbridge/core/tiomap3430.c
>> +++ b/drivers/staging/tidspbridge/core/tiomap3430.c
>> @@ -70,6 +70,7 @@
>>  #define MMU_LARGE_PAGE_MASK      0xFFFF0000
>>  #define MMU_SMALL_PAGE_MASK      0xFFFFF000
>>  #define OMAP3_IVA2_BOOTADDR_MASK 0xFFFFFC00
>> +#define MMU_L1_SIZE              0x4000
>>  #define PAGES_II_LVL_TABLE   512
>>  #define PHYS_TO_PAGE(phys)      pfn_to_page((phys) >> PAGE_SHIFT)
>>
>> @@ -786,10 +787,7 @@ static int bridge_dev_create(struct bridge_dev_context
>>
>>        pt_attrs = kzalloc(sizeof(struct pg_table_attrs), GFP_KERNEL);
>>        if (pt_attrs != NULL) {
>> -               /* Assuming that we use only DSP's memory map
>> -                * until 0x4000:0000 , we would need only 1024
>> -                * L1 enties i.e L1 size = 4K */
>> -               pt_attrs->l1_size = 0x1000;
>> +               pt_attrs->l1_size = MMU_L1_SIZE;
>
> How about using SZ_16K instead, struct member name already specifies
> it is L1 size, is the new define needed?

Ok, maybe a comment to state why 16K is used should be good.

I will make the changes.

Regards,
Fernando.

>
> Regards,
>
> Omar
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/