Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Sergey Sorokin]

Seems arm_cpu_do_unaligned_access() function could use it. It uses 
cpu_mmu_index() for now but I think use of mmu_idx is preferred. Anyway it's 
the subject for another patch.

13.06.2016, 10:47, "Aurelien Jarno" :
> On 2016-06-10 19:26, Sergey Sorokin wrote:
>>  There are functions cpu_unaligned_access() and do_unaligned_access() that
>>  are called with access type and mmu index arguments. But these arguments
>>  are named 'is_write' and 'is_user' in their declarations.
>>  The patch fixes the names to avoid a confusion.
>
> Unless I missed something, it seems that the is_user/mmu_idx argument is
> never used. Should we maybe just drop it?
>
> Otherwise it looks fine.
>
> --
> Aurelien Jarno GPG: 4096R/1DDD8C9B
> aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Paolo Bonzini]

On 10/06/2016 19:26, Sergey Sorokin wrote:
> cpu-common.h is not included in qom/cpu.h what do you think? Should
> it be included? Or may be MMUAccessType should be just moved into
> another header. For example into exec/memattrs.h

You can move it to qom/cpu.h.

Paolo

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Aurelien Jarno]

On 2016-06-10 19:26, Sergey Sorokin wrote:
> There are functions cpu_unaligned_access() and do_unaligned_access() that
> are called with access type and mmu index arguments. But these arguments
> are named 'is_write' and 'is_user' in their declarations.
> The patch fixes the names to avoid a confusion.

Unless I missed something, it seems that the is_user/mmu_idx argument is
never used. Should we maybe just drop it?

Otherwise it looks fine.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Sergey Sorokin]

cpu-common.h is not included in qom/cpu.h
what do you think? Should it be included? Or may be MMUAccessType should be 
just moved into another header. For example into exec/memattrs.h

10.06.2016, 19:44, "Peter Maydell" :
> On 10 June 2016 at 17:42, Sergey Sorokin  wrote:
>>  What if I combine both patches into single one?
>
> No particular objection.
>
> -- PMM

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Peter Maydell]

On 10 June 2016 at 17:42, Sergey Sorokin  wrote:
> What if I combine both patches into single one?

No particular objection.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Sergey Sorokin]

What if I combine both patches into single one?

10.06.2016, 19:33, "Peter Maydell" :
> On 10 June 2016 at 17:26, Sergey Sorokin  wrote:
>>  There are functions cpu_unaligned_access() and do_unaligned_access() that
>>  are called with access type and mmu index arguments. But these arguments
>>  are named 'is_write' and 'is_user' in their declarations.
>>  The patch fixes the names to avoid a confusion.
>>
>>  Signed-off-by: Sergey Sorokin 
>
> If we're going to touch all of these then we have an enum type
> we should be using instead of just 'int' for the old
> is_write argument: MMUAccessType (defined in cpu-common.h).
>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Sergey Sorokin]

I agree

10.06.2016, 19:33, "Peter Maydell" :
> On 10 June 2016 at 17:26, Sergey Sorokin  wrote:
>>  There are functions cpu_unaligned_access() and do_unaligned_access() that
>>  are called with access type and mmu index arguments. But these arguments
>>  are named 'is_write' and 'is_user' in their declarations.
>>  The patch fixes the names to avoid a confusion.
>>
>>  Signed-off-by: Sergey Sorokin 
>
> If we're going to touch all of these then we have an enum type
> we should be using instead of just 'int' for the old
> is_write argument: MMUAccessType (defined in cpu-common.h).
>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Peter Maydell]

On 10 June 2016 at 17:26, Sergey Sorokin  wrote:
> There are functions cpu_unaligned_access() and do_unaligned_access() that
> are called with access type and mmu index arguments. But these arguments
> are named 'is_write' and 'is_user' in their declarations.
> The patch fixes the names to avoid a confusion.
>
> Signed-off-by: Sergey Sorokin 

If we're going to touch all of these then we have an enum type
we should be using instead of just 'int' for the old
is_write argument: MMUAccessType (defined in cpu-common.h).

