[PATCH] firmware: scmi: Replace memcpy_from/toio() by memcpy() in scmi_read/write_resp_from_smt()

[Patrice CHOTARD]

Hi Mark

On 2/24/21 3:05 PM, Mark Kettenis wrote:
>> From: Patrice Chotard <patrice.chotard at foss.st.com>
>> Date: Wed, 24 Feb 2021 13:47:55 +0100
>>
>> To avoid "synchronous abort" in AARCH64 in case the "from" address
>> is not aligned, replace memcpy_toio() and memcpy_fromio() by memcpy().
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
>> Signed-off-by: Patrice Chotard <patrice.chotard at foss.st.com>
>> ---
>>
>>  drivers/firmware/scmi/smt.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> This doesnt really make sense.  There is no guarantee that memcpy()
> doesn't do an unaligned load or store either.
> 
> Unaligned loads and stores from/to normal memory should just work on
> arm64, and memcpy_toio() and memcpy_fromio() make sure they don't do
> unaligned loads and stores from/to "IO" memory (in this case the
> shared memory buffer).

You are right. After further analysis, i understood why the "synchronous abort" occurs.
At early stage, before U-Boot relocation, MMU is disabled and then all the "normal" memory 
space is not yet configured with MT_NORMAL type.
In this situation, all 64bits accesses to a none 64 bits aligned pointer in "normal memory"
triggers a synchronous abort.

Same accesses, with MMU configured (after U-Boot relocation), are OK.

I will propose a new patch which will update memcpy_from/toio() and forbid 64bits accesses to 
non aligned 64bits pointer in case MMU is not yet enabled.

Thanks

Patrice
> 
>> diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c
>> index d25478796a..b001163b62 100644
>> --- a/drivers/firmware/scmi/smt.c
>> +++ b/drivers/firmware/scmi/smt.c
>> @@ -93,7 +93,7 @@ int scmi_write_msg_to_smt(struct udevice *dev, struct scmi_smt *smt,
>>  			  SMT_HEADER_PROTOCOL_ID(msg->protocol_id) |
>>  			  SMT_HEADER_MESSAGE_ID(msg->message_id);
>>  
>> -	memcpy_toio(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
>> +	memcpy(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
>>  
>>  	return 0;
>>  }
>> @@ -124,7 +124,7 @@ int scmi_read_resp_from_smt(struct udevice *dev, struct scmi_smt *smt,
>>  
>>  	/* Get the data */
>>  	msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header);
>> -	memcpy_fromio(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
>> +	memcpy(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
>>  
>>  	return 0;
>>  }
>> -- 
>> 2.17.1
>>
>>