* Re: [PATCH V2] ocxl: Fix access to the AFU Descriptor Data
From: christophe lombard @ 2018-08-14 12:22 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, fbarrat, vaibhav,
andrew.donnellan
In-Reply-To: <87in4dljch.fsf@concordia.ellerman.id.au>
Le 14/08/2018 à 05:26, Michael Ellerman a écrit :
> Hi Christophe,
>
> The patch looks fine, just a nit about the change log:
>
> Christophe Lombard <clombard@linux.vnet.ibm.com> writes:
>> The AFU Information DVSEC capability is a means to extract common,
>> general information about all of the AFUs associated with a Function
>> independent of the specific functionality that each AFU provides.
>>
>> This patch fixes the access to the AFU Descriptor Data indexed by the
>> AFU Info Index field.
>
>> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
>> Cc: stable <stable@vger.kernel.org> # 4.16
>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
>
> When fixing a bug it's always good to describe how the bug manifests.
> ie. in this case we are clearly writing to the wrong location in config
> space, but what is the consequence of that? Does it kill the device, or
> just fails to initialise something correctly? How could I tell if I'm
> hitting this bug currently? How would I tell if the fix is applied
> correctly?
You are right, let me send a new version.
Thanks
>
> cheers
>
>> ---
>> Changelog[v2]
>> - Rebase to latest upstream.
>> - Use pci_write_config_byte instead of pci_write_config_word
>> ---
>> drivers/misc/ocxl/config.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
>> index 2e30de9..57a6bb1 100644
>> --- a/drivers/misc/ocxl/config.c
>> +++ b/drivers/misc/ocxl/config.c
>> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>> u32 val;
>> int rc, templ_major, templ_minor, len;
>>
>> - pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
>> + pci_write_config_byte(dev,
>> + fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
>> + afu_idx);
>> rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
>> if (rc)
>> return rc;
>> --
>> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v7 7/9] powerpc/pseries: Dump the SLB contents on SLB MCE errors.
From: Mahesh Jagannath Salgaonkar @ 2018-08-14 10:57 UTC (permalink / raw)
To: Nicholas Piggin, Aneesh Kumar K.V
Cc: Michal Suchanek, Ananth Narayan, linuxppc-dev, Laurent Dufour
In-Reply-To: <20180814002616.18546185@roar.ozlabs.ibm.com>
On 08/13/2018 07:57 PM, Nicholas Piggin wrote:
> On Mon, 13 Aug 2018 09:47:04 +0530
> Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>
>> On 08/11/2018 10:03 AM, Nicholas Piggin wrote:
>>> On Tue, 07 Aug 2018 19:47:39 +0530
>>> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>>>
>>>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>>
>>>> If we get a machine check exceptions due to SLB errors then dump the
>>>> current SLB contents which will be very much helpful in debugging the
>>>> root cause of SLB errors. Introduce an exclusive buffer per cpu to hold
>>>> faulty SLB entries. In real mode mce handler saves the old SLB contents
>>>> into this buffer accessible through paca and print it out later in virtual
>>>> mode.
>>>>
>>>> With this patch the console will log SLB contents like below on SLB MCE
>>>> errors:
>>>>
>>>> [ 507.297236] SLB contents of cpu 0x1
>>>> [ 507.297237] Last SLB entry inserted at slot 16
>>>> [ 507.297238] 00 c000000008000000 400ea1b217000500
>>>> [ 507.297239] 1T ESID= c00000 VSID= ea1b217 LLP:100
>>>> [ 507.297240] 01 d000000008000000 400d43642f000510
>>>> [ 507.297242] 1T ESID= d00000 VSID= d43642f LLP:110
>>>> [ 507.297243] 11 f000000008000000 400a86c85f000500
>>>> [ 507.297244] 1T ESID= f00000 VSID= a86c85f LLP:100
>>>> [ 507.297245] 12 00007f0008000000 4008119624000d90
>>>> [ 507.297246] 1T ESID= 7f VSID= 8119624 LLP:110
>>>> [ 507.297247] 13 0000000018000000 00092885f5150d90
>>>> [ 507.297247] 256M ESID= 1 VSID= 92885f5150 LLP:110
>>>> [ 507.297248] 14 0000010008000000 4009e7cb50000d90
>>>> [ 507.297249] 1T ESID= 1 VSID= 9e7cb50 LLP:110
>>>> [ 507.297250] 15 d000000008000000 400d43642f000510
>>>> [ 507.297251] 1T ESID= d00000 VSID= d43642f LLP:110
>>>> [ 507.297252] 16 d000000008000000 400d43642f000510
>>>> [ 507.297253] 1T ESID= d00000 VSID= d43642f LLP:110
>>>> [ 507.297253] ----------------------------------
>>>> [ 507.297254] SLB cache ptr value = 3
>>>> [ 507.297254] Valid SLB cache entries:
>>>> [ 507.297255] 00 EA[0-35]= 7f000
>>>> [ 507.297256] 01 EA[0-35]= 1
>>>> [ 507.297257] 02 EA[0-35]= 1000
>>>> [ 507.297257] Rest of SLB cache entries:
>>>> [ 507.297258] 03 EA[0-35]= 7f000
>>>> [ 507.297258] 04 EA[0-35]= 1
>>>> [ 507.297259] 05 EA[0-35]= 1000
>>>> [ 507.297260] 06 EA[0-35]= 12
>>>> [ 507.297260] 07 EA[0-35]= 7f000
>>>>
>>>> Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>>> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
>>>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>> Changes in V7:
>>>> - Print slb cache ptr value and slb cache data
>>>> ---
>>>> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 7 ++
>>>> arch/powerpc/include/asm/paca.h | 4 +
>>>> arch/powerpc/mm/slb.c | 73 +++++++++++++++++++++++++
>>>> arch/powerpc/platforms/pseries/ras.c | 10 +++
>>>> arch/powerpc/platforms/pseries/setup.c | 10 +++
>>>> 5 files changed, 103 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> index cc00a7088cf3..5a3fe282076d 100644
>>>> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
>>>> @@ -485,9 +485,16 @@ static inline void hpte_init_pseries(void) { }
>>>>
>>>> extern void hpte_init_native(void);
>>>>
>>>> +struct slb_entry {
>>>> + u64 esid;
>>>> + u64 vsid;
>>>> +};
>>>> +
>>>> extern void slb_initialize(void);
>>>> extern void slb_flush_and_rebolt(void);
>>>> extern void slb_flush_and_rebolt_realmode(void);
>>>> +extern void slb_save_contents(struct slb_entry *slb_ptr);
>>>> +extern void slb_dump_contents(struct slb_entry *slb_ptr);
>>>>
>>>> extern void slb_vmalloc_update(void);
>>>> extern void slb_set_size(u16 size);
>>>> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
>>>> index 7f22929ce915..233d25ff6f64 100644
>>>> --- a/arch/powerpc/include/asm/paca.h
>>>> +++ b/arch/powerpc/include/asm/paca.h
>>>> @@ -254,6 +254,10 @@ struct paca_struct {
>>>> #endif
>>>> #ifdef CONFIG_PPC_PSERIES
>>>> u8 *mce_data_buf; /* buffer to hold per cpu rtas errlog */
>>>> +
>>>> + /* Capture SLB related old contents in MCE handler. */
>>>> + struct slb_entry *mce_faulty_slbs;
>>>> + u16 slb_save_cache_ptr;
>>>> #endif /* CONFIG_PPC_PSERIES */
>>>> } ____cacheline_aligned;
>>>>
>>>> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
>>>> index e89f675f1b5e..16a53689ffd4 100644
>>>> --- a/arch/powerpc/mm/slb.c
>>>> +++ b/arch/powerpc/mm/slb.c
>>>> @@ -151,6 +151,79 @@ void slb_flush_and_rebolt_realmode(void)
>>>> get_paca()->slb_cache_ptr = 0;
>>>> }
>>>>
>>>> +void slb_save_contents(struct slb_entry *slb_ptr)
>>>> +{
>>>> + int i;
>>>> + unsigned long e, v;
>>>> +
>>>> + /* Save slb_cache_ptr value. */
>>>> + get_paca()->slb_save_cache_ptr = get_paca()->slb_cache_ptr;
>>>
>>> What's the point of saving this?
>>
>> This is to know how many valid cache entries were present at the time of
>> SLB mutlihit. We use this index value while dumping the slb cahce entries.
>
> Oh I see you're dumping that thing as well. I don't know if that's
> worth doing, it just gives you the first 8 SLB entries installed but
> you already have those (or they're overwritten and irrelevat).
Aneesh, Can you comment on this ?
>
>>
>>>
>>>> +
>>>> + if (!slb_ptr)
>>>> + return;
>>>
>>> Can this ever happen?
>>
>> May be Never. We allocate the memory at very early stage. But just added
>> as sanity check.
>
> Okay if you think it's needed.
>
>>
>>>
>>>> +
>>>> + for (i = 0; i < mmu_slb_size; i++) {
>>>> + asm volatile("slbmfee %0,%1" : "=r" (e) : "r" (i));
>>>> + asm volatile("slbmfev %0,%1" : "=r" (v) : "r" (i));
>>>
>>> Does the UM say these instructions can cause machine checks if the SLB
>>> is corrupted? It talks about mfslb instruction causing MCE, but there
>>> seems to be no such instruction so I wonder if that's a typo for slbmf?
>>>
>>> Seems like a parity error in the SLB should cause a MCE, at least,
>>> because it can't guarantee valid data for the instruction in that case
>>> (multi-hit may be different because you aren't searching by EA).
>>>
>>> You could limit slb saving to a single level of recursion to avoid
>>> the problem.
>>
>> Yeah, we could do this OR restrict slb saving only for SLB multi-hit.
>> Parity errors are anyway hardware errors. If parity error is transient
>> then saving of SLBs may not trigger another MCE. In that case old SLB
>> content would look ok even if we dump them on console. What do you say ?
>
> I'm not sure. A parity error I think can cause a multi hit. Can you be
> sure of a software caused multi hit? Would be a good idea if you can I
> think. It may be a good idea to avoid recursion as well, just in case.
yeah, you are right. Parity errors can also cause multi-hit. Will limit
slb saving to single level of recursion.
Thanks for your review.
-Mahesh.
>
> Thanks,
> Nick
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation
From: zhong jiang @ 2018-08-14 10:28 UTC (permalink / raw)
To: Michael Ellerman
Cc: tony.luck, fenghua.yu, benh, paulus, oleg, rppt, dhowells, akpm,
viro, linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <874lfxjnzl.fsf@concordia.ellerman.id.au>
On 2018/8/14 17:28, Michael Ellerman wrote:
> zhong jiang <zhongjiang@huawei.com> writes:
>> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
>> So just replace it.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
> This code is copied from binutils and we don't want to needlessly cause
> it to diverge from the binutils copy.
>
> So thanks but no thanks.
Thank you for clarification.
Sincerely
zhong jiang
> cheers
>
>> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
>> index ac2b55b..f3f57a1 100644
>> --- a/arch/powerpc/xmon/ppc-opc.c
>> +++ b/arch/powerpc/xmon/ppc-opc.c
>> @@ -966,8 +966,7 @@
>> { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
>> };
>>
>> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
>> - / sizeof (powerpc_operands[0]));
>> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
>>
>> /* The functions used to insert and extract complicated operands. */
>>
>> @@ -6980,8 +6979,7 @@
>> {"fcfidu.", XRC(63,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT, FRB}},
>> };
>>
>> -const int powerpc_num_opcodes =
>> - sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
>> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
>> \f
>> /* The VLE opcode table.
>>
>> @@ -7219,8 +7217,7 @@
>> {"se_bl", BD8(58,0,1), BD8_MASK, PPCVLE, 0, {B8}},
>> };
>>
>> -const int vle_num_opcodes =
>> - sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
>> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
>> \f
>> /* The macro table. This is only used by the assembler. */
>>
>> @@ -7288,5 +7285,4 @@
>> {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
>> };
>>
>> -const int powerpc_num_macros =
>> - sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
>> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
>> --
>> 1.7.12.4
> .
>
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Christophe LEROY @ 2018-08-14 10:28 UTC (permalink / raw)
To: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <40132e5c-e7a7-46d1-be1d-24cb1b91923a@linux.ibm.com>
Le 13/08/2018 à 15:44, Aneesh Kumar K.V a écrit :
> On 08/13/2018 06:57 PM, Christophe Leroy wrote:
>> While implementing TLB miss HW assistance on the 8xx, the following
>> warning was encountered:
>>
>> [ 423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412
>> ___slab_alloc.constprop.30+0x26c/0x46c
>> [ 423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted
>> 4.18.0-rc8-00664-g2dfff9121c55 #671
>> [ 423.733075] NIP: c0108f90 LR: c0109ad0 CTR: 00000004
>> [ 423.733121] REGS: c455bba0 TRAP: 0700 Not tainted
>> (4.18.0-rc8-00664-g2dfff9121c55)
>> [ 423.733147] MSR: 00021032 <ME,IR,DR,RI> CR: 24224848 XER: 20000000
>> [ 423.733319]
>> [ 423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0
>> c0011b34 c7fa41e0 c455be30
>> [ 423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842
>> 10018840 c079b37c 00000040
>> [ 423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000
>> 00000100 00000200 c455a000
>> [ 423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000
>> c7fa41e0 00000000 00009032
>> [ 423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
>> [ 423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
>> [ 423.734283] Call Trace:
>> [ 423.734326] [c455bc50] [00000100] 0x100 (unreliable)
>> [ 423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
>> [ 423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
>> [ 423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
>> [ 423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
>> [ 423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
>> [ 423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
>> [ 423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
>> [ 423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
>> [ 423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
>> [ 423.735271] Instruction dump:
>> [ 423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78
>> 4bfffe24 81370010
>> [ 423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 4bfffeb8
>> 81340010 712a0004
>> [ 423.735757] ---[ end trace e9b222919a470790 ]---
>>
>> This warning occurs when calling kmem_cache_zalloc() on a
>> cache having a constructor.
>>
>> In this case it happens because PGD cache and 512k hugepte cache are
>> the same size (4k). While a cache with constructor is created for
>> the PGD, hugepages create cache without constructor and uses
>> kmem_cache_zalloc(). As both expect a cache with the same size,
>> the hugepages reuse the cache created for PGD, hence the conflict.
>>
>> As the constructors only aim at zeroing the allocated memory, this
>> patch fixes this issue by removing the constructors and using
>> kmem_cache_zalloc() instead.
>>
>
> But that means we zero out on each alloc from the slab right? Earlier we
> allocated we we added memory to the slab. Also we have code that
> carefully zero things out when we free the page table back to slab.
> The idea there was, it is better take the cost of zeroing out during
> free rather than fault.
Ok, then it means we have to do it the other way round and change
hugetlb to use cache with constructors as well.
At first look, it seems quite tricky to do it though. The constructor
doesn't get the size of the cache, so it means we need one constructor
for each possible size.
Christophe
>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/include/asm/book3s/32/pgalloc.h | 2 +-
>> arch/powerpc/include/asm/book3s/64/pgalloc.h | 4 ++--
>> arch/powerpc/include/asm/nohash/32/pgalloc.h | 2 +-
>> arch/powerpc/include/asm/nohash/64/pgalloc.h | 6 +++---
>> arch/powerpc/mm/init-common.c | 21 +++------------------
>> 5 files changed, 10 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h
>> b/arch/powerpc/include/asm/book3s/32/pgalloc.h
>> index 82e44b1a00ae..4c23cc1ae7a1 100644
>> --- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
>> +++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
>> @@ -32,7 +32,7 @@ extern struct kmem_cache *pgtable_cache[];
>>
>> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>> {
>> - return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
>> + return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> }
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h
>> b/arch/powerpc/include/asm/book3s/64/pgalloc.h
>> index 76234a14b97d..074359cd632a 100644
>> --- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
>> +++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
>> @@ -81,7 +81,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>> if (radix_enabled())
>> return radix__pgd_alloc(mm);
>>
>> - pgd = kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
>> + pgd = kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> /*
>> * Don't scan the PGD for pointers, it contains references to
>> PUDs but
>> @@ -120,7 +120,7 @@ static inline pud_t *pud_alloc_one(struct
>> mm_struct *mm, unsigned long addr)
>> {
>> pud_t *pud;
>>
>> - pud = kmem_cache_alloc(PGT_CACHE(PUD_CACHE_INDEX),
>> + pud = kmem_cache_zalloc(PGT_CACHE(PUD_CACHE_INDEX),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> /*
>> * Tell kmemleak to ignore the PUD, that means don't scan it for
>> diff --git a/arch/powerpc/include/asm/nohash/32/pgalloc.h
>> b/arch/powerpc/include/asm/nohash/32/pgalloc.h
>> index 8825953c225b..766cf0c90d19 100644
>> --- a/arch/powerpc/include/asm/nohash/32/pgalloc.h
>> +++ b/arch/powerpc/include/asm/nohash/32/pgalloc.h
>> @@ -32,7 +32,7 @@ extern struct kmem_cache *pgtable_cache[];
>>
>> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>> {
>> - return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
>> + return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> }
>>
>> diff --git a/arch/powerpc/include/asm/nohash/64/pgalloc.h
>> b/arch/powerpc/include/asm/nohash/64/pgalloc.h
>> index e2d62d033708..54ee5ac02d81 100644
>> --- a/arch/powerpc/include/asm/nohash/64/pgalloc.h
>> +++ b/arch/powerpc/include/asm/nohash/64/pgalloc.h
>> @@ -43,7 +43,7 @@ extern struct kmem_cache *pgtable_cache[];
>>
>> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
>> {
>> - return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
>> + return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> }
>>
>> @@ -56,7 +56,7 @@ static inline void pgd_free(struct mm_struct *mm,
>> pgd_t *pgd)
>>
>> static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned
>> long addr)
>> {
>> - return kmem_cache_alloc(PGT_CACHE(PUD_INDEX_SIZE),
>> + return kmem_cache_zalloc(PGT_CACHE(PUD_INDEX_SIZE),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> }
>>
>> @@ -86,7 +86,7 @@ static inline void pmd_populate(struct mm_struct
>> *mm, pmd_t *pmd,
>>
>> static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned
>> long addr)
>> {
>> - return kmem_cache_alloc(PGT_CACHE(PMD_CACHE_INDEX),
>> + return kmem_cache_zalloc(PGT_CACHE(PMD_CACHE_INDEX),
>> pgtable_gfp_flags(mm, GFP_KERNEL));
>> }
>>
>> diff --git a/arch/powerpc/mm/init-common.c
>> b/arch/powerpc/mm/init-common.c
>> index 2b656e67f2ea..2ae15ff8f76f 100644
>> --- a/arch/powerpc/mm/init-common.c
>> +++ b/arch/powerpc/mm/init-common.c
>> @@ -25,21 +25,6 @@
>> #include <asm/pgalloc.h>
>> #include <asm/pgtable.h>
>>
>> -static void pgd_ctor(void *addr)
>> -{
>> - memset(addr, 0, PGD_TABLE_SIZE);
>> -}
>> -
>> -static void pud_ctor(void *addr)
>> -{
>> - memset(addr, 0, PUD_TABLE_SIZE);
>> -}
>> -
>> -static void pmd_ctor(void *addr)
>> -{
>> - memset(addr, 0, PMD_TABLE_SIZE);
>> -}
>> -
>> struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE];
>> EXPORT_SYMBOL_GPL(pgtable_cache); /* used by kvm_hv module */
>>
>> @@ -91,15 +76,15 @@ EXPORT_SYMBOL_GPL(pgtable_cache_add); /* used
>> by kvm_hv module */
>>
>> void pgtable_cache_init(void)
>> {
>> - pgtable_cache_add(PGD_INDEX_SIZE, pgd_ctor);
>> + pgtable_cache_add(PGD_INDEX_SIZE, NULL);
>>
>> if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
>> - pgtable_cache_add(PMD_CACHE_INDEX, pmd_ctor);
>> + pgtable_cache_add(PMD_CACHE_INDEX, NULL);
>> /*
>> * In all current configs, when the PUD index exists it's the
>> * same size as either the pgd or pmd index except with THP enabled
>> * on book3s 64
>> */
>> if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
>> - pgtable_cache_add(PUD_CACHE_INDEX, pud_ctor);
>> + pgtable_cache_add(PUD_CACHE_INDEX, NULL);
>> }
>>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation
From: Michael Ellerman @ 2018-08-14 9:28 UTC (permalink / raw)
To: zhong jiang, tony.luck, fenghua.yu, benh, paulus, oleg, rppt,
dhowells, akpm, viro
Cc: linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <1534214814-9043-3-git-send-email-zhongjiang@huawei.com>
zhong jiang <zhongjiang@huawei.com> writes:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
> So just replace it.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
This code is copied from binutils and we don't want to needlessly cause
it to diverge from the binutils copy.
So thanks but no thanks.
cheers
> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> index ac2b55b..f3f57a1 100644
> --- a/arch/powerpc/xmon/ppc-opc.c
> +++ b/arch/powerpc/xmon/ppc-opc.c
> @@ -966,8 +966,7 @@
> { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> };
>
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> - / sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
>
> /* The functions used to insert and extract complicated operands. */
>
> @@ -6980,8 +6979,7 @@
> {"fcfidu.", XRC(63,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT, FRB}},
> };
>
> -const int powerpc_num_opcodes =
> - sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> \f
> /* The VLE opcode table.
>
> @@ -7219,8 +7217,7 @@
> {"se_bl", BD8(58,0,1), BD8_MASK, PPCVLE, 0, {B8}},
> };
>
> -const int vle_num_opcodes =
> - sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
> \f
> /* The macro table. This is only used by the assembler. */
>
> @@ -7288,5 +7285,4 @@
> {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
> };
>
> -const int powerpc_num_macros =
> - sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
> --
> 1.7.12.4
^ permalink raw reply
* [PATCH 2/2] powerpc/process: Constify the number of insns printed by show instructions functions.
From: Christophe Leroy @ 2018-08-14 8:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, muriloo
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <718cc9c9bd1d4bb2b4c2596f1a7ee00334e77055.1534192631.git.christophe.leroy@c-s.fr>
instructions_to_print var is assigned value 16 and there is no
way to change it.
This patch replaces it by a constant.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/process.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index c722ce4ca1c0..6317f2ed04ab 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1259,17 +1259,16 @@ struct task_struct *__switch_to(struct task_struct *prev,
return last;
}
-static int instructions_to_print = 16;
+#define NR_INSN_TO_PRINT 16
static void show_instructions(struct pt_regs *regs)
{
int i;
- unsigned long pc = regs->nip - (instructions_to_print * 3 / 4 *
- sizeof(int));
+ unsigned long pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
printk("Instruction dump:");
- for (i = 0; i < instructions_to_print; i++) {
+ for (i = 0; i < NR_INSN_TO_PRINT; i++) {
int instr;
if (!(i % 8))
@@ -1306,9 +1305,9 @@ void show_user_instructions(struct pt_regs *regs)
char buf[96]; /* enough for 8 times 9 + 2 chars */
int l = 0;
- pc = regs->nip - (instructions_to_print * 3 / 4 * sizeof(int));
+ pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
- for (i = 0; i < instructions_to_print; i++) {
+ for (i = 0; i < NR_INSN_TO_PRINT; i++) {
int instr;
if (!(i % 8) && (i > 0)) {
--
2.13.3
^ permalink raw reply related
* [PATCH 1/2] powerpc/process: fix nested output in show_user_instructions()
From: Christophe Leroy @ 2018-08-14 8:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, muriloo
Cc: linux-kernel, linuxppc-dev
When two processes crash at the same time, we sometimes encounter
nesting in the middle of a line:
[ 4.365317] init[1]: segfault (11) at 0 nip 0 lr 0 code 1
[ 4.370452] init[1]: code: XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
[ 4.372042] init[74]: segfault (11) at 10a74 nip 1000c198 lr 100078c8 code 1 in sh[10000000+14000]
[ 4.386829] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
[ 4.391542] init[1]: code: XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
[ 4.400863] init[74]: code: 90010024 bf61000c 91490a7c 3fa01002 3be00000 7d3e4b78 3bbd0c20 3b600000
[ 4.409867] init[74]: code: 3b9d0040 7c7fe02e 2f830000 419e0028 <89230000> 2f890000 41be001c 4b7f6e79
This patch fixes it by preparing complete lines in a buffer and
printing it at once.
Fixes: 88b0fe1757359 ("powerpc: Add show_user_instructions()")
Cc: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/process.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 913c5725cdb2..c722ce4ca1c0 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1303,32 +1303,33 @@ void show_user_instructions(struct pt_regs *regs)
{
unsigned long pc;
int i;
+ char buf[96]; /* enough for 8 times 9 + 2 chars */
+ int l = 0;
pc = regs->nip - (instructions_to_print * 3 / 4 * sizeof(int));
- pr_info("%s[%d]: code: ", current->comm, current->pid);
-
for (i = 0; i < instructions_to_print; i++) {
int instr;
if (!(i % 8) && (i > 0)) {
- pr_cont("\n");
- pr_info("%s[%d]: code: ", current->comm, current->pid);
+ pr_info("%s[%d]: code: %s\n", current->comm, current->pid, buf);
+ l = 0;
}
if (probe_kernel_address((unsigned int __user *)pc, instr)) {
- pr_cont("XXXXXXXX ");
+ l += sprintf(buf + l, "XXXXXXXX ");
} else {
if (regs->nip == pc)
- pr_cont("<%08x> ", instr);
+ l += sprintf(buf + l, "<%08x> ", instr);
else
- pr_cont("%08x ", instr);
+ l += sprintf(buf + l, "%08x ", instr);
}
pc += sizeof(int);
}
- pr_cont("\n");
+ if (l)
+ pr_info("%s[%d]: code: %s\n", current->comm, current->pid, buf);
}
struct regbit {
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation
From: Joe Perches @ 2018-08-14 4:49 UTC (permalink / raw)
To: zhong jiang, tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt,
dhowells, akpm, viro
Cc: linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <1534214814-9043-3-git-send-email-zhongjiang@huawei.com>
On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
> So just replace it.
Better to remove the extern and the const altogether here as well.
$ git grep -w powerpc_num_opcodes
arch/powerpc/xmon/ppc-dis.c: opcode_end = powerpc_opcodes + powerpc_num_opcodes;
arch/powerpc/xmon/ppc-opc.c:const int powerpc_num_opcodes =
arch/powerpc/xmon/ppc.h:extern const int powerpc_num_opcodes;
And this one could be removed instead:
$ git grep -w vle_num_opcodes
arch/powerpc/xmon/ppc-opc.c:const int vle_num_opcodes =
arch/powerpc/xmon/ppc.h:extern const int vle_num_opcodes;
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> index ac2b55b..f3f57a1 100644
> --- a/arch/powerpc/xmon/ppc-opc.c
> +++ b/arch/powerpc/xmon/ppc-opc.c
> @@ -966,8 +966,7 @@
> { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> };
>
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> - / sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
>
> /* The functions used to insert and extract complicated operands. */
>
> @@ -6980,8 +6979,7 @@
> {"fcfidu.", XRC(63,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT, FRB}},
> };
>
> -const int powerpc_num_opcodes =
> - sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> \f
> /* The VLE opcode table.
>
> @@ -7219,8 +7217,7 @@
> {"se_bl", BD8(58,0,1), BD8_MASK, PPCVLE, 0, {B8}},
> };
>
> -const int vle_num_opcodes =
> - sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
> \f
> /* The macro table. This is only used by the assembler. */
>
> @@ -7288,5 +7285,4 @@
> {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
> };
>
> -const int powerpc_num_macros =
> - sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
^ permalink raw reply
* Re: [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation
From: zhong jiang @ 2018-08-14 4:57 UTC (permalink / raw)
To: Joe Perches
Cc: tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt, dhowells,
akpm, viro, linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <81ae2b1b7ea395217dbf3494457a232bdfdc79c5.camel@perches.com>
On 2018/8/14 12:45, Joe Perches wrote:
> On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
>> We prefer to ARRAY_SIZE rather than duplicating its implementation.
>> So just replace it.
> []
>> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
> []
>> @@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char *buffer, int buflen)
>> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
>> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
>> };
>> -#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
>> +#define PFM_CMD_COUNT ARRAY_SIZE(pfm_cmd_tab)
> Better would be to remove the #define altogether and change
> the one place where it's used to ARRAY_SIZE(...)
> ---
> arch/ia64/kernel/perfmon.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
> index a9d4dc6c0427..08ece2c7b6e1 100644
> --- a/arch/ia64/kernel/perfmon.c
> +++ b/arch/ia64/kernel/perfmon.c
> @@ -4645,7 +4645,6 @@ static pfm_cmd_desc_t pfm_cmd_tab[]={
> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
> };
> -#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
>
> static int
> pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
> @@ -4770,7 +4769,7 @@ sys_perfmonctl (int fd, int cmd, void __user *arg, int count)
> */
> if (unlikely(pmu_conf == NULL)) return -ENOSYS;
>
> - if (unlikely(cmd < 0 || cmd >= PFM_CMD_COUNT)) {
> + if (unlikely(cmd < 0 || cmd >= ARRAY_SIZE(pfm_cmd_tab)) {
> DPRINT(("invalid cmd=%d\n", cmd));
> return -EINVAL;
> }
>
>
> .
>
Thank you for suggestion. That's indeed better if just one palce use it. I will repost in v2.
Sincerely,
zhong jiang
^ permalink raw reply
* Re: [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation
From: Joe Perches @ 2018-08-14 4:45 UTC (permalink / raw)
To: zhong jiang, tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt,
dhowells, akpm, viro
Cc: linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <1534214814-9043-2-git-send-email-zhongjiang@huawei.com>
On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
> We prefer to ARRAY_SIZE rather than duplicating its implementation.
> So just replace it.
[]
> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
[]
> @@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char *buffer, int buflen)
> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
> };
> -#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
> +#define PFM_CMD_COUNT ARRAY_SIZE(pfm_cmd_tab)
Better would be to remove the #define altogether and change
the one place where it's used to ARRAY_SIZE(...)
---
arch/ia64/kernel/perfmon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index a9d4dc6c0427..08ece2c7b6e1 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -4645,7 +4645,6 @@ static pfm_cmd_desc_t pfm_cmd_tab[]={
/* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
/* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
};
-#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
static int
pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
@@ -4770,7 +4769,7 @@ sys_perfmonctl (int fd, int cmd, void __user *arg, int count)
*/
if (unlikely(pmu_conf == NULL)) return -ENOSYS;
- if (unlikely(cmd < 0 || cmd >= PFM_CMD_COUNT)) {
+ if (unlikely(cmd < 0 || cmd >= ARRAY_SIZE(pfm_cmd_tab)) {
DPRINT(("invalid cmd=%d\n", cmd));
return -EINVAL;
}
^ permalink raw reply related
* Re: [PATCH v2] selftests/powerpc: Avoid remaining process/threads
From: Michael Ellerman @ 2018-08-14 4:16 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Gustavo Romero
In-Reply-To: <a4f9f03b-51dd-55d4-cfec-9ae48fbbd9d6@debian.org>
Breno Leitao <leitao@debian.org> writes:
> Hello Michael,
>
> On 08/06/2018 08:06 AM, Michael Ellerman wrote:
>> Breno Leitao <leitao@debian.org> writes:
>>
>>> diff --git a/tools/testing/selftests/powerpc/harness.c b/tools/testing/selftests/powerpc/harness.c
>>> index 66d31de60b9a..06c51e8d8ccb 100644
>>> --- a/tools/testing/selftests/powerpc/harness.c
>>> +++ b/tools/testing/selftests/powerpc/harness.c
>>> @@ -85,13 +85,16 @@ int run_test(int (test_function)(void), char *name)
>>> return status;
>>> }
>>>
>>> -static void alarm_handler(int signum)
>>> +static void sig_handler(int signum)
>>> {
>>> - /* Jut wake us up from waitpid */
>>> + if (signum == SIGINT)
>>> + kill(-pid, SIGTERM);
>>
>> I don't think we need to do that here, if we just return then we'll pop
>> out of the waitpid() and go via the normal path.
>
> Correct, if we press ^C while the parent process is waiting at waitpid(),
> then waitpid() syscall will be interrupted (EINTR) and never restarted again
> (unless we set sa_flags = SA_RESTART), thus, the code will restart to execute
> the next instruction when the signal handler is done, as we had skipped
> waitpid().
>
> From a theoretical point of view, the user can press ^C before the process
> executes waitpid() syscall. In this case and the process will not 'skip' the
> waitpid(), which will continue to wait. We can clearly force this behavior
> putting a sleep(1) before waitpid() and pressing ^C in the very first
> second, it will 'skip' the nanosleep() syscall instead of waitpid() which
> will be there, and the ^C will be ignored (thus not calling kill(-pid, SIGTERM)).
True.
Though that race also exists vs us registering the SIGINT handler, so
it's basically not solvable, the user can always press ^C before we're
ready.
cheers
^ permalink raw reply
* Re: cxl: remove a dead branch
From: Michael Ellerman @ 2018-08-14 4:13 UTC (permalink / raw)
To: Mathieu Malaterre, Frederic Barrat
Cc: linuxppc-dev, Christophe Lombard, Andrew Donnellan
In-Reply-To: <CA+7wUsxG-Ys7snbY_=wgGRxcUYTN-tEMM9gqHOWidvEBmB5ogA@mail.gmail.com>
Mathieu Malaterre <malat@debian.org> writes:
> Frederic,
>
> Could you double check with Michael what is now best to do.
I decided it had been long enough (since March), so I just merged it.
If Fred et. al. want to do something better they can send me another
patch on top of it.
cheers
> On Mon, Aug 13, 2018 at 1:23 PM Michael Ellerman
> <patch-notifications@ellerman.id.au> wrote:
>>
>> On Thu, 2018-03-22 at 21:05:28 UTC, Mathieu Malaterre wrote:
>> > In commit 14baf4d9c739 ("cxl: Add guest-specific code") the following code
>> > was added:
>> >
>> > if (afu->crs_len < 0) {
>> > dev_err(&afu->dev, "Unexpected configuration record size value\n");
>> > return -EINVAL;
>> > }
>> >
>> > However the variable `crs_len` is of type u64 and cannot be compared < 0.
>> > Remove the dead code section. Fix the following warning treated as error
>> > with W=1:
>> >
>> > ../drivers/misc/cxl/guest.c:919:19: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>> >
>> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>
>> Applied to powerpc next, thanks.
>>
>> https://git.kernel.org/powerpc/c/e4ecafb14fd9cd77d8f4320af1922e
>>
>> cheers
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: Fix build error
From: Michael Ellerman @ 2018-08-14 4:09 UTC (permalink / raw)
To: Alexey Kardashevskiy, Aneesh Kumar K.V, npiggin, benh, paulus
Cc: linuxppc-dev
In-Reply-To: <48a5eb4c-66db-ba47-dd71-4bb898a958ee@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 10/08/2018 17:10, Michael Ellerman wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>
>>> Fix the below build error using strlcpy instead of strncpy
>>>
>>> In function 'pnv_parse_cpuidle_dt',
>>> inlined from 'pnv_init_idle_states' at arch/powerpc/platforms/powernv/idle.c:840:7,
>>> inlined from '__machine_initcall_powernv_pnv_init_idle_states' at arch/powerpc/platforms/powernv/idle.c:870:1:
>>> arch/powerpc/platforms/powernv/idle.c:820:3: error: 'strncpy' specified bound 16 equals destination size [-Werror=stringop-truncation]
>>> strncpy(pnv_idle_states[i].name, temp_string[i],
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> PNV_IDLE_NAME_LEN);
>>
>> I'm curious why I haven't seen this? What compiler are you using?
>
> gcc 8 does this.
Yeah you're right. It was hidden by another build failure in my build
scripts :/
cheers
^ permalink raw reply
* Re: [PATCH V2] ocxl: Fix access to the AFU Descriptor Data
From: Michael Ellerman @ 2018-08-14 3:26 UTC (permalink / raw)
To: Christophe Lombard, linuxppc-dev, fbarrat, vaibhav,
andrew.donnellan
In-Reply-To: <1534169362-9394-1-git-send-email-clombard@linux.vnet.ibm.com>
Hi Christophe,
The patch looks fine, just a nit about the change log:
Christophe Lombard <clombard@linux.vnet.ibm.com> writes:
> The AFU Information DVSEC capability is a means to extract common,
> general information about all of the AFUs associated with a Function
> independent of the specific functionality that each AFU provides.
>
> This patch fixes the access to the AFU Descriptor Data indexed by the
> AFU Info Index field.
> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
> Cc: stable <stable@vger.kernel.org> # 4.16
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
When fixing a bug it's always good to describe how the bug manifests.
ie. in this case we are clearly writing to the wrong location in config
space, but what is the consequence of that? Does it kill the device, or
just fails to initialise something correctly? How could I tell if I'm
hitting this bug currently? How would I tell if the fix is applied
correctly?
cheers
> ---
> Changelog[v2]
> - Rebase to latest upstream.
> - Use pci_write_config_byte instead of pci_write_config_word
> ---
> drivers/misc/ocxl/config.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index 2e30de9..57a6bb1 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
> u32 val;
> int rc, templ_major, templ_minor, len;
>
> - pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
> + pci_write_config_byte(dev,
> + fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
> + afu_idx);
> rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
> if (rc)
> return rc;
> --
> 2.7.4
^ permalink raw reply
* [PATCH] powerpc/mpc85xx: fix issues in clock node
From: Yuantian Tang @ 2018-08-14 2:50 UTC (permalink / raw)
To: robh+dt, mark.rutland
Cc: benh, paulus, mpe, devicetree, linuxppc-dev, Yuantian Tang
The compatible string is not correct in the clock node.
The clocks property refers to the wrong node too.
This patch is to fix them.
Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
---
arch/powerpc/boot/dts/fsl/t1023si-post.dtsi | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
index 4908af5..763caf4 100644
--- a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
@@ -348,7 +348,7 @@
mux0: mux0@0 {
#clock-cells = <0>;
reg = <0x0 4>;
- compatible = "fsl,core-mux-clock";
+ compatible = "fsl,qoriq-core-mux-2.0";
clocks = <&pll0 0>, <&pll0 1>;
clock-names = "pll0_0", "pll0_1";
clock-output-names = "cmux0";
@@ -356,9 +356,9 @@
mux1: mux1@20 {
#clock-cells = <0>;
reg = <0x20 4>;
- compatible = "fsl,core-mux-clock";
- clocks = <&pll0 0>, <&pll0 1>;
- clock-names = "pll0_0", "pll0_1";
+ compatible = "fsl,qoriq-core-mux-2.0";
+ clocks = <&pll1 0>, <&pll1 1>;
+ clock-names = "pll1_0", "pll1_1";
clock-output-names = "cmux1";
};
};
--
1.7.1
^ permalink raw reply related
* [PATCH 0/2] Use ARRAY_SIZE to replace its implementation
From: zhong jiang @ 2018-08-14 2:46 UTC (permalink / raw)
To: tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt, dhowells,
akpm, viro
Cc: linux-ia64, linux-kernel, linuxppc-dev
The issue is detected with the help of Coccinelle.
zhong jiang (2):
ia64: Use ARRAY_SIZE to replace its implementation
powerpc: Use ARRAY_SIZE to replace its implementation
arch/ia64/kernel/perfmon.c | 2 +-
arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
2 files changed, 5 insertions(+), 9 deletions(-)
--
1.7.12.4
^ permalink raw reply
* [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation
From: zhong jiang @ 2018-08-14 2:46 UTC (permalink / raw)
To: tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt, dhowells,
akpm, viro
Cc: linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <1534214814-9043-1-git-send-email-zhongjiang@huawei.com>
Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
So just replace it.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
arch/powerpc/xmon/ppc-opc.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
index ac2b55b..f3f57a1 100644
--- a/arch/powerpc/xmon/ppc-opc.c
+++ b/arch/powerpc/xmon/ppc-opc.c
@@ -966,8 +966,7 @@
{ 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
};
-const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
- / sizeof (powerpc_operands[0]));
+const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
/* The functions used to insert and extract complicated operands. */
@@ -6980,8 +6979,7 @@
{"fcfidu.", XRC(63,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT, FRB}},
};
-const int powerpc_num_opcodes =
- sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
+const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
\f
/* The VLE opcode table.
@@ -7219,8 +7217,7 @@
{"se_bl", BD8(58,0,1), BD8_MASK, PPCVLE, 0, {B8}},
};
-const int vle_num_opcodes =
- sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
+const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
\f
/* The macro table. This is only used by the assembler. */
@@ -7288,5 +7285,4 @@
{"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
};
-const int powerpc_num_macros =
- sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
+const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
--
1.7.12.4
^ permalink raw reply related
* [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation
From: zhong jiang @ 2018-08-14 2:46 UTC (permalink / raw)
To: tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt, dhowells,
akpm, viro
Cc: linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <1534214814-9043-1-git-send-email-zhongjiang@huawei.com>
We prefer to ARRAY_SIZE rather than duplicating its implementation.
So just replace it.
Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
arch/ia64/kernel/perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index a9d4dc6..6cbe6e0 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char *buffer, int buflen)
/* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL),
/* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY, pfarg_dbreg_t, NULL)
};
-#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
+#define PFM_CMD_COUNT ARRAY_SIZE(pfm_cmd_tab)
static int
pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
--
1.7.12.4
^ permalink raw reply related
* Re: [PATCH V2] ocxl: Fix access to the AFU Descriptor Data
From: Andrew Donnellan @ 2018-08-14 1:15 UTC (permalink / raw)
To: Christophe Lombard, linuxppc-dev, fbarrat, vaibhav
In-Reply-To: <1534169362-9394-1-git-send-email-clombard@linux.vnet.ibm.com>
On 14/08/18 00:09, Christophe Lombard wrote:
> The AFU Information DVSEC capability is a means to extract common,
> general information about all of the AFUs associated with a Function
> independent of the specific functionality that each AFU provides.
>
> This patch fixes the access to the AFU Descriptor Data indexed by the
> AFU Info Index field.
>
> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
> Cc: stable <stable@vger.kernel.org> # 4.16
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
Thanks
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> Changelog[v2]
> - Rebase to latest upstream.
> - Use pci_write_config_byte instead of pci_write_config_word
> ---
> drivers/misc/ocxl/config.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index 2e30de9..57a6bb1 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
> u32 val;
> int rc, templ_major, templ_minor, len;
>
> - pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
> + pci_write_config_byte(dev,
> + fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
> + afu_idx);
> rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
> if (rc)
> return rc;
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v6 00/11] hugetlb: Factorize hugetlb architecture primitives
From: Alex Ghiti @ 2018-08-13 18:35 UTC (permalink / raw)
To: linux-mm, mike.kravetz, linux, catalin.marinas, will.deacon,
tony.luck, fenghua.yu, ralf, paul.burton, jhogan, jejb, deller,
benh, paulus, mpe, ysato, dalias, davem, tglx, mingo, hpa, x86,
arnd, linux-arm-kernel, linux-kernel, linux-ia64, linux-mips,
linux-parisc, linuxppc-dev, linux-sh, sparclinux, linux-arch
In-Reply-To: <20180806175711.24438-1-alex@ghiti.fr>
Hi everyone,
Does someone need anything more to be done regarding this series ?
Thanks,
Alex
On 08/06/2018 05:57 PM, Alexandre Ghiti wrote:
> [CC linux-mm for inclusion in -mm tree]
>
> In order to reduce copy/paste of functions across architectures and then
> make riscv hugetlb port (and future ports) simpler and smaller, this
> patchset intends to factorize the numerous hugetlb primitives that are
> defined across all the architectures.
>
> Except for prepare_hugepage_range, this patchset moves the versions that
> are just pass-through to standard pte primitives into
> asm-generic/hugetlb.h by using the same #ifdef semantic that can be
> found in asm-generic/pgtable.h, i.e. __HAVE_ARCH_***.
>
> s390 architecture has not been tackled in this serie since it does not
> use asm-generic/hugetlb.h at all.
>
> This patchset has been compiled on all addressed architectures with
> success (except for parisc, but the problem does not come from this
> series).
>
> v6:
> - Remove nohash/32 and book3s/32 powerpc specific implementations in
> order to use the generic ones.
> - Add all the Reviewed-by, Acked-by and Tested-by in the commits,
> thanks to everyone.
>
> v5:
> As suggested by Mike Kravetz, no need to move the #include
> <asm-generic/hugetlb.h> for arm and x86 architectures, let it live at
> the top of the file.
>
> v4:
> Fix powerpc build error due to misplacing of #include
> <asm-generic/hugetlb.h> outside of #ifdef CONFIG_HUGETLB_PAGE, as
> pointed by Christophe Leroy.
>
> v1, v2, v3:
> Same version, just problems with email provider and misuse of
> --batch-size option of git send-email
>
> Alexandre Ghiti (11):
> hugetlb: Harmonize hugetlb.h arch specific defines with pgtable.h
> hugetlb: Introduce generic version of hugetlb_free_pgd_range
> hugetlb: Introduce generic version of set_huge_pte_at
> hugetlb: Introduce generic version of huge_ptep_get_and_clear
> hugetlb: Introduce generic version of huge_ptep_clear_flush
> hugetlb: Introduce generic version of huge_pte_none
> hugetlb: Introduce generic version of huge_pte_wrprotect
> hugetlb: Introduce generic version of prepare_hugepage_range
> hugetlb: Introduce generic version of huge_ptep_set_wrprotect
> hugetlb: Introduce generic version of huge_ptep_set_access_flags
> hugetlb: Introduce generic version of huge_ptep_get
>
> arch/arm/include/asm/hugetlb-3level.h | 32 +---------
> arch/arm/include/asm/hugetlb.h | 30 ----------
> arch/arm64/include/asm/hugetlb.h | 39 +++---------
> arch/ia64/include/asm/hugetlb.h | 47 ++-------------
> arch/mips/include/asm/hugetlb.h | 40 +++----------
> arch/parisc/include/asm/hugetlb.h | 33 +++--------
> arch/powerpc/include/asm/book3s/32/pgtable.h | 6 --
> arch/powerpc/include/asm/book3s/64/pgtable.h | 1 +
> arch/powerpc/include/asm/hugetlb.h | 43 ++------------
> arch/powerpc/include/asm/nohash/32/pgtable.h | 6 --
> arch/powerpc/include/asm/nohash/64/pgtable.h | 1 +
> arch/sh/include/asm/hugetlb.h | 54 ++---------------
> arch/sparc/include/asm/hugetlb.h | 40 +++----------
> arch/x86/include/asm/hugetlb.h | 69 ----------------------
> include/asm-generic/hugetlb.h | 88 +++++++++++++++++++++++++++-
> 15 files changed, 135 insertions(+), 394 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v2 3/3] powerpc/mce: Handle memcpy_mcsafe
From: Reza Arbab @ 2018-08-13 15:49 UTC (permalink / raw)
To: Balbir Singh; +Cc: linuxppc-dev, oohall, npiggin, linux-nvdimm
In-Reply-To: <20180405071500.22320-4-bsingharora@gmail.com>
On Thu, Apr 05, 2018 at 05:15:00PM +1000, Balbir Singh wrote:
>Add a blocking notifier callback to be called in real-mode
>on machine check exceptions for UE (ld/st) errors only.
It's been a while, but is this patchset still being pursued?
This patch in particular (callbacks for MCE handling) has other device
memory use cases and I'd like to move it along.
>The patch registers a callback on boot to be notified
>of machine check exceptions and returns a NOTIFY_STOP when
>a page of interest is seen as the source of the machine
>check exception. This page of interest is a ZONE_DEVICE
>page and hence for now, for memcpy_mcsafe to work, the page
>needs to belong to ZONE_DEVICE and memcpy_mcsafe should be
>used to access the memory.
>
>The patch also modifies the NIP of the exception context
>to go back to the fixup handler (in memcpy_mcsafe) and does
>not print any error message as the error is treated as
>returned via a return value and handled.
>
>Signed-off-by: Balbir Singh <bsingharora@gmail.com>
>---
> arch/powerpc/include/asm/mce.h | 3 +-
> arch/powerpc/kernel/mce.c | 77 ++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 77 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>index 3a1226e9b465..a76638e3e47e 100644
>--- a/arch/powerpc/include/asm/mce.h
>+++ b/arch/powerpc/include/asm/mce.h
>@@ -125,7 +125,8 @@ struct machine_check_event {
> enum MCE_UeErrorType ue_error_type:8;
> uint8_t effective_address_provided;
> uint8_t physical_address_provided;
>- uint8_t reserved_1[5];
>+ uint8_t error_return;
>+ uint8_t reserved_1[4];
> uint64_t effective_address;
> uint64_t physical_address;
> uint8_t reserved_2[8];
>diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
>index efdd16a79075..b9e4881fa8c5 100644
>--- a/arch/powerpc/kernel/mce.c
>+++ b/arch/powerpc/kernel/mce.c
>@@ -28,7 +28,9 @@
> #include <linux/percpu.h>
> #include <linux/export.h>
> #include <linux/irq_work.h>
>+#include <linux/extable.h>
>
>+#include <asm/extable.h>
> #include <asm/machdep.h>
> #include <asm/mce.h>
>
>@@ -54,6 +56,52 @@ static struct irq_work mce_event_process_work = {
>
> DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
>
>+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
>+
>+int register_mce_notifier(struct notifier_block *nb)
>+{
>+ return blocking_notifier_chain_register(&mce_notifier_list, nb);
>+}
>+EXPORT_SYMBOL_GPL(register_mce_notifier);
>+
>+int unregister_mce_notifier(struct notifier_block *nb)
>+{
>+ return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
>+}
>+EXPORT_SYMBOL_GPL(unregister_mce_notifier);
>+
>+
>+static int check_memcpy_mcsafe(struct notifier_block *nb,
>+ unsigned long val, void *data)
>+{
>+ /*
>+ * val contains the physical_address of the bad address
>+ */
>+ unsigned long pfn = val >> PAGE_SHIFT;
>+ struct page *page = realmode_pfn_to_page(pfn);
>+ int rc = NOTIFY_DONE;
>+
>+ if (!page)
>+ goto out;
>+
>+ if (is_zone_device_page(page)) /* for HMM and PMEM */
>+ rc = NOTIFY_STOP;
>+out:
>+ return rc;
>+}
>+
>+struct notifier_block memcpy_mcsafe_nb = {
>+ .priority = 0,
>+ .notifier_call = check_memcpy_mcsafe,
>+};
>+
>+int mce_mcsafe_register(void)
>+{
>+ register_mce_notifier(&memcpy_mcsafe_nb);
>+ return 0;
>+}
>+arch_initcall(mce_mcsafe_register);
>+
> static void mce_set_error_info(struct machine_check_event *mce,
> struct mce_error_info *mce_err)
> {
>@@ -151,9 +199,31 @@ void save_mce_event(struct pt_regs *regs, long handled,
> mce->u.ue_error.effective_address_provided = true;
> mce->u.ue_error.effective_address = addr;
> if (phys_addr != ULONG_MAX) {
>+ int rc;
>+ const struct exception_table_entry *entry;
>+
>+ /*
>+ * Once we have the physical address, we check to
>+ * see if the current nip has a fixup entry.
>+ * Having a fixup entry plus the notifier stating
>+ * that it can handle the exception is an indication
>+ * that we should return to the fixup entry and
>+ * return an error from there
>+ */
> mce->u.ue_error.physical_address_provided = true;
> mce->u.ue_error.physical_address = phys_addr;
>- machine_check_ue_event(mce);
>+
>+ rc = blocking_notifier_call_chain(&mce_notifier_list,
>+ phys_addr, NULL);
Could we pass mce entirely here instead of just phys_addr? It would
allow the callback itself to set error_return if needed.
>+ if (rc & NOTIFY_STOP_MASK) {
>+ entry = search_exception_tables(regs->nip);
>+ if (entry != NULL) {
>+ mce->u.ue_error.error_return = 1;
>+ regs->nip = extable_fixup(entry);
>+ } else
>+ machine_check_ue_event(mce);
>+ } else
>+ machine_check_ue_event(mce);
> }
> }
> return;
With the above, this logic would be simplified. So,
rc = blocking_notifier_call_chain(&mce_notifier_list,
(unsigned long)mce, NULL);
if (rc & NOTIFY_STOP_MASK) {
entry = search_exception_tables(regs->nip);
if (entry != NULL) {
mce->u.ue_error.error_return = 1;
regs->nip = extable_fixup(entry);
}
}
if (!mce->u.ue_error.error_return)
machine_check_ue_event(mce);
>@@ -208,7 +278,6 @@ void release_mce_event(void)
> get_mce_event(NULL, true);
> }
>
>-
> /*
> * Queue up the MCE event which then can be handled later.
> */
>@@ -239,6 +308,10 @@ void machine_check_queue_event(void)
> if (!get_mce_event(&evt, MCE_EVENT_RELEASE))
> return;
>
>+ if (evt.error_type == MCE_ERROR_TYPE_UE &&
>+ evt.u.ue_error.error_return == 1)
>+ return;
>+
> index = __this_cpu_inc_return(mce_queue_count) - 1;
> /* If queue is full, just return for now. */
> if (index >= MAX_MC_EVT) {
>--
>2.13.6
>
--
Reza Arbab
^ permalink raw reply
* Re: [PATCH V2] ocxl: Fix access to the AFU Descriptor Data
From: Frederic Barrat @ 2018-08-13 14:50 UTC (permalink / raw)
To: Christophe Lombard, linuxppc-dev, fbarrat, vaibhav,
andrew.donnellan
In-Reply-To: <1534169362-9394-1-git-send-email-clombard@linux.vnet.ibm.com>
Le 13/08/2018 à 16:09, Christophe Lombard a écrit :
> The AFU Information DVSEC capability is a means to extract common,
> general information about all of the AFUs associated with a Function
> independent of the specific functionality that each AFU provides.
>
> This patch fixes the access to the AFU Descriptor Data indexed by the
> AFU Info Index field.
>
> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
> Cc: stable <stable@vger.kernel.org> # 4.16
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---
Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Changelog[v2]
> - Rebase to latest upstream.
> - Use pci_write_config_byte instead of pci_write_config_word
> ---
> drivers/misc/ocxl/config.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index 2e30de9..57a6bb1 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
> u32 val;
> int rc, templ_major, templ_minor, len;
>
> - pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
> + pci_write_config_byte(dev,
> + fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
> + afu_idx);
> rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
> if (rc)
> return rc;
>
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/64s: move machine check SLB flushing to mm/slb.c
From: Nicholas Piggin @ 2018-08-13 14:41 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar
Cc: linuxppc-dev, Gautham R . Shenoy, kvm-ppc, Aneesh Kumar K.V,
Akshay Adiga
In-Reply-To: <a216c87b-99ac-3a64-1c92-9e90ac28c8b6@linux.vnet.ibm.com>
On Mon, 13 Aug 2018 09:57:33 +0530
Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
> On 08/10/2018 12:12 PM, Nicholas Piggin wrote:
> > The machine check code that flushes and restores bolted segments in
> > real mode belongs in mm/slb.c. This will also be used by pseries
> > machine check and idle code in future changes.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >
> > Since v1:
> > - Restore the test for slb_shadow (mpe)
> > ---
> > arch/powerpc/include/asm/book3s/64/mmu-hash.h | 3 ++
> > arch/powerpc/kernel/mce_power.c | 26 +++++--------
> > arch/powerpc/mm/slb.c | 39 +++++++++++++++++++
> > 3 files changed, 51 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > index 2f74bdc805e0..d4e398185b3a 100644
> > --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> > @@ -497,6 +497,9 @@ extern void hpte_init_native(void);
> >
> > extern void slb_initialize(void);
> > extern void slb_flush_and_rebolt(void);
> > +extern void slb_flush_all_realmode(void);
> > +extern void __slb_restore_bolted_realmode(void);
> > +extern void slb_restore_bolted_realmode(void);
> >
> > extern void slb_vmalloc_update(void);
> > extern void slb_set_size(u16 size);
> > diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> > index d6756af6ec78..3497c8329c1d 100644
> > --- a/arch/powerpc/kernel/mce_power.c
> > +++ b/arch/powerpc/kernel/mce_power.c
> > @@ -62,11 +62,8 @@ static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> > #ifdef CONFIG_PPC_BOOK3S_64
> > static void flush_and_reload_slb(void)
> > {
> > - struct slb_shadow *slb;
> > - unsigned long i, n;
> > -
> > /* Invalidate all SLBs */
> > - asm volatile("slbmte %0,%0; slbia" : : "r" (0));
> > + slb_flush_all_realmode();
> >
> > #ifdef CONFIG_KVM_BOOK3S_HANDLER
> > /*
> > @@ -76,22 +73,17 @@ static void flush_and_reload_slb(void)
> > if (get_paca()->kvm_hstate.in_guest)
> > return;
> > #endif
> > -
> > - /* For host kernel, reload the SLBs from shadow SLB buffer. */
> > - slb = get_slb_shadow();
> > - if (!slb)
> > + if (early_radix_enabled())
> > return;
>
> Would we ever get MCE for SLB errors when radix is enabled ?
Well I'm not 100% sure. I don't think the MMU should in radix mode,
but KVM will put guests into HPT mode and put entries into the SLB.
I'm not completely sure we would never get a MCE come through here.
>
> >
> > - n = min_t(u32, be32_to_cpu(slb->persistent), SLB_MIN_SIZE);
> > -
> > - /* Load up the SLB entries from shadow SLB */
> > - for (i = 0; i < n; i++) {
> > - unsigned long rb = be64_to_cpu(slb->save_area[i].esid);
> > - unsigned long rs = be64_to_cpu(slb->save_area[i].vsid);
> > + /*
> > + * This probably shouldn't happen, but it may be possible it's
> > + * called in early boot before SLB shadows are allocated.
> > + */
> > + if (!get_slb_shadow())
> > + return;
>
> Any reason you added above check here instead on mm/slb.c ? Should we
> move above check inside slb_restore_bolted_realmode() ? I guess mm/slb.c
> is right place for this check. This will also help pseries machine check
> to avoid calling this extra check explicitly.
I thought it was a corner case because the slb.c code should not
be called before it's initialised. I'd prefer these exceptional
machine check cases be tested in the MCE code. Anything else calling
flush_and_reload_slb so early would be a bad bug.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v7 7/9] powerpc/pseries: Dump the SLB contents on SLB MCE errors.
From: Nicholas Piggin @ 2018-08-13 14:27 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar
Cc: linuxppc-dev, Aneesh Kumar K.V, Michael Ellerman, Michal Suchanek,
Ananth Narayan, Laurent Dufour
In-Reply-To: <e188218a-f470-58ca-ebc9-0efb2d8e322e@linux.vnet.ibm.com>
On Mon, 13 Aug 2018 09:47:04 +0530
Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
> On 08/11/2018 10:03 AM, Nicholas Piggin wrote:
> > On Tue, 07 Aug 2018 19:47:39 +0530
> > Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
> >
> >> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >>
> >> If we get a machine check exceptions due to SLB errors then dump the
> >> current SLB contents which will be very much helpful in debugging the
> >> root cause of SLB errors. Introduce an exclusive buffer per cpu to hold
> >> faulty SLB entries. In real mode mce handler saves the old SLB contents
> >> into this buffer accessible through paca and print it out later in virtual
> >> mode.
> >>
> >> With this patch the console will log SLB contents like below on SLB MCE
> >> errors:
> >>
> >> [ 507.297236] SLB contents of cpu 0x1
> >> [ 507.297237] Last SLB entry inserted at slot 16
> >> [ 507.297238] 00 c000000008000000 400ea1b217000500
> >> [ 507.297239] 1T ESID= c00000 VSID= ea1b217 LLP:100
> >> [ 507.297240] 01 d000000008000000 400d43642f000510
> >> [ 507.297242] 1T ESID= d00000 VSID= d43642f LLP:110
> >> [ 507.297243] 11 f000000008000000 400a86c85f000500
> >> [ 507.297244] 1T ESID= f00000 VSID= a86c85f LLP:100
> >> [ 507.297245] 12 00007f0008000000 4008119624000d90
> >> [ 507.297246] 1T ESID= 7f VSID= 8119624 LLP:110
> >> [ 507.297247] 13 0000000018000000 00092885f5150d90
> >> [ 507.297247] 256M ESID= 1 VSID= 92885f5150 LLP:110
> >> [ 507.297248] 14 0000010008000000 4009e7cb50000d90
> >> [ 507.297249] 1T ESID= 1 VSID= 9e7cb50 LLP:110
> >> [ 507.297250] 15 d000000008000000 400d43642f000510
> >> [ 507.297251] 1T ESID= d00000 VSID= d43642f LLP:110
> >> [ 507.297252] 16 d000000008000000 400d43642f000510
> >> [ 507.297253] 1T ESID= d00000 VSID= d43642f LLP:110
> >> [ 507.297253] ----------------------------------
> >> [ 507.297254] SLB cache ptr value = 3
> >> [ 507.297254] Valid SLB cache entries:
> >> [ 507.297255] 00 EA[0-35]= 7f000
> >> [ 507.297256] 01 EA[0-35]= 1
> >> [ 507.297257] 02 EA[0-35]= 1000
> >> [ 507.297257] Rest of SLB cache entries:
> >> [ 507.297258] 03 EA[0-35]= 7f000
> >> [ 507.297258] 04 EA[0-35]= 1
> >> [ 507.297259] 05 EA[0-35]= 1000
> >> [ 507.297260] 06 EA[0-35]= 12
> >> [ 507.297260] 07 EA[0-35]= 7f000
> >>
> >> Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> >> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >> ---
> >>
> >> Changes in V7:
> >> - Print slb cache ptr value and slb cache data
> >> ---
> >> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 7 ++
> >> arch/powerpc/include/asm/paca.h | 4 +
> >> arch/powerpc/mm/slb.c | 73 +++++++++++++++++++++++++
> >> arch/powerpc/platforms/pseries/ras.c | 10 +++
> >> arch/powerpc/platforms/pseries/setup.c | 10 +++
> >> 5 files changed, 103 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> >> index cc00a7088cf3..5a3fe282076d 100644
> >> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> >> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> >> @@ -485,9 +485,16 @@ static inline void hpte_init_pseries(void) { }
> >>
> >> extern void hpte_init_native(void);
> >>
> >> +struct slb_entry {
> >> + u64 esid;
> >> + u64 vsid;
> >> +};
> >> +
> >> extern void slb_initialize(void);
> >> extern void slb_flush_and_rebolt(void);
> >> extern void slb_flush_and_rebolt_realmode(void);
> >> +extern void slb_save_contents(struct slb_entry *slb_ptr);
> >> +extern void slb_dump_contents(struct slb_entry *slb_ptr);
> >>
> >> extern void slb_vmalloc_update(void);
> >> extern void slb_set_size(u16 size);
> >> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> >> index 7f22929ce915..233d25ff6f64 100644
> >> --- a/arch/powerpc/include/asm/paca.h
> >> +++ b/arch/powerpc/include/asm/paca.h
> >> @@ -254,6 +254,10 @@ struct paca_struct {
> >> #endif
> >> #ifdef CONFIG_PPC_PSERIES
> >> u8 *mce_data_buf; /* buffer to hold per cpu rtas errlog */
> >> +
> >> + /* Capture SLB related old contents in MCE handler. */
> >> + struct slb_entry *mce_faulty_slbs;
> >> + u16 slb_save_cache_ptr;
> >> #endif /* CONFIG_PPC_PSERIES */
> >> } ____cacheline_aligned;
> >>
> >> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> >> index e89f675f1b5e..16a53689ffd4 100644
> >> --- a/arch/powerpc/mm/slb.c
> >> +++ b/arch/powerpc/mm/slb.c
> >> @@ -151,6 +151,79 @@ void slb_flush_and_rebolt_realmode(void)
> >> get_paca()->slb_cache_ptr = 0;
> >> }
> >>
> >> +void slb_save_contents(struct slb_entry *slb_ptr)
> >> +{
> >> + int i;
> >> + unsigned long e, v;
> >> +
> >> + /* Save slb_cache_ptr value. */
> >> + get_paca()->slb_save_cache_ptr = get_paca()->slb_cache_ptr;
> >
> > What's the point of saving this?
>
> This is to know how many valid cache entries were present at the time of
> SLB mutlihit. We use this index value while dumping the slb cahce entries.
Oh I see you're dumping that thing as well. I don't know if that's
worth doing, it just gives you the first 8 SLB entries installed but
you already have those (or they're overwritten and irrelevat).
>
> >
> >> +
> >> + if (!slb_ptr)
> >> + return;
> >
> > Can this ever happen?
>
> May be Never. We allocate the memory at very early stage. But just added
> as sanity check.
Okay if you think it's needed.
>
> >
> >> +
> >> + for (i = 0; i < mmu_slb_size; i++) {
> >> + asm volatile("slbmfee %0,%1" : "=r" (e) : "r" (i));
> >> + asm volatile("slbmfev %0,%1" : "=r" (v) : "r" (i));
> >
> > Does the UM say these instructions can cause machine checks if the SLB
> > is corrupted? It talks about mfslb instruction causing MCE, but there
> > seems to be no such instruction so I wonder if that's a typo for slbmf?
> >
> > Seems like a parity error in the SLB should cause a MCE, at least,
> > because it can't guarantee valid data for the instruction in that case
> > (multi-hit may be different because you aren't searching by EA).
> >
> > You could limit slb saving to a single level of recursion to avoid
> > the problem.
>
> Yeah, we could do this OR restrict slb saving only for SLB multi-hit.
> Parity errors are anyway hardware errors. If parity error is transient
> then saving of SLBs may not trigger another MCE. In that case old SLB
> content would look ok even if we dump them on console. What do you say ?
I'm not sure. A parity error I think can cause a multi hit. Can you be
sure of a software caused multi hit? Would be a good idea if you can I
think. It may be a good idea to avoid recursion as well, just in case.
Thanks,
Nick
^ permalink raw reply
* [PATCH V2] ocxl: Fix access to the AFU Descriptor Data
From: Christophe Lombard @ 2018-08-13 14:09 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, vaibhav, andrew.donnellan
The AFU Information DVSEC capability is a means to extract common,
general information about all of the AFUs associated with a Function
independent of the specific functionality that each AFU provides.
This patch fixes the access to the AFU Descriptor Data indexed by the
AFU Info Index field.
Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
Cc: stable <stable@vger.kernel.org> # 4.16
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
Changelog[v2]
- Rebase to latest upstream.
- Use pci_write_config_byte instead of pci_write_config_word
---
drivers/misc/ocxl/config.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 2e30de9..57a6bb1 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
u32 val;
int rc, templ_major, templ_minor, len;
- pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
+ pci_write_config_byte(dev,
+ fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
+ afu_idx);
rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
if (rc)
return rc;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox