Re: [PATCH v3 8/9] mailbox: mediatek: Add CMDQ secure mailbox driver

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

Hi CK,

Thanks for the reviews.

On Tue, 2023-12-26 at 08:28 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Fri, 2023-12-22 at 12:52 +0800, Jason-JH.Lin wrote:
> > To support secure video path feature, GCE have to read/write
> > registgers
> > in the secure world. GCE will enable the secure access permission
> > to
> > the
> > HW who wants to access the secure content buffer.
> > 
> > Add CMDQ secure mailbox driver to make CMDQ client user is able to
> > sending their HW settings to the secure world. So that GCE can
> > execute
> > all instructions to configure HW in the secure world.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> 
> [snip]
> 
> > +
> > +int cmdq_sec_pkt_write(struct cmdq_pkt *pkt, u32 addr, u64 base,
> > +		       const enum cmdq_iwc_addr_metadata_type type,
> > +		       const u32 offset, const u32 size, const u32
> > port)
> > +{
> > +	int ret;
> > +
> > +	ret = cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX0, addr);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = cmdq_pkt_write_s_reg_value(pkt, CMDQ_THR_SPR_IDX0,
> > (u32)base);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return cmdq_sec_append_metadata(pkt, type, base, offset, size,
> > port);
> 
> Why do you append this metadata? It looks like TEE would use this
> meta
> data to check command is secure or not. But this meta data is in
> normal
> world, so it may be modified by hacker. I think TEE should check
> command buffer directly not by the meta data.
> 
> Regards,
> CK
> 

OK, I'll try to remove this.

Regards,
Jason-JH.Lin

> > +}
> > +EXPORT_SYMBOL_GPL(cmdq_sec_pkt_write);
> > +