Messages in this thread | | | Date | Tue, 26 May 2015 09:30:05 -0600 | From | Khalid Aziz <> | Subject | Re: [PATCH v2] sparc: Resolve conflict between sparc v9 and M7 on usage of bit 9 of TTE |
| |
On 05/24/2015 02:00 PM, David Miller wrote: > From: Khalid Aziz <khalid.aziz@oracle.com> > Date: Fri, 15 May 2015 11:48:15 -0600 > >> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c >> index c38d19f..606e3f9 100644 >> --- a/arch/sparc/kernel/setup_64.c >> +++ b/arch/sparc/kernel/setup_64.c >> @@ -255,6 +255,30 @@ void sun4v_patch_2insn_range(struct sun4v_2insn_patch_entry *start, >> } >> } >> >> +void sun_m7_patch_2insn_range(struct sun4v_2insn_patch_entry *start, >> + struct sun4v_2insn_patch_entry *end) >> +{ >> + while (start < end) { >> + unsigned long addr = start->addr; >> + >> + *(unsigned int *) (addr + 0) = start->insns[0]; >> + /* We are updating an instruction. Make sure it is >> + * written out >> + */ >> + wmb(); >> + __asm__ __volatile__("flush %0" : : "r" (addr + 0)); >> + >> + *(unsigned int *) (addr + 4) = start->insns[1]; >> + /* We are updating an instruction. Make sure it is >> + * written out >> + */ >> + wmb(); >> + __asm__ __volatile__("flush %0" : : "r" (addr + 4)); > > There is no reason to say this in a comment, none of the other code > patching routines mention this, and it's even more excessive to say it > twice in quick succession basically in the same place. > > Please just remove these comments, anyone reading this code knows we > are code patching and will realize that there are memory barrier > requirements. > > Alternatively if you think it's worth mentioning, submit a separate > patch that adds the comment to all of the code patching routines for > consistency. > >> @@ -267,6 +291,9 @@ static void __init sun4v_patch(void) >> >> sun4v_patch_2insn_range(&__sun4v_2insn_patch, >> &__sun4v_2insn_patch_end); >> + if (sun4v_chip_type == SUN4V_CHIP_SPARC_M7) >> + sun_m7_patch_2insn_range(&__sun_m7_2insn_patch, >> + &__sun_m7_2insn_patch_end); > > When a function call spans multiple lines, the second and subsequent line > must start at exactly the first column after the openning parenthesis of > the first line. You must use the appropriate number of TAB and SPACE > characters necessary to achieve this. > > Look at how the sun4v_patch_2insn_range() call before the one you added is > indented. > >> @@ -2312,8 +2345,7 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend, >> _PAGE_P_4U | _PAGE_W_4U); >> if (tlb_type == hypervisor) >> pte_base = (_PAGE_VALID | _PAGE_SZ4MB_4V | >> - _PAGE_CP_4V | _PAGE_CV_4V | >> - _PAGE_P_4V | _PAGE_W_4V); >> + page_cache4v_flag | _PAGE_P_4V | _PAGE_W_4V); > > The existing indentation of these lines after the first of the > pte_base assignment were correct, please do not change how it is > indented. > >> @@ -2465,8 +2497,8 @@ static void __init sun4v_pgprot_init(void) >> kern_linear_pte_xor[0] = (_PAGE_VALID | _PAGE_SZ4MB_4V) ^ >> PAGE_OFFSET; >> #endif >> - kern_linear_pte_xor[0] |= (_PAGE_CP_4V | _PAGE_CV_4V | >> - _PAGE_P_4V | _PAGE_W_4V); >> + kern_linear_pte_xor[0] |= (page_cache4v_flag | _PAGE_P_4V | >> + _PAGE_W_4V); > > Likewise.
All of these changes were caused by my addressing checkpatch barfing, but the way you suggest indenting is more readable. I will clean all this up, ignore checkpatch, test the updated patch and send a new version.
Thanks, Khalid
| |