[for-6.2,05/43] target/microblaze: Implement do_unaligned_access for user-only

Message ID 20210729004647.282017-6-richard.henderson@linaro.org
State New
Headers show
Series
  • Unaligned accesses for user-only
Related show

Commit Message

Richard Henderson July 29, 2021, 12:46 a.m.
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/microblaze/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.1

Comments

Philippe Mathieu-Daudé July 29, 2021, 8:26 a.m. | #1
On 7/29/21 2:46 AM, Richard Henderson wrote:
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/microblaze/cpu.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Peter Maydell July 29, 2021, 1:26 p.m. | #2
On Thu, 29 Jul 2021 at 01:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/microblaze/cpu.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c

> index 72d8f2a0da..cbec062ed7 100644

> --- a/target/microblaze/cpu.c

> +++ b/target/microblaze/cpu.c

> @@ -367,11 +367,11 @@ static const struct TCGCPUOps mb_tcg_ops = {

>      .synchronize_from_tb = mb_cpu_synchronize_from_tb,

>      .cpu_exec_interrupt = mb_cpu_exec_interrupt,

>      .tlb_fill = mb_cpu_tlb_fill,

> +    .do_unaligned_access = mb_cpu_do_unaligned_access,

>

>  #ifndef CONFIG_USER_ONLY

>      .do_interrupt = mb_cpu_do_interrupt,

>      .do_transaction_failed = mb_cpu_transaction_failed,

> -    .do_unaligned_access = mb_cpu_do_unaligned_access,

>  #endif /* !CONFIG_USER_ONLY */

>  };


If I'm reading the kernel sources correctly, for Microblaze it always
fixes up unaligned accesses, so for our linux-user code we want
"ignore unaligned access errors" rather than reporting them up
to cpu-loop.c, I think ?

-- PMM
Richard Henderson July 29, 2021, 6 p.m. | #3
On 7/29/21 3:26 AM, Peter Maydell wrote:
> On Thu, 29 Jul 2021 at 01:54, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>   target/microblaze/cpu.c | 2 +-

>>   1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c

>> index 72d8f2a0da..cbec062ed7 100644

>> --- a/target/microblaze/cpu.c

>> +++ b/target/microblaze/cpu.c

>> @@ -367,11 +367,11 @@ static const struct TCGCPUOps mb_tcg_ops = {

>>       .synchronize_from_tb = mb_cpu_synchronize_from_tb,

>>       .cpu_exec_interrupt = mb_cpu_exec_interrupt,

>>       .tlb_fill = mb_cpu_tlb_fill,

>> +    .do_unaligned_access = mb_cpu_do_unaligned_access,

>>

>>   #ifndef CONFIG_USER_ONLY

>>       .do_interrupt = mb_cpu_do_interrupt,

>>       .do_transaction_failed = mb_cpu_transaction_failed,

>> -    .do_unaligned_access = mb_cpu_do_unaligned_access,

>>   #endif /* !CONFIG_USER_ONLY */

>>   };

> 

> If I'm reading the kernel sources correctly, for Microblaze it always

> fixes up unaligned accesses, so for our linux-user code we want

> "ignore unaligned access errors" rather than reporting them up

> to cpu-loop.c, I think ?


Ah, in that case we should not be setting MO_ALIGN for some -cpu xxx, I think?  Or does 
the MSR_EE bit cover that?  Anyway, it looked reachable at first glance.


r~
Edgar E. Iglesias July 29, 2021, 6:44 p.m. | #4
On Thu, Jul 29, 2021 at 08:00:50AM -1000, Richard Henderson wrote:
> On 7/29/21 3:26 AM, Peter Maydell wrote:

> > On Thu, 29 Jul 2021 at 01:54, Richard Henderson

> > <richard.henderson@linaro.org> wrote:

> > > 

> > > Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>

> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> > > ---

> > >   target/microblaze/cpu.c | 2 +-

> > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > 

> > > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c

> > > index 72d8f2a0da..cbec062ed7 100644

> > > --- a/target/microblaze/cpu.c

> > > +++ b/target/microblaze/cpu.c

> > > @@ -367,11 +367,11 @@ static const struct TCGCPUOps mb_tcg_ops = {

> > >       .synchronize_from_tb = mb_cpu_synchronize_from_tb,

> > >       .cpu_exec_interrupt = mb_cpu_exec_interrupt,

> > >       .tlb_fill = mb_cpu_tlb_fill,

> > > +    .do_unaligned_access = mb_cpu_do_unaligned_access,

> > > 

> > >   #ifndef CONFIG_USER_ONLY

> > >       .do_interrupt = mb_cpu_do_interrupt,

> > >       .do_transaction_failed = mb_cpu_transaction_failed,

> > > -    .do_unaligned_access = mb_cpu_do_unaligned_access,

> > >   #endif /* !CONFIG_USER_ONLY */

> > >   };

> > 

> > If I'm reading the kernel sources correctly, for Microblaze it always

> > fixes up unaligned accesses, so for our linux-user code we want

> > "ignore unaligned access errors" rather than reporting them up

> > to cpu-loop.c, I think ?


Yes, I think so.

> 

> Ah, in that case we should not be setting MO_ALIGN for some -cpu xxx, I

> think?  Or does the MSR_EE bit cover that?  Anyway, it looked reachable at

> first glance.


Hmm yeah, perhaps we shouldn't be setting MO_ALIGN for linux-user...

Best regards,
Edgar

Patch

diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 72d8f2a0da..cbec062ed7 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -367,11 +367,11 @@  static const struct TCGCPUOps mb_tcg_ops = {
     .synchronize_from_tb = mb_cpu_synchronize_from_tb,
     .cpu_exec_interrupt = mb_cpu_exec_interrupt,
     .tlb_fill = mb_cpu_tlb_fill,
+    .do_unaligned_access = mb_cpu_do_unaligned_access,
 
 #ifndef CONFIG_USER_ONLY
     .do_interrupt = mb_cpu_do_interrupt,
     .do_transaction_failed = mb_cpu_transaction_failed,
-    .do_unaligned_access = mb_cpu_do_unaligned_access,
 #endif /* !CONFIG_USER_ONLY */
 };