Unaligned accesses for user-only

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

Posted by Richard Henderson 4 years, 6 months ago

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 */
 };
 
-- 
2.25.1

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

Posted by Philippe Mathieu-Daudé 4 years, 6 months ago

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>

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

Posted by Peter Maydell 4 years, 6 months ago

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

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

Posted by Richard Henderson 4 years, 6 months ago

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~

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

Posted by Edgar E. Iglesias 4 years, 6 months ago

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