Re: [PATCH 31/37] misc: Add host side pci driver for pci test function device

[Kishon Vijay Abraham I]

Hi,

On Tuesday 24 January 2017 09:32 PM, Christoph Hellwig wrote:
> On Thu, Jan 12, 2017 at 03:56:20PM +0530, Kishon Vijay Abraham I wrote:
>> Add PCI endpoint test driver that can verify base address
>> register, legacy interrupt/MSI interrupt and read/write/copy
>> buffers between host and device. The corresponding pci-epf-test
>> function driver should be used on the EP side.
> 
> Just curious:  what would you think of a text based (e.g. debugfs)
> interface to avoid the need for a userspace tool here?

I felt having a userspace tool gives the flexibility to add more tests
(iterations, sizes etc..) while the driver can just focus on performing simple
tests. Say we'd like to perform infinite read/write tests, it's better if the
userspace tool invokes read/write tests repeatedly instead of that being
implemented in the driver.
> 
>> +static const struct pci_device_id pci_endpoint_test_tbl[] = {
>> +{ PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_ANY_ID) },
>> +{ }
>> +};
>> +MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl);
> 
> Also this looks really odd, and dangerous.  Probing for any
> TI device will bind to all kinds of legit devices.  It would
> be good if you could squeeze out a single id for this device

There is actually an id for the device, but I think we'll need an id for every
function right?

Having said that the id for the device is better than PCI_ANY_ID. Will fix it
in my next revision.

Thanks
Kishon

Re: [PATCH] powerpc/mm: Fix RECLAIM_DISTANCE

[Gavin Shan]

On Wed, Jan 25, 2017 at 09:27:44AM +0530, Balbir Singh wrote:
>On Tue, Jan 24, 2017 at 10:32:28AM +1100, Gavin Shan wrote:
>> When @node_reclaim_mode ("/proc/sys/vm/zone_reclaim_mode") is enabled,
>> the nodes in the specified distance (< RECLAIM_DISTANCE) to the preferred
>> one will be checked for page direct reclaim in the fast path, as below
>> function call chain indicates. Currently, RECLAIM_DISTANCE is set to 10,
>> equal to LOCAL_DISTANCE. It means no nodes, including the preferred one,
>> don't match the conditions. So no nodes are checked for direct reclaim
>> in the fast path.
>> 
>>__alloc_pages_nodemask
>>get_page_from_freelist
>>zone_allows_reclaim
>> 
>> This fixes it by setting RECLAIM_DISTANCE to 30. With it, the preferred
>> node and its directly adjacent nodes will be checked for page direct
>> reclaim. The comments explaining RECLAIM_DISTANCE is out of date. This
>> updates and makes it correct.
>> 
>> Signed-off-by: Gavin Shan 
>> ---
>
>I spoke about this at length with Anton and the larger kernel team.
>We need performance data before we can commit to the change. Do we
>benchmarks to show that the change does not introduce regression
>w.r.t runtime cost?
>

Thanks for review. I just found the problem when studying the code
last year. It sounds reasonable not to rely on the slow path for page
reclaim if the fast path can reclaim enough pages. From this point,
I believe the performance should be improved. In the meanwhile, the
page cache/buffer could be released, as part of the output of page
reclaim. It's going to affect fs's performance for sure. So do you
have recommended test examples to measure the improved performance
because of this?

Thanks,
Gavin

Re: [bug] stack protector panics on v4.10-rc1+

[Segher Boessenkool]

On Wed, Jan 25, 2017 at 09:24:53AM +0530, Balbir Singh wrote:
> > So my inclination is to revert the powerpc stack protector code for
> > 4.10, and we can try again for 4.11 or 12.
> 
> That makes sense. We then wait for the right gcc version? I guess we also
> push for per-task gaurd value as opposed to a global one?

Global value will work (with GCC 7) as-is.  Per-task will require some
kernel work, but yeah you want it, that is why the options for that exist ;-)

You don't have to revert the current stack protector code; just condition it
on the relevant GCC flags (you will need to do that later anyway).


Segher

Re: [PATCH] powerpc/mm: Fix RECLAIM_DISTANCE

[Balbir Singh]

On Tue, Jan 24, 2017 at 10:32:28AM +1100, Gavin Shan wrote:
> When @node_reclaim_mode ("/proc/sys/vm/zone_reclaim_mode") is enabled,
> the nodes in the specified distance (< RECLAIM_DISTANCE) to the preferred
> one will be checked for page direct reclaim in the fast path, as below
> function call chain indicates. Currently, RECLAIM_DISTANCE is set to 10,
> equal to LOCAL_DISTANCE. It means no nodes, including the preferred one,
> don't match the conditions. So no nodes are checked for direct reclaim
> in the fast path.
> 
>__alloc_pages_nodemask
>get_page_from_freelist
>zone_allows_reclaim
> 
> This fixes it by setting RECLAIM_DISTANCE to 30. With it, the preferred
> node and its directly adjacent nodes will be checked for page direct
> reclaim. The comments explaining RECLAIM_DISTANCE is out of date. This
> updates and makes it correct.
> 
> Signed-off-by: Gavin Shan 
> ---

I spoke about this at length with Anton and the larger kernel team.
We need performance data before we can commit to the change. Do we
benchmarks to show that the change does not introduce regression
w.r.t runtime cost?

Balbir Singh.

Re: [bug] stack protector panics on v4.10-rc1+

[Balbir Singh]

On Tue, Jan 24, 2017 at 03:09:40PM +1100, Michael Ellerman wrote:
> Michael Ellerman  writes:
> 
> > # zgrep STACKPROTECTOR /proc/config.gz 
> > CONFIG_HAVE_CC_STACKPROTECTOR=y
> > CONFIG_CC_STACKPROTECTOR=y
> > CONFIG_CC_STACKPROTECTOR_REGULAR=y
> >
> > I guess I'm just lucky?
> 
> No, I'm just using a gcc built without libc as Segher pointed out:
> 
>   https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg113181.html
> 
>   Right.  Tony's compilers are built using a (modified version of) buildall,
>   and buildall goes out of its way to build without libc whatsoever, even
>   if the configuration (powerpc64-linux, for example) expects one.
>   
>   Which leads to TARGET_LIBC_PROVIDES_SSP being undefined (it would normally
>   be true for glibc >= 2.4), and that is all.  Mystery solved.  Thanks!
> 
> 
> So my inclination is to revert the powerpc stack protector code for
> 4.10, and we can try again for 4.11 or 12.
>

That makes sense. We then wait for the right gcc version? I guess we also
push for per-task gaurd value as opposed to a global one?

Balbir Singh 