thanks
-- PMM

[Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions

[Sergey Sorokin]

There are functions cpu_unaligned_access() and do_unaligned_access() that
are called with access type and mmu index arguments. But these arguments
are named 'is_write' and 'is_user' in their declarations.
The patch fixes the names to avoid a confusion.

Signed-off-by: Sergey Sorokin 
---
 include/qom/cpu.h  |  8 
 target-alpha/cpu.h |  4 ++--
 target-alpha/mem_helper.c  |  4 ++--
 target-arm/internals.h |  4 ++--
 target-arm/op_helper.c | 17 +
 target-mips/cpu.h  |  4 ++--
 target-mips/op_helper.c|  2 +-
 target-sparc/cpu.h |  5 +++--
 target-sparc/ldst_helper.c |  5 +++--
 target-xtensa/cpu.h|  4 ++--
 target-xtensa/op_helper.c  |  3 ++-
 11 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 32f3af3..60985c2 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -141,8 +141,8 @@ typedef struct CPUClass {
 bool (*has_work)(CPUState *cpu);
 void (*do_interrupt)(CPUState *cpu);
 CPUUnassignedAccess do_unassigned_access;
-void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
-int is_write, int is_user, uintptr_t retaddr);
+void (*do_unaligned_access)(CPUState *cpu, vaddr addr, int access_type,
+int mmu_idx, uintptr_t retaddr);
 bool (*virtio_is_big_endian)(CPUState *cpu);
 int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
uint8_t *buf, int len, bool is_write);
@@ -716,12 +716,12 @@ static inline void cpu_unassigned_access(CPUState *cpu, 
hwaddr addr,
 }
 
 static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr,
-int is_write, int is_user,
+int access_type, int mmu_idx,
 uintptr_t retaddr)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-cc->do_unaligned_access(cpu, addr, is_write, is_user, retaddr);
+cc->do_unaligned_access(cpu, addr, access_type, mmu_idx, retaddr);
 }
 #endif
 
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index e71ea70..4e512a2 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -322,8 +322,8 @@ void alpha_cpu_dump_state(CPUState *cs, FILE *f, 
fprintf_function cpu_fprintf,
 hwaddr alpha_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int alpha_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int alpha_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-void alpha_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-   int is_write, int is_user, uintptr_t 
retaddr);
+void alpha_cpu_do_unaligned_access(CPUState *cpu, vaddr addr, int access_type,
+   int mmu_idx, uintptr_t retaddr);
 
 #define cpu_list alpha_cpu_list
 #define cpu_exec cpu_alpha_exec
diff --git a/target-alpha/mem_helper.c b/target-alpha/mem_helper.c
index 7f4d15f..cfb4898 100644
--- a/target-alpha/mem_helper.c
+++ b/target-alpha/mem_helper.c
@@ -98,8 +98,8 @@ uint64_t helper_stq_c_phys(CPUAlphaState *env, uint64_t p, 
uint64_t v)
 return ret;
 }
 
-void alpha_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
-   int is_write, int is_user, uintptr_t 
retaddr)
+void alpha_cpu_do_unaligned_access(CPUState *cs, vaddr addr, int access_type,
+   int mmu_idx, uintptr_t retaddr)
 {
 AlphaCPU *cpu = ALPHA_CPU(cs);
 CPUAlphaState *env = >env;
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 728ecba..6d469bf 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -476,7 +476,7 @@ bool arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int 
mmu_idx,
 bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx);
 
 /* Raise a data fault alignment exception for the specified virtual address */
-void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int is_write,
- int is_user, uintptr_t retaddr);
+void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int access_type,
+ int mmu_idx, uintptr_t retaddr);
 
 #endif
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 35912a1..04316b5 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -79,7 +79,7 @@ uint32_t HELPER(neon_tbl)(CPUARMState *env, uint32_t ireg, 
uint32_t def,
 static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
 unsigned int target_el,
 bool same_el,
-bool s1ptw, int is_write,
+bool s1ptw, bool is_write,
 int fsc)
 {
 uint32_t syn;
@@ -97,7 +97,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t 
template_syn,