* 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
* [PATCH V3] ocxl: Fix access to the AFU Descriptor Data
From: Christophe Lombard @ 2018-08-14 12:45 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.
Write in the AFU Index field allows to access to the descriptor data
for each AFU.
With the current code, we are not able to access to these specific data
when the index >= 1 because we are writing to the wrong location.
All requests to the data of each AFU are pointing to those of the AFU 0,
which could have impacts when using a card with more than one AFU per
function.
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>
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
Changelog[v3]
- Rebase to latest upstream.
- Update the commit message.
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
* Re: [PATCH v7 7/9] powerpc/pseries: Dump the SLB contents on SLB MCE errors.
From: Aneesh Kumar K.V @ 2018-08-14 12:47 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar, Nicholas Piggin, Aneesh Kumar K.V
Cc: Michal Suchanek, Ananth Narayan, linuxppc-dev, Laurent Dufour
In-Reply-To: <ccdcaaa0-48b3-bcda-10cf-804ee8fc0d1e@linux.vnet.ibm.com>
On 08/14/2018 04:27 PM, Mahesh Jagannath Salgaonkar wrote:
> 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 ?
>
>
We never clear slb_cache entries. We just update slb_cache_ptr. Now on
debug we would like to find which entries are the valid
slb_cache_entries for this run. slb_cache_ptr gives us that details.
One of the ways we could end up with a slb multi hit is if we have
slb_cache_ptr corruption. So instead of doing a flush_and_rebolt, we
invalidated a subset of valid slb entries. But I understand that in that
specific case, we context switched out with that corrupted value and the
value we are dumping above really won't help in isolating. But if we are
corrupting paca, we might continue to overwrite it again and we can
compare the slb contents against slb_cache contents and see if there is
any corruption.
-aneesh
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Update perf_regs structure to include SIER
From: Ravi Bangoria @ 2018-08-14 10:41 UTC (permalink / raw)
To: Madhavan Srinivasan, mpe, Arnaldo Carvalho de Melo
Cc: linuxppc-dev, Jiri Olsa, Namhyung Kim, Alexander Shishkin,
Anju T Sudhakar, Ravi Bangoria
In-Reply-To: <6284589d-d8ef-da3d-c220-b6d5abf8c50e@linux.vnet.ibm.com>
On 08/14/2018 03:46 PM, Madhavan Srinivasan wrote:
> Hi arnaldo,
>
> Any comments or ack for this patch.
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Thanks,
Ravi
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Update perf_regs structure to include SIER
From: Madhavan Srinivasan @ 2018-08-14 10:16 UTC (permalink / raw)
To: mpe, Arnaldo Carvalho de Melo
Cc: linuxppc-dev, Jiri Olsa, Namhyung Kim, Alexander Shishkin,
Anju T Sudhakar, Ravi Bangoria
In-Reply-To: <1531218587-20742-1-git-send-email-maddy@linux.vnet.ibm.com>
Hi arnaldo,
Any comments or ack for this patch.
With regards
Maddy
On Tuesday 10 July 2018 03:59 PM, Madhavan Srinivasan wrote:
> On each sample, Sample Instruction Event Register (SIER) content
> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
> but instead, SIER content is saved in the "dar" register of pt_regs.
>
> Patch adds another entry to the perf_regs structure to include the "SIER"
> printing which internally maps to the "dar" of pt_regs.
>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/uapi/asm/perf_regs.h | 1 +
> arch/powerpc/perf/perf_regs.c | 1 +
> tools/arch/powerpc/include/uapi/asm/perf_regs.h | 1 +
> tools/perf/arch/powerpc/include/perf_regs.h | 3 ++-
> tools/perf/arch/powerpc/util/perf_regs.c | 1 +
> 5 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index 9e52c86ccbd3..ff91192407d1 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_TRAP,
> PERF_REG_POWERPC_DAR,
> PERF_REG_POWERPC_DSISR,
> + PERF_REG_POWERPC_SIER,
> PERF_REG_POWERPC_MAX,
> };
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index 09ceea6175ba..c262aea22ad9 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -69,6 +69,7 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
> PT_REGS_OFFSET(PERF_REG_POWERPC_TRAP, trap),
> PT_REGS_OFFSET(PERF_REG_POWERPC_DAR, dar),
> PT_REGS_OFFSET(PERF_REG_POWERPC_DSISR, dsisr),
> + PT_REGS_OFFSET(PERF_REG_POWERPC_SIER, dar),
> };
>
> u64 perf_reg_value(struct pt_regs *regs, int idx)
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index 9e52c86ccbd3..ff91192407d1 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_TRAP,
> PERF_REG_POWERPC_DAR,
> PERF_REG_POWERPC_DSISR,
> + PERF_REG_POWERPC_SIER,
> PERF_REG_POWERPC_MAX,
> };
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index 00e37b106913..1076393e6f43 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -62,7 +62,8 @@ static const char *reg_names[] = {
> [PERF_REG_POWERPC_SOFTE] = "softe",
> [PERF_REG_POWERPC_TRAP] = "trap",
> [PERF_REG_POWERPC_DAR] = "dar",
> - [PERF_REG_POWERPC_DSISR] = "dsisr"
> + [PERF_REG_POWERPC_DSISR] = "dsisr",
> + [PERF_REG_POWERPC_SIER] = "sier"
> };
>
> static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index ec50939b0418..07fcd977d93e 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -52,6 +52,7 @@ const struct sample_reg sample_reg_masks[] = {
> SMPL_REG(trap, PERF_REG_POWERPC_TRAP),
> SMPL_REG(dar, PERF_REG_POWERPC_DAR),
> SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
> + SMPL_REG(sier, PERF_REG_POWERPC_SIER),
> SMPL_REG_END
> };
>
^ permalink raw reply
* [PATCH v2 1/4] powerpc/mm: enable the use of page table cache of order 0
From: Christophe Leroy @ 2018-08-14 14:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
hugepages uses a cache of order 0. Lets allow page tables
of order 0 in the common part in order to avoid open coding
in hugetlb
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/32/pgalloc.h | 5 +----
arch/powerpc/include/asm/book3s/64/pgalloc.h | 5 +----
arch/powerpc/include/asm/nohash/32/pgalloc.h | 5 +----
arch/powerpc/include/asm/nohash/64/pgalloc.h | 5 +----
arch/powerpc/mm/init-common.c | 6 +++---
5 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h b/arch/powerpc/include/asm/book3s/32/pgalloc.h
index 82e44b1a00ae..96138ab3ddd6 100644
--- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
@@ -25,10 +25,7 @@
extern void __bad_pte(pmd_t *pmd);
extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({ \
- BUG_ON(!(shift)); \
- pgtable_cache[(shift) - 1]; \
- })
+#define PGT_CACHE(shift) pgtable_cache[shift]
static inline pgd_t *pgd_alloc(struct mm_struct *mm)
{
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index 391ed2c3b697..bfed4cf3b2f3 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -37,10 +37,7 @@ extern struct vmemmap_backing *vmemmap_list;
#define MAX_PGTABLE_INDEX_SIZE 0xf
extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({ \
- BUG_ON(!(shift)); \
- pgtable_cache[(shift) - 1]; \
- })
+#define PGT_CACHE(shift) pgtable_cache[shift]
extern pte_t *pte_fragment_alloc(struct mm_struct *, unsigned long, int);
extern pmd_t *pmd_fragment_alloc(struct mm_struct *, unsigned long);
diff --git a/arch/powerpc/include/asm/nohash/32/pgalloc.h b/arch/powerpc/include/asm/nohash/32/pgalloc.h
index 8825953c225b..6fbbb90043c0 100644
--- a/arch/powerpc/include/asm/nohash/32/pgalloc.h
+++ b/arch/powerpc/include/asm/nohash/32/pgalloc.h
@@ -25,10 +25,7 @@
extern void __bad_pte(pmd_t *pmd);
extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({ \
- BUG_ON(!(shift)); \
- pgtable_cache[(shift) - 1]; \
- })
+#define PGT_CACHE(shift) pgtable_cache[shift]
static inline pgd_t *pgd_alloc(struct mm_struct *mm)
{
diff --git a/arch/powerpc/include/asm/nohash/64/pgalloc.h b/arch/powerpc/include/asm/nohash/64/pgalloc.h
index e2d62d033708..e95eb499a174 100644
--- a/arch/powerpc/include/asm/nohash/64/pgalloc.h
+++ b/arch/powerpc/include/asm/nohash/64/pgalloc.h
@@ -36,10 +36,7 @@ extern struct vmemmap_backing *vmemmap_list;
#define MAX_PGTABLE_INDEX_SIZE 0xf
extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({ \
- BUG_ON(!(shift)); \
- pgtable_cache[(shift) - 1]; \
- })
+#define PGT_CACHE(shift) pgtable_cache[shift]
static inline pgd_t *pgd_alloc(struct mm_struct *mm)
{
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 2b656e67f2ea..41190f2b60c2 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -40,7 +40,7 @@ static void pmd_ctor(void *addr)
memset(addr, 0, PMD_TABLE_SIZE);
}
-struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE];
+struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE + 1];
EXPORT_SYMBOL_GPL(pgtable_cache); /* used by kvm_hv module */
/*
@@ -71,7 +71,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
* moment, gcc doesn't seem to recognize is_power_of_2 as a
* constant expression, so so much for that. */
BUG_ON(!is_power_of_2(minalign));
- BUG_ON((shift < 1) || (shift > MAX_PGTABLE_INDEX_SIZE));
+ BUG_ON(shift > MAX_PGTABLE_INDEX_SIZE);
if (PGT_CACHE(shift))
return; /* Already have a cache of this size */
@@ -83,7 +83,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
panic("Could not allocate pgtable cache for order %d", shift);
kfree(name);
- pgtable_cache[shift - 1] = new;
+ pgtable_cache[shift] = new;
pr_debug("Allocated pgtable cache for order %d\n", shift);
}
--
2.13.3
^ permalink raw reply related
* [PATCH v2 2/4] powerpc/mm: replace hugetlb_cache by PGT_CACHE(PTE_T_ORDER)
From: Christophe Leroy @ 2018-08-14 14:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <2f96bf1a8df1091c642de099ed07c34b5ab9b90a.1534258290.git.christophe.leroy@c-s.fr>
Instead of opencoding cache handling for the special case
of hugepage tables having a single pte_t element, this
patch makes use of the common pgtable_cache helpers
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/hugetlb.h | 2 --
arch/powerpc/mm/hugetlbpage.c | 26 +++++++-------------------
2 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index 2d00cc530083..e13843556414 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -6,8 +6,6 @@
#include <asm/page.h>
#include <asm-generic/hugetlb.h>
-extern struct kmem_cache *hugepte_cache;
-
#ifdef CONFIG_PPC_BOOK3S_64
#include <asm/book3s/64/hugetlb.h>
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 7296a42eb62e..53b7a605c3a8 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -43,6 +43,8 @@ EXPORT_SYMBOL(HPAGE_SHIFT);
#define hugepd_none(hpd) (hpd_val(hpd) == 0)
+#define PTE_T_ORDER (__builtin_ffs(sizeof(pte_t)) - __builtin_ffs(sizeof(void *)))
+
pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr, unsigned long sz)
{
/*
@@ -62,7 +64,7 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
int num_hugepd;
if (pshift >= pdshift) {
- cachep = hugepte_cache;
+ cachep = PGT_CACHE(PTE_T_ORDER);
num_hugepd = 1 << (pshift - pdshift);
} else {
cachep = PGT_CACHE(pdshift - pshift);
@@ -265,7 +267,7 @@ static void hugepd_free_rcu_callback(struct rcu_head *head)
unsigned int i;
for (i = 0; i < batch->index; i++)
- kmem_cache_free(hugepte_cache, batch->ptes[i]);
+ kmem_cache_free(PGT_CACHE(PTE_T_ORDER), batch->ptes[i]);
free_page((unsigned long)batch);
}
@@ -278,7 +280,7 @@ static void hugepd_free(struct mmu_gather *tlb, void *hugepte)
if (atomic_read(&tlb->mm->mm_users) < 2 ||
mm_is_thread_local(tlb->mm)) {
- kmem_cache_free(hugepte_cache, hugepte);
+ kmem_cache_free(PGT_CACHE(PTE_T_ORDER), hugepte);
put_cpu_var(hugepd_freelist_cur);
return;
}
@@ -653,7 +655,6 @@ static int __init hugepage_setup_sz(char *str)
}
__setup("hugepagesz=", hugepage_setup_sz);
-struct kmem_cache *hugepte_cache;
static int __init hugetlbpage_init(void)
{
int psize;
@@ -703,21 +704,8 @@ static int __init hugetlbpage_init(void)
if (pdshift > shift)
pgtable_cache_add(pdshift - shift, NULL);
#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_8xx)
- else if (!hugepte_cache) {
- /*
- * Create a kmem cache for hugeptes. The bottom bits in
- * the pte have size information encoded in them, so
- * align them to allow this
- */
- hugepte_cache = kmem_cache_create("hugepte-cache",
- sizeof(pte_t),
- HUGEPD_SHIFT_MASK + 1,
- 0, NULL);
- if (hugepte_cache == NULL)
- panic("%s: Unable to create kmem cache "
- "for hugeptes\n", __func__);
-
- }
+ else
+ pgtable_cache_add(PTE_T_ORDER, NULL);
#endif
}
--
2.13.3
^ permalink raw reply related
* [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Christophe Leroy @ 2018-08-14 14:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <2f96bf1a8df1091c642de099ed07c34b5ab9b90a.1534258290.git.christophe.leroy@c-s.fr>
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.
In order to avoid this conflict, this patch:
- modifies pgtable_cache_add() so that a zeroising constructor is
added for any cache size.
- replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/pgtable.h | 2 +-
arch/powerpc/mm/hugetlbpage.c | 6 ++---
arch/powerpc/mm/init-common.c | 46 ++++++++++++++++++++++++++------------
3 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index 14c79a7dc855..1e6265dc6697 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -72,7 +72,7 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
/* can we use this in kvm */
unsigned long vmalloc_to_phys(void *vmalloc_addr);
-void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
+void pgtable_cache_add(unsigned int shift);
void pgtable_cache_init(void);
#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_PPC32)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 53b7a605c3a8..6cd90445b1f5 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -71,7 +71,7 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
num_hugepd = 1;
}
- new = kmem_cache_zalloc(cachep, pgtable_gfp_flags(mm, GFP_KERNEL));
+ new = kmem_cache_alloc(cachep, pgtable_gfp_flags(mm, GFP_KERNEL));
BUG_ON(pshift > HUGEPD_SHIFT_MASK);
BUG_ON((unsigned long)new & HUGEPD_SHIFT_MASK);
@@ -702,10 +702,10 @@ static int __init hugetlbpage_init(void)
* use pgt cache for hugepd.
*/
if (pdshift > shift)
- pgtable_cache_add(pdshift - shift, NULL);
+ pgtable_cache_add(pdshift - shift);
#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_8xx)
else
- pgtable_cache_add(PTE_T_ORDER, NULL);
+ pgtable_cache_add(PTE_T_ORDER);
#endif
}
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 41190f2b60c2..b7ca03643d0b 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -25,19 +25,37 @@
#include <asm/pgalloc.h>
#include <asm/pgtable.h>
-static void pgd_ctor(void *addr)
-{
- memset(addr, 0, PGD_TABLE_SIZE);
+#define CTOR(shift) static void ctor_##shift(void *addr) \
+{ \
+ memset(addr, 0, sizeof(void *) << (shift)); \
}
-static void pud_ctor(void *addr)
-{
- memset(addr, 0, PUD_TABLE_SIZE);
-}
+CTOR(0); CTOR(1); CTOR(2); CTOR(3); CTOR(4); CTOR(5); CTOR(6); CTOR(7);
+CTOR(8); CTOR(9); CTOR(10); CTOR(11); CTOR(12); CTOR(13); CTOR(14); CTOR(15);
-static void pmd_ctor(void *addr)
+static inline void (*ctor(int shift))(void *)
{
- memset(addr, 0, PMD_TABLE_SIZE);
+ BUILD_BUG_ON(MAX_PGTABLE_INDEX_SIZE != 15);
+
+ switch (shift) {
+ case 0: return ctor_0;
+ case 1: return ctor_1;
+ case 2: return ctor_2;
+ case 3: return ctor_3;
+ case 4: return ctor_4;
+ case 5: return ctor_5;
+ case 6: return ctor_6;
+ case 7: return ctor_7;
+ case 8: return ctor_8;
+ case 9: return ctor_9;
+ case 10: return ctor_10;
+ case 11: return ctor_11;
+ case 12: return ctor_12;
+ case 13: return ctor_13;
+ case 14: return ctor_14;
+ case 15: return ctor_15;
+ }
+ return NULL;
}
struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE + 1];
@@ -50,7 +68,7 @@ EXPORT_SYMBOL_GPL(pgtable_cache); /* used by kvm_hv module */
* everything else. Caches created by this function are used for all
* the higher level pagetables, and for hugepage pagetables.
*/
-void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
+void pgtable_cache_add(unsigned int shift)
{
char *name;
unsigned long table_size = sizeof(void *) << shift;
@@ -78,7 +96,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
align = max_t(unsigned long, align, minalign);
name = kasprintf(GFP_KERNEL, "pgtable-2^%d", shift);
- new = kmem_cache_create(name, table_size, align, 0, ctor);
+ new = kmem_cache_create(name, table_size, align, 0, ctor(shift));
if (!new)
panic("Could not allocate pgtable cache for order %d", shift);
@@ -91,15 +109,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);
if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
- pgtable_cache_add(PMD_CACHE_INDEX, pmd_ctor);
+ pgtable_cache_add(PMD_CACHE_INDEX);
/*
* 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);
}
--
2.13.3
^ permalink raw reply related
* [PATCH v2 4/4] powerpc/mm: remove unnecessary test in pgtable_cache_init()
From: Christophe Leroy @ 2018-08-14 14:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <2f96bf1a8df1091c642de099ed07c34b5ab9b90a.1534258290.git.christophe.leroy@c-s.fr>
pgtable_cache_add() gracefully handles the case when a cache that
size already exists by returning early with the following test:
if (PGT_CACHE(shift))
return; /* Already have a cache of this size */
It is then not needed to test the existence of the cache before.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/init-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index b7ca03643d0b..1e6910eb70ed 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -111,13 +111,13 @@ void pgtable_cache_init(void)
{
pgtable_cache_add(PGD_INDEX_SIZE);
- if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
+ if (PMD_CACHE_INDEX)
pgtable_cache_add(PMD_CACHE_INDEX);
/*
* 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))
+ if (PUD_CACHE_INDEX)
pgtable_cache_add(PUD_CACHE_INDEX);
}
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 0/2] Use ARRAY_SIZE to replace its implementation
From: David Howells @ 2018-08-14 16:18 UTC (permalink / raw)
To: zhong jiang
Cc: dhowells, tony.luck, fenghua.yu, benh, paulus, mpe, oleg, rppt,
akpm, viro, linux-ia64, linux-kernel, linuxppc-dev
In-Reply-To: <1534214814-9043-1-git-send-email-zhongjiang@huawei.com>
> "Use ARRAY_SIZE to replace its implementation"
Um, the subject line doesn't make sense.
David
^ permalink raw reply
* linux-next: manual merge of the powerpc tree with Linus' tree
From: Stephen Rothwell @ 2018-08-14 23:19 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, PowerPC
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Thomas Gleixner, Diana Craciun
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
Hi all,
Today's linux-next merge of the powerpc tree got a conflict in:
Documentation/admin-guide/kernel-parameters.txt
between commits:
05736e4ac13c ("cpu/hotplug: Provide knobs to control SMT")
506a66f37489 ("Revert "x86/apic: Ignore secondary threads if nosmt=force"")
from Linus' tree and commit:
26cb1f36c43e ("Documentation: Add nospectre_v1 parameter")
from the powerpc tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc Documentation/admin-guide/kernel-parameters.txt
index 5a67e409d370,4167bbea51e1..000000000000
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@@ -2761,10 -2687,10 +2761,14 @@@
nosmt [KNL,S390] Disable symmetric multithreading (SMT).
Equivalent to smt=1.
+ [KNL,x86] Disable symmetric multithreading (SMT).
+ nosmt=force: Force disable SMT, cannot be undone
+ via the sysfs control file.
+
+ nospectre_v1 [PPC] Disable mitigations for Spectre Variant 1 (bounds
+ check bypass). With this option data leaks are possible
+ in the system.
+
nospectre_v2 [X86] Disable all mitigations for the Spectre variant 2
(indirect branch prediction) vulnerability. System may
allow data leaks with this option, which is equivalent
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v7 4/9] powerpc/pseries: Define MCE error event section.
From: Michael Ellerman @ 2018-08-16 4:14 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar, linuxppc-dev
Cc: Aneesh Kumar K.V, Michal Suchanek, Ananth Narayan,
Nicholas Piggin, Laurent Dufour
In-Reply-To: <13f8280f-02c1-7b00-0a40-a045bf6ee1f6@linux.vnet.ibm.com>
Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> On 08/08/2018 08:12 PM, Michael Ellerman wrote:
...
>>
>>> + union {
>>> + struct {
>>> + uint8_t ue_err_type;
>>> + /* XXXXXXXX
>>> + * X 1: Permanent or Transient UE.
>>> + * X 1: Effective address provided.
>>> + * X 1: Logical address provided.
>>> + * XX 2: Reserved.
>>> + * XXX 3: Type of UE error.
>>> + */
>>
>> But which bit is bit 0? And is that the LSB or MSB?
>
> RTAS errorlog data in BE format, the leftmost bit is MSB 0 (1: Permanent
> or Transient UE.). I Will update the comment above that properly points
> out which one is MSB 0.
>
>>
>>
>>> + uint8_t reserved_1[6];
>>> + __be64 effective_address;
>>> + __be64 logical_address;
>>> + } ue_error;
>>> + struct {
>>> + uint8_t soft_err_type;
>>> + /* XXXXXXXX
>>> + * X 1: Effective address provided.
>>> + * XXXXX 5: Reserved.
>>> + * XX 2: Type of SLB/ERAT/TLB error.
>>> + */
>>> + uint8_t reserved_1[6];
>>> + __be64 effective_address;
>>> + uint8_t reserved_2[8];
>>> + } soft_error;
>>> + } u;
>>> +};
>>> +#pragma pack(pop)
>>
>> Why not __packed ?
>
> Because when used __packed it added 1 byte extra padding between
> reserved_1[6] and effective_address. That caused wrong effective address
> to be printed on the console. Hence I switched to #pragma pack to force
> 1 byte alignment for this structure alone.
OK, that's weird.
Do we really need to bother with all the union stuff? The only
difference is the field names, and whether logical address has a value
or not. What about:
struct pseries_mc_errorlog {
__be32 fru_id;
__be32 proc_id;
u8 error_type;
u8 sub_error_type;
u8 reserved_1[6];
__be64 effective_address;
__be64 logical_address;
} __packed;
cheers
^ permalink raw reply
* ptrace compile failure with gcc-8.2 on 32-bit powerpc
From: Meelis Roos @ 2018-08-16 7:44 UTC (permalink / raw)
To: linux-powerpc, Linux Kernel list
After upgrading my distro compiler to gcc-8.2, Linux fails to compile on=20
32-bit powerpc (tested with 4.17, 4.18 and v4.18-7873-gf91e654474d4).
CC arch/powerpc/kernel/ptrace.o
In file included from ./include/linux/bitmap.h:9,
from ./include/linux/cpumask.h:12,
from ./include/linux/rcupdate.h:44,
from ./include/linux/rculist.h:11,
from ./include/linux/pid.h:5,
from ./include/linux/sched.h:14,
from arch/powerpc/kernel/ptrace.c:19:
In function =A1memcpy=A2,
inlined from =A1user_regset_copyin=A2 at ./include/linux/regset.h:295=
:4,
inlined from =A1vr_set=A2 at arch/powerpc/kernel/ptrace.c:619:9:
./include/linux/string.h:345:9: error: =A1__builtin_memcpy=A2 offset [-52=
7, -529] is out of the bounds [0, 16] of object =A1vrsave=A2 with type =A1=
union <anonymous>=A2 [-Werror=3Darray-bounds]
return __builtin_memcpy(p, q, size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/ptrace.c: In function =A1vr_set=A2:
arch/powerpc/kernel/ptrace.c:614:5: note: =A1vrsave=A2 declared here
} vrsave;
^~~~~~
In file included from ./include/linux/bitmap.h:9,
from ./include/linux/cpumask.h:12,
from ./include/linux/rcupdate.h:44,
from ./include/linux/rculist.h:11,
from ./include/linux/pid.h:5,
from ./include/linux/sched.h:14,
from arch/powerpc/kernel/ptrace.c:19:
In function =A1memcpy=A2,
inlined from =A1user_regset_copyout=A2 at ./include/linux/regset.h:27=
0:4,
inlined from =A1vr_get=A2 at arch/powerpc/kernel/ptrace.c:572:9:
./include/linux/string.h:345:9: error: =A1__builtin_memcpy=A2 offset [-52=
7, -529] is out of the bounds [0, 16] of object =A1vrsave=A2 with type =A1=
union <anonymous>=A2 [-Werror=3Darray-bounds]
return __builtin_memcpy(p, q, size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/ptrace.c: In function =A1vr_get=A2:
arch/powerpc/kernel/ptrace.c:567:5: note: =A1vrsave=A2 declared here
} vrsave;
^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [scripts/Makefile.build:311: arch/powerpc/kernel/ptrace.o] E=
rror 1
--=20
Meelis Roos (mroos@linux.ee)
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/pseries/mm: call H_BLOCK_REMOVE
From: Laurent Dufour @ 2018-08-16 9:41 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, linux-kernel
Cc: aneesh.kumar, benh, paulus, npiggin
In-Reply-To: <877elcj0oa.fsf@concordia.ellerman.id.au>
On 30/07/2018 15:47, Michael Ellerman wrote:
> Hi Laurent,
>
> Just one comment below.
>
> Laurent Dufour <ldufour@linux.vnet.ibm.com> writes:
>> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
>> index 96b8cd8a802d..41ed03245eb4 100644
>> --- a/arch/powerpc/platforms/pseries/lpar.c
>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>> @@ -418,6 +418,73 @@ static void pSeries_lpar_hpte_invalidate(unsigned long slot, unsigned long vpn,
>> BUG_ON(lpar_rc != H_SUCCESS);
>> }
>>
>> +
>> +/*
>> + * As defined in the PAPR's section 14.5.4.1.8
>> + * The control mask doesn't include the returned reference and change bit from
>> + * the processed PTE.
>> + */
>> +#define HBLKR_AVPN 0x0100000000000000UL
>> +#define HBLKR_CTRL_MASK 0xf800000000000000UL
>> +#define HBLKR_CTRL_SUCCESS 0x8000000000000000UL
>> +#define HBLKR_CTRL_ERRNOTFOUND 0x8800000000000000UL
>> +#define HBLKR_CTRL_ERRBUSY 0xa000000000000000UL
>> +
>> +/**
>> + * H_BLOCK_REMOVE caller.
>> + * @idx should point to the latest @param entry set with a PTEX.
>> + * If PTE cannot be processed because another CPUs has already locked that
>> + * group, those entries are put back in @param starting at index 1.
>> + * If entries has to be retried and @retry_busy is set to true, these entries
>> + * are retried until success. If @retry_busy is set to false, the returned
>> + * is the number of entries yet to process.
>> + */
>> +static unsigned long call_block_remove(unsigned long idx, unsigned long *param,
>> + bool retry_busy)
>> +{
>> + unsigned long i, rc, new_idx;
>> + unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
>> +
>> +again:
>> + new_idx = 0;
>> + BUG_ON((idx < 2) || (idx > PLPAR_HCALL9_BUFSIZE));
>
> I count 1 ..
>
>> + if (idx < PLPAR_HCALL9_BUFSIZE)
>> + param[idx] = HBR_END;
>> +
>> + rc = plpar_hcall9(H_BLOCK_REMOVE, retbuf,
>> + param[0], /* AVA */
>> + param[1], param[2], param[3], param[4], /* TS0-7 */
>> + param[5], param[6], param[7], param[8]);
>> + if (rc == H_SUCCESS)
>> + return 0;
>> +
>> + BUG_ON(rc != H_PARTIAL);
>
> 2 ...
>
>> + /* Check that the unprocessed entries were 'not found' or 'busy' */
>> + for (i = 0; i < idx-1; i++) {
>> + unsigned long ctrl = retbuf[i] & HBLKR_CTRL_MASK;
>> +
>> + if (ctrl == HBLKR_CTRL_ERRBUSY) {
>> + param[++new_idx] = param[i+1];
>> + continue;
>> + }
>> +
>> + BUG_ON(ctrl != HBLKR_CTRL_SUCCESS
>> + && ctrl != HBLKR_CTRL_ERRNOTFOUND);
>
> 3 ...
>
> BUG_ON()s.
>
> I know the code in this file is already pretty liberal with the use of
> BUG_ON() but I'd prefer if we don't make it any worse.
The first one is clearly not required. But I would keep the following twos
because this call is not expected to fail except if there is a discrepancy
between the linux kernel HASH views and the hypervisor's one, which could be
dramatic in the consequences.
>
> Given this is an optimisation it seems like we should be able to fall
> back to the existing implementation in the case of error (which will
> probably then BUG_ON() 😂)
I don't think falling back to the H_BULK call will be helpfull since it is
doing the same so the same errors are expected. Furthermore, this hcall can do
a partial work which means complex code to fallback on H_BULK as we should
identify to already processed entries.
> If there's some reason we can't then I guess I can live with it.
I'm proposing to send a new series with _only_ 2 calls to BUG_ON().
Furthermore this patch is not correct on the way the huge pages are managed. I
was too hurry to push it last time.
Cheers,
Laurent.
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc/tm: Remove msr_tm_active()
From: Michael Neuling @ 2018-08-15 23:46 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev
In-Reply-To: <1529362784-14194-1-git-send-email-leitao@debian.org>
On Mon, 2018-06-18 at 19:59 -0300, Breno Leitao wrote:
> Currently msr_tm_active() is a wrapper around MSR_TM_ACTIVE() if
> CONFIG_PPC_TRANSACTIONAL_MEM is set, or it is just a function that
> returns false if CONFIG_PPC_TRANSACTIONAL_MEM is not set.
>=20
> This function is not necessary, since MSR_TM_ACTIVE() just do the same,
> checking for the TS bits and does not require any TM facility.
>=20
> This patchset remove every instance of msr_tm_active() and replaced it
> by MSR_TM_ACTIVE().
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
>=20
Patch looks good... one minor nit below...
> =20
> - if (!msr_tm_active(regs->msr) &&
> - !current->thread.load_fp && !loadvec(current->thread))
> + if (!current->thread.load_fp && !loadvec(current->thread)) {
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> + if (!MSR_TM_ACTIVE(regs->msr))
> + return;
Can you make a MSR_TM_ACTIVE() that returns false when
!CONFIG_PPC_TRANSACTIONAL_MEM. Then you don't need this inline #ifdef.
Mikey
> +#else
> return;
> +#endif
> + }
> =20
> msr =3D regs->msr;
> msr_check_and_set(msr_all_available);
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/tm: Adjust tm_reclaim_thread() parameters
From: Michael Neuling @ 2018-08-15 23:48 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Cyril Bur
In-Reply-To: <1529362784-14194-3-git-send-email-leitao@debian.org>
On Mon, 2018-06-18 at 19:59 -0300, Breno Leitao wrote:
> From: Cyril Bur <cyrilbur@gmail.com>
>=20
> tm_reclaim_thread() doesn't use the parameter anymore, both callers have
> to bother getting it as they have no need for a struct thread_info
> either.
>=20
> It was previously used but became unused in commit
> dc3106690b20 ("powerpc: tm: Always use fp_state and vr_state to store liv=
e
> registers")
>=20
> Just remove it and adjust the callers.
>=20
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Acked-by: Michael Neuling <mikey@neuling.org.>
> ---
> arch/powerpc/kernel/process.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index 9ef4aea9fffe..f8beee03f00a 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -866,8 +866,7 @@ static inline bool tm_enabled(struct task_struct *tsk=
)
> return tsk && tsk->thread.regs && (tsk->thread.regs->msr & MSR_TM);
> }
> =20
> -static void tm_reclaim_thread(struct thread_struct *thr,
> - struct thread_info *ti, uint8_t cause)
> +static void tm_reclaim_thread(struct thread_struct *thr, uint8_t cause)
> {
> /*
> * Use the current MSR TM suspended bit to track if we have
> @@ -914,7 +913,7 @@ static void tm_reclaim_thread(struct thread_struct *t=
hr,
> void tm_reclaim_current(uint8_t cause)
> {
> tm_enable();
> - tm_reclaim_thread(¤t->thread, current_thread_info(), cause);
> + tm_reclaim_thread(¤t->thread, cause);
> }
> =20
> static inline void tm_reclaim_task(struct task_struct *tsk)
> @@ -945,7 +944,7 @@ static inline void tm_reclaim_task(struct task_struct
> *tsk)
> thr->regs->ccr, thr->regs->msr,
> thr->regs->trap);
> =20
> - tm_reclaim_thread(thr, task_thread_info(tsk), TM_CAUSE_RESCHED);
> + tm_reclaim_thread(thr, TM_CAUSE_RESCHED);
> =20
> TM_DEBUG("--- tm_reclaim on pid %d complete\n",
> tsk->pid);
^ permalink raw reply
* Re: [PATCH v2 4/4] powerpc/tm: Do not recheckpoint non-tm task
From: Michael Neuling @ 2018-08-15 23:50 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev
In-Reply-To: <1529362784-14194-4-git-send-email-leitao@debian.org>
On Mon, 2018-06-18 at 19:59 -0300, Breno Leitao wrote:
> If __switch_to() tries to context switch from task A to task B, and task =
A
> had task->thread->regs->msr[TM] enabled, then __switch_to_tm() will call
> tm_recheckpoint_new_task(), which will call trecheckpoint, for task B, wh=
ich
> is clearly wrong since task B might not be an active TM user.
>=20
> This does not cause a lot of damage because tm_recheckpoint() will abort
> the call since it realize that the current task does not have msr[TM] bit
> set.
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/process.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>=20
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index f8beee03f00a..d26a150766ef 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1036,7 +1036,8 @@ static inline void __switch_to_tm(struct task_struc=
t
> *prev,
> prev->thread.regs->msr &=3D ~MSR_TM;
> }
> =20
> - tm_recheckpoint_new_task(new);
> + if (tm_enabled(new))
> + tm_recheckpoint_new_task(new);
I'm not sure we need this patch as tm_recheckpoint_new_task() does this its=
elf.
---
static inline void tm_recheckpoint_new_task(struct task_struct *new)
{
if (!cpu_has_feature(CPU_FTR_TM))
return;
/* Recheckpoint the registers of the thread we're about to switch to.
*
* If the task was using FP, we non-lazily reload both the original and
* the speculative FP register states. This is because the kernel
* doesn't see if/when a TM rollback occurs, so if we take an FP
* unavailable later, we are unable to determine which set of FP regs
* need to be restored.
*/
if (!tm_enabled(new))
return;
---
Mikey
^ permalink raw reply
* Re: [PATCH v2 2/4] powerpc/tm: Fix HTM documentation
From: Michael Neuling @ 2018-08-15 23:46 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev
In-Reply-To: <1529362784-14194-2-git-send-email-leitao@debian.org>
On Mon, 2018-06-18 at 19:59 -0300, Breno Leitao wrote:
> This patch simply fix part of the documentation on the HTM code.
>=20
> This fixes reference to old fields that were renamed in commit
> 000ec280e3dd ("powerpc: tm: Rename transct_(*) to ck(\1)_state")
>=20
> It also documents better the flow after commit eb5c3f1c8647 ("powerpc:
> Always save/restore checkpointed regs during treclaim/trecheckpoint"),
> where tm_recheckpoint can recheckpoint what is in ck{fp,vr}_state blindly=
.
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
Acked-By: Michael Neuling <mikey@neuling.org>
Thanks
> ---
> arch/powerpc/kernel/tm.S | 10 +++++-----
> arch/powerpc/kernel/traps.c | 15 +++++++++------
> 2 files changed, 14 insertions(+), 11 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
> index ff12f47a96b6..019d73053cd3 100644
> --- a/arch/powerpc/kernel/tm.S
> +++ b/arch/powerpc/kernel/tm.S
> @@ -95,9 +95,9 @@ EXPORT_SYMBOL_GPL(tm_abort);
> * uint8_t cause)
> *
> * - Performs a full reclaim. This destroys outstanding
> - * transactions and updates thread->regs.tm_ckpt_* with the
> - * original checkpointed state. Note that thread->regs is
> - * unchanged.
> + * transactions and updates thread.ckpt_regs, thread.ckfp_state and
> + * thread.ckvr_state with the original checkpointed state. Note that
> + * thread->regs is unchanged.
> *
> * Purpose is to both abort transactions of, and preserve the state of,
> * a transactions at a context switch. We preserve/restore both sets of
> process
> @@ -260,7 +260,7 @@ _GLOBAL(tm_reclaim)
> =20
> /* Altivec (VEC/VMX/VR)*/
> addi r7, r3, THREAD_CKVRSTATE
> - SAVE_32VRS(0, r6, r7) /* r6 scratch, r7 transact vr state */
> + SAVE_32VRS(0, r6, r7) /* r6 scratch, r7 ckvr_state */
> mfvscr v0
> li r6, VRSTATE_VSCR
> stvx v0, r7, r6
> @@ -271,7 +271,7 @@ _GLOBAL(tm_reclaim)
> =20
> /* Floating Point (FP) */
> addi r7, r3, THREAD_CKFPSTATE
> - SAVE_32FPRS_VSRS(0, R6, R7) /* r6 scratch, r7 transact fp state */
> + SAVE_32FPRS_VSRS(0, R6, R7) /* r6 scratch, r7 ckfp_state */
> mffs fr0
> stfd fr0,FPSTATE_FPSCR(r7)
> =20
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 0e17dcb48720..6742b6b3eb37 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1719,16 +1719,19 @@ void fp_unavailable_tm(struct pt_regs *regs)
> * checkpointed FP registers need to be loaded.
> */
> tm_reclaim_current(TM_CAUSE_FAC_UNAV);
> - /* Reclaim didn't save out any FPRs to transact_fprs. */
> +
> + /* Reclaim initially saved out bogus (lazy) FPRs to ckfp_state, and
> + * then it was overwrite by the thr->fp_state by tm_reclaim_thread().
> + *
> + * At this point, ck{fp,vr}_state contains the exact values we want to
> + * recheckpoint.
> + */
> =20
> /* Enable FP for the task: */
> current->thread.load_fp =3D 1;
> =20
> - /* This loads and recheckpoints the FP registers from
> - * thread.fpr[]. They will remain in registers after the
> - * checkpoint so we don't need to reload them after.
> - * If VMX is in use, the VRs now hold checkpointed values,
> - * so we don't want to load the VRs from the thread_struct.
> + /*
> + * Recheckpoint all the checkpointed ckpt, ck{fp, vr}_state registers.
> */
> tm_recheckpoint(¤t->thread);
> }
^ permalink raw reply
* Re: [PATCH v2 4/4] powerpc/tm: Do not recheckpoint non-tm task
From: Breno Leitao @ 2018-08-16 14:19 UTC (permalink / raw)
To: Michael Neuling, linuxppc-dev
In-Reply-To: <cb89f5cf8d751da3de587b33cc07cc9292f5b324.camel@neuling.org>
Hey Mikey,
Thanks for the review.
On 08/15/2018 08:50 PM, Michael Neuling wrote:
> On Mon, 2018-06-18 at 19:59 -0300, Breno Leitao wrote:
>> If __switch_to() tries to context switch from task A to task B, and task A
>> had task->thread->regs->msr[TM] enabled, then __switch_to_tm() will call
>> tm_recheckpoint_new_task(), which will call trecheckpoint, for task B, which
>> is clearly wrong since task B might not be an active TM user.
>>
>> This does not cause a lot of damage because tm_recheckpoint() will abort
>> the call since it realize that the current task does not have msr[TM] bit
>> set.
>>
>> Signed-off-by: Breno Leitao <leitao@debian.org>
>> ---
>> arch/powerpc/kernel/process.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index f8beee03f00a..d26a150766ef 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -1036,7 +1036,8 @@ static inline void __switch_to_tm(struct task_struct
>> *prev,
>> prev->thread.regs->msr &= ~MSR_TM;
>> }
>>
>> - tm_recheckpoint_new_task(new);
>> + if (tm_enabled(new))
>> + tm_recheckpoint_new_task(new);
>
> I'm not sure we need this patch as tm_recheckpoint_new_task() does this itself.
My plan is to move this check prior to calling these TM functions, doing
early checking and avoiding calling tm_recheckpoint on a non-tm enabled task.
It is very weird when you see, during a tracing, a kernel thread (PF_KTHREAD)
being tm_recheckpointed. :-/
That said, the TM function would do the operation other than "check and do
it" mode.
Ideally I would like to check the thread before calling any TM functions,
and warning (WARN_ON) if we detect, later in the game, that a thread is not
TM enabled.
This helps on two different fronts, in my opinion:
* Code readability
* Understanding tracing (ftrace) outputs.
^ permalink raw reply
* Re: [PATCH v7 4/9] powerpc/pseries: Define MCE error event section.
From: Segher Boessenkool @ 2018-08-16 14:44 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mahesh Jagannath Salgaonkar, linuxppc-dev, Laurent Dufour,
Michal Suchanek, Aneesh Kumar K.V, Nicholas Piggin,
Ananth Narayan
In-Reply-To: <87lg97aqxc.fsf@concordia.ellerman.id.au>
Hi!
On Thu, Aug 16, 2018 at 02:14:39PM +1000, Michael Ellerman wrote:
> Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> > On 08/08/2018 08:12 PM, Michael Ellerman wrote:
> >>> + uint8_t reserved_1[6];
> >>> + __be64 effective_address;
> >>> + __be64 logical_address;
> >>> + } ue_error;
> >>> + struct {
> >>> + uint8_t soft_err_type;
> >>> + /* XXXXXXXX
> >>> + * X 1: Effective address provided.
> >>> + * XXXXX 5: Reserved.
> >>> + * XX 2: Type of SLB/ERAT/TLB error.
> >>> + */
> >>> + uint8_t reserved_1[6];
> >>> + __be64 effective_address;
> >>> + uint8_t reserved_2[8];
> >>> + } soft_error;
> >>> + } u;
> >>> +};
> >>> +#pragma pack(pop)
> >>
> >> Why not __packed ?
> >
> > Because when used __packed it added 1 byte extra padding between
> > reserved_1[6] and effective_address. That caused wrong effective address
> > to be printed on the console. Hence I switched to #pragma pack to force
> > 1 byte alignment for this structure alone.
>
> OK, that's weird.
Yes, if that is true, then please open a GCC bugzilla report.
Segher
^ permalink raw reply
* [PATCH v3] powerpc/tm: Remove msr_tm_active()
From: Breno Leitao @ 2018-08-16 17:21 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mikey, Breno Leitao
In-Reply-To: <e2f169e3ce57e5b092642088127f67a52da2bf93.camel@neuling.org>
Currently msr_tm_active() is a wrapper around MSR_TM_ACTIVE() if
CONFIG_PPC_TRANSACTIONAL_MEM is set, or it is just a function that
returns false if CONFIG_PPC_TRANSACTIONAL_MEM is not set.
This function is not necessary, since MSR_TM_ACTIVE() just do the same and
could be used, removing the dualism and simplifying the code.
This patchset remove every instance of msr_tm_active() and replaced it
by MSR_TM_ACTIVE().
Signed-off-by: Breno Leitao <leitao@debian.org>
---
arch/powerpc/include/asm/reg.h | 7 ++++++-
arch/powerpc/kernel/process.c | 21 +++++++++------------
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 562568414cf4..58393bc6c964 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -116,11 +116,16 @@
#define MSR_TS_S __MASK(MSR_TS_S_LG) /* Transaction Suspended */
#define MSR_TS_T __MASK(MSR_TS_T_LG) /* Transaction Transactional */
#define MSR_TS_MASK (MSR_TS_T | MSR_TS_S) /* Transaction State bits */
-#define MSR_TM_ACTIVE(x) (((x) & MSR_TS_MASK) != 0) /* Transaction active? */
#define MSR_TM_RESV(x) (((x) & MSR_TS_MASK) == MSR_TS_MASK) /* Reserved */
#define MSR_TM_TRANSACTIONAL(x) (((x) & MSR_TS_MASK) == MSR_TS_T)
#define MSR_TM_SUSPENDED(x) (((x) & MSR_TS_MASK) == MSR_TS_S)
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+#define MSR_TM_ACTIVE(x) (((x) & MSR_TS_MASK) != 0) /* Transaction active? */
+#else
+#define MSR_TM_ACTIVE(x) 0
+#endif
+
#if defined(CONFIG_PPC_BOOK3S_64)
#define MSR_64BIT MSR_SF
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 9ef4aea9fffe..f18dba07ea16 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -102,24 +102,18 @@ static void check_if_tm_restore_required(struct task_struct *tsk)
}
}
-static inline bool msr_tm_active(unsigned long msr)
-{
- return MSR_TM_ACTIVE(msr);
-}
-
static bool tm_active_with_fp(struct task_struct *tsk)
{
- return msr_tm_active(tsk->thread.regs->msr) &&
+ return MSR_TM_ACTIVE(tsk->thread.regs->msr) &&
(tsk->thread.ckpt_regs.msr & MSR_FP);
}
static bool tm_active_with_altivec(struct task_struct *tsk)
{
- return msr_tm_active(tsk->thread.regs->msr) &&
+ return MSR_TM_ACTIVE(tsk->thread.regs->msr) &&
(tsk->thread.ckpt_regs.msr & MSR_VEC);
}
#else
-static inline bool msr_tm_active(unsigned long msr) { return false; }
static inline void check_if_tm_restore_required(struct task_struct *tsk) { }
static inline bool tm_active_with_fp(struct task_struct *tsk) { return false; }
static inline bool tm_active_with_altivec(struct task_struct *tsk) { return false; }
@@ -247,7 +241,8 @@ void enable_kernel_fp(void)
* giveup as this would save to the 'live' structure not the
* checkpointed structure.
*/
- if(!msr_tm_active(cpumsr) && msr_tm_active(current->thread.regs->msr))
+ if (!MSR_TM_ACTIVE(cpumsr) &&
+ MSR_TM_ACTIVE(current->thread.regs->msr))
return;
__giveup_fpu(current);
}
@@ -311,7 +306,8 @@ void enable_kernel_altivec(void)
* giveup as this would save to the 'live' structure not the
* checkpointed structure.
*/
- if(!msr_tm_active(cpumsr) && msr_tm_active(current->thread.regs->msr))
+ if (!MSR_TM_ACTIVE(cpumsr) &&
+ MSR_TM_ACTIVE(current->thread.regs->msr))
return;
__giveup_altivec(current);
}
@@ -397,7 +393,8 @@ void enable_kernel_vsx(void)
* giveup as this would save to the 'live' structure not the
* checkpointed structure.
*/
- if(!msr_tm_active(cpumsr) && msr_tm_active(current->thread.regs->msr))
+ if (!MSR_TM_ACTIVE(cpumsr) &&
+ MSR_TM_ACTIVE(current->thread.regs->msr))
return;
__giveup_vsx(current);
}
@@ -530,7 +527,7 @@ void restore_math(struct pt_regs *regs)
{
unsigned long msr;
- if (!msr_tm_active(regs->msr) &&
+ if (!MSR_TM_ACTIVE(regs->msr) &&
!current->thread.load_fp && !loadvec(current->thread))
return;
--
2.16.3
^ permalink raw reply related
* Re: [PATCH 3/3] powerpc/pseries/mm: call H_BLOCK_REMOVE
From: Laurent Dufour @ 2018-08-16 17:27 UTC (permalink / raw)
To: Aneesh Kumar K.V, Michael Ellerman, linuxppc-dev, linux-kernel
Cc: benh, paulus, npiggin
In-Reply-To: <87fu00olaf.fsf@linux.ibm.com>
On 30/07/2018 16:22, Aneesh Kumar K.V wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>
>> Hi Laurent,
>>
>> Just one comment below.
>>
>> Laurent Dufour <ldufour@linux.vnet.ibm.com> writes:
>>> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
>>> index 96b8cd8a802d..41ed03245eb4 100644
>>> --- a/arch/powerpc/platforms/pseries/lpar.c
>>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>>> @@ -418,6 +418,73 @@ static void pSeries_lpar_hpte_invalidate(unsigned long slot, unsigned long vpn,
>>> BUG_ON(lpar_rc != H_SUCCESS);
>>> }
>>>
>>> +
>>> +/*
>>> + * As defined in the PAPR's section 14.5.4.1.8
>>> + * The control mask doesn't include the returned reference and change bit from
>>> + * the processed PTE.
>>> + */
>>> +#define HBLKR_AVPN 0x0100000000000000UL
>>> +#define HBLKR_CTRL_MASK 0xf800000000000000UL
>>> +#define HBLKR_CTRL_SUCCESS 0x8000000000000000UL
>>> +#define HBLKR_CTRL_ERRNOTFOUND 0x8800000000000000UL
>>> +#define HBLKR_CTRL_ERRBUSY 0xa000000000000000UL
>>> +
>>> +/**
>>> + * H_BLOCK_REMOVE caller.
>>> + * @idx should point to the latest @param entry set with a PTEX.
>>> + * If PTE cannot be processed because another CPUs has already locked that
>>> + * group, those entries are put back in @param starting at index 1.
>>> + * If entries has to be retried and @retry_busy is set to true, these entries
>>> + * are retried until success. If @retry_busy is set to false, the returned
>>> + * is the number of entries yet to process.
>>> + */
>>> +static unsigned long call_block_remove(unsigned long idx, unsigned long *param,
>>> + bool retry_busy)
>>> +{
>>> + unsigned long i, rc, new_idx;
>>> + unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
>>> +
>>> +again:
>>> + new_idx = 0;
>>> + BUG_ON((idx < 2) || (idx > PLPAR_HCALL9_BUFSIZE));
>>
>> I count 1 ..
>>
>>> + if (idx < PLPAR_HCALL9_BUFSIZE)
>>> + param[idx] = HBR_END;
>>> +
>>> + rc = plpar_hcall9(H_BLOCK_REMOVE, retbuf,
>>> + param[0], /* AVA */
>>> + param[1], param[2], param[3], param[4], /* TS0-7 */
>>> + param[5], param[6], param[7], param[8]);
>>> + if (rc == H_SUCCESS)
>>> + return 0;
>>> +
>>> + BUG_ON(rc != H_PARTIAL);
>>
>> 2 ...
>>
>>> + /* Check that the unprocessed entries were 'not found' or 'busy' */
>>> + for (i = 0; i < idx-1; i++) {
>>> + unsigned long ctrl = retbuf[i] & HBLKR_CTRL_MASK;
>>> +
>>> + if (ctrl == HBLKR_CTRL_ERRBUSY) {
>>> + param[++new_idx] = param[i+1];
>>> + continue;
>>> + }
>>> +
>>> + BUG_ON(ctrl != HBLKR_CTRL_SUCCESS
>>> + && ctrl != HBLKR_CTRL_ERRNOTFOUND);
>>
>> 3 ...
>>
>> BUG_ON()s.
>>
>> I know the code in this file is already pretty liberal with the use of
>> BUG_ON() but I'd prefer if we don't make it any worse.
>>
>> Given this is an optimisation it seems like we should be able to fall
>> back to the existing implementation in the case of error (which will
>> probably then BUG_ON() 😂)
>>
>> If there's some reason we can't then I guess I can live with it.
>
> It would be nice to log the error in case we are not expecting the
> error return. We recently did
> https://marc.info/?i=20180629083904.29250-1-aneesh.kumar@linux.ibm.com
I'm not sure that a failure during an invalidation should just result in an
error message being displayed because the page remains accessible and could
potentially be accessed later.
A comment in the caller hash__tlb_flush(), is quite explicit about that:
/* If there's a TLB batch pending, then we must flush it because the
* pages are going to be freed and we really don't want to have a CPU
* access a freed page because it has a stale TLB
*/
Getting an error when adding an entry may not be fatal but when removing one,
this could lead to data being exposed.
Laurent.
^ permalink raw reply
* [PATCH] powerpc/powernv/pci: Work around races in PCI bridge enabling
From: Benjamin Herrenschmidt @ 2018-08-17 0:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Michael Ellerman
The generic code is race when multiple children of a PCI bridge try to
enable it simultaneously.
This leads to drivers trying to access a device through a not-yet-enabled
bridge, and this EEH errors under various circumstances when using parallel
driver probing.
There is work going on to fix that properly in the PCI core but it will
take some time.
x86 gets away with it because (outside of hotplug), the BIOS enables all
the bridges at boot time.
This patch does the same thing on powernv by enabling all bridges that
have child devices at boot time, thus avoiding subsequent races. It's suitable
for backporting to stable and distros, while the proper PCI fix will probably
be significantly more invasive.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: stable@vger.kernel.org
---
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 70b2e1e0f23c..0975b0aaf210 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3368,12 +3368,52 @@ static void pnv_pci_ioda_create_dbgfs(void)
#endif /* CONFIG_DEBUG_FS */
}
+static void pnv_pci_enable_bridge(struct pci_bus *bus)
+{
+ struct pci_dev *dev = bus->self;
+ struct pci_bus *child;
+
+ /* Empty bus ? bail */
+ if (list_empty(&bus->devices))
+ return;
+
+ /*
+ * If there's a bridge associated with that bus enable it. This works
+ * around races in the generic code if the enabling is done during
+ * parallel probing. This can be removed once those races have been
+ * fixed.
+ */
+ if (dev) {
+ int rc = pci_enable_device(dev);
+ if (rc)
+ pci_err(dev, "Error enabling bridge (%d)\n", rc);
+ pci_set_master(dev);
+ }
+
+ /* Perform the same to child busses */
+ list_for_each_entry(child, &bus->children, node)
+ pnv_pci_enable_bridge(child);
+}
+
+static void pnv_pci_enable_bridges(void)
+{
+ struct pci_controller *hose;
+ struct pnv_phb *phb;
+ struct pci_bus *bus;
+ struct pci_dev *pdev;
+
+ list_for_each_entry(hose, &hose_list, list_node)
+ pnv_pci_enable_bridge(hose->bus);
+}
+
static void pnv_pci_ioda_fixup(void)
{
pnv_pci_ioda_setup_PEs();
pnv_pci_ioda_setup_iommu_api();
pnv_pci_ioda_create_dbgfs();
+ pnv_pci_enable_bridges();
+
#ifdef CONFIG_EEH
pnv_eeh_post_init();
#endif
^ permalink raw reply related
* Re: [PATCH v2 1/4] powerpc/tm: Remove msr_tm_active()
From: Michael Ellerman @ 2018-08-17 0:49 UTC (permalink / raw)
To: Michael Neuling, Breno Leitao, linuxppc-dev
In-Reply-To: <e2f169e3ce57e5b092642088127f67a52da2bf93.camel@neuling.org>
Michael Neuling <mikey@neuling.org> writes:
> On Mon, 2018-06-18 at 19:59 -0300, Breno Leitao wrote:
>> Currently msr_tm_active() is a wrapper around MSR_TM_ACTIVE() if
>> CONFIG_PPC_TRANSACTIONAL_MEM is set, or it is just a function that
>> returns false if CONFIG_PPC_TRANSACTIONAL_MEM is not set.
>>
>> This function is not necessary, since MSR_TM_ACTIVE() just do the same,
>> checking for the TS bits and does not require any TM facility.
>>
>> This patchset remove every instance of msr_tm_active() and replaced it
>> by MSR_TM_ACTIVE().
>>
>> Signed-off-by: Breno Leitao <leitao@debian.org>
>>
>
> Patch looks good... one minor nit below...
>
>>
>> - if (!msr_tm_active(regs->msr) &&
>> - !current->thread.load_fp && !loadvec(current->thread))
>> + if (!current->thread.load_fp && !loadvec(current->thread)) {
>> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> + if (!MSR_TM_ACTIVE(regs->msr))
>> + return;
>
> Can you make a MSR_TM_ACTIVE() that returns false when
> !CONFIG_PPC_TRANSACTIONAL_MEM. Then you don't need this inline #ifdef.
Is that safe?
I see ~50 callers of MSR_TM_ACTIVE(), are they all inside #ifdef TM ?
cheers
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Aneesh Kumar K.V @ 2018-08-17 3:32 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <4aaca2d27429e6bdadc340fd3b96e7c350c4b2f4.1534258290.git.christophe.leroy@c-s.fr>
On 08/14/2018 08:24 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.
>
> In order to avoid this conflict, this patch:
> - modifies pgtable_cache_add() so that a zeroising constructor is
> added for any cache size.
> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
>
Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and remove
the constructor completely?
-aneesh
^ permalink raw reply
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