Re: [PATCH] powerpc: opal-msglog: Report size of memcons log

[Michael Ellerman]

Michael Ellerman  writes:

> Joel Stanley  writes:
>
>> The OPAL memory console is reported to be size zero, as we do not
>> initialise the struct attr with any size information due to the size
>> being variable. This leads users to think that the console is empty.
>
> Hmm OK. That is a general property of /proc and /sys files that are
> dynamically generated, so users probably need to get used to it :)
>
>> Instead report the maximum size.
>
> But OK. That sounds sane enough. My only worry is that it might confuse
> some tools, ie. the file claims to be x bytes but is actually smaller.
> But I guess that can actually happen anyway with any file.
>
> So I'll merge this and stop blabbing :)

Hmm, but then I get:

  $ ls -la msglog 
  -r--r--r-- 1 root root 4503599627370496 Jan 25 13:09 msglog

I know firmware likes to spit out lots of messages, but 4PB seems a bit
large :P

I fixed it with the patch below which I'll fold in, resulting in:

  $ ls -la msglog 
  -r--r--r-- 1 root root 1048576 Jan 25 13:30 msglog


Which seems more likely.

cheers

diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c 
b/arch/powerpc/platforms/powernv/opal-msglog.c
index 8486b2ceb510..7a9cde0cfbd1 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -124,8 +124,8 @@ void __init opal_msglog_init(void)
}
 
/* Report maximum size */
-   opal_msglog_attr.size =  be64_to_cpu(mc->ibuf_size) +
-   be64_to_cpu(mc->obuf_size);
+   opal_msglog_attr.size =  be32_to_cpu(mc->ibuf_size) +
+   be32_to_cpu(mc->obuf_size);
 
opal_memcons = mc;
 }

Re: [PATCH] powerpc/powernv: Validate memcons descriptor and output buffer

[Gavin Shan]

On Tue, Jan 24, 2017 at 09:21:22PM +1100, Michael Ellerman wrote:
>Gavin Shan  writes:
>
>> Currently, it's assumed that memcons and its output buffer are included
>> in the linear mapping. It's not true when "mem=384M" is included in
>> bootargs. The system runs into kernel crash eventually.
>>
>># od -x /proc/device-tree/ibm,opal/ibm,opal-memcons
>>000   0b30 0010
>>010
>>
>> This validates memcons descriptor and its output buffer to ensure they
>> are valid in linear mapping. Otherwise, the interface won't be populated
>> to avoid kernel crash during system boot.
>>
>> Cc: sta...@vger.kernel.org #3.15+
>> Fixes: bfc36894a48 ("powerpc/powernv: Add OPAL message log interface")
>> Signed-off-by: Gavin Shan 
>
>Hmm.
>
>Arguably the memcons shouldn't be in the linear map at all.
>

Thanks for review. I agree it shouldn't be in linear map as it is for logs
produced by skiboot, not kernel. Currently, the regions (including the
reserved ones) in memblock are covered by linear mapping. As I can see,
there is no flag to differentiate the cases - the region is reserved by
skiboot or kernel itself. With the flag, we can skip mapping the regions
that were reserved by skiboot to linear mapping area in htab_initialize()
or radix_init_pgtable(). I will see if I can introduce on without too much
efforts.

As I can see, the only benefit is to avoid randomly writing to the area
and data corruption, if the region is invisible from linear mapping.

>AFAICS the kernel only ever reads from it, so really it should be
>explicitly mapped, and mapped read only.
>
>That would also fix this problem :D
>

Yes, I think your proposed solution is better. This patch just adds more
checks to avoid the problem, meaning the logs is inaccessible if the buffer
is out of range. With your suggestion, it's still accessible.

I believe the log buffer should be mapped to VMALLOC area. In order to
setup mapping, we need page frame (struct page). However, it's possible
the page frame isn't existing (e.g. with mem= in bootargs).

It seems the best option would be mapping it into IOREMAP area with
ioremap_prot(). No page frame is needed and allow to set the region
to READONLY. Michael, what's your thoughts about this? :-)

Thanks,
Gavin

Re: [PATCH] cxl: drop unused header asm/pnv-pci.h

[Andrew Donnellan]

On 24/01/17 20:57, Michael Ellerman wrote:

Greg Kurz  writes:


The kernel API does not use anything from this header file.


That's actually hard to prove in general, because pnv-pci.h includes
linux/pci.h, linux/pci_hotplug.h, linux/irq.h, misc/cxl-base.h and
asm/opal-api.h. At least four of which aren't included directly by
api.c.

But I'll take you word for it :)


I did compile test it before giving my Reviewed-by.

--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

Re: [PATCH] ibmveth: Add a proper check for the availability of the checksum features

[Thomas Falcon]

On 01/24/2017 12:28 AM, Thomas Huth wrote:
> When using the ibmveth driver in a KVM/QEMU based VM, it currently
> always prints out a scary error message like this when it is started:
>
>  ibmveth 7103 (unregistered net_device): unable to change
>  checksum offload settings. 1 rc=-2 ret_attr=7103
>
> This happens because the driver always tries to enable the checksum
> offloading without checking for the availability of this feature first.
> QEMU does not support checksum offloading for the spapr-vlan device,
> thus we always get the error message here.
> According to the LoPAPR specification, the "ibm,illan-options" property
> of the corresponding device tree node should be checked first to see
> whether the H_ILLAN_ATTRIUBTES hypercall and thus the checksum offloading
> feature is available. Thus let's do this in the ibmveth driver, too, so
> that the error message is really only limited to cases where something
> goes wrong, and does not occur if the feature is just missing.

Thanks a lot for this patch, Thomas.  Was going to give an Ack, but its already 
been applied :)

>
> Signed-off-by: Thomas Huth 
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
> b/drivers/net/ethernet/ibm/ibmveth.c
> index a831f94..309f5c6 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -1601,8 +1601,11 @@ static int ibmveth_probe(struct vio_dev *dev, const 
> struct vio_device_id *id)
>   netdev->netdev_ops = _netdev_ops;
>   netdev->ethtool_ops = _ethtool_ops;
>   SET_NETDEV_DEV(netdev, >dev);
> - netdev->hw_features = NETIF_F_SG | NETIF_F_RXCSUM |
> - NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> + netdev->hw_features = NETIF_F_SG;
> + if (vio_get_attribute(dev, "ibm,illan-options", NULL) != NULL) {
> + netdev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +NETIF_F_RXCSUM;
> + }
>
>   netdev->features |= netdev->hw_features;
>

[PATCH] mtd/ifc: Fix location of eccstat registers for IFC V1.0

[mark.marshall]

From: Mark Marshall 

