RE: [PATCH 2/2] DSPBRIDGE: add checking 128 byte alignment for dsp cache line size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@xxxxxxxxx] 
> Sent: Tuesday, May 12, 2009 12:39 AM
> To: Kanigeri, Hari
> Cc: felipe.contreras@xxxxxxxxx; Menon, Nishanth; 
> linux-omap@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/2] DSPBRIDGE: add checking 128 byte 
> alignment for dsp cache line size
> 
> From: "ext Kanigeri, Hari" <h-kanigeri2@xxxxxx>
> Subject: RE: [PATCH 2/2] DSPBRIDGE: add checking 128 byte 
> alignment for dsp cache line size
> Date: Mon, 11 May 2009 20:26:11 +0200
> 
> > To summarize the discussion.
> 
> Thanks
> 
> > We need to have the check for 128 bytes alignment (upper 
> and lower). 
> > The 2 places that this can be done are in flush function or in Map 
> > function.
> > 
> > 	- I prefer the check is done in Map function as Felipe 
> mentioned this 
> > function is bound to be called by MM components as opposed to Flush 
> > function.
> 
> Mapping is totally another thing from this problem.
> 
> Any page mapping is being done, forcing PAGE_SIZE as below, because
> PAGE_SIZE(4KB) is the mininum unit from (io)mmu H/W 
> perspective. This is same as ARM too. They've been aligned on 
> 4KB already. So no need to worry about 128-byte alignement 
> for mapping.
> 

-- pMpuAddr that is passed to the PROC_Map is not aligned on 4KB boundary.
The alignemnt is taken care within the function. If this function is passed
the information of the buffer is read-only (from MPU point of view) then we
can enforce the cache alignment check. 


> DSP_STATUS PROC_Map(DSP_HPROCESSOR hProcessor, void 
> *pMpuAddr, u32 ulSize,
> 		   void *pReqAddr, void **ppMapAddr, u32 ulMapAttr) {
> 	.....
> 	/* Calculate the page-aligned PA, VA and size */
> 	vaAlign = PG_ALIGN_LOW((u32) pReqAddr, PG_SIZE_4K);
> 	paAlign = PG_ALIGN_LOW((u32) pMpuAddr, PG_SIZE_4K);
> 	sizeAlign = PG_ALIGN_HIGH(ulSize + (u32)pMpuAddr - paAlign,
> 				 PG_SIZE_4K);
> 
> The points which we should take care of are bridge cache
> operations("PROC_Flush/Invalidate..()") and we should/can 
> handle this problem independently. It would cause another 
> dependency and increase another complexity again if we have 
> to assume something(ex:
> "read-only" or "write-only" buffer) on other modules or 
> expecting something on its "protocol".
> 
> > Also, another reason is the flush
> > function in Bridge driver is going to be removed eventually.
> 
> Maybe this should be discussed separately from the current topic.
> 
> > The open issue is the impact on the OMX components. Once if we 
> > finalize that there is no impact on the existing OMX 
> components or we 
> > have a plan to take this into account we can enforce this check.
> 
> This modification impact is a compatibility issue with the 
> existing S/W. Fixing it is the way to go rather than working 
> around the potential risk that userland can cause kernel 
> crash / memory corruption.
> 
> 

-- I agree with you that fixing is the way to go, but I think we should
give some transition time for the applications to adapt to this change.--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux