| Message ID | 20240626-san-v1-5-f3cc42302189@daynix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix check-qtest-ppc64 sanitizer errors | expand |
On 26/6/24 13:06, Akihiko Odaki wrote: > FDT properties are aligned by 4 bytes, not 8 bytes. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > hw/ppc/vof.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c > index e3b430a81f4f..b5b6514d79fc 100644 > --- a/hw/ppc/vof.c > +++ b/hw/ppc/vof.c > @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base) > mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen); > g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc)); > if (sc == 2) { > - mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) * ac)); > + mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac); > } else { > mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) * ac)); OK but please keep API uses consistent, so convert other uses please. > } >
On 2024/06/26 21:03, Philippe Mathieu-Daudé wrote: > On 26/6/24 13:06, Akihiko Odaki wrote: >> FDT properties are aligned by 4 bytes, not 8 bytes. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> hw/ppc/vof.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c >> index e3b430a81f4f..b5b6514d79fc 100644 >> --- a/hw/ppc/vof.c >> +++ b/hw/ppc/vof.c >> @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, >> GArray *claimed, uint64_t base) >> mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen); >> g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc)); >> if (sc == 2) { >> - mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + >> sizeof(uint32_t) * ac)); >> + mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac); >> } else { >> mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + >> sizeof(uint32_t) * ac)); > > OK but please keep API uses consistent, so convert other uses please. This is the only unaligned access.
On 27/6/24 15:12, Akihiko Odaki wrote: > On 2024/06/26 21:03, Philippe Mathieu-Daudé wrote: >> On 26/6/24 13:06, Akihiko Odaki wrote: >>> FDT properties are aligned by 4 bytes, not 8 bytes. >>> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>> --- >>> hw/ppc/vof.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c >>> index e3b430a81f4f..b5b6514d79fc 100644 >>> --- a/hw/ppc/vof.c >>> +++ b/hw/ppc/vof.c >>> @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, >>> GArray *claimed, uint64_t base) >>> mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen); >>> g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc)); >>> if (sc == 2) { >>> - mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + >>> sizeof(uint32_t) * ac)); >>> + mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac); >>> } else { >>> mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + >>> sizeof(uint32_t) * ac)); >> >> OK but please keep API uses consistent, so convert other uses please. > > This is the only unaligned access. What I mean with consistent API use is either use the be64_to_cpu and be32_to_cpu API, or ldq_be_p and ldl_be_p. A mix of both makes review more confusing.
On 2024/06/27 23:02, Philippe Mathieu-Daudé wrote: > On 27/6/24 15:12, Akihiko Odaki wrote: >> On 2024/06/26 21:03, Philippe Mathieu-Daudé wrote: >>> On 26/6/24 13:06, Akihiko Odaki wrote: >>>> FDT properties are aligned by 4 bytes, not 8 bytes. >>>> >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>>> --- >>>> hw/ppc/vof.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c >>>> index e3b430a81f4f..b5b6514d79fc 100644 >>>> --- a/hw/ppc/vof.c >>>> +++ b/hw/ppc/vof.c >>>> @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, >>>> GArray *claimed, uint64_t base) >>>> mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen); >>>> g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc)); >>>> if (sc == 2) { >>>> - mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + >>>> sizeof(uint32_t) * ac)); >>>> + mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac); >>>> } else { >>>> mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + >>>> sizeof(uint32_t) * ac)); >>> >>> OK but please keep API uses consistent, so convert other uses please. >> >> This is the only unaligned access. > > What I mean with consistent API use is either use the be64_to_cpu and > be32_to_cpu API, or ldq_be_p and ldl_be_p. A mix of both makes review > more confusing. The desired semantics are different in these two cases so I believe it is natural to use different APIs; ldq_be_p() for an unaligned 64-bit access and be32_to_cpu(*(uint32_t *)) for an aligned 32-bit access.
diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c index e3b430a81f4f..b5b6514d79fc 100644 --- a/hw/ppc/vof.c +++ b/hw/ppc/vof.c @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base) mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen); g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc)); if (sc == 2) { - mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) * ac)); + mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac); } else { mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) * ac)); }
FDT properties are aligned by 4 bytes, not 8 bytes. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- hw/ppc/vof.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)