The commit 7a654172161c ("mtd/ifc: Add support for IFC controller
version 2.0") added support for version 2.0 of the IFC controller.
The version 2.0 controller has the ECC status registers at a different
location to the previous versions.  This broke the code for IFC all
versions less than 2.0.

Correct the fsl_ifc_nand structure so that the ECC status can be read
from the correct location for both version 1.0 and 2.0 of the controller.

Cc: sta...@vger.kernel.org
Signed-off-by: Mark Marshall 
---
 drivers/mtd/nand/fsl_ifc_nand.c | 5 -
 include/linux/fsl_ifc.h | 8 ++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 0a177b1..123c0f8 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -260,7 +260,10 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
int sector_end = sector + chip->ecc.steps - 1;
 
for (i = sector / 4; i <= sector_end / 4; i++)
-   eccstat[i] = ifc_in32(>ifc_nand.nand_eccstat[i]);
+   eccstat[i] = ifc_in32(
+   (ctrl->version >= FSL_IFC_VERSION_2_0_0) ?
+   >ifc_nand.v2_nand_eccstat[i] :
+   >ifc_nand.v1_nand_eccstat[i]);
 
for (i = sector; i <= sector_end; i++) {
errors = check_read_ecc(mtd, ctrl, eccstat, i);
diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
index 3f9778c..c332f0a 100644
--- a/include/linux/fsl_ifc.h
+++ b/include/linux/fsl_ifc.h
@@ -733,8 +733,12 @@ struct fsl_ifc_nand {
__be32 nand_erattr1;
u32 res19[0x10];
__be32 nand_fsr;
-   u32 res20[0x3];
-   __be32 nand_eccstat[6];
+   u32 res20;
+   /* The V1 nand_eccstat is actually 4 words that overlaps the
+* V2 nand_eccstat.
+*/
+   __be32 v1_nand_eccstat[2];
+   __be32 v2_nand_eccstat[6];
u32 res21[0x1c];
__be32 nanndcr;
u32 res22[0x2];
-- 
2.7.4

Re: [PATCH] ibmveth: Add a proper check for the availability of the checksum features

[David Miller]

From: Thomas Huth 
Date: Tue, 24 Jan 2017 07:28:41 +0100

> When using the ibmveth driver in a KVM/QEMU based VM, it currently
> always prints out a scary error message like this when it is started:
> 
>  ibmveth 7103 (unregistered net_device): unable to change
>  checksum offload settings. 1 rc=-2 ret_attr=7103
> 
> This happens because the driver always tries to enable the checksum
> offloading without checking for the availability of this feature first.
> QEMU does not support checksum offloading for the spapr-vlan device,
> thus we always get the error message here.
> According to the LoPAPR specification, the "ibm,illan-options" property
> of the corresponding device tree node should be checked first to see
> whether the H_ILLAN_ATTRIUBTES hypercall and thus the checksum offloading
> feature is available. Thus let's do this in the ibmveth driver, too, so
> that the error message is really only limited to cases where something
> goes wrong, and does not occur if the feature is just missing.
> 
> Signed-off-by: Thomas Huth 

Applied.

Re: [PATCH V9 0/8] powerpc/devtree: Add support for 2 new DRC properties

[Michael Bringmann]

Ping

On 12/07/2016 01:13 AM, Michael Bringmann wrote:
> Several properties in the DRC device tree format are replaced by
> more compact representations to allow, for example, for the encoding
> of vast amounts of memory, and or reduced duplication of information
> in related data structures.
> 
> "ibm,drc-info": This property, when present, replaces the following
> four properties: "ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
> and "ibm,drc-power-domains".  This property is defined for all
> dynamically reconfigurable platform nodes.  The "ibm,drc-info" elements
> are intended to provide a more compact representation, and reduce some
> search overhead.
> 
> "ibm,dynamic-memory-v2": This property replaces the "ibm,dynamic-memory"
> node representation within the "ibm,dynamic-reconfiguration-memory"
> property provided by the BMC.  This element format is intended to provide
> a more compact representation of memory, especially, for systems with
> massive amounts of RAM.  To simplify portability, this property is
> converted to the "ibm,dynamic-memory" property during system boot.
> 
> "ibm,architecture.vec": Bidirectional communication mechanism between
> the host system and the front end processor indicating what features
> the host system supports and what features the front end processor will
> actually provide.  In this case, we are indicating that the host system
> can support the new device tree structures "ibm,drc-info" and
> "ibm,dynamic-memory-v2".
> 
> Signed-off-by: Michael Bringmann 
> 
> Michael Bringmann (8):
>   powerpc/firmware: Add definitions for new firmware features.
>   powerpc/memory: Parse new memory property to register blocks.
>   powerpc/memory: Parse new memory property to initialize structures.
>   pseries/hotplug init: Convert new DRC memory property for hotplug runtime
>   pseries/drc-info: Search new DRC properties for CPU indexes
>   hotplug/drc-info: Add code to search new devtree properties
>   powerpc: Check arch.vec earlier during boot for memory features
>   powerpc: Enable support for new DRC devtree properties
> ---
> Changes in V9:
>   -- Various code cleanup measures
> 
>  arch/powerpc/include/asm/firmware.h |5 -
>  arch/powerpc/include/asm/prom.h |   38 
>  arch/powerpc/kernel/prom.c  |  103 ++--
>  arch/powerpc/kernel/prom_init.c |3 
>  arch/powerpc/mm/numa.c  |  168 ---
>  arch/powerpc/platforms/pseries/firmware.c   |2 
>  arch/powerpc/platforms/pseries/pseries_energy.c |  202 
> ---
>  drivers/pci/hotplug/rpadlpar_core.c |   13 +
>  drivers/pci/hotplug/rpaphp.h|4 
>  drivers/pci/hotplug/rpaphp_core.c   |  108 +---
>  10 files changed, 543 insertions(+), 103 deletions(-)
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:   (512) 466-0650
m...@linux.vnet.ibm.com

Re: [PATCH v4 2/5] ia64: reuse append_elf_note() and final_note() functions

[Tony Luck]

On Tue, Jan 24, 2017 at 10:11 AM, Hari Bathini
 wrote:

> Hello IA64 folks,
>
> Could you please review this patch..?

It looks OK in principal.  My lab is in partial disarray at the
moment (just got back from a sabbatical) so I can't test
build and boot. Have you cross-compiled it (or gotten a success
build report from zero-day)?

If you have ... then add an Acked-by: Tony Luck 

-Tony

Re: [PATCH v4 2/5] ia64: reuse append_elf_note() and final_note() functions

[Hari Bathini]

On Tuesday 17 January 2017 10:36 PM, Hari Bathini wrote:



On Friday 06 January 2017 07:33 AM, Dave Young wrote:

On 01/05/17 at 11:01pm, Hari Bathini wrote:

Get rid of multiple definitions of append_elf_note() & final_note()
functions. Reuse these functions compiled under CONFIG_CRASH_CORE
Also, define Elf_Word and use it instead of generic u32 or the more
specific Elf64_Word.

Signed-off-by: Hari Bathini 
---

Changes from v3:
* Dropped hard-coded values and used DIV_ROUND_UP().

Changes from v2:
* Added a definition for Elf_Word.
* Used IA64 version of append_elf_note() and final_note() functions.


  arch/ia64/kernel/crash.c   |   22 --
  include/linux/crash_core.h |4 
  include/linux/elf.h|2 ++
  kernel/crash_core.c|   34 ++
  kernel/kexec_core.c|   28 
  5 files changed, 20 insertions(+), 70 deletions(-)

diff --git a/arch/ia64/kernel/crash.c b/arch/ia64/kernel/crash.c
index 2955f35..75859a0 100644
--- a/arch/ia64/kernel/crash.c
+++ b/arch/ia64/kernel/crash.c
@@ -27,28 +27,6 @@ static int kdump_freeze_monarch;
  static int kdump_on_init = 1;
  static int kdump_on_fatal_mca = 1;
  -static inline Elf64_Word
-*append_elf_note(Elf64_Word *buf, char *name, unsigned type, void 
*data,

-size_t data_len)
-{
-struct elf_note *note = (struct elf_note *)buf;
-note->n_namesz = strlen(name) + 1;
-note->n_descsz = data_len;
-note->n_type   = type;
-buf += (sizeof(*note) + 3)/4;
-memcpy(buf, name, note->n_namesz);
-buf += (note->n_namesz + 3)/4;
-memcpy(buf, data, data_len);
-buf += (data_len + 3)/4;
-return buf;
-}
-
-static void
-final_note(void *buf)
-{
-memset(buf, 0, sizeof(struct elf_note));
-}
-
  extern void ia64_dump_cpu_regs(void *);
static DEFINE_PER_CPU(struct elf_prstatus, elf_prstatus);
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 18d0f94..541a197 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -55,6 +55,10 @@ extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
  extern size_t vmcoreinfo_size;
  extern size_t vmcoreinfo_max_size;
  +Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int 
type,

+  void *data, size_t data_len);
+void final_note(Elf_Word *buf);
+
  int __init parse_crashkernel(char *cmdline, unsigned long long 
system_ram,
  unsigned long long *crash_size, unsigned long long 
*crash_base);
  int parse_crashkernel_high(char *cmdline, unsigned long long 
system_ram,

diff --git a/include/linux/elf.h b/include/linux/elf.h
index 20fa8d8..ba069e8 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -29,6 +29,7 @@ extern Elf32_Dyn _DYNAMIC [];
  #define elf_noteelf32_note
  #define elf_addr_tElf32_Off
  #define Elf_HalfElf32_Half
+#define Elf_WordElf32_Word
#else
  @@ -39,6 +40,7 @@ extern Elf64_Dyn _DYNAMIC [];
  #define elf_noteelf64_note
  #define elf_addr_tElf64_Off
  #define Elf_HalfElf64_Half
+#define Elf_WordElf64_Word
#endif
  diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 80b441d..362dace 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -291,32 +291,26 @@ int __init parse_crashkernel_low(char *cmdline,
  "crashkernel=", suffix_tbl[SUFFIX_LOW]);
  }
  -static u32 *append_elf_note(u32 *buf, char *name, unsigned int type,
-void *data, size_t data_len)
+Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int 
type,

+  void *data, size_t data_len)
  {
-struct elf_note note;
-
-note.n_namesz = strlen(name) + 1;
-note.n_descsz = data_len;
-note.n_type   = type;
-memcpy(buf, , sizeof(note));
-buf += (sizeof(note) + 3)/4;
-memcpy(buf, name, note.n_namesz);
-buf += (note.n_namesz + 3)/4;
-memcpy(buf, data, note.n_descsz);
-buf += (note.n_descsz + 3)/4;
+struct elf_note *note = (struct elf_note *)buf;
+
+note->n_namesz = strlen(name) + 1;
+note->n_descsz = data_len;
+note->n_type   = type;
+buf += DIV_ROUND_UP(sizeof(*note), sizeof(Elf_Word));
+memcpy(buf, name, note->n_namesz);
+buf += DIV_ROUND_UP(note->n_namesz, sizeof(Elf_Word));
+memcpy(buf, data, data_len);
+buf += DIV_ROUND_UP(data_len, sizeof(Elf_Word));
return buf;
  }
  -static void final_note(u32 *buf)
+void final_note(Elf_Word *buf)
  {
-struct elf_note note;
-
-note.n_namesz = 0;
-note.n_descsz = 0;
-note.n_type   = 0;
-memcpy(buf, , sizeof(note));
+memset(buf, 0, sizeof(struct elf_note));
  }
static void update_vmcoreinfo_note(void)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 2179a16..263d764 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -990,34 +990,6 @@ int crash_shrink_memory(unsigned long new_size)
  return ret;
  }
  -static u32 *append_elf_note(u32 *buf, char 

Re: [PATCH v4 2/5] ia64: reuse append_elf_note() and final_note() functions

[Hari Bathini]

On Friday 20 January 2017 11:17 AM, Michael Ellerman wrote:

Hari Bathini  writes:


Get rid of multiple definitions of append_elf_note() & final_note()
functions. Reuse these functions compiled under CONFIG_CRASH_CORE
Also, define Elf_Word and use it instead of generic u32 or the more
specific Elf64_Word.

Signed-off-by: Hari Bathini 
---

Changes from v3:
* Dropped hard-coded values and used DIV_ROUND_UP().

Changes from v2:
* Added a definition for Elf_Word.
* Used IA64 version of append_elf_note() and final_note() functions.


  arch/ia64/kernel/crash.c   |   22 --
  include/linux/crash_core.h |4 
  include/linux/elf.h|2 ++
  kernel/crash_core.c|   34 ++
  kernel/kexec_core.c|   28 
  5 files changed, 20 insertions(+), 70 deletions(-)

Do the powerpc patches later in the series actually depend on this one?
Or is this just an unrelated cleanup?

As it is I can't merge the series until we at least get an ack on this
from the ia64 folks.

If you can just split this out as a separate patch that would make it a
lot easier to get the rest merged.



Hi Michael,

append_elf_note() & final_note() functions were defined statically at 
three different places,
arch/powerpc/kernel/fadump.c being one of them. With my changes, I would 
need to add
a fourth static definition if I ignore this cleanup. So, I preferred to 
clean this up...


Let me ping IA64 folks one last time. Will do a respin without the 
cleanup if I don't get

any response from them by end of this week..

Thanks
Hari

Re: [PATCH 3/3] powerpc: bpf: implement in-register swap for 64-bit endian operations

['Naveen N. Rao']

On 2017/01/24 04:13PM, David Laight wrote:
> From: 'Naveen N. Rao'
> > Sent: 23 January 2017 19:22
> > On 2017/01/15 09:00AM, Benjamin Herrenschmidt wrote:
> > > On Fri, 2017-01-13 at 23:22 +0530, 'Naveen N. Rao' wrote:
> > > > > That rather depends on whether the processor has a store to load 
> > > > > forwarder
> > > > > that will satisfy the read from the store buffer.
> > > > > I don't know about ppc, but at least some x86 will do that.
> > > >
> > > > Interesting - good to know that.
> > > >
> > > > However, I don't think powerpc does that and in-register swap is likely
> > > > faster regardless. Note also that gcc prefers this form at higher
> > > > optimization levels.
> > >
> > > Of course powerpc has a load-store forwarder these days, however, I
> > > wouldn't be surprised if the in-register form was still faster on some
> > > implementations, but this needs to be tested.
> > 
> > Thanks for clarifying! To test this, I wrote a simple (perhaps naive)
> > test that just issues a whole lot of endian swaps and in _that_ test, it
> > does look like the load-store forwarder is doing pretty well.
> ...
> > This is all in a POWER8 vm. On POWER7, the in-register variant is around
> > 4 times faster than the ldbrx variant.
> ...
> 
> I wonder which is faster on the little 1GHz embedded ppc we use here.

Worth a test, for sure.
FWIW, this patch won't matter since eBPF JIT is for ppc64.

Thanks,
Naveen

[PATCH v4 2/2] modversions: treat symbol CRCs as 32 bit quantities

[Ard Biesheuvel]

The modversion symbol CRCs are emitted as ELF symbols, which allows us to
easily populate the kcrctab sections by relying on the linker to associate
each kcrctab slot with the correct value.

This has a couple of downsides:
- Given that the CRCs are treated as memory addresses, we waste 4 bytes
  for each CRC on 64 bit architectures,
- On architectures that support runtime relocation, a R__RELATIVE
  relocation entry is emitted for each CRC value, which identifies it as
  a quantity that requires fixing up based on the actual runtime load
  offset of the kernel. This results in corrupted CRCs unless we
  explicitly undo the fixup (and this is currently being handled in the
  core module code)
- Such runtime relocation entries take up 24 bytes of __init space each,
  resulting in a x8 overhead in [uncompressed] kernel size for CRCs.

Switching to explicit 32 bit values on 64 bit architectures fixes most
of these issues, given that 32 bit values are not treated as quantities
that require fixing up based on the actual runtime load offset. Note that
on some ELF64 architectures [such as PPC64], these 32-bit values are still
emitted as [absolute] runtime relocatable quantities, even if the value
resolves to a build time constant. Since relative relocations are always
resolved at build time, this patch enables MODULE_REL_CRCS on powerpc when
CONFIG_RELOCATABLE=y, which turns the core kernel's absolute CRC references
into relative references to where the actual CRC value is stored in .rodata

So redefine all CRC fields and variables as s32, and redefine the
__CRC_SYMBOL() macro for 64 bit builds to emit the CRC reference using
inline assembler (which is necessary since 64-bit C code cannot use
32-bit types to hold memory addresses, even if they are ultimately
resolved using values that do not exceed 0x). To avoid potential
problems with legacy 32-bit architectures using legacy toolchains, the
equivalent C definition of the kcrctab entry is retained for 32-bit
architectures.

Note that this mostly reverts commit d4703aefdbc8 ("module: handle ppc64
relocating kcrctabs when CONFIG_RELOCATABLE=y")

Signed-off-by: Ard Biesheuvel 
---
 arch/powerpc/Kconfig  |  1 +
 arch/powerpc/include/asm/module.h |  4 --
 arch/powerpc/kernel/module_64.c   |  8 
 include/asm-generic/export.h  | 11 ++---
 include/linux/export.h| 14 ++
 include/linux/module.h| 14 +++---
 kernel/module.c   | 45 ++--
 7 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..db8a1ef6bfaf 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -484,6 +484,7 @@ config RELOCATABLE
bool "Build a relocatable kernel"
depends on (PPC64 && !COMPILE_TEST) || (FLATMEM && (44x || FSL_BOOKE))
select NONSTATIC_KERNEL
+   select MODULE_REL_CRCS if MODVERSIONS
help
  This builds a kernel image that is capable of running at the
  location the kernel is loaded at. For ppc32, there is no any
diff --git a/arch/powerpc/include/asm/module.h 
b/arch/powerpc/include/asm/module.h
index cc12c61ef315..53885512b8d3 100644
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -90,9 +90,5 @@ static inline int module_finalize_ftrace(struct module *mod, 
const Elf_Shdr *sec
 }
 #endif
 
-#if defined(CONFIG_MODVERSIONS) && defined(CONFIG_PPC64)
-#define ARCH_RELOCATES_KCRCTAB
-#define reloc_start PHYSICAL_START
-#endif
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_MODULE_H */
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index bb1807184bad..0b0f89685b67 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -286,14 +286,6 @@ static void dedotify_versions(struct modversion_info *vers,
for (end = (void *)vers + size; vers < end; vers++)
if (vers->name[0] == '.') {
memmove(vers->name, vers->name+1, strlen(vers->name));
-#ifdef ARCH_RELOCATES_KCRCTAB
-   /* The TOC symbol has no CRC computed. To avoid CRC
-* check failing, we must force it to the expected
-* value (see CRC check in module.c).
-*/
-   if (!strcmp(vers->name, "TOC."))
-   vers->crc = -(unsigned long)reloc_start;
-#endif
}
 }
 
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 63554e9f6e0c..0dc6da121c49 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -9,18 +9,15 @@
 #ifndef KSYM_ALIGN
 #define KSYM_ALIGN 8
 #endif
-#ifndef KCRC_ALIGN
-#define KCRC_ALIGN 8
-#endif
 #else
 #define __put .long
 #ifndef KSYM_ALIGN
 #define KSYM_ALIGN 4
 #endif
+#endif
 #ifndef KCRC_ALIGN
 #define KCRC_ALIGN 4
 #endif
-#endif
 
 #ifdef 

[PATCH v4 1/2] kbuild: modversions: add infrastructure for emitting relative CRCs

[Ard Biesheuvel]

This add the kbuild infrastructure that will allow architectures to emit
vmlinux symbol CRCs as 32-bit offsets to other locations in the kernel
where the actual values are stored. This works around problems with CRCs
being mistaken for relocatable symbols on kernels that self relocate at
runtime (i.e., powerpc with CONFIG_RELOCATABLE=y)

For the kbuild side of things, this comes down to the following:
- introducing a Kconfig symbol MODULE_REL_CRCS
- adding a -R switch to genksyms to instruct it to emit the CRC symbols
  as references into the .rodata section, and emitting the actual value
  into it
- making modpost distinguish such references from absolute CRC symbols by
  the section index (SHN_ABS)
- making kallsyms disregard non-absolute symbols with a __crc_ prefix

Signed-off-by: Ard Biesheuvel 
---
 init/Kconfig|  4 
 scripts/Makefile.build  |  2 ++
 scripts/genksyms/genksyms.c | 19 ++-
 scripts/kallsyms.c  | 12 
 scripts/mod/modpost.c   | 10 ++
 5 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index e1a937348a3e..4dd8bd232a1d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1987,6 +1987,10 @@ config MODVERSIONS
  make them incompatible with the kernel you are running.  If
  unsure, say N.
 
+config MODULE_REL_CRCS
+   bool
+   depends on MODVERSIONS
+
 config MODULE_SRCVERSION_ALL
bool "Source checksum for all modules"
help
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index eadcd4d359d9..a9cfab2ee7c1 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -164,6 +164,7 @@ cmd_gensymtypes_c = 
\
 $(CPP) -D__GENKSYMS__ $(c_flags) $< |   \
 $(GENKSYMS) $(if $(1), -T $(2)) \
  $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \
+ $(if $(part-of-module),,$(patsubst y,-R,$(CONFIG_MODULE_REL_CRCS)))\
  $(if $(KBUILD_PRESERVE),-p)\
  -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))
 
