* [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
@ 2016-06-17 22:08 Aaron Larson
2016-06-17 22:55 ` Scott Wood
0 siblings, 1 reply; 7+ messages in thread
From: Aaron Larson @ 2016-06-17 22:08 UTC (permalink / raw)
To: agraf, david, qemu-devel, qemu-ppc
When e500 PPC is booted multi-core, the non-boot cores are started via
the spin table. ppce500_spin.c:spin_kick() calls
mmubooke_create_initial_mapping() to allocate a 64MB TLB entry, but
the created TLB entry is only 256KB.
The root cause is that the function computing the size of the TLB
entry, namely booke206_page_size_to_tlb assumes MAS1.TSIZE as defined
by latter PPC cores, specifically (n**4)KB. The result is then used by
mmubooke_create_initial_mapping using MAS1_TSIZE_SHIFT, but
MAS1_TSIZE_SHIFT is defined assuming TLB entries are (n**2)KB. I.e., a
difference of shift=7 or shift=8.
Simply changing MAS1_TSIZE_SHIFT from 7 to 8 is not appropriate since
the macro is used elsewhere.
Signed-off-by: Aaron Larson <alarson@ddci.com>
---
hw/ppc/ppce500_spin.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index 76bd78b..7e38f0c 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
/* Create -kernel TLB entries for BookE, linearly spanning 256MB. */
static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
{
- return ctz32(size >> 10) >> 1;
+ /* The EREF indicates that TLB pages are (4 to the power of 2)KB, which
+ * corresponds to MAS1_TSIZE_SHIFT=8, but to support legacy processors that
+ * assume TLB pages are (2 to the power of 2)KB MAS1_TSIZE_SHIFT is
+ * currently 7. */
+ return ctz32(size >> 10) >> (MAS1_TSIZE_SHIFT - 7);
}
static void mmubooke_create_initial_mapping(CPUPPCState *env,
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
2016-06-17 22:08 [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size Aaron Larson
@ 2016-06-17 22:55 ` Scott Wood
2016-06-20 2:15 ` david
0 siblings, 1 reply; 7+ messages in thread
From: Scott Wood @ 2016-06-17 22:55 UTC (permalink / raw)
To: Aaron Larson, agraf, david, qemu-devel, qemu-ppc
On 06/17/2016 05:13 PM, Aaron Larson wrote:
> When e500 PPC is booted multi-core, the non-boot cores are started via
> the spin table. ppce500_spin.c:spin_kick() calls
> mmubooke_create_initial_mapping() to allocate a 64MB TLB entry, but
> the created TLB entry is only 256KB.
>
> The root cause is that the function computing the size of the TLB
> entry, namely booke206_page_size_to_tlb assumes MAS1.TSIZE as defined
> by latter PPC cores, specifically (n**4)KB. The result is then used by
> mmubooke_create_initial_mapping using MAS1_TSIZE_SHIFT, but
> MAS1_TSIZE_SHIFT is defined assuming TLB entries are (n**2)KB. I.e., a
> difference of shift=7 or shift=8.
>
> Simply changing MAS1_TSIZE_SHIFT from 7 to 8 is not appropriate since
> the macro is used elsewhere.
>
> Signed-off-by: Aaron Larson <alarson@ddci.com>
> ---
> hw/ppc/ppce500_spin.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
> index 76bd78b..7e38f0c 100644
> --- a/hw/ppc/ppce500_spin.c
> +++ b/hw/ppc/ppce500_spin.c
> @@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
> /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */
> static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
> {
> - return ctz32(size >> 10) >> 1;
> + /* The EREF indicates that TLB pages are (4 to the power of 2)KB, which
> + * corresponds to MAS1_TSIZE_SHIFT=8, but to support legacy processors that
> + * assume TLB pages are (2 to the power of 2)KB MAS1_TSIZE_SHIFT is
> + * currently 7. */
This is backwards. It's the old processors that can only handle
power-of-4 sizes.
> + return ctz32(size >> 10) >> (MAS1_TSIZE_SHIFT - 7);
The patch that changed MAS1_TSIZE_SHIFT from 8 to 7 was around the same
time as the patch that added this code, which is probably why adjusting
it got missed. Commit 2bd9543cd3 did update the equivalent code in
ppce500_mpc8544ds.c, which now resides in hw/ppc/e500.c and has been
changed to not assume a power-of-2 size. The ppce500_spin version
should be eliminated.
-Scott
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
2016-06-17 22:55 ` Scott Wood
@ 2016-06-20 2:15 ` david
2016-06-20 17:04 ` Scott Wood
0 siblings, 1 reply; 7+ messages in thread
From: david @ 2016-06-20 2:15 UTC (permalink / raw)
To: Scott Wood; +Cc: Aaron Larson, agraf, qemu-devel, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 2825 bytes --]
On Fri, Jun 17, 2016 at 10:55:47PM +0000, Scott Wood wrote:
> On 06/17/2016 05:13 PM, Aaron Larson wrote:
> > When e500 PPC is booted multi-core, the non-boot cores are started via
> > the spin table. ppce500_spin.c:spin_kick() calls
> > mmubooke_create_initial_mapping() to allocate a 64MB TLB entry, but
> > the created TLB entry is only 256KB.
> >
> > The root cause is that the function computing the size of the TLB
> > entry, namely booke206_page_size_to_tlb assumes MAS1.TSIZE as defined
> > by latter PPC cores, specifically (n**4)KB. The result is then used by
> > mmubooke_create_initial_mapping using MAS1_TSIZE_SHIFT, but
> > MAS1_TSIZE_SHIFT is defined assuming TLB entries are (n**2)KB. I.e., a
> > difference of shift=7 or shift=8.
> >
> > Simply changing MAS1_TSIZE_SHIFT from 7 to 8 is not appropriate since
> > the macro is used elsewhere.
> >
> > Signed-off-by: Aaron Larson <alarson@ddci.com>
> > ---
> > hw/ppc/ppce500_spin.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
> > index 76bd78b..7e38f0c 100644
> > --- a/hw/ppc/ppce500_spin.c
> > +++ b/hw/ppc/ppce500_spin.c
> > @@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
> > /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */
> > static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
> > {
> > - return ctz32(size >> 10) >> 1;
> > + /* The EREF indicates that TLB pages are (4 to the power of 2)KB, which
> > + * corresponds to MAS1_TSIZE_SHIFT=8, but to support legacy processors that
> > + * assume TLB pages are (2 to the power of 2)KB MAS1_TSIZE_SHIFT is
> > + * currently 7. */
>
> This is backwards. It's the old processors that can only handle
> power-of-4 sizes.
To clarify, is this a problem in the code, or just in the comment?
> > + return ctz32(size >> 10) >> (MAS1_TSIZE_SHIFT - 7);
>
> The patch that changed MAS1_TSIZE_SHIFT from 8 to 7 was around the same
> time as the patch that added this code, which is probably why adjusting
> it got missed. Commit 2bd9543cd3 did update the equivalent code in
> ppce500_mpc8544ds.c, which now resides in hw/ppc/e500.c and has been
> changed to not assume a power-of-2 size. The ppce500_spin version
> should be eliminated.
Sounds sensible.
Aaron, for some reason I got multiple copies of your patch mail - a
couple of full ones and then a couple more extras which had 0 size.
Was that just something going wrong with your mailer, or did you
attempt to send a couple of different versions?
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
2016-06-20 2:15 ` david
@ 2016-06-20 17:04 ` Scott Wood
0 siblings, 0 replies; 7+ messages in thread
From: Scott Wood @ 2016-06-20 17:04 UTC (permalink / raw)
To: david; +Cc: Aaron Larson, agraf, qemu-devel, qemu-ppc
On 06/19/2016 09:13 PM, david@gibson.dropbear.id.au wrote:
> On Fri, Jun 17, 2016 at 10:55:47PM +0000, Scott Wood wrote:
>> On 06/17/2016 05:13 PM, Aaron Larson wrote:
>>> When e500 PPC is booted multi-core, the non-boot cores are started via
>>> the spin table. ppce500_spin.c:spin_kick() calls
>>> mmubooke_create_initial_mapping() to allocate a 64MB TLB entry, but
>>> the created TLB entry is only 256KB.
>>>
>>> The root cause is that the function computing the size of the TLB
>>> entry, namely booke206_page_size_to_tlb assumes MAS1.TSIZE as defined
>>> by latter PPC cores, specifically (n**4)KB. The result is then used by
>>> mmubooke_create_initial_mapping using MAS1_TSIZE_SHIFT, but
>>> MAS1_TSIZE_SHIFT is defined assuming TLB entries are (n**2)KB. I.e., a
>>> difference of shift=7 or shift=8.
>>>
>>> Simply changing MAS1_TSIZE_SHIFT from 7 to 8 is not appropriate since
>>> the macro is used elsewhere.
>>>
>>> Signed-off-by: Aaron Larson <alarson@ddci.com>
>>> ---
>>> hw/ppc/ppce500_spin.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
>>> index 76bd78b..7e38f0c 100644
>>> --- a/hw/ppc/ppce500_spin.c
>>> +++ b/hw/ppc/ppce500_spin.c
>>> @@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
>>> /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */
>>> static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
>>> {
>>> - return ctz32(size >> 10) >> 1;
>>> + /* The EREF indicates that TLB pages are (4 to the power of 2)KB, which
>>> + * corresponds to MAS1_TSIZE_SHIFT=8, but to support legacy processors that
>>> + * assume TLB pages are (2 to the power of 2)KB MAS1_TSIZE_SHIFT is
>>> + * currently 7. */
>>
>> This is backwards. It's the old processors that can only handle
>> power-of-4 sizes.
>
> To clarify, is this a problem in the code, or just in the comment?
Just the comment.
-Scott
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
@ 2016-06-18 1:20 alarson
0 siblings, 0 replies; 7+ messages in thread
From: alarson @ 2016-06-18 1:20 UTC (permalink / raw)
To: agraf, alarson, david, qemu-devel, qemu-ppc
When e500 PPC is booted multi-core, the non-boot cores are started via
the spin table. ppce500_spin.c:spin_kick() calls
mmubooke_create_initial_mapping() to allocate a 64MB TLB entry, but
the created TLB entry is only 256KB.
The root cause is that the function computing the size of the TLB
entry, namely booke206_page_size_to_tlb assumes MAS1.TSIZE as defined
by latter PPC cores, specifically (n**4)KB. The result is then used by
mmubooke_create_initial_mapping using MAS1_TSIZE_SHIFT, but
MAS1_TSIZE_SHIFT is defined assuming TLB entries are (n**2)KB. I.e., a
difference of shift=7 or shift=8.
Simply changing MAS1_TSIZE_SHIFT from 7 to 8 is not appropriate since
the macro is used elsewhere.
Signed-off-by: Aaron Larson <alarson@ddci.com>
---
hw/ppc/ppce500_spin.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index 76bd78b..7e38f0c 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
/* Create -kernel TLB entries for BookE, linearly spanning 256MB. */
static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
{
- return ctz32(size >> 10) >> 1;
+ /* The EREF indicates that TLB pages are (4 to the power of 2)KB, which
+ * corresponds to MAS1_TSIZE_SHIFT=8, but to support legacy processors that
+ * assume TLB pages are (2 to the power of 2)KB MAS1_TSIZE_SHIFT is
+ * currently 7. */
+ return ctz32(size >> 10) >> (MAS1_TSIZE_SHIFT - 7);
}
static void mmubooke_create_initial_mapping(CPUPPCState *env,
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
@ 2016-06-17 23:54 alarson
0 siblings, 0 replies; 7+ messages in thread
From: alarson @ 2016-06-17 23:54 UTC (permalink / raw)
To: agraf, david, qemu-devel, qemu-ppc
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size
@ 2016-06-17 23:54 alarson
0 siblings, 0 replies; 7+ messages in thread
From: alarson @ 2016-06-17 23:54 UTC (permalink / raw)
To: agraf, david, qemu-devel, qemu-ppc
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-20 18:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17 22:08 [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size Aaron Larson
2016-06-17 22:55 ` Scott Wood
2016-06-20 2:15 ` david
2016-06-20 17:04 ` Scott Wood
2016-06-17 23:54 alarson
2016-06-17 23:54 alarson
2016-06-18 1:20 alarson
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.