* [PATCH] firmware: scmi: Replace memcpy_from/toio() by memcpy() in scmi_read/write_resp_from_smt()
@ 2021-02-24 12:47 Patrice Chotard
2021-02-24 14:05 ` Mark Kettenis
0 siblings, 1 reply; 3+ messages in thread
From: Patrice Chotard @ 2021-02-24 12:47 UTC (permalink / raw)
To: u-boot
From: Patrice Chotard <patrice.chotard@st.com>
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@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
drivers/firmware/scmi/smt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] firmware: scmi: Replace memcpy_from/toio() by memcpy() in scmi_read/write_resp_from_smt()
2021-02-24 12:47 [PATCH] firmware: scmi: Replace memcpy_from/toio() by memcpy() in scmi_read/write_resp_from_smt() Patrice Chotard
@ 2021-02-24 14:05 ` Mark Kettenis
2021-02-26 7:54 ` Patrice CHOTARD
0 siblings, 1 reply; 3+ messages in thread
From: Mark Kettenis @ 2021-02-24 14:05 UTC (permalink / raw)
To: u-boot
> From: Patrice Chotard <patrice.chotard@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@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@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).
> 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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] firmware: scmi: Replace memcpy_from/toio() by memcpy() in scmi_read/write_resp_from_smt()
2021-02-24 14:05 ` Mark Kettenis
@ 2021-02-26 7:54 ` Patrice CHOTARD
0 siblings, 0 replies; 3+ messages in thread
From: Patrice CHOTARD @ 2021-02-26 7:54 UTC (permalink / raw)
To: u-boot
Hi Mark
On 2/24/21 3:05 PM, Mark Kettenis wrote:
>> From: Patrice Chotard <patrice.chotard@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@st.com>
>> Signed-off-by: Patrice Chotard <patrice.chotard@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
>>
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-26 7:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-24 12:47 [PATCH] firmware: scmi: Replace memcpy_from/toio() by memcpy() in scmi_read/write_resp_from_smt() Patrice Chotard
2021-02-24 14:05 ` Mark Kettenis
2021-02-26 7:54 ` Patrice CHOTARD
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).