@@ -337,6 +338,7 @@ cmd_gensymtypes_S = 
\
 $(CPP) -D__GENKSYMS__ $(c_flags) -xc - |\
 $(GENKSYMS) $(if $(1), -T $(2)) \
  $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \
+ $(if $(part-of-module),,$(patsubst y,-R,$(CONFIG_MODULE_REL_CRCS)))\
  $(if $(KBUILD_PRESERVE),-p)\
  -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))
 
diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
index 06121ce524a7..c9235d8340f1 100644
--- a/scripts/genksyms/genksyms.c
+++ b/scripts/genksyms/genksyms.c
@@ -44,7 +44,7 @@ char *cur_filename, *source_file;
 int in_source_file;
 
 static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types,
-  flag_preserve, flag_warnings;
+  flag_preserve, flag_warnings, flag_rel_crcs;
 static const char *mod_prefix = "";
 
 static int errors;
@@ -693,7 +693,10 @@ void export_symbol(const char *name)
fputs(">\n", debugfile);
 
/* Used as a linker script. */
-   printf("%s__crc_%s = 0x%08lx ;\n", mod_prefix, name, crc);
+   printf(!flag_rel_crcs ? "%s__crc_%s = 0x%08lx;\n" :
+  "SECTIONS { .rodata : ALIGN(4) { "
+  "%s__crc_%s = .; LONG(0x%08lx); } }\n",
+  mod_prefix, name, crc);
}
 }
 
