* [PATCH] powerpc/64s: Fix dt_cpu_ftrs to have restore_cpu clear unwanted LPCR bits
From: Nicholas Piggin @ 2018-04-05 5:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, stable
Presently the dt_cpu_ftrs restore_cpu will only add bits to the LPCR
for secondaries, but some bits must be removed (e.g., UPRT for HPT).
Not clearing these bits on secondaries causes checkstops when booting
with disable_radix.
restore_cpu can not just set LPCR, because it is also called by the
idle wakeup code which relies on opal_slw_set_reg to restore the value
of LPCR, at least on P8 which does not save LPCR to stack in the idle
code.
Fix this by including a mask of bits to clear from LPCR as well, which
is used by restore_cpu.
This is a little messy now, but it's a minimal fix that can be
backported. Longer term, the idle SPR save/restore code can be
reworked to completely avoid calls to restore_cpu, then restore_cpu
would be able to unconditionally set LPCR to match boot processor
environment.
Fixes: 5a61ef74f269f ("powerpc/64s: Support new device tree binding for discovering CPU features")
Cc: stable@vger.kernel.org # v4.12+
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
I tested this fix and it boots a POWER9 with disable_radix, where
previously it checkstopped. Deeper idle states seem to work too,
after they're enabled with a firmware override.
arch/powerpc/kernel/dt_cpu_ftrs.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 11a3a4fed3fb..ed7605d8fd2d 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -83,6 +83,7 @@ static int hv_mode;
static struct {
u64 lpcr;
+ u64 lpcr_clear;
u64 hfscr;
u64 fscr;
} system_registers;
@@ -91,6 +92,8 @@ static void (*init_pmu_registers)(void);
static void __restore_cpu_cpufeatures(void)
{
+ u64 lpcr;
+
/*
* LPCR is restored by the power on engine already. It can be changed
* after early init e.g., by radix enable, and we have no unified API
@@ -103,8 +106,10 @@ static void __restore_cpu_cpufeatures(void)
* The best we can do to accommodate secondary boot and idle restore
* for now is "or" LPCR with existing.
*/
-
- mtspr(SPRN_LPCR, system_registers.lpcr | mfspr(SPRN_LPCR));
+ lpcr = mfspr(SPRN_LPCR);
+ lpcr |= system_registers.lpcr;
+ lpcr &= ~system_registers.lpcr_clear;
+ mtspr(SPRN_LPCR, lpcr);
if (hv_mode) {
mtspr(SPRN_LPID, 0);
mtspr(SPRN_HFSCR, system_registers.hfscr);
@@ -324,8 +329,9 @@ static int __init feat_enable_mmu_hash_v3(struct dt_cpu_feature *f)
{
u64 lpcr;
+ system_registers.lpcr_clear |= (LPCR_ISL | LPCR_UPRT | LPCR_HR);
lpcr = mfspr(SPRN_LPCR);
- lpcr &= ~LPCR_ISL;
+ lpcr &= ~(LPCR_ISL | LPCR_UPRT | LPCR_HR);
mtspr(SPRN_LPCR, lpcr);
cur_cpu_spec->mmu_features |= MMU_FTRS_HASH_BASE;
--
2.16.3
^ permalink raw reply related
* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Nicholas Piggin @ 2018-04-05 5:04 UTC (permalink / raw)
To: Dan Williams
Cc: Balbir Singh, Michael Ellerman, linuxppc-dev, linux-nvdimm,
Christoph Hellwig, Matthew Wilcox, Luck, Tony
In-Reply-To: <CAPcyv4hqKsOy9GU=NcMGj38pXnuEMRhXfox6jVwQ37+mcjnvhA@mail.gmail.com>
On Wed, 4 Apr 2018 20:00:52 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> [ adding Matthew, Christoph, and Tony ]
>
> On Wed, Apr 4, 2018 at 4:57 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > On Thu, 5 Apr 2018 09:19:42 +1000
> > Balbir Singh <bsingharora@gmail.com> wrote:
> >
> >> The pmem infrastructure uses memcpy_mcsafe in the pmem
> >> layer so as to convert machine check excpetions into
> >> a return value on failure in case a machine check
> >> exception is encoutered during the memcpy.
> >>
> >> This patch largely borrows from the copyuser_power7
> >> logic and does not add the VMX optimizations, largely
> >> to keep the patch simple. If needed those optimizations
> >> can be folded in.
> >
> > So memcpy_mcsafe doesn't return number of bytes copied?
> > Huh, well that makes it simple.
>
> Well, not in current kernels, but we need to add that support or
> remove the direct call to copy_to_iter() in fs/dax.c. I'm looking
> right now to add "bytes remaining" support to the x86 memcpy_mcsafe(),
> but for copy_to_user we also need to handle bytes remaining for write
> faults. That fix is hopefully something that can land in an early
> 4.17-rc, but it won't be ready for -rc1.
I wonder if the powerpc implementation should just go straight to
counting bytes. Backporting to this interface would be trivial, but
it would just mean there's only one variant of the code to support.
That's up to Balbir though.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 1/7] powerpc/fadump: Move the metadata region to start of the reserved area.
From: Mahesh Jagannath Salgaonkar @ 2018-04-05 4:50 UTC (permalink / raw)
To: Hari Bathini, linuxppc-dev
Cc: Ananth Narayan, kernelfans, Aneesh Kumar K.V, Nathan Fontenot,
Anshuman Khandual, Srikar Dronamraju
In-Reply-To: <f5a8a0f8-0b97-ac3e-dd4b-4796d6976725@linux.vnet.ibm.com>
On 04/04/2018 12:56 AM, Hari Bathini wrote:
> Mahesh, I think we should explicitly document that production and
> capture kernel
> versions should be same. For changes like below, older/newer production
> kernel vs
> capture kernel is bound to fail. Of course, production and capture
> kernel versions
> would be the same case usually but for the uninitiated, mentioning that
> explicitly
> in documentation would help..?
Yeah we could do that. In ideal case production and capture kernel will
be same. But yes, there can be cases where we may end up in older/newer
kernel scenario. My earlier versions had backward compatibility which I
broke in v3. I can fix that in v4. Thanks for catching it. Will also
update documentation mentioning working kernel combinations.
Thanks,
-Mahesh.
>
> Thanks
> Hari
>
>
> On Monday 02 April 2018 11:59 AM, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> Currently the metadata region that holds crash info structure and ELF
>> core
>> header is placed towards the end of reserved memory area. This patch
>> places
>> it at the beginning of the reserved memory area.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/fadump.h | 4 ++
>> arch/powerpc/kernel/fadump.c | 92
>> ++++++++++++++++++++++++++++++++-----
>> 2 files changed, 83 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/fadump.h
>> b/arch/powerpc/include/asm/fadump.h
>> index 5a23010af600..44b6ebfe9be6 100644
>> --- a/arch/powerpc/include/asm/fadump.h
>> +++ b/arch/powerpc/include/asm/fadump.h
>> @@ -61,6 +61,9 @@
>> #define FADUMP_UNREGISTER 2
>> #define FADUMP_INVALIDATE 3
>>
>> +/* Number of dump sections requested by kernel */
>> +#define FADUMP_NUM_SECTIONS 4
>> +
>> /* Dump status flag */
>> #define FADUMP_ERROR_FLAG 0x2000
>>
>> @@ -119,6 +122,7 @@ struct fadump_mem_struct {
>> struct fadump_section cpu_state_data;
>> struct fadump_section hpte_region;
>> struct fadump_section rmr_region;
>> + struct fadump_section metadata_region;
>> };
>>
>> /* Firmware-assisted dump configuration details. */
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 3c2c2688918f..80552fd7d5f8 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -188,17 +188,48 @@ static void fadump_show_config(void)
>> pr_debug("Boot memory size : %lx\n", fw_dump.boot_memory_size);
>> }
>>
>> +static unsigned long get_fadump_metadata_size(void)
>> +{
>> + unsigned long size = 0;
>> +
>> + /*
>> + * If fadump is active then look into fdm_active to get metadata
>> + * size set by previous kernel.
>> + */
>> + if (fw_dump.dump_active) {
>> + size = fdm_active->cpu_state_data.destination_address -
>> + fdm_active->metadata_region.source_address;
>> + goto out;
>> + }
>> + size += sizeof(struct fadump_crash_info_header);
>> + size += sizeof(struct elfhdr); /* ELF core header.*/
>> + size += sizeof(struct elf_phdr); /* place holder for cpu notes */
>> + /* Program headers for crash memory regions. */
>> + size += sizeof(struct elf_phdr) * (memblock_num_regions(memory) +
>> 2);
>> +
>> + size = PAGE_ALIGN(size);
>> +out:
>> + pr_debug("fadump Metadata size is %ld\n", size);
>> + return size;
>> +}
>> +
>> static unsigned long init_fadump_mem_struct(struct fadump_mem_struct
>> *fdm,
>> unsigned long addr)
>> {
>> + uint16_t num_sections = 0;
>> + unsigned long metadata_base = 0;
>> +
>> if (!fdm)
>> return 0;
>>
>> + /* Skip the fadump metadata area. */
>> + metadata_base = addr;
>> + addr += get_fadump_metadata_size();
>> +
>> memset(fdm, 0, sizeof(struct fadump_mem_struct));
>> addr = addr & PAGE_MASK;
>>
>> fdm->header.dump_format_version = cpu_to_be32(0x00000001);
>> - fdm->header.dump_num_sections = cpu_to_be16(3);
>> fdm->header.dump_status_flag = 0;
>> fdm->header.offset_first_dump_section =
>> cpu_to_be32((u32)offsetof(struct fadump_mem_struct,
>> cpu_state_data));
>> @@ -222,6 +253,7 @@ static unsigned long init_fadump_mem_struct(struct
>> fadump_mem_struct *fdm,
>> fdm->cpu_state_data.source_address = 0;
>> fdm->cpu_state_data.source_len =
>> cpu_to_be64(fw_dump.cpu_state_data_size);
>> fdm->cpu_state_data.destination_address = cpu_to_be64(addr);
>> + num_sections++;
>> addr += fw_dump.cpu_state_data_size;
>>
>> /* hpte region section */
>> @@ -230,6 +262,7 @@ static unsigned long init_fadump_mem_struct(struct
>> fadump_mem_struct *fdm,
>> fdm->hpte_region.source_address = 0;
>> fdm->hpte_region.source_len =
>> cpu_to_be64(fw_dump.hpte_region_size);
>> fdm->hpte_region.destination_address = cpu_to_be64(addr);
>> + num_sections++;
>> addr += fw_dump.hpte_region_size;
>>
>> /* RMA region section */
>> @@ -238,8 +271,25 @@ static unsigned long
>> init_fadump_mem_struct(struct fadump_mem_struct *fdm,
>> fdm->rmr_region.source_address = cpu_to_be64(RMA_START);
>> fdm->rmr_region.source_len = cpu_to_be64(fw_dump.boot_memory_size);
>> fdm->rmr_region.destination_address = cpu_to_be64(addr);
>> + num_sections++;
>> addr += fw_dump.boot_memory_size;
>>
>> + /*
>> + * fadump metadata section.
>> + * Add an entry with source len a zero. We are only intereseted in
>> + * source address which will help us to detect the location of
>> + * metadata area where faump header and elf core header is placed.
>> + */
>> + fdm->metadata_region.request_flag =
>> cpu_to_be32(FADUMP_REQUEST_FLAG);
>> + fdm->metadata_region.source_data_type =
>> + cpu_to_be16(FADUMP_REAL_MODE_REGION);
>> + fdm->metadata_region.source_address = cpu_to_be64(metadata_base);
>> + fdm->metadata_region.source_len = 0;
>> + fdm->metadata_region.destination_address = cpu_to_be64(addr);
>> + num_sections++;
>> +
>> + fdm->header.dump_num_sections = cpu_to_be16(num_sections);
>> + BUILD_BUG_ON(num_sections != FADUMP_NUM_SECTIONS);
>> return addr;
>> }
>>
>> @@ -325,16 +375,17 @@ static unsigned long get_fadump_area_size(void)
>> size += fw_dump.cpu_state_data_size;
>> size += fw_dump.hpte_region_size;
>> size += fw_dump.boot_memory_size;
>> - size += sizeof(struct fadump_crash_info_header);
>> - size += sizeof(struct elfhdr); /* ELF core header.*/
>> - size += sizeof(struct elf_phdr); /* place holder for cpu notes */
>> - /* Program headers for crash memory regions. */
>> - size += sizeof(struct elf_phdr) * (memblock_num_regions(memory) +
>> 2);
>> -
>> + size += get_fadump_metadata_size();
>> size = PAGE_ALIGN(size);
>> return size;
>> }
>>
>> +static inline unsigned long get_fadump_metadata_base(
>> + const struct fadump_mem_struct *fdm)
>> +{
>> + return be64_to_cpu(fdm->metadata_region.source_address);
>> +}
>> +
>> int __init fadump_reserve_mem(void)
>> {
>> unsigned long base, size, memory_boundary;
>> @@ -395,9 +446,9 @@ int __init fadump_reserve_mem(void)
>> (unsigned long)(size >> 20),
>> (unsigned long)(base >> 20));
>>
>> - fw_dump.fadumphdr_addr =
>> -
>> be64_to_cpu(fdm_active->rmr_region.destination_address) +
>> - be64_to_cpu(fdm_active->rmr_region.source_len);
>> + pr_info("Number of kernel Dump sections: %d\n",
>> + be16_to_cpu(fdm_active->header.dump_num_sections));
>> + fw_dump.fadumphdr_addr = get_fadump_metadata_base(fdm_active);
>> pr_debug("fadumphdr_addr = %p\n",
>> (void *) fw_dump.fadumphdr_addr);
>> } else {
>> @@ -803,14 +854,24 @@ static int __init fadump_build_cpu_notes(const
>> struct fadump_mem_struct *fdm)
>> static int __init process_fadump(const struct fadump_mem_struct
>> *fdm_active)
>> {
>> struct fadump_crash_info_header *fdh;
>> + uint16_t num_sections = 0;
>> int rc = 0;
>>
>> if (!fdm_active || !fw_dump.fadumphdr_addr)
>> return -EINVAL;
>>
>> + /* Check if platform has honored all 4 dump sections requested. */
>> + num_sections = be16_to_cpu(fdm_active->header.dump_num_sections);
>> + if (num_sections < FADUMP_NUM_SECTIONS) {
>> + printk(KERN_ERR "Dump taken by platform does not "
>> + "contain all requested sections\n");
>> + return -EINVAL;
>> + }
>> +
>> /* Check if the dump data is valid. */
>> if ((be16_to_cpu(fdm_active->header.dump_status_flag) ==
>> FADUMP_ERROR_FLAG) ||
>> (fdm_active->cpu_state_data.error_flags != 0) ||
>> + (fdm_active->metadata_region.error_flags != 0) ||
>> (fdm_active->rmr_region.error_flags != 0)) {
>> printk(KERN_ERR "Dump taken by platform is not valid\n");
>> return -EINVAL;
>> @@ -821,6 +882,11 @@ static int __init process_fadump(const struct
>> fadump_mem_struct *fdm_active)
>> printk(KERN_ERR "Dump taken by platform is incomplete\n");
>> return -EINVAL;
>> }
>> + if ((fdm_active->metadata_region.bytes_dumped !=
>> + fdm_active->metadata_region.source_len)) {
>> + printk(KERN_ERR "Dump taken by platform is incomplete\n");
>> + return -EINVAL;
>> + }
>>
>> /* Validate the fadump crash info header */
>> fdh = __va(fw_dump.fadumphdr_addr);
>> @@ -1082,7 +1148,7 @@ static int register_fadump(void)
>>
>> fadump_setup_crash_memory_ranges();
>>
>> - addr = be64_to_cpu(fdm.rmr_region.destination_address) +
>> be64_to_cpu(fdm.rmr_region.source_len);
>> + addr = get_fadump_metadata_base(&fdm);
>> /* Initialize fadump crash info header. */
>> addr = init_fadump_header(addr);
>> vaddr = __va(addr);
>> @@ -1153,7 +1219,7 @@ void fadump_cleanup(void)
>> /* Invalidate the registration only if dump is active. */
>> if (fw_dump.dump_active) {
>> init_fadump_mem_struct(&fdm,
>> -
>> be64_to_cpu(fdm_active->cpu_state_data.destination_address));
>> + get_fadump_metadata_base(fdm_active));
>> fadump_invalidate_dump(&fdm);
>> }
>> }
>> @@ -1236,7 +1302,7 @@ static void fadump_invalidate_release_mem(void)
>> return;
>> }
>>
>> - destination_address =
>> be64_to_cpu(fdm_active->cpu_state_data.destination_address);
>> + destination_address = get_fadump_metadata_base(fdm_active);
>> fadump_cleanup();
>> mutex_unlock(&fadump_mutex);
>>
>>
>
^ permalink raw reply
* Re: [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings
From: Balbir Singh @ 2018-04-05 4:43 UTC (permalink / raw)
To: Dan Williams; +Cc: Oliver, Device Tree, linuxppc-dev, linux-nvdimm
In-Reply-To: <CAPcyv4if4un9mgFon4ttjKDYEtHC_GP6VoxUENp7YNS50T3afA@mail.gmail.com>
On Wed, 4 Apr 2018 07:21:32 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> On Wed, Apr 4, 2018 at 7:04 AM, Oliver <oohall@gmail.com> wrote:
> > On Wed, Apr 4, 2018 at 10:07 PM, Balbir Singh <bsingharora@gmail.com> wrote:
> >> On Tue, 3 Apr 2018 10:37:51 -0700
> >> Dan Williams <dan.j.williams@intel.com> wrote:
> >>
> >>> On Tue, Apr 3, 2018 at 7:24 AM, Oliver O'Halloran <oohall@gmail.com> wrote:
> >>> > Add device-tree binding documentation for the nvdimm region driver.
> >>> >
> >>> > Cc: devicetree@vger.kernel.org
> >>> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> >>> > ---
> >>> > v2: Changed name from nvdimm-region to pmem-region.
> >>> > Cleaned up the example binding and fixed the overlapping regions.
> >>> > Added support for multiple regions in a single reg.
> >>> > ---
> >>> > .../devicetree/bindings/pmem/pmem-region.txt | 80 ++++++++++++++++++++++
> >>> > MAINTAINERS | 1 +
> >>> > 2 files changed, 81 insertions(+)
> >>> > create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt
> >>>
> >>> Device-tree folks, does this look, ok?
> >>>
> >>> Oliver, is there any concept of a management interface to the
> >>> device(s) backing these regions? libnvdimm calls these "nmem" devices
> >>> and support operations like health status and namespace label
> >>> management.
> >
> > It's something I'm planning on implementing as soon as someone gives
> > me some hardware that isn't hacked up lab crap. I'm posting this
> > version with just regions since people have been asking for something
> > in upstream even if it's not fully featured.
> >
> > Grumbling aside, the plan is to have separate drivers for the DIMM
> > type. Discovering DIMM devices happens via the normal discovery
> > mechanisms (e.g. an NVDIMM supporting the JEDEC interface is an I2C
> > device) and when binding to a specific DIMM device it registers a DIMM
> > descriptor structure and a ndctl implementation for that DIMM type
> > with of_pmem. When of_pmem binds to a region it can plug everything
> > into the region specific bus. There's a few details to work out, but I
> > think it's a reasonable approach.
>
> Yeah, that sounds reasonable. It would mean that your management
> interface would need to understand that nmems on different buses could
> potentially move to another bus after a reconfiguration, but that's
> not too much different than the ACPI case where nmems can join and
> leave regions after a reset / reconfig.
>
> >> We would need a way to have nmem and pmem-regions find each other. Since we
> >> don't have the ACPI abstractions, the nmem region would need to add the
> >> ability for a driver to have a phandle to the interleaving and nmem properties.
> >>
> >> I guess that would be a separate driver, that would manage the nmem devices
> >> and there would be a way to relate the pmem and nmems. Oliver?
> >
> > Yes, that's the plan.
>
> So Balbir, is that enough for an Acked-by for this device-tree proposal?
I don't see any major problems with the binding, but I think Oliver wanted
to send in a few clarifications based on a private discussion.
Balbir Singh.
^ permalink raw reply
* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Dan Williams @ 2018-04-05 3:00 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Balbir Singh, Michael Ellerman, linuxppc-dev, linux-nvdimm,
Christoph Hellwig, Matthew Wilcox, Luck, Tony
In-Reply-To: <20180405095755.58b3891f@roar.ozlabs.ibm.com>
[ adding Matthew, Christoph, and Tony ]
On Wed, Apr 4, 2018 at 4:57 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> On Thu, 5 Apr 2018 09:19:42 +1000
> Balbir Singh <bsingharora@gmail.com> wrote:
>
>> The pmem infrastructure uses memcpy_mcsafe in the pmem
>> layer so as to convert machine check excpetions into
>> a return value on failure in case a machine check
>> exception is encoutered during the memcpy.
>>
>> This patch largely borrows from the copyuser_power7
>> logic and does not add the VMX optimizations, largely
>> to keep the patch simple. If needed those optimizations
>> can be folded in.
>
> So memcpy_mcsafe doesn't return number of bytes copied?
> Huh, well that makes it simple.
Well, not in current kernels, but we need to add that support or
remove the direct call to copy_to_iter() in fs/dax.c. I'm looking
right now to add "bytes remaining" support to the x86 memcpy_mcsafe(),
but for copy_to_user we also need to handle bytes remaining for write
faults. That fix is hopefully something that can land in an early
4.17-rc, but it won't be ready for -rc1.
^ permalink raw reply
* Re: [RESEND 1/3] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Balbir Singh @ 2018-04-05 1:11 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, linux-nvdimm, mpe
In-Reply-To: <20180405094900.3ce4be8b@roar.ozlabs.ibm.com>
On Thu, 5 Apr 2018 09:49:00 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:
> On Thu, 5 Apr 2018 09:19:41 +1000
> Balbir Singh <bsingharora@gmail.com> wrote:
>
> > The code currently assumes PAGE_SHIFT as the shift value of
> > the pfn, this works correctly (mostly) for user space pages,
> > but the correct thing to do is
>
> It would be good to actually explain the problem in the
> changelog. I would have thought pte_pfn returns a
> PAGE_SIZE based pfn value?
>
The issue is hidden inside of hugepte_offset() as invoked by __find_linux_pte().
I will send a new version because the code needs to do
<< (shift - PAGE_SHIFT) for instruction address.
> >
> > 1. Extrace the shift value returned via the pte-walk API's
>
> ^^^ extract?
Thanks, yes, typo!
Balbir Singh.
^ permalink raw reply
* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Nicholas Piggin @ 2018-04-04 23:57 UTC (permalink / raw)
To: Balbir Singh; +Cc: linuxppc-dev, linux-nvdimm, mpe
In-Reply-To: <20180404231943.29581-3-bsingharora@gmail.com>
On Thu, 5 Apr 2018 09:19:42 +1000
Balbir Singh <bsingharora@gmail.com> wrote:
> The pmem infrastructure uses memcpy_mcsafe in the pmem
> layer so as to convert machine check excpetions into
> a return value on failure in case a machine check
> exception is encoutered during the memcpy.
>
> This patch largely borrows from the copyuser_power7
> logic and does not add the VMX optimizations, largely
> to keep the patch simple. If needed those optimizations
> can be folded in.
So memcpy_mcsafe doesn't return number of bytes copied?
Huh, well that makes it simple.
Would be nice if there was an easy way to share this with
the regular memcpy code... that's probably for another day
though, probably better to let this settle down first.
I didn't review exact instructions, but the approach looks
right to me.
Acked-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
> arch/powerpc/include/asm/string.h | 2 +
> arch/powerpc/lib/Makefile | 2 +-
> arch/powerpc/lib/memcpy_mcsafe_64.S | 212 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 215 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
>
> diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
> index 9b8cedf618f4..b7e872a64726 100644
> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -30,7 +30,9 @@ extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
> #ifdef CONFIG_PPC64
> #define __HAVE_ARCH_MEMSET32
> #define __HAVE_ARCH_MEMSET64
> +#define __HAVE_ARCH_MEMCPY_MCSAFE
>
> +extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
> extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
> extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
> extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 3c29c9009bbf..048afee9f518 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -24,7 +24,7 @@ endif
>
> obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
> copyuser_power7.o string_64.o copypage_power7.o memcpy_power7.o \
> - memcpy_64.o memcmp_64.o pmem.o
> + memcpy_64.o memcmp_64.o pmem.o memcpy_mcsafe_64.o
>
> obj64-$(CONFIG_SMP) += locks.o
> obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
> diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
> new file mode 100644
> index 000000000000..e7eaa9b6cded
> --- /dev/null
> +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
> @@ -0,0 +1,212 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) IBM Corporation, 2011
> + * Derived from copyuser_power7.s by Anton Blanchard <anton@au.ibm.com>
> + * Author - Balbir Singh <bsingharora@gmail.com>
> + */
> +#include <asm/ppc_asm.h>
> +#include <asm/errno.h>
> +
> + .macro err1
> +100:
> + EX_TABLE(100b,.Ldo_err1)
> + .endm
> +
> + .macro err2
> +200:
> + EX_TABLE(200b,.Ldo_err2)
> + .endm
> +
> +.Ldo_err2:
> + ld r22,STK_REG(R22)(r1)
> + ld r21,STK_REG(R21)(r1)
> + ld r20,STK_REG(R20)(r1)
> + ld r19,STK_REG(R19)(r1)
> + ld r18,STK_REG(R18)(r1)
> + ld r17,STK_REG(R17)(r1)
> + ld r16,STK_REG(R16)(r1)
> + ld r15,STK_REG(R15)(r1)
> + ld r14,STK_REG(R14)(r1)
> + addi r1,r1,STACKFRAMESIZE
> +.Ldo_err1:
> + li r3,-EFAULT
> + blr
> +
> +
> +_GLOBAL(memcpy_mcsafe)
> + cmpldi r5,16
> + blt .Lshort_copy
> +
> +.Lcopy:
> + /* Get the source 8B aligned */
> + neg r6,r4
> + mtocrf 0x01,r6
> + clrldi r6,r6,(64-3)
> +
> + bf cr7*4+3,1f
> +err1; lbz r0,0(r4)
> + addi r4,r4,1
> +err1; stb r0,0(r3)
> + addi r3,r3,1
> +
> +1: bf cr7*4+2,2f
> +err1; lhz r0,0(r4)
> + addi r4,r4,2
> +err1; sth r0,0(r3)
> + addi r3,r3,2
> +
> +2: bf cr7*4+1,3f
> +err1; lwz r0,0(r4)
> + addi r4,r4,4
> +err1; stw r0,0(r3)
> + addi r3,r3,4
> +
> +3: sub r5,r5,r6
> + cmpldi r5,128
> + blt 5f
> +
> + mflr r0
> + stdu r1,-STACKFRAMESIZE(r1)
> + std r14,STK_REG(R14)(r1)
> + std r15,STK_REG(R15)(r1)
> + std r16,STK_REG(R16)(r1)
> + std r17,STK_REG(R17)(r1)
> + std r18,STK_REG(R18)(r1)
> + std r19,STK_REG(R19)(r1)
> + std r20,STK_REG(R20)(r1)
> + std r21,STK_REG(R21)(r1)
> + std r22,STK_REG(R22)(r1)
> + std r0,STACKFRAMESIZE+16(r1)
> +
> + srdi r6,r5,7
> + mtctr r6
> +
> + /* Now do cacheline (128B) sized loads and stores. */
> + .align 5
> +4:
> +err2; ld r0,0(r4)
> +err2; ld r6,8(r4)
> +err2; ld r7,16(r4)
> +err2; ld r8,24(r4)
> +err2; ld r9,32(r4)
> +err2; ld r10,40(r4)
> +err2; ld r11,48(r4)
> +err2; ld r12,56(r4)
> +err2; ld r14,64(r4)
> +err2; ld r15,72(r4)
> +err2; ld r16,80(r4)
> +err2; ld r17,88(r4)
> +err2; ld r18,96(r4)
> +err2; ld r19,104(r4)
> +err2; ld r20,112(r4)
> +err2; ld r21,120(r4)
> + addi r4,r4,128
> +err2; std r0,0(r3)
> +err2; std r6,8(r3)
> +err2; std r7,16(r3)
> +err2; std r8,24(r3)
> +err2; std r9,32(r3)
> +err2; std r10,40(r3)
> +err2; std r11,48(r3)
> +err2; std r12,56(r3)
> +err2; std r14,64(r3)
> +err2; std r15,72(r3)
> +err2; std r16,80(r3)
> +err2; std r17,88(r3)
> +err2; std r18,96(r3)
> +err2; std r19,104(r3)
> +err2; std r20,112(r3)
> +err2; std r21,120(r3)
> + addi r3,r3,128
> + bdnz 4b
> +
> + clrldi r5,r5,(64-7)
> +
> + ld r14,STK_REG(R14)(r1)
> + ld r15,STK_REG(R15)(r1)
> + ld r16,STK_REG(R16)(r1)
> + ld r17,STK_REG(R17)(r1)
> + ld r18,STK_REG(R18)(r1)
> + ld r19,STK_REG(R19)(r1)
> + ld r20,STK_REG(R20)(r1)
> + ld r21,STK_REG(R21)(r1)
> + ld r22,STK_REG(R22)(r1)
> + addi r1,r1,STACKFRAMESIZE
> +
> + /* Up to 127B to go */
> +5: srdi r6,r5,4
> + mtocrf 0x01,r6
> +
> +6: bf cr7*4+1,7f
> +err1; ld r0,0(r4)
> +err1; ld r6,8(r4)
> +err1; ld r7,16(r4)
> +err1; ld r8,24(r4)
> +err1; ld r9,32(r4)
> +err1; ld r10,40(r4)
> +err1; ld r11,48(r4)
> +err1; ld r12,56(r4)
> + addi r4,r4,64
> +err1; std r0,0(r3)
> +err1; std r6,8(r3)
> +err1; std r7,16(r3)
> +err1; std r8,24(r3)
> +err1; std r9,32(r3)
> +err1; std r10,40(r3)
> +err1; std r11,48(r3)
> +err1; std r12,56(r3)
> + addi r3,r3,64
> +
> + /* Up to 63B to go */
> +7: bf cr7*4+2,8f
> +err1; ld r0,0(r4)
> +err1; ld r6,8(r4)
> +err1; ld r7,16(r4)
> +err1; ld r8,24(r4)
> + addi r4,r4,32
> +err1; std r0,0(r3)
> +err1; std r6,8(r3)
> +err1; std r7,16(r3)
> +err1; std r8,24(r3)
> + addi r3,r3,32
> +
> + /* Up to 31B to go */
> +8: bf cr7*4+3,9f
> +err1; ld r0,0(r4)
> +err1; ld r6,8(r4)
> + addi r4,r4,16
> +err1; std r0,0(r3)
> +err1; std r6,8(r3)
> + addi r3,r3,16
> +
> +9: clrldi r5,r5,(64-4)
> +
> + /* Up to 15B to go */
> +.Lshort_copy:
> + mtocrf 0x01,r5
> + bf cr7*4+0,12f
> +err1; lwz r0,0(r4) /* Less chance of a reject with word ops */
> +err1; lwz r6,4(r4)
> + addi r4,r4,8
> +err1; stw r0,0(r3)
> +err1; stw r6,4(r3)
> + addi r3,r3,8
> +
> +12: bf cr7*4+1,13f
> +err1; lwz r0,0(r4)
> + addi r4,r4,4
> +err1; stw r0,0(r3)
> + addi r3,r3,4
> +
> +13: bf cr7*4+2,14f
> +err1; lhz r0,0(r4)
> + addi r4,r4,2
> +err1; sth r0,0(r3)
> + addi r3,r3,2
> +
> +14: bf cr7*4+3,15f
> +err1; lbz r0,0(r4)
> +err1; stb r0,0(r3)
> +
> +15: li r3,0
> + blr
^ permalink raw reply
* Re: [RESEND 1/3] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Nicholas Piggin @ 2018-04-04 23:49 UTC (permalink / raw)
To: Balbir Singh; +Cc: linuxppc-dev, linux-nvdimm, mpe
In-Reply-To: <20180404231943.29581-2-bsingharora@gmail.com>
On Thu, 5 Apr 2018 09:19:41 +1000
Balbir Singh <bsingharora@gmail.com> wrote:
> The code currently assumes PAGE_SHIFT as the shift value of
> the pfn, this works correctly (mostly) for user space pages,
> but the correct thing to do is
It would be good to actually explain the problem in the
changelog. I would have thought pte_pfn returns a
PAGE_SIZE based pfn value?
>
> 1. Extrace the shift value returned via the pte-walk API's
^^^ extract?
> 2. Use the shift value to access the instruction address.
>
> Note, the final physical address still use PAGE_SHIFT for
> computation. handle_ierror() is not modified and handle_derror()
> is modified just for extracting the correct instruction
> address.
>
> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
> arch/powerpc/kernel/mce_power.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> index fe6fc63251fe..69c8cc1e8e4f 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -36,7 +36,8 @@
> * Convert an address related to an mm to a PFN. NOTE: we are in real
> * mode, we could potentially race with page table updates.
> */
> -static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> +static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
> + unsigned int *shift)
> {
> pte_t *ptep;
> unsigned long flags;
> @@ -49,9 +50,9 @@ static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>
> local_irq_save(flags);
> if (mm == current->mm)
> - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
> + ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift);
> else
> - ptep = find_init_mm_pte(addr, NULL);
> + ptep = find_init_mm_pte(addr, shift);
> local_irq_restore(flags);
> if (!ptep || pte_special(*ptep))
> return ULONG_MAX;
> @@ -353,13 +354,14 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
> unsigned long pfn, instr_addr;
> struct instruction_op op;
> struct pt_regs tmp = *regs;
> + unsigned int shift;
>
> - pfn = addr_to_pfn(regs, regs->nip);
> + pfn = addr_to_pfn(regs, regs->nip, &shift);
> if (pfn != ULONG_MAX) {
> - instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
> + instr_addr = (pfn << shift) + (regs->nip & ((1 << shift) - 1));
> instr = *(unsigned int *)(instr_addr);
> if (!analyse_instr(&op, &tmp, instr)) {
> - pfn = addr_to_pfn(regs, op.ea);
> + pfn = addr_to_pfn(regs, op.ea, &shift);
> *addr = op.ea;
> *phys_addr = (pfn << PAGE_SHIFT);
> return 0;
> @@ -437,7 +439,8 @@ static int mce_handle_ierror(struct pt_regs *regs,
> unsigned long pfn;
>
> if (get_paca()->in_mce < MAX_MCE_DEPTH) {
> - pfn = addr_to_pfn(regs, regs->nip);
> + pfn = addr_to_pfn(regs, regs->nip,
> + NULL);
> if (pfn != ULONG_MAX) {
> *phys_addr =
> (pfn << PAGE_SHIFT);
^ permalink raw reply
* [RESEND 3/3] powerpc/mce: Handle memcpy_mcsafe
From: Balbir Singh @ 2018-04-04 23:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, mpe, npiggin, Balbir Singh
In-Reply-To: <20180404231943.29581-1-bsingharora@gmail.com>
Add a blocking notifier callback to be called in real-mode
on machine check exceptions for UE (ld/st) errors only.
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);
+ 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;
@@ -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
^ permalink raw reply related
* [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Balbir Singh @ 2018-04-04 23:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, mpe, npiggin, Balbir Singh
In-Reply-To: <20180404231943.29581-1-bsingharora@gmail.com>
The pmem infrastructure uses memcpy_mcsafe in the pmem
layer so as to convert machine check excpetions into
a return value on failure in case a machine check
exception is encoutered during the memcpy.
This patch largely borrows from the copyuser_power7
logic and does not add the VMX optimizations, largely
to keep the patch simple. If needed those optimizations
can be folded in.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 212 ++++++++++++++++++++++++++++++++++++
3 files changed, 215 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 9b8cedf618f4..b7e872a64726 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -30,7 +30,9 @@ extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
#ifdef CONFIG_PPC64
#define __HAVE_ARCH_MEMSET32
#define __HAVE_ARCH_MEMSET64
+#define __HAVE_ARCH_MEMCPY_MCSAFE
+extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3c29c9009bbf..048afee9f518 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -24,7 +24,7 @@ endif
obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
copyuser_power7.o string_64.o copypage_power7.o memcpy_power7.o \
- memcpy_64.o memcmp_64.o pmem.o
+ memcpy_64.o memcmp_64.o pmem.o memcpy_mcsafe_64.o
obj64-$(CONFIG_SMP) += locks.o
obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
new file mode 100644
index 000000000000..e7eaa9b6cded
--- /dev/null
+++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) IBM Corporation, 2011
+ * Derived from copyuser_power7.s by Anton Blanchard <anton@au.ibm.com>
+ * Author - Balbir Singh <bsingharora@gmail.com>
+ */
+#include <asm/ppc_asm.h>
+#include <asm/errno.h>
+
+ .macro err1
+100:
+ EX_TABLE(100b,.Ldo_err1)
+ .endm
+
+ .macro err2
+200:
+ EX_TABLE(200b,.Ldo_err2)
+ .endm
+
+.Ldo_err2:
+ ld r22,STK_REG(R22)(r1)
+ ld r21,STK_REG(R21)(r1)
+ ld r20,STK_REG(R20)(r1)
+ ld r19,STK_REG(R19)(r1)
+ ld r18,STK_REG(R18)(r1)
+ ld r17,STK_REG(R17)(r1)
+ ld r16,STK_REG(R16)(r1)
+ ld r15,STK_REG(R15)(r1)
+ ld r14,STK_REG(R14)(r1)
+ addi r1,r1,STACKFRAMESIZE
+.Ldo_err1:
+ li r3,-EFAULT
+ blr
+
+
+_GLOBAL(memcpy_mcsafe)
+ cmpldi r5,16
+ blt .Lshort_copy
+
+.Lcopy:
+ /* Get the source 8B aligned */
+ neg r6,r4
+ mtocrf 0x01,r6
+ clrldi r6,r6,(64-3)
+
+ bf cr7*4+3,1f
+err1; lbz r0,0(r4)
+ addi r4,r4,1
+err1; stb r0,0(r3)
+ addi r3,r3,1
+
+1: bf cr7*4+2,2f
+err1; lhz r0,0(r4)
+ addi r4,r4,2
+err1; sth r0,0(r3)
+ addi r3,r3,2
+
+2: bf cr7*4+1,3f
+err1; lwz r0,0(r4)
+ addi r4,r4,4
+err1; stw r0,0(r3)
+ addi r3,r3,4
+
+3: sub r5,r5,r6
+ cmpldi r5,128
+ blt 5f
+
+ mflr r0
+ stdu r1,-STACKFRAMESIZE(r1)
+ std r14,STK_REG(R14)(r1)
+ std r15,STK_REG(R15)(r1)
+ std r16,STK_REG(R16)(r1)
+ std r17,STK_REG(R17)(r1)
+ std r18,STK_REG(R18)(r1)
+ std r19,STK_REG(R19)(r1)
+ std r20,STK_REG(R20)(r1)
+ std r21,STK_REG(R21)(r1)
+ std r22,STK_REG(R22)(r1)
+ std r0,STACKFRAMESIZE+16(r1)
+
+ srdi r6,r5,7
+ mtctr r6
+
+ /* Now do cacheline (128B) sized loads and stores. */
+ .align 5
+4:
+err2; ld r0,0(r4)
+err2; ld r6,8(r4)
+err2; ld r7,16(r4)
+err2; ld r8,24(r4)
+err2; ld r9,32(r4)
+err2; ld r10,40(r4)
+err2; ld r11,48(r4)
+err2; ld r12,56(r4)
+err2; ld r14,64(r4)
+err2; ld r15,72(r4)
+err2; ld r16,80(r4)
+err2; ld r17,88(r4)
+err2; ld r18,96(r4)
+err2; ld r19,104(r4)
+err2; ld r20,112(r4)
+err2; ld r21,120(r4)
+ addi r4,r4,128
+err2; std r0,0(r3)
+err2; std r6,8(r3)
+err2; std r7,16(r3)
+err2; std r8,24(r3)
+err2; std r9,32(r3)
+err2; std r10,40(r3)
+err2; std r11,48(r3)
+err2; std r12,56(r3)
+err2; std r14,64(r3)
+err2; std r15,72(r3)
+err2; std r16,80(r3)
+err2; std r17,88(r3)
+err2; std r18,96(r3)
+err2; std r19,104(r3)
+err2; std r20,112(r3)
+err2; std r21,120(r3)
+ addi r3,r3,128
+ bdnz 4b
+
+ clrldi r5,r5,(64-7)
+
+ ld r14,STK_REG(R14)(r1)
+ ld r15,STK_REG(R15)(r1)
+ ld r16,STK_REG(R16)(r1)
+ ld r17,STK_REG(R17)(r1)
+ ld r18,STK_REG(R18)(r1)
+ ld r19,STK_REG(R19)(r1)
+ ld r20,STK_REG(R20)(r1)
+ ld r21,STK_REG(R21)(r1)
+ ld r22,STK_REG(R22)(r1)
+ addi r1,r1,STACKFRAMESIZE
+
+ /* Up to 127B to go */
+5: srdi r6,r5,4
+ mtocrf 0x01,r6
+
+6: bf cr7*4+1,7f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+err1; ld r7,16(r4)
+err1; ld r8,24(r4)
+err1; ld r9,32(r4)
+err1; ld r10,40(r4)
+err1; ld r11,48(r4)
+err1; ld r12,56(r4)
+ addi r4,r4,64
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+err1; std r7,16(r3)
+err1; std r8,24(r3)
+err1; std r9,32(r3)
+err1; std r10,40(r3)
+err1; std r11,48(r3)
+err1; std r12,56(r3)
+ addi r3,r3,64
+
+ /* Up to 63B to go */
+7: bf cr7*4+2,8f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+err1; ld r7,16(r4)
+err1; ld r8,24(r4)
+ addi r4,r4,32
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+err1; std r7,16(r3)
+err1; std r8,24(r3)
+ addi r3,r3,32
+
+ /* Up to 31B to go */
+8: bf cr7*4+3,9f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+ addi r4,r4,16
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+ addi r3,r3,16
+
+9: clrldi r5,r5,(64-4)
+
+ /* Up to 15B to go */
+.Lshort_copy:
+ mtocrf 0x01,r5
+ bf cr7*4+0,12f
+err1; lwz r0,0(r4) /* Less chance of a reject with word ops */
+err1; lwz r6,4(r4)
+ addi r4,r4,8
+err1; stw r0,0(r3)
+err1; stw r6,4(r3)
+ addi r3,r3,8
+
+12: bf cr7*4+1,13f
+err1; lwz r0,0(r4)
+ addi r4,r4,4
+err1; stw r0,0(r3)
+ addi r3,r3,4
+
+13: bf cr7*4+2,14f
+err1; lhz r0,0(r4)
+ addi r4,r4,2
+err1; sth r0,0(r3)
+ addi r3,r3,2
+
+14: bf cr7*4+3,15f
+err1; lbz r0,0(r4)
+err1; stb r0,0(r3)
+
+15: li r3,0
+ blr
--
2.13.6
^ permalink raw reply related
* [RESEND 1/3] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Balbir Singh @ 2018-04-04 23:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, mpe, npiggin, Balbir Singh
In-Reply-To: <20180404231943.29581-1-bsingharora@gmail.com>
The code currently assumes PAGE_SHIFT as the shift value of
the pfn, this works correctly (mostly) for user space pages,
but the correct thing to do is
1. Extrace the shift value returned via the pte-walk API's
2. Use the shift value to access the instruction address.
Note, the final physical address still use PAGE_SHIFT for
computation. handle_ierror() is not modified and handle_derror()
is modified just for extracting the correct instruction
address.
Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/mce_power.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index fe6fc63251fe..69c8cc1e8e4f 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -36,7 +36,8 @@
* Convert an address related to an mm to a PFN. NOTE: we are in real
* mode, we could potentially race with page table updates.
*/
-static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
+static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
+ unsigned int *shift)
{
pte_t *ptep;
unsigned long flags;
@@ -49,9 +50,9 @@ static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
local_irq_save(flags);
if (mm == current->mm)
- ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
+ ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift);
else
- ptep = find_init_mm_pte(addr, NULL);
+ ptep = find_init_mm_pte(addr, shift);
local_irq_restore(flags);
if (!ptep || pte_special(*ptep))
return ULONG_MAX;
@@ -353,13 +354,14 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
unsigned long pfn, instr_addr;
struct instruction_op op;
struct pt_regs tmp = *regs;
+ unsigned int shift;
- pfn = addr_to_pfn(regs, regs->nip);
+ pfn = addr_to_pfn(regs, regs->nip, &shift);
if (pfn != ULONG_MAX) {
- instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
+ instr_addr = (pfn << shift) + (regs->nip & ((1 << shift) - 1));
instr = *(unsigned int *)(instr_addr);
if (!analyse_instr(&op, &tmp, instr)) {
- pfn = addr_to_pfn(regs, op.ea);
+ pfn = addr_to_pfn(regs, op.ea, &shift);
*addr = op.ea;
*phys_addr = (pfn << PAGE_SHIFT);
return 0;
@@ -437,7 +439,8 @@ static int mce_handle_ierror(struct pt_regs *regs,
unsigned long pfn;
if (get_paca()->in_mce < MAX_MCE_DEPTH) {
- pfn = addr_to_pfn(regs, regs->nip);
+ pfn = addr_to_pfn(regs, regs->nip,
+ NULL);
if (pfn != ULONG_MAX) {
*phys_addr =
(pfn << PAGE_SHIFT);
--
2.13.6
^ permalink raw reply related
* [RESEND 0/3] Add support for memcpy_mcsafe
From: Balbir Singh @ 2018-04-04 23:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, mpe, npiggin, Balbir Singh
memcpy_mcsafe() is an API currently used by the pmem subsystem to convert
errors while doing a memcpy (machine check exception errors) to a return
value. This patchset consists of three patches
1. The first patch is a bug fix to handle machine check errors correctly
while walking the page tables in kernel mode, due to huge pmd/pud sizes
2. The second patch adds memcpy_mcsafe() support, this is largely derived
from existing code
3. The third patch registers for callbacks on machine check exceptions and
in them uses specialized knowledge of the type of page to decide whether
to handle the MCE as is or to return to a fixup address present in
memcpy_mcsafe(). If a fixup address is used, then we return an error
value of -EFAULT to the caller.
Testing
A large part of the testing was done under a simulator by selectively
inserting machine check exceptions in a test driver doing memcpy_mcsafe
via ioctls.
Balbir Singh (3):
powerpc/mce: Bug fixes for MCE handling in kernel space
powerpc/memcpy: Add memcpy_mcsafe for pmem
powerpc/mce: Handle memcpy_mcsafe
arch/powerpc/include/asm/mce.h | 3 +-
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/kernel/mce.c | 76 +++++++++++-
arch/powerpc/kernel/mce_power.c | 17 +--
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 225 ++++++++++++++++++++++++++++++++++++
6 files changed, 314 insertions(+), 11 deletions(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
--
2.13.6
^ permalink raw reply
* Re: [PATCH v9 15/24] mm: Introduce __vm_normal_page()
From: Jerome Glisse @ 2018-04-04 21:59 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <a3446d86-8a29-a9f2-a1fe-b8cc1b748132@linux.vnet.ibm.com>
On Wed, Apr 04, 2018 at 06:26:44PM +0200, Laurent Dufour wrote:
>
>
> On 03/04/2018 21:39, Jerome Glisse wrote:
> > On Tue, Mar 13, 2018 at 06:59:45PM +0100, Laurent Dufour wrote:
> >> When dealing with the speculative fault path we should use the VMA's field
> >> cached value stored in the vm_fault structure.
> >>
> >> Currently vm_normal_page() is using the pointer to the VMA to fetch the
> >> vm_flags value. This patch provides a new __vm_normal_page() which is
> >> receiving the vm_flags flags value as parameter.
> >>
> >> Note: The speculative path is turned on for architecture providing support
> >> for special PTE flag. So only the first block of vm_normal_page is used
> >> during the speculative path.
> >
> > Might be a good idea to explicitly have SPECULATIVE Kconfig option depends
> > on ARCH_PTE_SPECIAL and a comment for !HAVE_PTE_SPECIAL in the function
> > explaining that speculative page fault should never reach that point.
>
> Unfortunately there is no ARCH_PTE_SPECIAL in the config file, it is defined in
> the per architecture header files.
> So I can't do anything in the Kconfig file
Maybe adding a new Kconfig symbol for ARCH_PTE_SPECIAL very much like
others ARCH_HAS_
>
> However, I can check that at build time, and doing such a check in
> __vm_normal_page sounds to be a good place, like that:
>
> @@ -869,6 +870,14 @@ struct page *__vm_normal_page(struct vm_area_struct *vma,
> unsigned long addr,
>
> /* !HAVE_PTE_SPECIAL case follows: */
>
> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> + /* This part should never get called when the speculative page fault
> + * handler is turned on. This is mainly because we can't rely on
> + * vm_start.
> + */
> +#error CONFIG_SPECULATIVE_PAGE_FAULT requires HAVE_PTE_SPECIAL
> +#endif
> +
> if (unlikely(vma_flags & (VM_PFNMAP|VM_MIXEDMAP))) {
> if (vma_flags & VM_MIXEDMAP) {
> if (!pfn_valid(pfn))
>
I am not a fan of #if/#else/#endif in code. But that's a taste thing.
I honnestly think that adding a Kconfig for special pte is the cleanest
solution.
Cheers,
Jérôme
^ permalink raw reply
* Re: [mm] b1f0502d04: INFO:trying_to_register_non-static_key
From: David Rientjes @ 2018-04-04 21:53 UTC (permalink / raw)
To: Laurent Dufour
Cc: kernel test robot, paulmck, peterz, akpm, kirill, ak, mhocko,
dave, jack, Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner,
Ingo Molnar, hpa, Will Deacon, Sergey Senozhatsky,
Andrea Arcangeli, Alexei Starovoitov, kemi.wang,
sergey.senozhatsky.work, Daniel Jordan, linux-kernel, linux-mm,
haren, khandual, npiggin, bsingharora, Tim Chen, linuxppc-dev,
x86, lkp
In-Reply-To: <147e8be3-2c33-2111-aacc-dc2bb932fa8c@linux.vnet.ibm.com>
On Wed, 4 Apr 2018, Laurent Dufour wrote:
> > I also think the following is needed:
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -312,6 +312,10 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
> > vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
> > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> > INIT_LIST_HEAD(&vma->anon_vma_chain);
> > +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> > + seqcount_init(&vma->vm_sequence);
> > + atomic_set(&vma->vm_ref_count, 0);
> > +#endif
> >
> > err = insert_vm_struct(mm, vma);
> > if (err)
>
> No, this not needed because the vma is allocated with kmem_cache_zalloc() so
> vm_ref_count is 0, and insert_vm_struc() will later call
> __vma_link_rb() which will call seqcount_init().
>
> Furhtermore, in case of error, the vma structure is freed without calling
> get_vma() so there is risk of lockdep warning.
>
Perhaps you're working from a different tree than I am, or you fixed the
lockdep warning differently when adding to dup_mmap() and mmap_region().
I got the following two lockdep errors.
I fixed it locally by doing the seqcount_init() and atomic_set()
everywhere a vma could be initialized.
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 12 PID: 1 Comm: init Not tainted
Call Trace:
[<ffffffff8b12026f>] dump_stack+0x67/0x98
[<ffffffff8a92b616>] register_lock_class+0x1e6/0x4e0
[<ffffffff8a92cfe9>] __lock_acquire+0xb9/0x1710
[<ffffffff8a92ef3a>] lock_acquire+0xba/0x200
[<ffffffff8aa827df>] mprotect_fixup+0x10f/0x310
[<ffffffff8aade3fd>] setup_arg_pages+0x12d/0x230
[<ffffffff8ab4564a>] load_elf_binary+0x44a/0x1740
[<ffffffff8aadde9b>] search_binary_handler+0x9b/0x1e0
[<ffffffff8ab44e96>] load_script+0x206/0x270
[<ffffffff8aadde9b>] search_binary_handler+0x9b/0x1e0
[<ffffffff8aae0355>] do_execveat_common.isra.32+0x6b5/0x9d0
[<ffffffff8aae069c>] do_execve+0x2c/0x30
[<ffffffff8a80047b>] run_init_process+0x2b/0x30
[<ffffffff8b1358d4>] kernel_init+0x54/0x110
[<ffffffff8b2001ca>] ret_from_fork+0x3a/0x50
and
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 21 PID: 1926 Comm: mkdir Not tainted
Call Trace:
[<ffffffff985202af>] dump_stack+0x67/0x98
[<ffffffff97d2b616>] register_lock_class+0x1e6/0x4e0
[<ffffffff97d2cfe9>] __lock_acquire+0xb9/0x1710
[<ffffffff97d2ef3a>] lock_acquire+0xba/0x200
[<ffffffff97e73c09>] unmap_page_range+0x89/0xaa0
[<ffffffff97e746af>] unmap_single_vma+0x8f/0x100
[<ffffffff97e74a1b>] unmap_vmas+0x4b/0x90
[<ffffffff97e7f833>] exit_mmap+0xa3/0x1c0
[<ffffffff97cc1b23>] mmput+0x73/0x120
[<ffffffff97ccbacd>] do_exit+0x2bd/0xd60
[<ffffffff97ccc5b7>] SyS_exit+0x17/0x20
[<ffffffff97c01f1d>] do_syscall_64+0x6d/0x1a0
[<ffffffff9860005a>] entry_SYSCALL_64_after_hwframe+0x26/0x9b
I think it would just be better to generalize vma allocation to initialize
certain fields and init both spf fields properly for
CONFIG_SPECULATIVE_PAGE_FAULT. It's obviously too delicate as is.
^ permalink raw reply
* Re: [PATCH v2] powerpc, pkey: make protection key 0 less special
From: Thiago Jung Bauermann @ 2018-04-04 21:41 UTC (permalink / raw)
To: Ram Pai
Cc: mpe, mingo, akpm, fweimer, shuah, msuchanek, linux-kernel, mhocko,
dave.hansen, paulus, aneesh.kumar, tglx, linuxppc-dev
In-Reply-To: <1522112702-27853-1-git-send-email-linuxram@us.ibm.com>
Hello Ram,
Ram Pai <linuxram@us.ibm.com> writes:
> Applications need the ability to associate an address-range with some
> key and latter revert to its initial default key. Pkey-0 comes close to
> providing this function but falls short, because the current
> implementation disallows applications to explicitly associate pkey-0 to
> the address range.
>
> Lets make pkey-0 less special and treat it almost like any other key.
> Thus it can be explicitly associated with any address range, and can be
> freed. This gives the application more flexibility and power. The
> ability to free pkey-0 must be used responsibily, since pkey-0 is
> associated with almost all address-range by default.
>
> Even with this change pkey-0 continues to be slightly more special
> from the following point of view.
> (a) it is implicitly allocated.
> (b) it is the default key assigned to any address-range.
It's also special in more ways (and if intentional, these should be part
of the commit message as well):
(c) it's not possible to change permissions for key 0
This has two causes: this patch explicitly forbids it in
arch_set_user_pkey_access(), and also because even if it's allocated,
the bits for key 0 in AMOR and UAMOR aren't set.
(d) it can be freed, but can't be allocated again later.
This is because mm_pkey_alloc() only calls __arch_activate_pkey(ret)
if ret > 0.
It looks like (d) is a bug. Either mm_pkey_free() should fail with key
0, or mm_pkey_alloc() should work with it.
(c) could be a measure to prevent users from shooting themselves in
their feet. But if that is the case, then mm_pkey_free() should forbid
freeing key 0 too.
> Tested on powerpc.
>
> cc: Thomas Gleixner <tglx@linutronix.de>
> cc: Dave Hansen <dave.hansen@intel.com>
> cc: Michael Ellermen <mpe@ellerman.id.au>
> cc: Ingo Molnar <mingo@kernel.org>
> cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
> History:
> v2: mm_pkey_is_allocated() continued to treat pkey-0 special.
> fixed it.
>
> arch/powerpc/include/asm/pkeys.h | 20 ++++++++++++++++----
> arch/powerpc/mm/pkeys.c | 20 ++++++++++++--------
> 2 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 0409c80..b598fa9 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -101,10 +101,14 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
>
> static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
> {
> - /* A reserved key is never considered as 'explicitly allocated' */
> - return ((pkey < arch_max_pkey()) &&
> - !__mm_pkey_is_reserved(pkey) &&
> - __mm_pkey_is_allocated(mm, pkey));
> + if (pkey < 0 || pkey >= arch_max_pkey())
> + return false;
> +
> + /* Reserved keys are never allocated. */
> + if (__mm_pkey_is_reserved(pkey))
> + return false;
> +
> + return __mm_pkey_is_allocated(mm, pkey);
> }
>
> extern void __arch_activate_pkey(int pkey);
> @@ -200,6 +204,14 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> {
> if (static_branch_likely(&pkey_disabled))
> return -EINVAL;
> +
> + /*
> + * userspace is discouraged from changing permissions of
> + * pkey-0.
They're not discouraged. They're not allowed to. :-)
> + * powerpc hardware does not support it anyway.
It doesn't? I don't get that impression from reading the ISA, but
perhaps I'm missing something.
> + */
> + if (!pkey)
> + return init_val ? -EINVAL : 0;
> +
> return __arch_set_user_pkey_access(tsk, pkey, init_val);
> }
>
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index ba71c54..e7a9e34 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -119,19 +119,21 @@ int pkey_initialize(void)
> #else
> os_reserved = 0;
> #endif
> - /*
> - * Bits are in LE format. NOTE: 1, 0 are reserved.
> - * key 0 is the default key, which allows read/write/execute.
> - * key 1 is recommended not to be used. PowerISA(3.0) page 1015,
> - * programming note.
> - */
> + /* Bits are in LE format. */
> initial_allocation_mask = ~0x0;
>
> /* register mask is in BE format */
> pkey_amr_uamor_mask = ~0x0ul;
> pkey_iamr_mask = ~0x0ul;
>
> - for (i = 2; i < (pkeys_total - os_reserved); i++) {
> + for (i = 0; i < (pkeys_total - os_reserved); i++) {
> + /*
There's a space between the tabs here.
> + * key 1 is recommended not to be used.
> + * PowerISA(3.0) page 1015,
> + */
> + if (i == 1)
> + continue;
> +
> initial_allocation_mask &= ~(0x1 << i);
> pkey_amr_uamor_mask &= ~(0x3ul << pkeyshift(i));
> pkey_iamr_mask &= ~(0x1ul << pkeyshift(i));
> @@ -145,7 +147,9 @@ void pkey_mm_init(struct mm_struct *mm)
> {
> if (static_branch_likely(&pkey_disabled))
> return;
> - mm_pkey_allocation_map(mm) = initial_allocation_mask;
> +
> + /* allocate key-0 by default */
> + mm_pkey_allocation_map(mm) = initial_allocation_mask | 0x1;
> /* -1 means unallocated or invalid */
> mm->context.execute_only_pkey = -1;
> }
I think we should also set the AMOR and UAMOR bits for key 0. Otherwise,
key 0 will be in allocated-but-not-enabled state which is yet another
subtle way in which it will be special.
Also, pkey_access_permitted() has a special case for key 0. Should it?
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v2 01/19] powerpc/powermac: Mark variable x as unused
From: Mathieu Malaterre @ 2018-04-04 20:21 UTC (permalink / raw)
To: LEROY Christophe
Cc: LKML, linuxppc-dev, Paul Mackerras, Benjamin Herrenschmidt,
Michael Ellerman
In-Reply-To: <20180329180743.Horde.0e_zt3S4Pqcv7Z0dJwOgOg9@messagerie.si.c-s.fr>
On Thu, Mar 29, 2018 at 6:07 PM, LEROY Christophe
<christophe.leroy@c-s.fr> wrote:
> Mathieu Malaterre <malat@debian.org> a =C3=A9crit :
>
>
>> Since the value of x is never intended to be read, declare it with gcc
>> attribute as unused. Fix warning treated as error with W=3D1:
>>
>> arch/powerpc/platforms/powermac/bootx_init.c:471:21: error: variable =
=E2=80=98x=E2=80=99
>> set but not used [-Werror=3Dunused-but-set-variable]
>>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>> v2: move x variable within its local scope
>>
>> arch/powerpc/platforms/powermac/bootx_init.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/powermac/bootx_init.c
>> b/arch/powerpc/platforms/powermac/bootx_init.c
>> index c3c9bbb3573a..d44e8571c1ec 100644
>> --- a/arch/powerpc/platforms/powermac/bootx_init.c
>> +++ b/arch/powerpc/platforms/powermac/bootx_init.c
>> @@ -468,7 +468,7 @@ void __init bootx_init(unsigned long r3, unsigned lo=
ng
>> r4)
>> boot_infos_t *bi =3D (boot_infos_t *) r4;
>> unsigned long hdr;
>> unsigned long space;
>> - unsigned long ptr, x;
>> + unsigned long ptr;
>> char *model;
>> unsigned long offset =3D reloc_offset();
>>
>> @@ -562,6 +562,7 @@ void __init bootx_init(unsigned long r3, unsigned lo=
ng
>> r4)
>> * MMU switched OFF, so this should not be useful anymore.
>> */
>> if (bi->version < 4) {
>> + unsigned long x __maybe_unused;
>> bootx_printf("Touching pages...\n");
>
>
> Stylewise, there should be an empty line after your declaration.
See v3
> But I believe you should remove that ugly loop and replace it by a call t=
o
> fault_in_pages_readable()
I have no idea on how to test the the new code, and it did not look
straightforward.
> Christophe
>>
>>
>> /*
>> --
>> 2.11.0
>
>
>
^ permalink raw reply
* Re: [PATCH 15/19] powerpc: Add missing prototype
From: Mathieu Malaterre @ 2018-04-04 20:18 UTC (permalink / raw)
To: LEROY Christophe; +Cc: linuxppc-dev, Paul Mackerras, LKML, Michael Ellerman
In-Reply-To: <20180329180246.Horde.AZE7BIRE2MuhcbAJb5Gg0A3@messagerie.si.c-s.fr>
On Thu, Mar 29, 2018 at 6:02 PM, LEROY Christophe
<christophe.leroy@c-s.fr> wrote:
> Mathieu Malaterre <malat@debian.org> a =C3=A9crit :
>
>
>> On Fri, Mar 23, 2018 at 1:20 PM, christophe leroy
>> <christophe.leroy@c-s.fr> wrote:
>>>
>>>
>>>
>>> Le 22/03/2018 =C3=A0 21:20, Mathieu Malaterre a =C3=A9crit :
>>>>
>>>>
>>>> Add one missing prototype for function rh_dump_blk. Fix warning treate=
d
>>>> as
>>>> error in W=3D1:
>>>>
>>>> arch/powerpc/lib/rheap.c:740:6: error: no previous prototype for
>>>> =E2=80=98rh_dump_blk=E2=80=99 [-Werror=3Dmissing-prototypes]
>>>>
>>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>>> ---
>>>> arch/powerpc/include/asm/rheap.h | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/rheap.h
>>>> b/arch/powerpc/include/asm/rheap.h
>>>> index 172381769cfc..e75d96de19a0 100644
>>>> --- a/arch/powerpc/include/asm/rheap.h
>>>> +++ b/arch/powerpc/include/asm/rheap.h
>>>> @@ -83,6 +83,9 @@ extern int rh_get_stats(rh_info_t * info, int what,
>>>> int
>>>> max_stats,
>>>> /* Simple dump of remote heap info */
>>>> extern void rh_dump(rh_info_t * info);
>>>> +/* Simple dump of remote info block */
>>>> +extern void rh_dump_blk(rh_info_t *info, rh_block_t *blk);
>>>> +
>>>
>>>
>>>
>>> Only used in one place, should be static
>>
>>
>> Well here is what I see over here:
>>
>> $ git grep rh_dump_blk
>> ...
>> arch/powerpc/lib/rheap.c:EXPORT_SYMBOL_GPL(rh_dump_blk);
>
>
> If it was really used by anybody in a module, it would already be in a .h=
so
> I think we should simply delete the function
>
Sent a v3 without the extern keyword. I did not feel bold enough to
remove completely a function. Could one of the maintainers confirm
removal of the function ?
> Christophe
>
>
>>
>>
>>
>>> Christophe
>>>
>>>> /* Set owner of taken block */
>>>> extern int rh_set_owner(rh_info_t * info, unsigned long start, const
>>>> char *owner);
>>>>
>>>
>>>
>>> ---
>>> L'absence de virus dans ce courrier =C3=A9lectronique a =C3=A9t=C3=A9 v=
=C3=A9rifi=C3=A9e par le
>>> logiciel antivirus Avast.
>>> https://www.avast.com/antivirus
>>>
>
>
^ permalink raw reply
* [PATCH v3 09/19] powerpc/chrp/pci: Make some functions static
From: Mathieu Malaterre @ 2018-04-04 20:15 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180322202007.23088-10-malat@debian.org>
These functions can all be static, make it so. Fix warnings treated as
errors with W=1:
arch/powerpc/platforms/chrp/pci.c:34:5: error: no previous prototype for ‘gg2_read_config’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/pci.c:61:5: error: no previous prototype for ‘gg2_write_config’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/pci.c:97:5: error: no previous prototype for ‘rtas_read_config’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/pci.c:112:5: error: no previous prototype for ‘rtas_write_config’ [-Werror=missing-prototypes]
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: style: fix alignement issue (checkpatch --strict)
v2: no change
arch/powerpc/platforms/chrp/pci.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/chrp/pci.c b/arch/powerpc/platforms/chrp/pci.c
index 0f512d35f7c5..5ddb57b82921 100644
--- a/arch/powerpc/platforms/chrp/pci.c
+++ b/arch/powerpc/platforms/chrp/pci.c
@@ -31,7 +31,7 @@ void __iomem *gg2_pci_config_base;
* limit the bus number to 3 bits
*/
-int gg2_read_config(struct pci_bus *bus, unsigned int devfn, int off,
+static int gg2_read_config(struct pci_bus *bus, unsigned int devfn, int off,
int len, u32 *val)
{
volatile void __iomem *cfg_data;
@@ -58,7 +58,7 @@ int gg2_read_config(struct pci_bus *bus, unsigned int devfn, int off,
return PCIBIOS_SUCCESSFUL;
}
-int gg2_write_config(struct pci_bus *bus, unsigned int devfn, int off,
+static int gg2_write_config(struct pci_bus *bus, unsigned int devfn, int off,
int len, u32 val)
{
volatile void __iomem *cfg_data;
@@ -94,8 +94,8 @@ static struct pci_ops gg2_pci_ops =
/*
* Access functions for PCI config space using RTAS calls.
*/
-int rtas_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
- int len, u32 *val)
+static int rtas_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
+ int len, u32 *val)
{
struct pci_controller *hose = pci_bus_to_host(bus);
unsigned long addr = (offset & 0xff) | ((devfn & 0xff) << 8)
@@ -109,8 +109,8 @@ int rtas_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
return rval? PCIBIOS_DEVICE_NOT_FOUND: PCIBIOS_SUCCESSFUL;
}
-int rtas_write_config(struct pci_bus *bus, unsigned int devfn, int offset,
- int len, u32 val)
+static int rtas_write_config(struct pci_bus *bus, unsigned int devfn, int offset,
+ int len, u32 val)
{
struct pci_controller *hose = pci_bus_to_host(bus);
unsigned long addr = (offset & 0xff) | ((devfn & 0xff) << 8)
--
2.11.0
^ permalink raw reply related
* [PATCH v3 11/19] powerpc/powermac: Move pmac_pfunc_base_install prototype to header file
From: Mathieu Malaterre @ 2018-04-04 20:13 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180322202007.23088-12-malat@debian.org>
The pmac_pfunc_base_install prototype was declared in powermac/smp.c since
function was used there, move it to pmac_pfunc.h header to be visible in
pfunc_base.c. Fix a warning treated as error with W=1:
arch/powerpc/platforms/powermac/pfunc_base.c:330:12: error: no previous prototype for ‘pmac_pfunc_base_install’ [-Werror=missing-prototypes]
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: remove extern keyword
v2: no change
arch/powerpc/include/asm/pmac_pfunc.h | 1 +
arch/powerpc/platforms/powermac/smp.c | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/pmac_pfunc.h b/arch/powerpc/include/asm/pmac_pfunc.h
index 73bd8f28f2a8..cee4e9f5b8cf 100644
--- a/arch/powerpc/include/asm/pmac_pfunc.h
+++ b/arch/powerpc/include/asm/pmac_pfunc.h
@@ -245,6 +245,7 @@ extern void pmf_put_function(struct pmf_function *func);
extern int pmf_call_one(struct pmf_function *func, struct pmf_args *args);
+int pmac_pfunc_base_install(void);
/* Suspend/resume code called by via-pmu directly for now */
extern void pmac_pfunc_base_suspend(void);
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index 95275e0e2efa..447da6db450a 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -65,7 +65,6 @@
#endif
extern void __secondary_start_pmac_0(void);
-extern int pmac_pfunc_base_install(void);
static void (*pmac_tb_freeze)(int freeze);
static u64 timebase;
--
2.11.0
^ permalink raw reply related
* [PATCH v3 12/19] powerpc/powermac: Add missing prototype for note_bootable_part()
From: Mathieu Malaterre @ 2018-04-04 20:13 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180322202007.23088-13-malat@debian.org>
Add a missing prototype for function `note_bootable_part` to silence a
warning treated as error with W=1:
arch/powerpc/platforms/powermac/setup.c:361:12: error: no previous prototype for ‘note_bootable_part’ [-Werror=missing-prototypes]
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: remove extern keyword
v2: no change
arch/powerpc/platforms/powermac/setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index ab668cb72263..8b2eab1340f4 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -352,6 +352,7 @@ static int pmac_late_init(void)
}
machine_late_initcall(powermac, pmac_late_init);
+void note_bootable_part(dev_t dev, int part, int goodness);
/*
* This is __ref because we check for "initializing" before
* touching any of the __init sensitive things and "initializing"
--
2.11.0
^ permalink raw reply related
* [PATCH v3 13/19] powerpc/52xx: Add missing functions prototypes
From: Mathieu Malaterre @ 2018-04-04 20:12 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180322202007.23088-14-malat@debian.org>
The function prototypes were declared within a `#ifdef CONFIG_PPC_LITE5200`
block which would prevent them from being visible when compiling
`mpc52xx_pm.c`. Move the prototypes outside of the `#ifdef` block to fix
the following warnings treated as errors with W=1:
arch/powerpc/platforms/52xx/mpc52xx_pm.c:58:5: error: no previous prototype for ‘mpc52xx_pm_prepare’ [-Werror=missing-prototypes]
arch/powerpc/platforms/52xx/mpc52xx_pm.c:113:5: error: no previous prototype for ‘mpc52xx_pm_enter’ [-Werror=missing-prototypes]
arch/powerpc/platforms/52xx/mpc52xx_pm.c:181:6: error: no previous prototype for ‘mpc52xx_pm_finish’ [-Werror=missing-prototypes]
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: remove extern keyword
v2: no change
arch/powerpc/include/asm/mpc52xx.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/mpc52xx.h
index e94cede14522..ce1e0aabaa64 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -350,14 +350,14 @@ extern struct mpc52xx_suspend mpc52xx_suspend;
extern int __init mpc52xx_pm_init(void);
extern int mpc52xx_set_wakeup_gpio(u8 pin, u8 level);
-#ifdef CONFIG_PPC_LITE5200
-extern int __init lite5200_pm_init(void);
-
/* lite5200 calls mpc5200 suspend functions, so here they are */
extern int mpc52xx_pm_prepare(void);
extern int mpc52xx_pm_enter(suspend_state_t);
extern void mpc52xx_pm_finish(void);
extern char saved_sram[0x4000]; /* reuse buffer from mpc52xx suspend */
+
+#ifdef CONFIG_PPC_LITE5200
+int __init lite5200_pm_init(void);
#endif
#endif /* CONFIG_PM */
--
2.11.0
^ permalink raw reply related
* [PATCH v3 15/19] powerpc: Add missing prototype
From: Mathieu Malaterre @ 2018-04-04 20:11 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180322202007.23088-16-malat@debian.org>
Add one missing prototype for function rh_dump_blk. Fix warning treated as
error in W=1:
arch/powerpc/lib/rheap.c:740:6: error: no previous prototype for ‘rh_dump_blk’ [-Werror=missing-prototypes]
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: remove extern keyword
v2: no change
arch/powerpc/include/asm/rheap.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/include/asm/rheap.h b/arch/powerpc/include/asm/rheap.h
index 172381769cfc..8e83703d6736 100644
--- a/arch/powerpc/include/asm/rheap.h
+++ b/arch/powerpc/include/asm/rheap.h
@@ -83,6 +83,9 @@ extern int rh_get_stats(rh_info_t * info, int what, int max_stats,
/* Simple dump of remote heap info */
extern void rh_dump(rh_info_t * info);
+/* Simple dump of remote info block */
+void rh_dump_blk(rh_info_t *info, rh_block_t *blk);
+
/* Set owner of taken block */
extern int rh_set_owner(rh_info_t * info, unsigned long start, const char *owner);
--
2.11.0
^ permalink raw reply related
* [PATCH v3 19/19] powerpc/tau: Synchronize function prototypes and body
From: Mathieu Malaterre @ 2018-04-04 20:10 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180322202007.23088-20-malat@debian.org>
Some function prototypes and body for Thermal Assist Units were not in
sync. Update the function definition to match the existing function
declaration found in `setup-common.c`, changing an `int` return type to a
`u32` return type. Move the prototypes to a header file. Fix the following
warnings, treated as error with W=1:
arch/powerpc/kernel/tau_6xx.c:257:5: error: no previous prototype for ‘cpu_temp_both’ [-Werror=missing-prototypes]
arch/powerpc/kernel/tau_6xx.c:262:5: error: no previous prototype for ‘cpu_temp’ [-Werror=missing-prototypes]
arch/powerpc/kernel/tau_6xx.c:267:5: error: no previous prototype for ‘tau_interrupts’ [-Werror=missing-prototypes]
Compile tested with CONFIG_TAU_INT.
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: remove extern keyword
v2: no change
arch/powerpc/kernel/irq.c | 2 +-
arch/powerpc/kernel/setup-common.c | 6 ------
arch/powerpc/kernel/setup.h | 6 ++++++
arch/powerpc/kernel/tau_6xx.c | 7 +++++--
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 061aa0f47bb1..bbf7ec582d60 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -89,7 +89,7 @@ atomic_t ppc_n_lost_interrupts;
#ifdef CONFIG_TAU_INT
extern int tau_initialized;
-extern int tau_interrupts(int);
+u32 tau_interrupts(unsigned long cpu);
#endif
#endif /* CONFIG_PPC32 */
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index d73ec518ef80..bd3675c3984b 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -192,12 +192,6 @@ void machine_halt(void)
machine_hang();
}
-
-#ifdef CONFIG_TAU
-extern u32 cpu_temp(unsigned long cpu);
-extern u32 cpu_temp_both(unsigned long cpu);
-#endif /* CONFIG_TAU */
-
#ifdef CONFIG_SMP
DEFINE_PER_CPU(unsigned int, cpu_pvr);
#endif
diff --git a/arch/powerpc/kernel/setup.h b/arch/powerpc/kernel/setup.h
index d768023a04bd..76845c925bf5 100644
--- a/arch/powerpc/kernel/setup.h
+++ b/arch/powerpc/kernel/setup.h
@@ -70,4 +70,10 @@ void kvm_cma_reserve(void);
static inline void kvm_cma_reserve(void) { };
#endif
+#ifdef CONFIG_TAU
+u32 cpu_temp(unsigned long cpu);
+u32 cpu_temp_both(unsigned long cpu);
+u32 tau_interrupts(unsigned long cpu);
+#endif /* CONFIG_TAU */
+
#endif /* __ARCH_POWERPC_KERNEL_SETUP_H */
diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 1fc6a89a978e..e2ab8a111b69 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -27,6 +27,9 @@
#include <asm/cache.h>
#include <asm/8xx_immap.h>
#include <asm/machdep.h>
+#include <asm/asm-prototypes.h>
+
+#include "setup.h"
static struct tau_temp
{
@@ -259,12 +262,12 @@ u32 cpu_temp_both(unsigned long cpu)
return ((tau[cpu].high << 16) | tau[cpu].low);
}
-int cpu_temp(unsigned long cpu)
+u32 cpu_temp(unsigned long cpu)
{
return ((tau[cpu].high + tau[cpu].low) / 2);
}
-int tau_interrupts(unsigned long cpu)
+u32 tau_interrupts(unsigned long cpu)
{
return (tau[cpu].interrupts);
}
--
2.11.0
^ permalink raw reply related
* [PATCH v3 05/19] powerpc/chrp/setup: Remove idu_size variable and make some functions static
From: Mathieu Malaterre @ 2018-04-04 20:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180328193553.1122-1-malat@debian.org>
Remove variable declaration idu_size and associated code since not used.
These functions can all be static, make it so. Fix warnings treated as
errors with W=1:
arch/powerpc/platforms/chrp/setup.c:97:6: error: no previous prototype for ‘chrp_show_cpuinfo’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/setup.c:302:13: error: no previous prototype for ‘chrp_setup_arch’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/setup.c:385:16: error: variable ‘idu_size’ set but not used [-Werror=unused-but-set-variable]
arch/powerpc/platforms/chrp/setup.c:526:13: error: no previous prototype for ‘chrp_init_IRQ’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/setup.c:559:1: error: no previous prototype for ‘chrp_init2’ [-Werror=missing-prototypes]
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: update subject line after v2 change
v2: Remove totally variable idu_size
arch/powerpc/platforms/chrp/setup.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/chrp/setup.c b/arch/powerpc/platforms/chrp/setup.c
index 481ed133e04b..d6d8ffc0271e 100644
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -94,7 +94,7 @@ static const char *chrp_names[] = {
"Total Impact Briq"
};
-void chrp_show_cpuinfo(struct seq_file *m)
+static void chrp_show_cpuinfo(struct seq_file *m)
{
int i, sdramen;
unsigned int t;
@@ -299,7 +299,7 @@ static __init void chrp_init(void)
of_node_put(node);
}
-void __init chrp_setup_arch(void)
+static void __init chrp_setup_arch(void)
{
struct device_node *root = of_find_node_by_path("/");
const char *machine = NULL;
@@ -382,7 +382,7 @@ static void __init chrp_find_openpic(void)
{
struct device_node *np, *root;
int len, i, j;
- int isu_size, idu_size;
+ int isu_size;
const unsigned int *iranges, *opprop = NULL;
int oplen = 0;
unsigned long opaddr;
@@ -427,11 +427,9 @@ static void __init chrp_find_openpic(void)
}
isu_size = 0;
- idu_size = 0;
if (len > 0 && iranges[1] != 0) {
printk(KERN_INFO "OpenPIC irqs %d..%d in IDU\n",
iranges[0], iranges[0] + iranges[1] - 1);
- idu_size = iranges[1];
}
if (len > 1)
isu_size = iranges[3];
@@ -523,7 +521,7 @@ static void __init chrp_find_8259(void)
}
}
-void __init chrp_init_IRQ(void)
+static void __init chrp_init_IRQ(void)
{
#if defined(CONFIG_VT) && defined(CONFIG_INPUT_ADBHID) && defined(CONFIG_XMON)
struct device_node *kbd;
@@ -555,7 +553,7 @@ void __init chrp_init_IRQ(void)
#endif
}
-void __init
+static void __init
chrp_init2(void)
{
#ifdef CONFIG_NVRAM
--
2.11.0
^ permalink raw reply related
* [PATCH v3 03/19] powerpc: Move `path` variable inside DEBUG_PROM
From: Mathieu Malaterre @ 2018-04-04 20:08 UTC (permalink / raw)
To: Michael Ellerman
Cc: Christophe Leroy, Mathieu Malaterre, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180328193307.978-1-malat@debian.org>
Add gcc attribute unused for two variables. Fix warnings treated as errors
with W=1:
arch/powerpc/kernel/prom_init.c:1388:8: error: variable ‘path’ set but not used [-Werror=unused-but-set-variable]
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
v3: really move path within ifdef DEBUG_PROM
v2: move path within ifdef DEBUG_PROM
arch/powerpc/kernel/prom_init.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index acf4b2e0530c..223b35acbbdd 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -603,7 +603,7 @@ static void __init early_cmdline_parse(void)
const char *opt;
char *p;
- int l = 0;
+ int l __maybe_unused = 0;
prom_cmd_line[0] = 0;
p = prom_cmd_line;
@@ -1385,7 +1385,10 @@ static void __init reserve_mem(u64 base, u64 size)
static void __init prom_init_mem(void)
{
phandle node;
- char *path, type[64];
+#ifdef DEBUG_PROM
+ char *path;
+#endif
+ char type[64];
unsigned int plen;
cell_t *p, *endp;
__be32 val;
@@ -1406,7 +1409,9 @@ static void __init prom_init_mem(void)
prom_debug("root_size_cells: %x\n", rsc);
prom_debug("scanning memory:\n");
+#ifdef DEBUG_PROM
path = prom_scratch;
+#endif
for (node = 0; prom_next_node(&node); ) {
type[0] = 0;
--
2.11.0
^ 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