| Message ID | 1569415572-19635-2-git-send-email-aleksandar.markovic@rt-rk.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/mips: Misc cleanups for September/October 2019 | expand |
On 9/25/19 2:45 PM, Aleksandar Markovic wrote: > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > Mostly fix errors and warnings reported by 'checkpatch.pl -f'. > > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > --- > target/mips/helper.c | 132 +++++++++++++++++++++++++++++++-------------------- > 1 file changed, 80 insertions(+), 52 deletions(-) > > diff --git a/target/mips/helper.c b/target/mips/helper.c > index a2b6459..3dd1aae 100644 > --- a/target/mips/helper.c > +++ b/target/mips/helper.c > @@ -39,8 +39,8 @@ enum { > #if !defined(CONFIG_USER_ONLY) > > /* no MMU emulation */ > -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > - target_ulong address, int rw, int access_type) > +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > + target_ulong address, int rw, int access_type) > { > *physical = address; > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > } > > /* fixed mapping MMU emulation */ > -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > - target_ulong address, int rw, int access_type) > +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > + target_ulong address, int rw, int access_type) > { > if (address <= (int32_t)0x7FFFFFFFUL) { > - if (!(env->CP0_Status & (1 << CP0St_ERL))) > + if (!(env->CP0_Status & (1 << CP0St_ERL))) { > *physical = address + 0x40000000UL; > - else > + } else { > *physical = address; > - } else if (address <= (int32_t)0xBFFFFFFFUL) > + } > + } else if (address <= (int32_t)0xBFFFFFFFUL) { > *physical = address & 0x1FFFFFFF; > - else > + } else { > *physical = address; > + } > > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > return TLBRET_MATCH; > } > > /* MIPS32/MIPS64 R4000-style MMU emulation */ > -int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > - target_ulong address, int rw, int access_type) > +int r4k_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > + target_ulong address, int rw, int access_type) > { > uint16_t ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask; > int i; > @@ -99,8 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > if (rw != MMU_DATA_STORE || (n ? tlb->D1 : tlb->D0)) { > *physical = tlb->PFN[n] | (address & (mask >> 1)); > *prot = PAGE_READ; > - if (n ? tlb->D1 : tlb->D0) > + if (n ? tlb->D1 : tlb->D0) { > *prot |= PAGE_WRITE; > + } > if (!(n ? tlb->XI1 : tlb->XI0)) { > *prot |= PAGE_EXEC; > } > @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx) > int32_t adetlb_mask; > > switch (mmu_idx) { > - case 3 /* ERL */: > - /* If EU is set, always unmapped */ > + case 3: > + /* > + * ERL > + * If EU is set, always unmapped > + */ My IDE show the current form nicer when the switch is folded. Are these comment really bothering checkpatch? > if (eu) { > return 0; > } > @@ -204,9 +210,9 @@ static int get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical, > pa & ~(hwaddr)segmask); > } > > -static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > - int *prot, target_ulong real_address, > - int rw, int access_type, int mmu_idx) > +static int get_physical_address(CPUMIPSState *env, hwaddr *physical, > + int *prot, target_ulong real_address, > + int rw, int access_type, int mmu_idx) > { > /* User mode can only access useg/xuseg */ > #if defined(TARGET_MIPS64) > @@ -252,14 +258,15 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > } else { > segctl = env->CP0_SegCtl2 >> 16; > } > - ret = get_segctl_physical_address(env, physical, prot, real_address, rw, > - access_type, mmu_idx, segctl, > - 0x3FFFFFFF); > + ret = get_segctl_physical_address(env, physical, prot, > + real_address, rw, access_type, > + mmu_idx, segctl, 0x3FFFFFFF); > #if defined(TARGET_MIPS64) > } else if (address < 0x4000000000000000ULL) { > /* xuseg */ > if (UX && address <= (0x3FFFFFFFFFFFFFFFULL & env->SEGMask)) { > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); > + ret = env->tlb->map_address(env, physical, prot, > + real_address, rw, access_type); > } else { > ret = TLBRET_BADADDR; > } > @@ -267,7 +274,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > /* xsseg */ > if ((supervisor_mode || kernel_mode) && > SX && address <= (0x7FFFFFFFFFFFFFFFULL & env->SEGMask)) { > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); > + ret = env->tlb->map_address(env, physical, prot, > + real_address, rw, access_type); > } else { > ret = TLBRET_BADADDR; > } > @@ -307,7 +315,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > /* xkseg */ > if (kernel_mode && KX && > address <= (0xFFFFFFFF7FFFFFFFULL & env->SEGMask)) { > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); > + ret = env->tlb->map_address(env, physical, prot, > + real_address, rw, access_type); > } else { > ret = TLBRET_BADADDR; > } > @@ -328,8 +337,10 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > access_type, mmu_idx, > env->CP0_SegCtl0 >> 16, 0x1FFFFFFF); > } else { > - /* kseg3 */ > - /* XXX: debug segment is not emulated */ > + /* > + * kseg3 > + * XXX: debug segment is not emulated > + */ Ditto. > ret = get_segctl_physical_address(env, physical, prot, real_address, rw, > access_type, mmu_idx, > env->CP0_SegCtl0, 0x1FFFFFFF); > @@ -515,9 +526,9 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, > #if defined(TARGET_MIPS64) > env->CP0_EntryHi &= env->SEGMask; > env->CP0_XContext = > - /* PTEBase */ (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | > - /* R */ (extract64(address, 62, 2) << (env->SEGBITS - 9)) | > - /* BadVPN2 */ (extract64(address, 13, env->SEGBITS - 13) << 4); > + (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | /* PTEBase */ > + (extract64(address, 62, 2) << (env->SEGBITS - 9)) | /* R */ > + (extract64(address, 13, env->SEGBITS - 13) << 4); /* BadVPN2 */ > #endif > cs->exception_index = exception; > env->error_code = error_code; > @@ -945,7 +956,8 @@ bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > } > > #ifndef CONFIG_USER_ONLY > -hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int rw) > +hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, > + int rw) > { > hwaddr physical; > int prot; > @@ -1005,7 +1017,7 @@ static const char * const excp_names[EXCP_LAST + 1] = { > }; > #endif > > -target_ulong exception_resume_pc (CPUMIPSState *env) > +target_ulong exception_resume_pc(CPUMIPSState *env) > { > target_ulong bad_pc; > target_ulong isa_mode; > @@ -1013,8 +1025,10 @@ target_ulong exception_resume_pc (CPUMIPSState *env) > isa_mode = !!(env->hflags & MIPS_HFLAG_M16); > bad_pc = env->active_tc.PC | isa_mode; > if (env->hflags & MIPS_HFLAG_BMASK) { > - /* If the exception was raised from a delay slot, come back to > - the jump. */ > + /* > + * If the exception was raised from a delay slot, come back to > + * the jump. > + */ > bad_pc -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4); > } > > @@ -1022,14 +1036,14 @@ target_ulong exception_resume_pc (CPUMIPSState *env) > } > > #if !defined(CONFIG_USER_ONLY) > -static void set_hflags_for_handler (CPUMIPSState *env) > +static void set_hflags_for_handler(CPUMIPSState *env) > { > /* Exception handlers are entered in 32-bit mode. */ > env->hflags &= ~(MIPS_HFLAG_M16); > /* ...except that microMIPS lets you choose. */ > if (env->insn_flags & ASE_MICROMIPS) { > - env->hflags |= (!!(env->CP0_Config3 > - & (1 << CP0C3_ISA_ON_EXC)) > + env->hflags |= (!!(env->CP0_Config3 & > + (1 << CP0C3_ISA_ON_EXC)) > << MIPS_HFLAG_M16_SHIFT); > } > } > @@ -1096,10 +1110,12 @@ void mips_cpu_do_interrupt(CPUState *cs) > switch (cs->exception_index) { > case EXCP_DSS: > env->CP0_Debug |= 1 << CP0DB_DSS; > - /* Debug single step cannot be raised inside a delay slot and > - resume will always occur on the next instruction > - (but we assume the pc has always been updated during > - code translation). */ > + /* > + * Debug single step cannot be raised inside a delay slot and > + * resume will always occur on the next instruction > + * (but we assume the pc has always been updated during > + * code translation). > + */ > env->CP0_DEPC = env->active_tc.PC | !!(env->hflags & MIPS_HFLAG_M16); > goto enter_debug_mode; > case EXCP_DINT: > @@ -1111,7 +1127,8 @@ void mips_cpu_do_interrupt(CPUState *cs) > case EXCP_DBp: > env->CP0_Debug |= 1 << CP0DB_DBp; > /* Setup DExcCode - SDBBP instruction */ > - env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << CP0DB_DEC; > + env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | > + (9 << CP0DB_DEC); > goto set_DEPC; > case EXCP_DDBS: > env->CP0_Debug |= 1 << CP0DB_DDBS; > @@ -1132,8 +1149,9 @@ void mips_cpu_do_interrupt(CPUState *cs) > env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_CP0; > env->hflags &= ~(MIPS_HFLAG_KSU); > /* EJTAG probe trap enable is not implemented... */ > - if (!(env->CP0_Status & (1 << CP0St_EXL))) > + if (!(env->CP0_Status & (1 << CP0St_EXL))) { > env->CP0_Cause &= ~(1U << CP0Ca_BD); > + } > env->active_tc.PC = env->exception_base + 0x480; > set_hflags_for_handler(env); > break; > @@ -1159,8 +1177,9 @@ void mips_cpu_do_interrupt(CPUState *cs) > } > env->hflags |= MIPS_HFLAG_CP0; > env->hflags &= ~(MIPS_HFLAG_KSU); > - if (!(env->CP0_Status & (1 << CP0St_EXL))) > + if (!(env->CP0_Status & (1 << CP0St_EXL))) { > env->CP0_Cause &= ~(1U << CP0Ca_BD); > + } > env->active_tc.PC = env->exception_base; > set_hflags_for_handler(env); > break; > @@ -1176,12 +1195,16 @@ void mips_cpu_do_interrupt(CPUState *cs) > uint32_t pending = (env->CP0_Cause & CP0Ca_IP_mask) >> CP0Ca_IP; > > if (env->CP0_Config3 & (1 << CP0C3_VEIC)) { > - /* For VEIC mode, the external interrupt controller feeds > - * the vector through the CP0Cause IP lines. */ > + /* > + * For VEIC mode, the external interrupt controller feeds > + * the vector through the CP0Cause IP lines. > + */ > vector = pending; > } else { > - /* Vectored Interrupts > - * Mask with Status.IM7-IM0 to get enabled interrupts. */ > + /* > + * Vectored Interrupts > + * Mask with Status.IM7-IM0 to get enabled interrupts. > + */ > pending &= (env->CP0_Status >> CP0St_IM) & 0xff; > /* Find the highest-priority interrupt. */ > while (pending >>= 1) { > @@ -1354,7 +1377,8 @@ void mips_cpu_do_interrupt(CPUState *cs) > > env->active_tc.PC += offset; > set_hflags_for_handler(env); > - env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC); > + env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | > + (cause << CP0Ca_EC); > break; > default: > abort(); > @@ -1390,7 +1414,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request) > } > > #if !defined(CONFIG_USER_ONLY) > -void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > +void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra) > { > CPUState *cs = env_cpu(env); > r4k_tlb_t *tlb; > @@ -1400,16 +1424,20 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > target_ulong mask; > > tlb = &env->tlb->mmu.r4k.tlb[idx]; > - /* The qemu TLB is flushed when the ASID changes, so no need to > - flush these entries again. */ > + /* > + * The qemu TLB is flushed when the ASID changes, so no need to > + * flush these entries again. > + */ > if (tlb->G == 0 && tlb->ASID != ASID) { > return; > } > > if (use_extra && env->tlb->tlb_in_use < MIPS_TLB_MAX) { > - /* For tlbwr, we can shadow the discarded entry into > - a new (fake) TLB entry, as long as the guest can not > - tell that it's there. */ > + /* > + * For tlbwr, we can shadow the discarded entry into > + * a new (fake) TLB entry, as long as the guest can not > + * tell that it's there. > + */ > env->tlb->mmu.r4k.tlb[env->tlb->tlb_in_use] = *tlb; > env->tlb->tlb_in_use++; > return; > Except 2 comments, OK for the rest. Another patch that makes rebasing very painful :(
25.09.2019. 17.53, "Philippe Mathieu-Daudé" <philmd@redhat.com> је написао/ла: > > On 9/25/19 2:45 PM, Aleksandar Markovic wrote: > > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > > > Mostly fix errors and warnings reported by 'checkpatch.pl -f'. > > > > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > > --- > > target/mips/helper.c | 132 +++++++++++++++++++++++++++++++-------------------- > > 1 file changed, 80 insertions(+), 52 deletions(-) > > > > diff --git a/target/mips/helper.c b/target/mips/helper.c > > index a2b6459..3dd1aae 100644 > > --- a/target/mips/helper.c > > +++ b/target/mips/helper.c > > @@ -39,8 +39,8 @@ enum { > > #if !defined(CONFIG_USER_ONLY) > > > > /* no MMU emulation */ > > -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > - target_ulong address, int rw, int access_type) > > +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > > + target_ulong address, int rw, int access_type) > > { > > *physical = address; > > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > > @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > } > > > > /* fixed mapping MMU emulation */ > > -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > - target_ulong address, int rw, int access_type) > > +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > > + target_ulong address, int rw, int access_type) > > { > > if (address <= (int32_t)0x7FFFFFFFUL) { > > - if (!(env->CP0_Status & (1 << CP0St_ERL))) > > + if (!(env->CP0_Status & (1 << CP0St_ERL))) { > > *physical = address + 0x40000000UL; > > - else > > + } else { > > *physical = address; > > - } else if (address <= (int32_t)0xBFFFFFFFUL) > > + } > > + } else if (address <= (int32_t)0xBFFFFFFFUL) { > > *physical = address & 0x1FFFFFFF; > > - else > > + } else { > > *physical = address; > > + } > > > > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > > return TLBRET_MATCH; > > } > > > > /* MIPS32/MIPS64 R4000-style MMU emulation */ > > -int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > - target_ulong address, int rw, int access_type) > > +int r4k_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > > + target_ulong address, int rw, int access_type) > > { > > uint16_t ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask; > > int i; > > @@ -99,8 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > if (rw != MMU_DATA_STORE || (n ? tlb->D1 : tlb->D0)) { > > *physical = tlb->PFN[n] | (address & (mask >> 1)); > > *prot = PAGE_READ; > > - if (n ? tlb->D1 : tlb->D0) > > + if (n ? tlb->D1 : tlb->D0) { > > *prot |= PAGE_WRITE; > > + } > > if (!(n ? tlb->XI1 : tlb->XI0)) { > > *prot |= PAGE_EXEC; > > } > > @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx) > > int32_t adetlb_mask; > > > > switch (mmu_idx) { > > - case 3 /* ERL */: > > - /* If EU is set, always unmapped */ > > + case 3: > > + /* > > + * ERL > > + * If EU is set, always unmapped > > + */ > > My IDE show the current form nicer when the switch is folded. > > Are these comment really bothering checkpatch? > While being sintaxically correct, interleaving comments and code in a single code line is considered a bad practice by many. > > if (eu) { > > return 0; > > } > > @@ -204,9 +210,9 @@ static int get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical, > > pa & ~(hwaddr)segmask); > > } > > > > -static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > > - int *prot, target_ulong real_address, > > - int rw, int access_type, int mmu_idx) > > +static int get_physical_address(CPUMIPSState *env, hwaddr *physical, > > + int *prot, target_ulong real_address, > > + int rw, int access_type, int mmu_idx) > > { > > /* User mode can only access useg/xuseg */ > > #if defined(TARGET_MIPS64) > > @@ -252,14 +258,15 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > > } else { > > segctl = env->CP0_SegCtl2 >> 16; > > } > > - ret = get_segctl_physical_address(env, physical, prot, real_address, rw, > > - access_type, mmu_idx, segctl, > > - 0x3FFFFFFF); > > + ret = get_segctl_physical_address(env, physical, prot, > > + real_address, rw, access_type, > > + mmu_idx, segctl, 0x3FFFFFFF); > > #if defined(TARGET_MIPS64) > > } else if (address < 0x4000000000000000ULL) { > > /* xuseg */ > > if (UX && address <= (0x3FFFFFFFFFFFFFFFULL & env->SEGMask)) { > > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); > > + ret = env->tlb->map_address(env, physical, prot, > > + real_address, rw, access_type); > > } else { > > ret = TLBRET_BADADDR; > > } > > @@ -267,7 +274,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > > /* xsseg */ > > if ((supervisor_mode || kernel_mode) && > > SX && address <= (0x7FFFFFFFFFFFFFFFULL & env->SEGMask)) { > > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); > > + ret = env->tlb->map_address(env, physical, prot, > > + real_address, rw, access_type); > > } else { > > ret = TLBRET_BADADDR; > > } > > @@ -307,7 +315,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > > /* xkseg */ > > if (kernel_mode && KX && > > address <= (0xFFFFFFFF7FFFFFFFULL & env->SEGMask)) { > > - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); > > + ret = env->tlb->map_address(env, physical, prot, > > + real_address, rw, access_type); > > } else { > > ret = TLBRET_BADADDR; > > } > > @@ -328,8 +337,10 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, > > access_type, mmu_idx, > > env->CP0_SegCtl0 >> 16, 0x1FFFFFFF); > > } else { > > - /* kseg3 */ > > - /* XXX: debug segment is not emulated */ > > + /* > > + * kseg3 > > + * XXX: debug segment is not emulated > > + */ > > Ditto. > > > ret = get_segctl_physical_address(env, physical, prot, real_address, rw, > > access_type, mmu_idx, > > env->CP0_SegCtl0, 0x1FFFFFFF); > > @@ -515,9 +526,9 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, > > #if defined(TARGET_MIPS64) > > env->CP0_EntryHi &= env->SEGMask; > > env->CP0_XContext = > > - /* PTEBase */ (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | > > - /* R */ (extract64(address, 62, 2) << (env->SEGBITS - 9)) | > > - /* BadVPN2 */ (extract64(address, 13, env->SEGBITS - 13) << 4); > > + (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | /* PTEBase */ > > + (extract64(address, 62, 2) << (env->SEGBITS - 9)) | /* R */ > > + (extract64(address, 13, env->SEGBITS - 13) << 4); /* BadVPN2 */ > > #endif > > cs->exception_index = exception; > > env->error_code = error_code; > > @@ -945,7 +956,8 @@ bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > > } > > > > #ifndef CONFIG_USER_ONLY > > -hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int rw) > > +hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, > > + int rw) > > { > > hwaddr physical; > > int prot; > > @@ -1005,7 +1017,7 @@ static const char * const excp_names[EXCP_LAST + 1] = { > > }; > > #endif > > > > -target_ulong exception_resume_pc (CPUMIPSState *env) > > +target_ulong exception_resume_pc(CPUMIPSState *env) > > { > > target_ulong bad_pc; > > target_ulong isa_mode; > > @@ -1013,8 +1025,10 @@ target_ulong exception_resume_pc (CPUMIPSState *env) > > isa_mode = !!(env->hflags & MIPS_HFLAG_M16); > > bad_pc = env->active_tc.PC | isa_mode; > > if (env->hflags & MIPS_HFLAG_BMASK) { > > - /* If the exception was raised from a delay slot, come back to > > - the jump. */ > > + /* > > + * If the exception was raised from a delay slot, come back to > > + * the jump. > > + */ > > bad_pc -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4); > > } > > > > @@ -1022,14 +1036,14 @@ target_ulong exception_resume_pc (CPUMIPSState *env) > > } > > > > #if !defined(CONFIG_USER_ONLY) > > -static void set_hflags_for_handler (CPUMIPSState *env) > > +static void set_hflags_for_handler(CPUMIPSState *env) > > { > > /* Exception handlers are entered in 32-bit mode. */ > > env->hflags &= ~(MIPS_HFLAG_M16); > > /* ...except that microMIPS lets you choose. */ > > if (env->insn_flags & ASE_MICROMIPS) { > > - env->hflags |= (!!(env->CP0_Config3 > > - & (1 << CP0C3_ISA_ON_EXC)) > > + env->hflags |= (!!(env->CP0_Config3 & > > + (1 << CP0C3_ISA_ON_EXC)) > > << MIPS_HFLAG_M16_SHIFT); > > } > > } > > @@ -1096,10 +1110,12 @@ void mips_cpu_do_interrupt(CPUState *cs) > > switch (cs->exception_index) { > > case EXCP_DSS: > > env->CP0_Debug |= 1 << CP0DB_DSS; > > - /* Debug single step cannot be raised inside a delay slot and > > - resume will always occur on the next instruction > > - (but we assume the pc has always been updated during > > - code translation). */ > > + /* > > + * Debug single step cannot be raised inside a delay slot and > > + * resume will always occur on the next instruction > > + * (but we assume the pc has always been updated during > > + * code translation). > > + */ > > env->CP0_DEPC = env->active_tc.PC | !!(env->hflags & MIPS_HFLAG_M16); > > goto enter_debug_mode; > > case EXCP_DINT: > > @@ -1111,7 +1127,8 @@ void mips_cpu_do_interrupt(CPUState *cs) > > case EXCP_DBp: > > env->CP0_Debug |= 1 << CP0DB_DBp; > > /* Setup DExcCode - SDBBP instruction */ > > - env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << CP0DB_DEC; > > + env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | > > + (9 << CP0DB_DEC); > > goto set_DEPC; > > case EXCP_DDBS: > > env->CP0_Debug |= 1 << CP0DB_DDBS; > > @@ -1132,8 +1149,9 @@ void mips_cpu_do_interrupt(CPUState *cs) > > env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_CP0; > > env->hflags &= ~(MIPS_HFLAG_KSU); > > /* EJTAG probe trap enable is not implemented... */ > > - if (!(env->CP0_Status & (1 << CP0St_EXL))) > > + if (!(env->CP0_Status & (1 << CP0St_EXL))) { > > env->CP0_Cause &= ~(1U << CP0Ca_BD); > > + } > > env->active_tc.PC = env->exception_base + 0x480; > > set_hflags_for_handler(env); > > break; > > @@ -1159,8 +1177,9 @@ void mips_cpu_do_interrupt(CPUState *cs) > > } > > env->hflags |= MIPS_HFLAG_CP0; > > env->hflags &= ~(MIPS_HFLAG_KSU); > > - if (!(env->CP0_Status & (1 << CP0St_EXL))) > > + if (!(env->CP0_Status & (1 << CP0St_EXL))) { > > env->CP0_Cause &= ~(1U << CP0Ca_BD); > > + } > > env->active_tc.PC = env->exception_base; > > set_hflags_for_handler(env); > > break; > > @@ -1176,12 +1195,16 @@ void mips_cpu_do_interrupt(CPUState *cs) > > uint32_t pending = (env->CP0_Cause & CP0Ca_IP_mask) >> CP0Ca_IP; > > > > if (env->CP0_Config3 & (1 << CP0C3_VEIC)) { > > - /* For VEIC mode, the external interrupt controller feeds > > - * the vector through the CP0Cause IP lines. */ > > + /* > > + * For VEIC mode, the external interrupt controller feeds > > + * the vector through the CP0Cause IP lines. > > + */ > > vector = pending; > > } else { > > - /* Vectored Interrupts > > - * Mask with Status.IM7-IM0 to get enabled interrupts. */ > > + /* > > + * Vectored Interrupts > > + * Mask with Status.IM7-IM0 to get enabled interrupts. > > + */ > > pending &= (env->CP0_Status >> CP0St_IM) & 0xff; > > /* Find the highest-priority interrupt. */ > > while (pending >>= 1) { > > @@ -1354,7 +1377,8 @@ void mips_cpu_do_interrupt(CPUState *cs) > > > > env->active_tc.PC += offset; > > set_hflags_for_handler(env); > > - env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC); > > + env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | > > + (cause << CP0Ca_EC); > > break; > > default: > > abort(); > > @@ -1390,7 +1414,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request) > > } > > > > #if !defined(CONFIG_USER_ONLY) > > -void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > > +void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra) > > { > > CPUState *cs = env_cpu(env); > > r4k_tlb_t *tlb; > > @@ -1400,16 +1424,20 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) > > target_ulong mask; > > > > tlb = &env->tlb->mmu.r4k.tlb[idx]; > > - /* The qemu TLB is flushed when the ASID changes, so no need to > > - flush these entries again. */ > > + /* > > + * The qemu TLB is flushed when the ASID changes, so no need to > > + * flush these entries again. > > + */ > > if (tlb->G == 0 && tlb->ASID != ASID) { > > return; > > } > > > > if (use_extra && env->tlb->tlb_in_use < MIPS_TLB_MAX) { > > - /* For tlbwr, we can shadow the discarded entry into > > - a new (fake) TLB entry, as long as the guest can not > > - tell that it's there. */ > > + /* > > + * For tlbwr, we can shadow the discarded entry into > > + * a new (fake) TLB entry, as long as the guest can not > > + * tell that it's there. > > + */ > > env->tlb->mmu.r4k.tlb[env->tlb->tlb_in_use] = *tlb; > > env->tlb->tlb_in_use++; > > return; > > > > Except 2 comments, OK for the rest. > > Another patch that makes rebasing very painful :( > It would be fantastic if you apply the same reasoning to your patches, that spread all over the code base, and happen so frequently, and certainly create enormously more rebasing problems for multitude of people than this patch or series does. Yours, Aleksandar >
On 9/27/19 6:32 AM, Aleksandar Markovic wrote: > 25.09.2019. 17.53, "Philippe Mathieu-Daudé" <philmd@redhat.com > <mailto:philmd@redhat.com>> је написао/ла: >> >> On 9/25/19 2:45 PM, Aleksandar Markovic wrote: >> > From: Aleksandar Markovic <amarkovic@wavecomp.com > <mailto:amarkovic@wavecomp.com>> >> > >> > Mostly fix errors and warnings reported by 'checkpatch.pl > <http://checkpatch.pl> -f'. >> > >> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com > <mailto:amarkovic@wavecomp.com>> >> > --- >> > target/mips/helper.c | 132 > +++++++++++++++++++++++++++++++-------------------- >> > 1 file changed, 80 insertions(+), 52 deletions(-) >> > >> > diff --git a/target/mips/helper.c b/target/mips/helper.c >> > index a2b6459..3dd1aae 100644 >> > --- a/target/mips/helper.c >> > +++ b/target/mips/helper.c >> > @@ -39,8 +39,8 @@ enum { >> > #if !defined(CONFIG_USER_ONLY) >> > >> > /* no MMU emulation */ >> > -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, >> > - target_ulong address, int rw, int access_type) >> > +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, >> > + target_ulong address, int rw, int access_type) >> > { >> > *physical = address; >> > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; >> > @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, > hwaddr *physical, int *prot, >> > } >> > >> > /* fixed mapping MMU emulation */ >> > -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int > *prot, >> > - target_ulong address, int rw, int > access_type) >> > +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int > *prot, >> > + target_ulong address, int rw, int > access_type) >> > { >> > if (address <= (int32_t)0x7FFFFFFFUL) { >> > - if (!(env->CP0_Status & (1 << CP0St_ERL))) >> > + if (!(env->CP0_Status & (1 << CP0St_ERL))) { >> > *physical = address + 0x40000000UL; >> > - else >> > + } else { >> > *physical = address; >> > - } else if (address <= (int32_t)0xBFFFFFFFUL) >> > + } >> > + } else if (address <= (int32_t)0xBFFFFFFFUL) { >> > *physical = address & 0x1FFFFFFF; >> > - else >> > + } else { >> > *physical = address; >> > + } >> > >> > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; >> > return TLBRET_MATCH; >> > } >> > >> > /* MIPS32/MIPS64 R4000-style MMU emulation */ >> > -int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, >> > - target_ulong address, int rw, int access_type) >> > +int r4k_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, >> > + target_ulong address, int rw, int access_type) >> > { >> > uint16_t ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask; >> > int i; >> > @@ -99,8 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr > *physical, int *prot, >> > if (rw != MMU_DATA_STORE || (n ? tlb->D1 : tlb->D0)) { >> > *physical = tlb->PFN[n] | (address & (mask >> 1)); >> > *prot = PAGE_READ; >> > - if (n ? tlb->D1 : tlb->D0) >> > + if (n ? tlb->D1 : tlb->D0) { >> > *prot |= PAGE_WRITE; >> > + } >> > if (!(n ? tlb->XI1 : tlb->XI0)) { >> > *prot |= PAGE_EXEC; >> > } >> > @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, > bool eu, int mmu_idx) >> > int32_t adetlb_mask; >> > >> > switch (mmu_idx) { >> > - case 3 /* ERL */: >> > - /* If EU is set, always unmapped */ >> > + case 3: >> > + /* >> > + * ERL >> > + * If EU is set, always unmapped >> > + */ >> >> My IDE show the current form nicer when the switch is folded. >> >> Are these comment really bothering checkpatch? >> > > While being sintaxically correct, interleaving comments and code in a > single code line is considered a bad practice by many. > >> > if (eu) { >> > return 0; >> > } >> > @@ -204,9 +210,9 @@ static int > get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical, >> > pa & ~(hwaddr)segmask); >> > } >> > >> > -static int get_physical_address (CPUMIPSState *env, hwaddr *physical, >> > - int *prot, target_ulong real_address, >> > - int rw, int access_type, int mmu_idx) >> > +static int get_physical_address(CPUMIPSState *env, hwaddr *physical, >> > + int *prot, target_ulong real_address, >> > + int rw, int access_type, int mmu_idx) >> > { >> > /* User mode can only access useg/xuseg */ >> > #if defined(TARGET_MIPS64) >> > @@ -252,14 +258,15 @@ static int get_physical_address (CPUMIPSState > *env, hwaddr *physical, >> > } else { >> > segctl = env->CP0_SegCtl2 >> 16; >> > } >> > - ret = get_segctl_physical_address(env, physical, prot, > real_address, rw, >> > - access_type, mmu_idx, segctl, >> > - 0x3FFFFFFF); >> > + ret = get_segctl_physical_address(env, physical, prot, >> > + real_address, rw, > access_type, >> > + mmu_idx, segctl, 0x3FFFFFFF); >> > #if defined(TARGET_MIPS64) >> > } else if (address < 0x4000000000000000ULL) { >> > /* xuseg */ >> > if (UX && address <= (0x3FFFFFFFFFFFFFFFULL & env->SEGMask)) { >> > - ret = env->tlb->map_address(env, physical, prot, > real_address, rw, access_type); >> > + ret = env->tlb->map_address(env, physical, prot, >> > + real_address, rw, access_type); >> > } else { >> > ret = TLBRET_BADADDR; >> > } >> > @@ -267,7 +274,8 @@ static int get_physical_address (CPUMIPSState > *env, hwaddr *physical, >> > /* xsseg */ >> > if ((supervisor_mode || kernel_mode) && >> > SX && address <= (0x7FFFFFFFFFFFFFFFULL & env->SEGMask)) { >> > - ret = env->tlb->map_address(env, physical, prot, > real_address, rw, access_type); >> > + ret = env->tlb->map_address(env, physical, prot, >> > + real_address, rw, access_type); >> > } else { >> > ret = TLBRET_BADADDR; >> > } >> > @@ -307,7 +315,8 @@ static int get_physical_address (CPUMIPSState > *env, hwaddr *physical, >> > /* xkseg */ >> > if (kernel_mode && KX && >> > address <= (0xFFFFFFFF7FFFFFFFULL & env->SEGMask)) { >> > - ret = env->tlb->map_address(env, physical, prot, > real_address, rw, access_type); >> > + ret = env->tlb->map_address(env, physical, prot, >> > + real_address, rw, access_type); >> > } else { >> > ret = TLBRET_BADADDR; >> > } >> > @@ -328,8 +337,10 @@ static int get_physical_address (CPUMIPSState > *env, hwaddr *physical, >> > access_type, mmu_idx, >> > env->CP0_SegCtl0 >> 16, > 0x1FFFFFFF); >> > } else { >> > - /* kseg3 */ >> > - /* XXX: debug segment is not emulated */ >> > + /* >> > + * kseg3 >> > + * XXX: debug segment is not emulated >> > + */ >> >> Ditto. >> >> > ret = get_segctl_physical_address(env, physical, prot, > real_address, rw, >> > access_type, mmu_idx, >> > env->CP0_SegCtl0, > 0x1FFFFFFF); >> > @@ -515,9 +526,9 @@ static void raise_mmu_exception(CPUMIPSState > *env, target_ulong address, >> > #if defined(TARGET_MIPS64) >> > env->CP0_EntryHi &= env->SEGMask; >> > env->CP0_XContext = >> > - /* PTEBase */ (env->CP0_XContext & ((~0ULL) << > (env->SEGBITS - 7))) | >> > - /* R */ (extract64(address, 62, 2) << (env->SEGBITS > - 9)) | >> > - /* BadVPN2 */ (extract64(address, 13, env->SEGBITS - 13) > << 4); >> > + (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | /* > PTEBase */ >> > + (extract64(address, 62, 2) << (env->SEGBITS - 9)) | /* > R */ >> > + (extract64(address, 13, env->SEGBITS - 13) << 4); /* > BadVPN2 */ >> > #endif >> > cs->exception_index = exception; >> > env->error_code = error_code; >> > @@ -945,7 +956,8 @@ bool mips_cpu_tlb_fill(CPUState *cs, vaddr > address, int size, >> > } >> > >> > #ifndef CONFIG_USER_ONLY >> > -hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong > address, int rw) >> > +hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong > address, >> > + int rw) >> > { >> > hwaddr physical; >> > int prot; >> > @@ -1005,7 +1017,7 @@ static const char * const excp_names[EXCP_LAST > + 1] = { >> > }; >> > #endif >> > >> > -target_ulong exception_resume_pc (CPUMIPSState *env) >> > +target_ulong exception_resume_pc(CPUMIPSState *env) >> > { >> > target_ulong bad_pc; >> > target_ulong isa_mode; >> > @@ -1013,8 +1025,10 @@ target_ulong exception_resume_pc > (CPUMIPSState *env) >> > isa_mode = !!(env->hflags & MIPS_HFLAG_M16); >> > bad_pc = env->active_tc.PC | isa_mode; >> > if (env->hflags & MIPS_HFLAG_BMASK) { >> > - /* If the exception was raised from a delay slot, come back to >> > - the jump. */ >> > + /* >> > + * If the exception was raised from a delay slot, come back to >> > + * the jump. >> > + */ >> > bad_pc -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4); >> > } >> > >> > @@ -1022,14 +1036,14 @@ target_ulong exception_resume_pc > (CPUMIPSState *env) >> > } >> > >> > #if !defined(CONFIG_USER_ONLY) >> > -static void set_hflags_for_handler (CPUMIPSState *env) >> > +static void set_hflags_for_handler(CPUMIPSState *env) >> > { >> > /* Exception handlers are entered in 32-bit mode. */ >> > env->hflags &= ~(MIPS_HFLAG_M16); >> > /* ...except that microMIPS lets you choose. */ >> > if (env->insn_flags & ASE_MICROMIPS) { >> > - env->hflags |= (!!(env->CP0_Config3 >> > - & (1 << CP0C3_ISA_ON_EXC)) >> > + env->hflags |= (!!(env->CP0_Config3 & >> > + (1 << CP0C3_ISA_ON_EXC)) >> > << MIPS_HFLAG_M16_SHIFT); >> > } >> > } >> > @@ -1096,10 +1110,12 @@ void mips_cpu_do_interrupt(CPUState *cs) >> > switch (cs->exception_index) { >> > case EXCP_DSS: >> > env->CP0_Debug |= 1 << CP0DB_DSS; >> > - /* Debug single step cannot be raised inside a delay slot and >> > - resume will always occur on the next instruction >> > - (but we assume the pc has always been updated during >> > - code translation). */ >> > + /* >> > + * Debug single step cannot be raised inside a delay slot and >> > + * resume will always occur on the next instruction >> > + * (but we assume the pc has always been updated during >> > + * code translation). >> > + */ >> > env->CP0_DEPC = env->active_tc.PC | !!(env->hflags & > MIPS_HFLAG_M16); >> > goto enter_debug_mode; >> > case EXCP_DINT: >> > @@ -1111,7 +1127,8 @@ void mips_cpu_do_interrupt(CPUState *cs) >> > case EXCP_DBp: >> > env->CP0_Debug |= 1 << CP0DB_DBp; >> > /* Setup DExcCode - SDBBP instruction */ >> > - env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) > | 9 << CP0DB_DEC; >> > + env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | >> > + (9 << CP0DB_DEC); >> > goto set_DEPC; >> > case EXCP_DDBS: >> > env->CP0_Debug |= 1 << CP0DB_DDBS; >> > @@ -1132,8 +1149,9 @@ void mips_cpu_do_interrupt(CPUState *cs) >> > env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_CP0; >> > env->hflags &= ~(MIPS_HFLAG_KSU); >> > /* EJTAG probe trap enable is not implemented... */ >> > - if (!(env->CP0_Status & (1 << CP0St_EXL))) >> > + if (!(env->CP0_Status & (1 << CP0St_EXL))) { >> > env->CP0_Cause &= ~(1U << CP0Ca_BD); >> > + } >> > env->active_tc.PC = env->exception_base + 0x480; >> > set_hflags_for_handler(env); >> > break; >> > @@ -1159,8 +1177,9 @@ void mips_cpu_do_interrupt(CPUState *cs) >> > } >> > env->hflags |= MIPS_HFLAG_CP0; >> > env->hflags &= ~(MIPS_HFLAG_KSU); >> > - if (!(env->CP0_Status & (1 << CP0St_EXL))) >> > + if (!(env->CP0_Status & (1 << CP0St_EXL))) { >> > env->CP0_Cause &= ~(1U << CP0Ca_BD); >> > + } >> > env->active_tc.PC = env->exception_base; >> > set_hflags_for_handler(env); >> > break; >> > @@ -1176,12 +1195,16 @@ void mips_cpu_do_interrupt(CPUState *cs) >> > uint32_t pending = (env->CP0_Cause & CP0Ca_IP_mask) >>> CP0Ca_IP; >> > >> > if (env->CP0_Config3 & (1 << CP0C3_VEIC)) { >> > - /* For VEIC mode, the external interrupt > controller feeds >> > - * the vector through the CP0Cause IP lines. */ >> > + /* >> > + * For VEIC mode, the external interrupt > controller feeds >> > + * the vector through the CP0Cause IP lines. >> > + */ >> > vector = pending; >> > } else { >> > - /* Vectored Interrupts >> > - * Mask with Status.IM7-IM0 to get enabled > interrupts. */ >> > + /* >> > + * Vectored Interrupts >> > + * Mask with Status.IM7-IM0 to get enabled > interrupts. >> > + */ >> > pending &= (env->CP0_Status >> CP0St_IM) & 0xff; >> > /* Find the highest-priority interrupt. */ >> > while (pending >>= 1) { >> > @@ -1354,7 +1377,8 @@ void mips_cpu_do_interrupt(CPUState *cs) >> > >> > env->active_tc.PC += offset; >> > set_hflags_for_handler(env); >> > - env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | > (cause << CP0Ca_EC); >> > + env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | >> > + (cause << CP0Ca_EC); >> > break; >> > default: >> > abort(); >> > @@ -1390,7 +1414,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int > interrupt_request) >> > } >> > >> > #if !defined(CONFIG_USER_ONLY) >> > -void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) >> > +void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra) >> > { >> > CPUState *cs = env_cpu(env); >> > r4k_tlb_t *tlb; >> > @@ -1400,16 +1424,20 @@ void r4k_invalidate_tlb (CPUMIPSState *env, > int idx, int use_extra) >> > target_ulong mask; >> > >> > tlb = &env->tlb->mmu.r4k.tlb[idx]; >> > - /* The qemu TLB is flushed when the ASID changes, so no need to >> > - flush these entries again. */ >> > + /* >> > + * The qemu TLB is flushed when the ASID changes, so no need to >> > + * flush these entries again. >> > + */ >> > if (tlb->G == 0 && tlb->ASID != ASID) { >> > return; >> > } >> > >> > if (use_extra && env->tlb->tlb_in_use < MIPS_TLB_MAX) { >> > - /* For tlbwr, we can shadow the discarded entry into >> > - a new (fake) TLB entry, as long as the guest can not >> > - tell that it's there. */ >> > + /* >> > + * For tlbwr, we can shadow the discarded entry into >> > + * a new (fake) TLB entry, as long as the guest can not >> > + * tell that it's there. >> > + */ >> > env->tlb->mmu.r4k.tlb[env->tlb->tlb_in_use] = *tlb; >> > env->tlb->tlb_in_use++; >> > return; >> > >> >> Except 2 comments, OK for the rest. >> >> Another patch that makes rebasing very painful :( >> > > It would be fantastic if you apply the same reasoning to your patches, > that spread all over the code base, and happen so frequently, and > certainly create enormously more rebasing problems for multitude of > people than this patch or series does. Fair enough :D You are free to question and Nack them. Regards, Phil.
Aleksandar, your message is hard to read, because you sent Content-Type: multipart/alternative, and both the test/html and the test/plain alternative ride roughshot over the quoted text's line structure. Quoted patches become unreadable garbage. Please check your e-mail setup. Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes: > 25.09.2019. 17.53, "Philippe Mathieu-Daudé" <philmd@redhat.com> је > написао/ла: >> >> On 9/25/19 2:45 PM, Aleksandar Markovic wrote: >> > From: Aleksandar Markovic <amarkovic@wavecomp.com> >> > >> > Mostly fix errors and warnings reported by 'checkpatch.pl -f'. >> > >> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> >> > --- >> > target/mips/helper.c | 132 > +++++++++++++++++++++++++++++++-------------------- >> > 1 file changed, 80 insertions(+), 52 deletions(-) >> > >> > diff --git a/target/mips/helper.c b/target/mips/helper.c >> > index a2b6459..3dd1aae 100644 >> > --- a/target/mips/helper.c >> > +++ b/target/mips/helper.c >> > @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool > eu, int mmu_idx) >> > int32_t adetlb_mask; >> > >> > switch (mmu_idx) { >> > - case 3 /* ERL */: >> > - /* If EU is set, always unmapped */ >> > + case 3: >> > + /* >> > + * ERL >> > + * If EU is set, always unmapped >> > + */ >> >> My IDE show the current form nicer when the switch is folded. >> >> Are these comment really bothering checkpatch? >> > > While being sintaxically correct, interleaving comments and code in a > single code line is considered a bad practice by many. I take that as a "no". The preferences of "many" (whoever they may be) are a lot less relevant than the specific project's prevailing style. "git-grep ' case .*/\*'" shows thousands of hits. If you want to pursue this change, please put it in a separate patch, so this one is really about fixing "errors and warnings reported by 'checkpatch.pl -f'", as your commit message promises. >> > if (eu) { >> > return 0; >> > } [...] >> Except 2 comments, OK for the rest. >> >> Another patch that makes rebasing very painful :( >> > > It would be fantastic if you apply the same reasoning to your patches, that > spread all over the code base, and happen so frequently, and certainly > create enormously more rebasing problems for multitude of people than this > patch or series does. Please tone down the aggression a notch. Philippe merely observed a trade-off. We deal with such trade-offs all the time. When your reviewer challenges one, you consider it (unless you did that already), then tell him what you concluded. "Tu quoque" is not a proper reply to such an observation (or to anything else for that matter). When you have an issue with Philippe's patches, address it in review of Philippe's patches. Thank you.
On Fri, 27 Sep 2019 at 13:03, Markus Armbruster <armbru@redhat.com> wrote: > > Aleksandar, your message is hard to read, because you sent Content-Type: > multipart/alternative, and both the test/html and the test/plain > alternative ride roughshot over the quoted text's line structure. > Quoted patches become unreadable garbage. Please check your e-mail > setup. Note that among the recent changes to the mailing list config to handle DMARC/DKIM signed emails better is one where we no longer let the list server try to fix up multipart/alternative or html emails into plain text. This is because we want to have the mailing list server never edit emails, in case they're signed and editing would cause DMARC/DKIM failures. So it's possible that an email sending config that previously produced acceptable emails on the list receipient/archive end might no longer do so. thanks -- PMM
-------- Original Message -------- Subject: Re: [PATCH v2 01/20] target/mips: Clean up helper.c Date: Friday, September 27, 2019 14:22 CEST From: Peter Maydell <peter.maydell@linaro.org> On Fri, 27 Sep 2019 at 13:03, Markus Armbruster <armbru@redhat.com> wrote: > > Aleksandar, your message is hard to read, because you sent Content-Type: > multipart/alternative, and both the test/html and the test/plain > alternative ride roughshot over the quoted text's line structure. > Quoted patches become unreadable garbage. Please check your e-mail > setup. Note that among the recent changes to the mailing list config to handle DMARC/DKIM signed emails better is one where we no longer let the list server try to fix up multipart/alternative or html emails into plain text. This is because we want to have the mailing list server never edit emails, in case they're signed and editing would cause DMARC/DKIM failures. So it's possible that an email sending config that previously produced acceptable emails on the list receipient/archive end might no longer do so. thanks -- PMM =gdkqvtcx5dxczmgpum5xioxr7j51bqm0k8vosjpa-tcq@mail.gmail.com>OK, my case belongs to "used to work before" group. I used GMail Android app to send this particular mail, and have been using that app for several months without problema. I am going to find an alternative way of sending mails to the list. I am sorry for any incovenience I caused. Aleksandar
"Aleksandar Markovic" <Aleksandar.Markovic@rt-rk.com> writes: > OK, my case belongs to "used to work before" group. I used GMail > Android app to send this particular mail, and have been using that app > for several months without problema. I am going to find an alternative > way of sending mails to the list. Appreciated! [...]
diff --git a/target/mips/helper.c b/target/mips/helper.c index a2b6459..3dd1aae 100644 --- a/target/mips/helper.c +++ b/target/mips/helper.c @@ -39,8 +39,8 @@ enum { #if !defined(CONFIG_USER_ONLY) /* no MMU emulation */ -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, - target_ulong address, int rw, int access_type) +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, + target_ulong address, int rw, int access_type) { *physical = address; *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, } /* fixed mapping MMU emulation */ -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, - target_ulong address, int rw, int access_type) +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, + target_ulong address, int rw, int access_type) { if (address <= (int32_t)0x7FFFFFFFUL) { - if (!(env->CP0_Status & (1 << CP0St_ERL))) + if (!(env->CP0_Status & (1 << CP0St_ERL))) { *physical = address + 0x40000000UL; - else + } else { *physical = address; - } else if (address <= (int32_t)0xBFFFFFFFUL) + } + } else if (address <= (int32_t)0xBFFFFFFFUL) { *physical = address & 0x1FFFFFFF; - else + } else { *physical = address; + } *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; return TLBRET_MATCH; } /* MIPS32/MIPS64 R4000-style MMU emulation */ -int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, - target_ulong address, int rw, int access_type) +int r4k_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, + target_ulong address, int rw, int access_type) { uint16_t ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask; int i; @@ -99,8 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, if (rw != MMU_DATA_STORE || (n ? tlb->D1 : tlb->D0)) { *physical = tlb->PFN[n] | (address & (mask >> 1)); *prot = PAGE_READ; - if (n ? tlb->D1 : tlb->D0) + if (n ? tlb->D1 : tlb->D0) { *prot |= PAGE_WRITE; + } if (!(n ? tlb->XI1 : tlb->XI0)) { *prot |= PAGE_EXEC; } @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx) int32_t adetlb_mask; switch (mmu_idx) { - case 3 /* ERL */: - /* If EU is set, always unmapped */ + case 3: + /* + * ERL + * If EU is set, always unmapped + */ if (eu) { return 0; } @@ -204,9 +210,9 @@ static int get_segctl_physical_address(CPUMIPSState *env, hwaddr *physical, pa & ~(hwaddr)segmask); } -static int get_physical_address (CPUMIPSState *env, hwaddr *physical, - int *prot, target_ulong real_address, - int rw, int access_type, int mmu_idx) +static int get_physical_address(CPUMIPSState *env, hwaddr *physical, + int *prot, target_ulong real_address, + int rw, int access_type, int mmu_idx) { /* User mode can only access useg/xuseg */ #if defined(TARGET_MIPS64) @@ -252,14 +258,15 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, } else { segctl = env->CP0_SegCtl2 >> 16; } - ret = get_segctl_physical_address(env, physical, prot, real_address, rw, - access_type, mmu_idx, segctl, - 0x3FFFFFFF); + ret = get_segctl_physical_address(env, physical, prot, + real_address, rw, access_type, + mmu_idx, segctl, 0x3FFFFFFF); #if defined(TARGET_MIPS64) } else if (address < 0x4000000000000000ULL) { /* xuseg */ if (UX && address <= (0x3FFFFFFFFFFFFFFFULL & env->SEGMask)) { - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); + ret = env->tlb->map_address(env, physical, prot, + real_address, rw, access_type); } else { ret = TLBRET_BADADDR; } @@ -267,7 +274,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, /* xsseg */ if ((supervisor_mode || kernel_mode) && SX && address <= (0x7FFFFFFFFFFFFFFFULL & env->SEGMask)) { - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); + ret = env->tlb->map_address(env, physical, prot, + real_address, rw, access_type); } else { ret = TLBRET_BADADDR; } @@ -307,7 +315,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, /* xkseg */ if (kernel_mode && KX && address <= (0xFFFFFFFF7FFFFFFFULL & env->SEGMask)) { - ret = env->tlb->map_address(env, physical, prot, real_address, rw, access_type); + ret = env->tlb->map_address(env, physical, prot, + real_address, rw, access_type); } else { ret = TLBRET_BADADDR; } @@ -328,8 +337,10 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical, access_type, mmu_idx, env->CP0_SegCtl0 >> 16, 0x1FFFFFFF); } else { - /* kseg3 */ - /* XXX: debug segment is not emulated */ + /* + * kseg3 + * XXX: debug segment is not emulated + */ ret = get_segctl_physical_address(env, physical, prot, real_address, rw, access_type, mmu_idx, env->CP0_SegCtl0, 0x1FFFFFFF); @@ -515,9 +526,9 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, #if defined(TARGET_MIPS64) env->CP0_EntryHi &= env->SEGMask; env->CP0_XContext = - /* PTEBase */ (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | - /* R */ (extract64(address, 62, 2) << (env->SEGBITS - 9)) | - /* BadVPN2 */ (extract64(address, 13, env->SEGBITS - 13) << 4); + (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) | /* PTEBase */ + (extract64(address, 62, 2) << (env->SEGBITS - 9)) | /* R */ + (extract64(address, 13, env->SEGBITS - 13) << 4); /* BadVPN2 */ #endif cs->exception_index = exception; env->error_code = error_code; @@ -945,7 +956,8 @@ bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size, } #ifndef CONFIG_USER_ONLY -hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int rw) +hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, + int rw) { hwaddr physical; int prot; @@ -1005,7 +1017,7 @@ static const char * const excp_names[EXCP_LAST + 1] = { }; #endif -target_ulong exception_resume_pc (CPUMIPSState *env) +target_ulong exception_resume_pc(CPUMIPSState *env) { target_ulong bad_pc; target_ulong isa_mode; @@ -1013,8 +1025,10 @@ target_ulong exception_resume_pc (CPUMIPSState *env) isa_mode = !!(env->hflags & MIPS_HFLAG_M16); bad_pc = env->active_tc.PC | isa_mode; if (env->hflags & MIPS_HFLAG_BMASK) { - /* If the exception was raised from a delay slot, come back to - the jump. */ + /* + * If the exception was raised from a delay slot, come back to + * the jump. + */ bad_pc -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4); } @@ -1022,14 +1036,14 @@ target_ulong exception_resume_pc (CPUMIPSState *env) } #if !defined(CONFIG_USER_ONLY) -static void set_hflags_for_handler (CPUMIPSState *env) +static void set_hflags_for_handler(CPUMIPSState *env) { /* Exception handlers are entered in 32-bit mode. */ env->hflags &= ~(MIPS_HFLAG_M16); /* ...except that microMIPS lets you choose. */ if (env->insn_flags & ASE_MICROMIPS) { - env->hflags |= (!!(env->CP0_Config3 - & (1 << CP0C3_ISA_ON_EXC)) + env->hflags |= (!!(env->CP0_Config3 & + (1 << CP0C3_ISA_ON_EXC)) << MIPS_HFLAG_M16_SHIFT); } } @@ -1096,10 +1110,12 @@ void mips_cpu_do_interrupt(CPUState *cs) switch (cs->exception_index) { case EXCP_DSS: env->CP0_Debug |= 1 << CP0DB_DSS; - /* Debug single step cannot be raised inside a delay slot and - resume will always occur on the next instruction - (but we assume the pc has always been updated during - code translation). */ + /* + * Debug single step cannot be raised inside a delay slot and + * resume will always occur on the next instruction + * (but we assume the pc has always been updated during + * code translation). + */ env->CP0_DEPC = env->active_tc.PC | !!(env->hflags & MIPS_HFLAG_M16); goto enter_debug_mode; case EXCP_DINT: @@ -1111,7 +1127,8 @@ void mips_cpu_do_interrupt(CPUState *cs) case EXCP_DBp: env->CP0_Debug |= 1 << CP0DB_DBp; /* Setup DExcCode - SDBBP instruction */ - env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | 9 << CP0DB_DEC; + env->CP0_Debug = (env->CP0_Debug & ~(0x1fULL << CP0DB_DEC)) | + (9 << CP0DB_DEC); goto set_DEPC; case EXCP_DDBS: env->CP0_Debug |= 1 << CP0DB_DDBS; @@ -1132,8 +1149,9 @@ void mips_cpu_do_interrupt(CPUState *cs) env->hflags |= MIPS_HFLAG_DM | MIPS_HFLAG_CP0; env->hflags &= ~(MIPS_HFLAG_KSU); /* EJTAG probe trap enable is not implemented... */ - if (!(env->CP0_Status & (1 << CP0St_EXL))) + if (!(env->CP0_Status & (1 << CP0St_EXL))) { env->CP0_Cause &= ~(1U << CP0Ca_BD); + } env->active_tc.PC = env->exception_base + 0x480; set_hflags_for_handler(env); break; @@ -1159,8 +1177,9 @@ void mips_cpu_do_interrupt(CPUState *cs) } env->hflags |= MIPS_HFLAG_CP0; env->hflags &= ~(MIPS_HFLAG_KSU); - if (!(env->CP0_Status & (1 << CP0St_EXL))) + if (!(env->CP0_Status & (1 << CP0St_EXL))) { env->CP0_Cause &= ~(1U << CP0Ca_BD); + } env->active_tc.PC = env->exception_base; set_hflags_for_handler(env); break; @@ -1176,12 +1195,16 @@ void mips_cpu_do_interrupt(CPUState *cs) uint32_t pending = (env->CP0_Cause & CP0Ca_IP_mask) >> CP0Ca_IP; if (env->CP0_Config3 & (1 << CP0C3_VEIC)) { - /* For VEIC mode, the external interrupt controller feeds - * the vector through the CP0Cause IP lines. */ + /* + * For VEIC mode, the external interrupt controller feeds + * the vector through the CP0Cause IP lines. + */ vector = pending; } else { - /* Vectored Interrupts - * Mask with Status.IM7-IM0 to get enabled interrupts. */ + /* + * Vectored Interrupts + * Mask with Status.IM7-IM0 to get enabled interrupts. + */ pending &= (env->CP0_Status >> CP0St_IM) & 0xff; /* Find the highest-priority interrupt. */ while (pending >>= 1) { @@ -1354,7 +1377,8 @@ void mips_cpu_do_interrupt(CPUState *cs) env->active_tc.PC += offset; set_hflags_for_handler(env); - env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC); + env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | + (cause << CP0Ca_EC); break; default: abort(); @@ -1390,7 +1414,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request) } #if !defined(CONFIG_USER_ONLY) -void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) +void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra) { CPUState *cs = env_cpu(env); r4k_tlb_t *tlb; @@ -1400,16 +1424,20 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra) target_ulong mask; tlb = &env->tlb->mmu.r4k.tlb[idx]; - /* The qemu TLB is flushed when the ASID changes, so no need to - flush these entries again. */ + /* + * The qemu TLB is flushed when the ASID changes, so no need to + * flush these entries again. + */ if (tlb->G == 0 && tlb->ASID != ASID) { return; } if (use_extra && env->tlb->tlb_in_use < MIPS_TLB_MAX) { - /* For tlbwr, we can shadow the discarded entry into - a new (fake) TLB entry, as long as the guest can not - tell that it's there. */ + /* + * For tlbwr, we can shadow the discarded entry into + * a new (fake) TLB entry, as long as the guest can not + * tell that it's there. + */ env->tlb->mmu.r4k.tlb[env->tlb->tlb_in_use] = *tlb; env->tlb->tlb_in_use++; return;