@@ -730,7 +733,7 @@ void error_with_pos(const char *fmt, ...)
 
 static void genksyms_usage(void)
 {
-   fputs("Usage:\n" "genksyms [-adDTwqhV] > /path/to/.tmp_obj.ver\n" "\n"
+   fputs("Usage:\n" "genksyms [-adDTwqhVR] > /path/to/.tmp_obj.ver\n" "\n"
 #ifdef __GNU_LIBRARY__
  "  -s, --symbol-prefix   Select symbol prefix\n"
  "  -d, --debug   Increment the debug level (repeatable)\n"
@@ -742,6 +745,7 @@ static void genksyms_usage(void)
  "  -q, --quiet   Disable warnings (default)\n"
  "  -h, --helpPrint this message\n"
  "  -V, --version Print the release version\n"
+ "  -R, --relative-crcEmit section relative symbol CRCs\n"
 #else  /* __GNU_LIBRARY__ */
  "  -sSelect symbol prefix\n"
  "  -dIncrement the debug level (repeatable)\n"
@@ -753,6 +757,7 @@ static void genksyms_usage(void)
  "  -qDisable warnings (default)\n"
  "  -hPrint this message\n"
  "  -VPrint the release version\n"
+ 

[PATCH v4 0/2] modversions: redefine kcrctab entries as 32-bit values

[Ard Biesheuvel]

This v4 is a followup to [0] 'modversions: redefine kcrctab entries as
relative CRC pointers', but since relative CRC pointers do not work in
modules, and are actually only needed by powerpc with CONFIG_RELOCATABLE=y,
I have made it a Kconfig selectable feature instead.

Patch #1 introduces the MODULE_REL_CRCS Kconfig symbol, and adds the kbuild
handling of it, i.e., modpost, genksyms and kallsyms.

Patch #2 switches all architectures to 32-bit CRC entries in kcrctab, where
all architectures except powerpc with CONFIG_RELOCATABLE=y use absolute ELF
symbol references as before.

v4: make relative CRCs kconfig selectable
use absolute CRC symbols in modules regardless of kconfig selection
split into two patches

v3: emit CRCs into .rodata rather than .rodata.modver, given that the latter
will be emitted with read-write permissions, making the CRCs end up in a
writable module segment.

fold the modpost fix to ensure that the section address is only substracted
from the symbol address when the ELF object in question is fully linked
(i.e., ET_DYN or ET_EXEC, and not ET_REL)

v2: update modpost as well, so that genksyms no longer has to emit symbols
for both the actual CRC value and the reference to where it is stored
in the image

[0] http://marc.info/?l=linux-arch=148493613415294=2

Ard Biesheuvel (2):
  kbuild: modversions: add infrastructure for emitting relative CRCs
  modversions: treat symbol CRCs as 32 bit quantities

 arch/powerpc/Kconfig  |  1 +
 arch/powerpc/include/asm/module.h |  4 --
 arch/powerpc/kernel/module_64.c   |  8 
 include/asm-generic/export.h  | 11 ++---
 include/linux/export.h| 14 ++
 include/linux/module.h| 14 +++---
 init/Kconfig  |  4 ++
 kernel/module.c   | 45 ++--
 scripts/Makefile.build|  2 +
 scripts/genksyms/genksyms.c   | 19 ++---
 scripts/kallsyms.c| 12 ++
 scripts/mod/modpost.c | 10 +
 12 files changed, 93 insertions(+), 51 deletions(-)

-- 
2.7.4

RE: [PATCH 3/3] powerpc: bpf: implement in-register swap for 64-bit endian operations

[David Laight]

From: 'Naveen N. Rao'
> Sent: 23 January 2017 19:22
> On 2017/01/15 09:00AM, Benjamin Herrenschmidt wrote:
> > On Fri, 2017-01-13 at 23:22 +0530, 'Naveen N. Rao' wrote:
> > > > That rather depends on whether the processor has a store to load 
> > > > forwarder
> > > > that will satisfy the read from the store buffer.
> > > > I don't know about ppc, but at least some x86 will do that.
> > >
> > > Interesting - good to know that.
> > >
> > > However, I don't think powerpc does that and in-register swap is likely
> > > faster regardless. Note also that gcc prefers this form at higher
> > > optimization levels.
> >
> > Of course powerpc has a load-store forwarder these days, however, I
> > wouldn't be surprised if the in-register form was still faster on some
> > implementations, but this needs to be tested.
> 
> Thanks for clarifying! To test this, I wrote a simple (perhaps naive)
> test that just issues a whole lot of endian swaps and in _that_ test, it
> does look like the load-store forwarder is doing pretty well.
...
> This is all in a POWER8 vm. On POWER7, the in-register variant is around
> 4 times faster than the ldbrx variant.
...

I wonder which is faster on the little 1GHz embedded ppc we use here.

David

Re: [PATCH 31/37] misc: Add host side pci driver for pci test function device

[Christoph Hellwig]

On Thu, Jan 12, 2017 at 03:56:20PM +0530, Kishon Vijay Abraham I wrote:
> Add PCI endpoint test driver that can verify base address
> register, legacy interrupt/MSI interrupt and read/write/copy
> buffers between host and device. The corresponding pci-epf-test
> function driver should be used on the EP side.

Just curious:  what would you think of a text based (e.g. debugfs)
interface to avoid the need for a userspace tool here?

> +static const struct pci_device_id pci_endpoint_test_tbl[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_ANY_ID) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(pci, pci_endpoint_test_tbl);

Also this looks really odd, and dangerous.  Probing for any
TI device will bind to all kinds of legit devices.  It would
be good if you could squeeze out a single id for this device
out of the TI group responsible for allocating it.  Otherwise
we might try some other venues, e.g. Red Hat through Qumranet
has PCI IDs available for virtio, which might have some left
for other Linux uses.

In general I fear the PCI ID allocation will become a worse
and worse issue once your framework goes in and we'll grow
more PCI device models in the kernel.

Re: [PATCH] cxl: drop unused header asm/pnv-pci.h

[Greg Kurz]

On Tue, 24 Jan 2017 20:57:06 +1100
Michael Ellerman  wrote:

> Greg Kurz  writes:
> 
> > The kernel API does not use anything from this header file.  
> 
> That's actually hard to prove in general, because pnv-pci.h includes
> linux/pci.h, linux/pci_hotplug.h, linux/irq.h, misc/cxl-base.h and
> asm/opal-api.h. At least four of which aren't included directly by
> api.c.
> 
> But I'll take you word for it :)
> 

Sorry for the lack of details...

api.c doesn't do anything related to pci hotplug, nor to OPAL. This
happens in pci.c.

It includes cxl.h which includes misc/cxl-base.h.

The only generic irq bits are irq_hw_number_t, provided by linux/types.h,
and irq_find_mapping(), provided by linux/irqdomain.h, which is included
by misc/cxl-base.h -> linux/interrupt.h -> asm/irq.h.

Cheers (and congrats :)

--
Greg


> cheers
> 
> > diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> > index 1b35e33d2434..bcc030eacab7 100644
> > --- a/drivers/misc/cxl/api.c
> > +++ b/drivers/misc/cxl/api.c
> > @@ -11,7 +11,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >  #include 
> >  #include 
> >  #include   

[PATCH] i2c/pasemi: Remove hardcoded bus numbers on smbus

[Darren Stevens]

The pasemi smbus controller uses PCI_FUNC(dev->devfn) to define which
number bus to attach to, however this fails when something else is 
probed first, for example an ATI Radeon graphics card will claim 9 or
10 busses, including the ones the pasemi wants.
Patch the driver to call i2c_add_adapter rather than
i2c_add_numbered_adapter.

Signed-off-by: Darren Stevens 

---
diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index df1dbc9..05847fd 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -365,7 +365,6 @@ static int pasemi_smb_probe(struct pci_dev *dev,
smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
smbus->adapter.algo = _algorithm;
smbus->adapter.algo_data = smbus;
-   smbus->adapter.nr = PCI_FUNC(dev->devfn);
 
/* set up the sysfs linkage to our parent device */
smbus->adapter.dev.parent = >dev;
@@ -373,7 +372,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
  (CLK_100K_DIV & CTL_CLK_M)));
 
-   error = i2c_add_numbered_adapter(>adapter);
+   error = i2c_add_adapter(>adapter);
if (error)
goto out_release_region;
 

Re: [PATCH] mtd/ifc: Fix location of eccstat registers for IFC V1.0

[Boris Brezillon]

On Tue, 24 Jan 2017 12:01:22 +0100
 wrote:

> From: Mark Marshall 
> 
> The commit 7a654172161c ("mtd/ifc: Add support for IFC controller
> version 2.0") added support for version 2.0 of the IFC controller.
> The version 2.0 controller has the ECC status registers at a different
> location to the previous versions.  This broke the code for IFC all
> versions less than 2.0.
> 
> Correct the fsl_ifc_nand structure so that the ECC status can be read
> from the correct location for both version 1.0 and 2.0 of the controller.
> 
> Cc: sta...@vger.kernel.org

Fixes: 7a654172161c ("mtd/ifc: Add support for IFC controller version 2.0")

> Signed-off-by: Mark Marshall 
> ---
>  drivers/mtd/nand/fsl_ifc_nand.c | 5 -
>  include/linux/fsl_ifc.h | 8 ++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 0a177b1..123c0f8 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -260,7 +260,10 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
>   int sector_end = sector + chip->ecc.steps - 1;
>  
>   for (i = sector / 4; i <= sector_end / 4; i++)
> - eccstat[i] = ifc_in32(>ifc_nand.nand_eccstat[i]);
> + eccstat[i] = ifc_in32(
> + (ctrl->version >= FSL_IFC_VERSION_2_0_0) ?
> + >ifc_nand.v2_nand_eccstat[i] :
> + >ifc_nand.v1_nand_eccstat[i]);

I'm nitpicking, but how about doing the following to improve
readability:

__be32 *eccstat_regs;

if (ctrl->version >= FSL_IFC_VERSION_2_0_0)
eccstat_regs = >ifc_nand.v2_nand_eccstat;
else
eccstat_regs = >ifc_nand.v1_nand_eccstat;

for (i = sector / 4; i <= sector_end / 4; i++)
eccstat[i] = ifc_in32(_regs[i]);

>  
>   for (i = sector; i <= sector_end; i++) {
>   errors = check_read_ecc(mtd, ctrl, eccstat, i);
> diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h
> index 3f9778c..c332f0a 100644
> --- a/include/linux/fsl_ifc.h
> +++ b/include/linux/fsl_ifc.h
> @@ -733,8 +733,12 @@ struct fsl_ifc_nand {
>   __be32 nand_erattr1;
>   u32 res19[0x10];
>   __be32 nand_fsr;
> - u32 res20[0x3];
> - __be32 nand_eccstat[6];
> + u32 res20;
> + /* The V1 nand_eccstat is actually 4 words that overlaps the
> +  * V2 nand_eccstat.
> +  */
> + __be32 v1_nand_eccstat[2];
> + __be32 v2_nand_eccstat[6];
>   u32 res21[0x1c];
>   __be32 nanndcr;
>   u32 res22[0x2];

Re: BUILD_BUG_ON(!__builtin_constant_p(feature)) breaks bcc trace tool

[Naveen N. Rao]

On 2017/01/24 04:36PM, Michael Ellerman wrote:
> Anton Blanchard  writes:
> > We added:
> >
> > BUILD_BUG_ON(!__builtin_constant_p(feature)) 
> >
> > to cpu_has_feature() and mmu_has_feature() in order to catch usage
> > issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately LLVM
> > isn't smart enough to resolve this, and it errors out.
> >
> > I work around it in my clang/LLVM builds of the kernel, but I have just
> > discovered that it causes a lot of issues for the bcc (eBPF) trace tool
> > (which uses LLVM).
> 
> I didn't understand that part, but Aneesh explained to me that it's
> because bcc pulls in the kernel-internal headers.
> 
> I guess as a quick fix we just have to #ifdef it, can you confirm this
> works?

This works and solves the issue for me. In the absence of a better 
approach, can you please push this for v4.10?

Tested-by: Naveen N. Rao 

Thanks,
Naveen

> 
> 
> diff --git a/arch/powerpc/include/asm/cpu_has_feature.h 
> b/arch/powerpc/include/asm/cpu_has_feature.h
> index b312b152461b..6e834caa3720 100644
> --- a/arch/powerpc/include/asm/cpu_has_feature.h
> +++ b/arch/powerpc/include/asm/cpu_has_feature.h
> @@ -23,7 +23,9 @@ static __always_inline bool cpu_has_feature(unsigned long 
> feature)
>  {
>   int i;
> 
> +#ifndef __clang__ /* clang can't cope with this */
>   BUILD_BUG_ON(!__builtin_constant_p(feature));
> +#endif
> 
>  #ifdef CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
>   if (!static_key_initialized) {
> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index a34c764ca8dd..233a7e8cc8e3 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -160,7 +160,9 @@ static __always_inline bool mmu_has_feature(unsigned long 
> feature)
>  {
>   int i;
> 
> +#ifndef __clang__ /* clang can't cope with this */
>   BUILD_BUG_ON(!__builtin_constant_p(feature));
> +#endif
> 
>  #ifdef CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
>   if (!static_key_initialized) {
> 
> 
> cheers
> 

Re: [PATCH v2] powerpc/pasemi: Remove hardcoded bus numbers on smbus

[Michael Ellerman]

Darren Stevens  writes:
> The pasemi smbus controller uses PCI_FUNC(dev->devfn) to define which
> number bus to attach to, however this fails when something else is 
> probed first, for example an ATI Radeon graphics card will claim 9 or
> 10 busses, including the ones the pasemi wants.
> Patch the driver to call i2c_add_adapter rather than
> i2c_add_numbered_adapter.

This is an i2c patch so the subject should be something like:

i2c/pasemi: Remove hardcoded bus numbers on smbus

And it should be sent to:

w...@the-dreams.de
linux-...@vger.kernel.org
linuxppc-dev@lists.ozlabs.org
linux-ker...@vger.kernel.org

cheers

Re: [PATCH] powerpc/powernv: Validate memcons descriptor and output buffer

[Michael Ellerman]

Gavin Shan  writes:

> Currently, it's assumed that memcons and its output buffer are included
> in the linear mapping. It's not true when "mem=384M" is included in
> bootargs. The system runs into kernel crash eventually.
>
># od -x /proc/device-tree/ibm,opal/ibm,opal-memcons
>000   0b30 0010
>010
>
> This validates memcons descriptor and its output buffer to ensure they
> are valid in linear mapping. Otherwise, the interface won't be populated
> to avoid kernel crash during system boot.
>
> Cc: sta...@vger.kernel.org #3.15+
> Fixes: bfc36894a48 ("powerpc/powernv: Add OPAL message log interface")
> Signed-off-by: Gavin Shan 

Hmm.

Arguably the memcons shouldn't be in the linear map at all.

AFAICS the kernel only ever reads from it, so really it should be
explicitly mapped, and mapped read only.

That would also fix this problem :D

cheers

Re: [PATCH] cxl: drop unused header asm/pnv-pci.h

[Michael Ellerman]

Greg Kurz  writes:

> The kernel API does not use anything from this header file.

That's actually hard to prove in general, because pnv-pci.h includes
linux/pci.h, linux/pci_hotplug.h, linux/irq.h, misc/cxl-base.h and
asm/opal-api.h. At least four of which aren't included directly by
api.c.

But I'll take you word for it :)

cheers

> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 1b35e33d2434..bcc030eacab7 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 

Re: BUILD_BUG_ON(!__builtin_constant_p(feature)) breaks bcc trace tool

[Arnd Bergmann]

On Tue, Jan 24, 2017 at 7:15 AM, Michael Ellerman  wrote:

> Hmm, in fact it seems because we don't define
> CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING and CONFIG_OPTIMIZE_INLINING, we
> get:
>
> #define inline  inline  __attribute__((always_inline)) notrace
>
> So in fact every inline function is marked always_inline all the time,
> which seems dubious.
>
> But still, it seems clang is ignoring always_inline.

I think the problem is that __builtin_constant_p() is usually false on
clang, at least for arguments
that are not constant pointers.

Arnd