On 9/27/19 6:32 AM, Aleksandar Markovic wrote: > 25.09.2019. 17.53, "Philippe Mathieu-Daudé" <phi...@redhat.com > <mailto:phi...@redhat.com>> је написао/ла: >> >> On 9/25/19 2:45 PM, Aleksandar Markovic wrote: >> > From: Aleksandar Markovic <amarko...@wavecomp.com > <mailto:amarko...@wavecomp.com>> >> > >> > Mostly fix errors and warnings reported by 'checkpatch.pl > <http://checkpatch.pl> -f'. >> > >> > Signed-off-by: Aleksandar Markovic <amarko...@wavecomp.com > <mailto:amarko...@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.
