> -----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