Re: [PATCH v3 09/14] ASoC: SOF: Add firmware, loader support

[Liam Girdwood <liam.r.girdwood@linux.intel.com>]

On Fri, 2018-12-21 at 10:57 +0100, Takashi Iwai wrote:
> On Fri, 21 Dec 2018 10:05:04 +0100,
> Bard liao wrote:
> > 
> > On 12/20/2018 11:07 PM, Pierre-Louis Bossart wrote:
> > > > > > > + snd_sof_dsp_block_write(sdev, offset,
> > > > > > > +                    (void *)block + sizeof(*block),
> > > > > > > +                    block->size);
> > > > > > > +
> > > > > > > +        /* next block */
> > > > > > > +        block = (void *)block + sizeof(*block) + block->size;
> > > > > > This may lead to an unaligned access.
> > > > > Did you mean we should double check the block->size to
> > > > > prevent access to an invalid address?
> > > > You need two types of checks for the given data:
> > > > - The bounce check of block->size;
> > > >    We need to avoid out-of-bounce access.
> > > s/bounce/bounds ?
> > > > - Alignment of block->size;
> > > >    For some non-x86 platforms, the access to an unaligned address might
> > > >    be illegal.
> > > 
> > > Maybe I am missing something but I don't see any sort of explicit
> > > restriction on alignment in the SOF tools. it looks implicit based
> > > on address offsets and bases.
> > 
> > Maybe get_unaligned() is good enough to avoid unaligned access?
> 
> That's another option, but you'd need to put everywhere.
> 
> An alternative is to just copy the data on a new block header on the
> stack temporarily and refer it.
> 
> Or simply assuring the alignment by checking block->size as Pierre
> suggested...

Yep, checking size is good for me too.

> 
> 
> Takashi
> 
> 
> > > Liam, do you see any negative side effects if we enforce a 32-bit
> > > alignment for all blocks (which essentially means all block sizes
> > > are multiple of 4)? we can try and experiment but it's better if we
> > > have an agreement on the design.


This shouldn't cause any problems, iirc all block sizes are rounded up to
nearest uint32_t anyway (as we have exception handler blocks that are 3 bytes of
assembly in size and rounded up to 4).

Liam

> > > 
> > > 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel