* [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
@ 2021-11-24 12:00 Leandro Lupori
2021-11-24 13:40 ` Daniel Henrique Barboza
2021-11-25 3:03 ` David Gibson
0 siblings, 2 replies; 10+ messages in thread
From: Leandro Lupori @ 2021-11-24 12:00 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: groug, danielhb413, Leandro Lupori, clg, david
When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
offset, causing the first byte of the adjacent PTE to be corrupted.
This caused a panic when booting FreeBSD, using the Hash MMU.
Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
---
target/ppc/mmu-hash64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 19832c4b46..f165ac691a 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
{
- hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
+ hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
if (cpu->vhyp) {
PPCVirtualHypervisorClass *vhc =
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 12:00 [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R Leandro Lupori
@ 2021-11-24 13:40 ` Daniel Henrique Barboza
2021-11-24 18:42 ` Cédric Le Goater
2021-11-25 3:03 ` David Gibson
1 sibling, 1 reply; 10+ messages in thread
From: Daniel Henrique Barboza @ 2021-11-24 13:40 UTC (permalink / raw)
To: Leandro Lupori, qemu-devel, qemu-ppc; +Cc: groug, clg, david
On 11/24/21 09:00, Leandro Lupori wrote:
> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
> offset, causing the first byte of the adjacent PTE to be corrupted.
> This caused a panic when booting FreeBSD, using the Hash MMU.
If you add a "Fixes:" tag with the commit that introduced the code you're
fixing, we can push it right away as a bug fix in 6.2 (assuming it doesn't
break anything else, of course).
The commit to be fixed in the case seems to be a2dd4e83e76b ("ppc/hash64:
Rework R and C bit updates")
One more comment below:
>
> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> ---
> target/ppc/mmu-hash64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 19832c4b46..f165ac691a 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
>
> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
> {
> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
Instead of adding a '14' you should add a new #define in mmu-hash64.h with this
value, something like "HPTE64_R_R_SHIFT". This will avoid hardcoding literals
around the code and forcing us to go to the ISA every time we wonder what's
an apparently random number represents. There's also a "HPTE64_R_R" defined
there but I'm not sure if it's usable here, so feel free to create a new
macro if needed.
In that note, the original commit that added this code also added a lot of
hardcoded "15" values for the C bit update in spapr_hpte_set_c() and
ppc_hash64_set_c(), and a "14" value like you're changing here in spapr_hpte_set_r().
If you're feeling generous I believe that another patch replacing these hardcoded values
with bit shift macros is warranted as well.
Thanks,
Daniel
>
> if (cpu->vhyp) {
> PPCVirtualHypervisorClass *vhc =
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 13:40 ` Daniel Henrique Barboza
@ 2021-11-24 18:42 ` Cédric Le Goater
2021-11-24 19:02 ` Daniel Henrique Barboza
2021-11-24 19:17 ` Leandro Lupori
0 siblings, 2 replies; 10+ messages in thread
From: Cédric Le Goater @ 2021-11-24 18:42 UTC (permalink / raw)
To: Daniel Henrique Barboza, Leandro Lupori, qemu-devel, qemu-ppc
Cc: groug, david
On 11/24/21 14:40, Daniel Henrique Barboza wrote:
>
>
> On 11/24/21 09:00, Leandro Lupori wrote:
>> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
>> offset, causing the first byte of the adjacent PTE to be corrupted.
>> This caused a panic when booting FreeBSD, using the Hash MMU.
I wonder how we never hit this issue before. Are you testing PowerNV
and/or pSeries ?
Could you share a FreeBDS image with us ?
> If you add a "Fixes:" tag with the commit that introduced the code you're
> fixing, we can push it right away as a bug fix in 6.2 (assuming it doesn't
> break anything else, of course).
>
> The commit to be fixed in the case seems to be a2dd4e83e76b ("ppc/hash64:
> Rework R and C bit updates")
Indeed.
> One more comment below:
>
>>
>> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
>> ---
>> target/ppc/mmu-hash64.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
>> index 19832c4b46..f165ac691a 100644
>> --- a/target/ppc/mmu-hash64.c
>> +++ b/target/ppc/mmu-hash64.c
>> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
>> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
>> {
>> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
>> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
>
> Instead of adding a '14' you should add a new #define in mmu-hash64.h with this
> value, something like "HPTE64_R_R_SHIFT". This will avoid hardcoding literals
> around the code and forcing us to go to the ISA every time we wonder what's
> an apparently random number represents. There's also a "HPTE64_R_R" defined
> there but I'm not sure if it's usable here, so feel free to create a new
> macro if needed.
>
> In that note, the original commit that added this code also added a lot of
> hardcoded "15" values for the C bit update in spapr_hpte_set_c() and
> ppc_hash64_set_c(), and a "14" value like you're changing here in spapr_hpte_set_r().
> If you're feeling generous I believe that another patch replacing these hardcoded values
> with bit shift macros is warranted as well.
May be for 7.0 though ?
Thanks,
C.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 18:42 ` Cédric Le Goater
@ 2021-11-24 19:02 ` Daniel Henrique Barboza
2021-11-24 19:17 ` Leandro Lupori
1 sibling, 0 replies; 10+ messages in thread
From: Daniel Henrique Barboza @ 2021-11-24 19:02 UTC (permalink / raw)
To: Cédric Le Goater, Leandro Lupori, qemu-devel, qemu-ppc; +Cc: groug, david
On 11/24/21 15:42, Cédric Le Goater wrote:
> On 11/24/21 14:40, Daniel Henrique Barboza wrote:
>>
>>
>> On 11/24/21 09:00, Leandro Lupori wrote:
>>> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
>>> offset, causing the first byte of the adjacent PTE to be corrupted.
>>> This caused a panic when booting FreeBSD, using the Hash MMU.
>
> I wonder how we never hit this issue before. Are you testing PowerNV
> and/or pSeries ?
>
> Could you share a FreeBDS image with us ?
>
>> If you add a "Fixes:" tag with the commit that introduced the code you're
>> fixing, we can push it right away as a bug fix in 6.2 (assuming it doesn't
>> break anything else, of course).
>>
>> The commit to be fixed in the case seems to be a2dd4e83e76b ("ppc/hash64:
>> Rework R and C bit updates")
>
> Indeed.
>
>> One more comment below:
>>
>>>
>>> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
>>> ---
>>> target/ppc/mmu-hash64.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
>>> index 19832c4b46..f165ac691a 100644
>>> --- a/target/ppc/mmu-hash64.c
>>> +++ b/target/ppc/mmu-hash64.c
>>> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
>>> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
>>> {
>>> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
>>> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
>>
>> Instead of adding a '14' you should add a new #define in mmu-hash64.h with this
>> value, something like "HPTE64_R_R_SHIFT". This will avoid hardcoding literals
>> around the code and forcing us to go to the ISA every time we wonder what's
>> an apparently random number represents. There's also a "HPTE64_R_R" defined
>> there but I'm not sure if it's usable here, so feel free to create a new
>> macro if needed.
>>
>> In that note, the original commit that added this code also added a lot of
>> hardcoded "15" values for the C bit update in spapr_hpte_set_c() and
>> ppc_hash64_set_c(), and a "14" value like you're changing here in spapr_hpte_set_r().
>> If you're feeling generous I believe that another patch replacing these hardcoded values
>> with bit shift macros is warranted as well.
>
> May be for 7.0 though ?
Yeah, this extra cleanup I proposed can be postponed to 7.0 in case someone wants
to give it a go.
Daniel
>
> Thanks,
>
> C.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 18:42 ` Cédric Le Goater
2021-11-24 19:02 ` Daniel Henrique Barboza
@ 2021-11-24 19:17 ` Leandro Lupori
2021-11-24 19:42 ` Daniel Henrique Barboza
2021-11-24 19:52 ` Cédric Le Goater
1 sibling, 2 replies; 10+ messages in thread
From: Leandro Lupori @ 2021-11-24 19:17 UTC (permalink / raw)
To: Cédric Le Goater, Daniel Henrique Barboza, qemu-devel, qemu-ppc
Cc: groug, david
[-- Attachment #1: Type: text/plain, Size: 2841 bytes --]
On 11/24/21 14:40, Daniel Henrique Barboza wrote:
>
>
> On 11/24/21 09:00, Leandro Lupori wrote:
>> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
>> offset, causing the first byte of the adjacent PTE to be corrupted.
>> This caused a panic when booting FreeBSD, using the Hash MMU.
I wonder how we never hit this issue before. Are you testing PowerNV
and/or pSeries ?
Could you share a FreeBDS image with us ?
I've hit this issue while testing PowerNV. With pSeries it doesn't happen.
It can be reproduced by trying to boot this iso: https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
It is easier to reproduce it using power8/powernv8.
> If you add a "Fixes:" tag with the commit that introduced the code you're
> fixing, we can push it right away as a bug fix in 6.2 (assuming it doesn't
> break anything else, of course).
>
> The commit to be fixed in the case seems to be a2dd4e83e76b ("ppc/hash64:
> Rework R and C bit updates")
Indeed.
Right.
> One more comment below:
>
>>
>> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
>> ---
>> target/ppc/mmu-hash64.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
>> index 19832c4b46..f165ac691a 100644
>> --- a/target/ppc/mmu-hash64.c
>> +++ b/target/ppc/mmu-hash64.c
>> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
>> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
>> {
>> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
>> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
>
> Instead of adding a '14' you should add a new #define in mmu-hash64.h with this
> value, something like "HPTE64_R_R_SHIFT". This will avoid hardcoding literals
> around the code and forcing us to go to the ISA every time we wonder what's
> an apparently random number represents. There's also a "HPTE64_R_R" defined
> there but I'm not sure if it's usable here, so feel free to create a new
> macro if needed.
>
> In that note, the original commit that added this code also added a lot of
> hardcoded "15" values for the C bit update in spapr_hpte_set_c() and
> ppc_hash64_set_c(), and a "14" value like you're changing here in spapr_hpte_set_r().
> If you're feeling generous I believe that another patch replacing these hardcoded values
> with bit shift macros is warranted as well.
What about creating HPTE64_R_R_BYTE and HPTE64_R_C_BYTE, with the values 14 and 15, respectively,
to make it clear that these are byte offsets within a PTE?
May be for 7.0 though ?
Thanks,
C.
[-- Attachment #2: Type: text/html, Size: 13225 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 19:17 ` Leandro Lupori
@ 2021-11-24 19:42 ` Daniel Henrique Barboza
2021-11-24 20:09 ` Cédric Le Goater
2021-11-24 19:52 ` Cédric Le Goater
1 sibling, 1 reply; 10+ messages in thread
From: Daniel Henrique Barboza @ 2021-11-24 19:42 UTC (permalink / raw)
To: Leandro Lupori, Cédric Le Goater, qemu-devel, qemu-ppc; +Cc: groug, david
On 11/24/21 16:17, Leandro Lupori wrote:
>
>
>
> On 11/24/21 14:40, Daniel Henrique Barboza wrote:
> >
> >
> > On 11/24/21 09:00, Leandro Lupori wrote:
> >> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
> >> offset, causing the first byte of the adjacent PTE to be corrupted.
> >> This caused a panic when booting FreeBSD, using the Hash MMU.
>
> I wonder how we never hit this issue before. Are you testing PowerNV
> and/or pSeries ?
>
> Could you share a FreeBDS image with us ?
>
> I've hit this issue while testing PowerNV. With pSeries it doesn't happen.
>
> It can be reproduced by trying to boot this iso: https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
>
> It is easier to reproduce it using power8/powernv8.
>
>
> > If you add a "Fixes:" tag with the commit that introduced the code you're
> > fixing, we can push it right away as a bug fix in 6.2 (assuming it doesn't
> > break anything else, of course).
> >
> > The commit to be fixed in the case seems to be a2dd4e83e76b ("ppc/hash64:
> > Rework R and C bit updates")
>
> Indeed.
>
> Right.
>
> > One more comment below:
> >
> >>
> >> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> >> ---
> >> target/ppc/mmu-hash64.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> >> index 19832c4b46..f165ac691a 100644
> >> --- a/target/ppc/mmu-hash64.c
> >> +++ b/target/ppc/mmu-hash64.c
> >> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
> >> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
> >> {
> >> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
> >> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
> >
> > Instead of adding a '14' you should add a new #define in mmu-hash64.h with this
> > value, something like "HPTE64_R_R_SHIFT". This will avoid hardcoding literals
> > around the code and forcing us to go to the ISA every time we wonder what's
> > an apparently random number represents. There's also a "HPTE64_R_R" defined
> > there but I'm not sure if it's usable here, so feel free to create a new
> > macro if needed.
> >
> > In that note, the original commit that added this code also added a lot of
> > hardcoded "15" values for the C bit update in spapr_hpte_set_c() and
> > ppc_hash64_set_c(), and a "14" value like you're changing here in spapr_hpte_set_r().
> > If you're feeling generous I believe that another patch replacing these hardcoded values
> > with bit shift macros is warranted as well.
>
> What about creating HPTE64_R_R_BYTEand HPTE64_R_C_BYTE, with the values 14 and 15, respectively,
> to make it clear that these are byte offsets within a PTE?
Looks good to me.
Daniel
>
> May be for 7.0 though ?
>
> Thanks,
>
> C.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 19:17 ` Leandro Lupori
2021-11-24 19:42 ` Daniel Henrique Barboza
@ 2021-11-24 19:52 ` Cédric Le Goater
2021-11-24 21:12 ` Leandro Lupori
1 sibling, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2021-11-24 19:52 UTC (permalink / raw)
To: Leandro Lupori, Daniel Henrique Barboza, qemu-devel, qemu-ppc
Cc: groug, david
> It can be reproduced by trying to boot this iso: https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
>
> It is easier to reproduce it using power8/powernv8.
power8 only has Hash MMU. I understand that FreeBSD also supports power9
processor. with radix ? and XIVE ?
Thanks,
C.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 19:42 ` Daniel Henrique Barboza
@ 2021-11-24 20:09 ` Cédric Le Goater
0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2021-11-24 20:09 UTC (permalink / raw)
To: Daniel Henrique Barboza, Leandro Lupori, qemu-devel, qemu-ppc
Cc: groug, david
On 11/24/21 20:42, Daniel Henrique Barboza wrote:
>
>
> On 11/24/21 16:17, Leandro Lupori wrote:
>>
>>
>>
>> On 11/24/21 14:40, Daniel Henrique Barboza wrote:
>> >
>> >
>> > On 11/24/21 09:00, Leandro Lupori wrote:
>> >> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
>> >> offset, causing the first byte of the adjacent PTE to be corrupted.
>> >> This caused a panic when booting FreeBSD, using the Hash MMU.
>>
>> I wonder how we never hit this issue before. Are you testing PowerNV
>> and/or pSeries ?
>>
>> Could you share a FreeBDS image with us ?
>>
>> I've hit this issue while testing PowerNV. With pSeries it doesn't happen.
>>
>> It can be reproduced by trying to boot this iso: https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
>>
>> It is easier to reproduce it using power8/powernv8.
>>
>>
>> > If you add a "Fixes:" tag with the commit that introduced the code you're
>> > fixing, we can push it right away as a bug fix in 6.2 (assuming it doesn't
>> > break anything else, of course).
>> >
>> > The commit to be fixed in the case seems to be a2dd4e83e76b ("ppc/hash64:
>> > Rework R and C bit updates")
>>
>> Indeed.
>>
>> Right.
>>
>> > One more comment below:
>> >
>> >>
>> >> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
>> >> ---
>> >> target/ppc/mmu-hash64.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
>> >> index 19832c4b46..f165ac691a 100644
>> >> --- a/target/ppc/mmu-hash64.c
>> >> +++ b/target/ppc/mmu-hash64.c
>> >> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
>> >> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
>> >> {
>> >> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
>> >> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
>> >
>> > Instead of adding a '14' you should add a new #define in mmu-hash64.h with this
>> > value, something like "HPTE64_R_R_SHIFT". This will avoid hardcoding literals
>> > around the code and forcing us to go to the ISA every time we wonder what's
>> > an apparently random number represents. There's also a "HPTE64_R_R" defined
>> > there but I'm not sure if it's usable here, so feel free to create a new
>> > macro if needed.
>> >
>> > In that note, the original commit that added this code also added a lot of
>> > hardcoded "15" values for the C bit update in spapr_hpte_set_c() and
>> > ppc_hash64_set_c(), and a "14" value like you're changing here in spapr_hpte_set_r().
>> > If you're feeling generous I believe that another patch replacing these hardcoded values
>> > with bit shift macros is warranted as well.
>>
>> What about creating HPTE64_R_R_BYTEand HPTE64_R_C_BYTE, with the values 14 and 15, respectively,
>> to make it clear that these are byte offsets within a PTE?
_BYTE_OFFSET may be ?
> Looks good to me.
Please update pSeries while you are it. I think HASH_PTE_SIZE_64 / 2
deserves an offset.
Thanks,
C.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 19:52 ` Cédric Le Goater
@ 2021-11-24 21:12 ` Leandro Lupori
0 siblings, 0 replies; 10+ messages in thread
From: Leandro Lupori @ 2021-11-24 21:12 UTC (permalink / raw)
To: Cédric Le Goater, Daniel Henrique Barboza, qemu-devel, qemu-ppc
Cc: groug, david
On 24/11/2021 16:52, Cédric Le Goater wrote:
>> It can be reproduced by trying to boot this iso:
>> https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
>>
>>
>> It is easier to reproduce it using power8/powernv8.
>
> power8 only has Hash MMU. I understand that FreeBSD also supports power9
> processor. with radix ? and XIVE ?
>
Right, FreeBSD supports POWER9 with Radix, that is now the default MMU
choice. To select Hash MMU, you need to pass radix_mmu=0 to kernel
command line.
FreeBSD supports XIVE too, but only for PowerNV.
BTW, when trying to boot with Radix instead of Hash, a different issue
happens. Boot goes further, but then programs start to get SIGILL or
SIGSEGV.
> Thanks,
>
> C.
Thanks,
Leandro
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R
2021-11-24 12:00 [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R Leandro Lupori
2021-11-24 13:40 ` Daniel Henrique Barboza
@ 2021-11-25 3:03 ` David Gibson
1 sibling, 0 replies; 10+ messages in thread
From: David Gibson @ 2021-11-25 3:03 UTC (permalink / raw)
To: Leandro Lupori; +Cc: groug, danielhb413, qemu-ppc, qemu-devel, clg
[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]
On Wed, Nov 24, 2021 at 09:00:46AM -0300, Leandro Lupori wrote:
> When updating the R bit of a PTE, the Hash64 MMU was using a wrong byte
> offset, causing the first byte of the adjacent PTE to be corrupted.
> This caused a panic when booting FreeBSD, using the Hash MMU.
>
> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
Ouch, that's an embarrassing error :/.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target/ppc/mmu-hash64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 19832c4b46..f165ac691a 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -786,7 +786,7 @@ static void ppc_hash64_set_dsi(CPUState *cs, int mmu_idx, uint64_t dar, uint64_t
>
> static void ppc_hash64_set_r(PowerPCCPU *cpu, hwaddr ptex, uint64_t pte1)
> {
> - hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 16;
> + hwaddr base, offset = ptex * HASH_PTE_SIZE_64 + 14;
>
> if (cpu->vhyp) {
> PPCVirtualHypervisorClass *vhc =
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-25 3:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24 12:00 [PATCH] target/ppc: fix Hash64 MMU update of PTE bit R Leandro Lupori
2021-11-24 13:40 ` Daniel Henrique Barboza
2021-11-24 18:42 ` Cédric Le Goater
2021-11-24 19:02 ` Daniel Henrique Barboza
2021-11-24 19:17 ` Leandro Lupori
2021-11-24 19:42 ` Daniel Henrique Barboza
2021-11-24 20:09 ` Cédric Le Goater
2021-11-24 19:52 ` Cédric Le Goater
2021-11-24 21:12 ` Leandro Lupori
2021-11-25 3:03 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.