Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [arm-soc:to-build 4/4] arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function
From: kbuild test robot @ 2016-10-13 13:25 UTC (permalink / raw)
  To: linux-arm-kernel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git to-build
head:   346c22ea9cb8d0ee331c73529b205414e43a4655
commit: 346c22ea9cb8d0ee331c73529b205414e43a4655 [4/4] Revert "Disable "maybe-uninitialized" warning globally"
config: s390-default_defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 346c22ea9cb8d0ee331c73529b205414e43a4655
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
>> arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]
     } __packed data = {rc, addr};
                ^~~~
   arch/s390/pci/pci_dma.c:426:16: note: 'pa' was declared here
     unsigned long pa;
                   ^~

vim +/pa +309 arch/s390/pci/pci_dma.c

828b35f6 Jan Glauber         2012-11-29  293  		goto out;
13954fd6 Sebastian Ott       2016-09-08  294  
13954fd6 Sebastian Ott       2016-09-08  295  	if (zdev->tlb_refresh || s390_iommu_strict)
828b35f6 Jan Glauber         2012-11-29  296  		bitmap_clear(zdev->iommu_bitmap, offset, size);
13954fd6 Sebastian Ott       2016-09-08  297  	else
13954fd6 Sebastian Ott       2016-09-08  298  		bitmap_set(zdev->lazy_bitmap, offset, size);
13954fd6 Sebastian Ott       2016-09-08  299  
828b35f6 Jan Glauber         2012-11-29  300  out:
828b35f6 Jan Glauber         2012-11-29  301  	spin_unlock_irqrestore(&zdev->iommu_bitmap_lock, flags);
828b35f6 Jan Glauber         2012-11-29  302  }
828b35f6 Jan Glauber         2012-11-29  303  
52d43d81 Sebastian Ott       2015-10-26  304  static inline void zpci_err_dma(unsigned long rc, unsigned long addr)
52d43d81 Sebastian Ott       2015-10-26  305  {
52d43d81 Sebastian Ott       2015-10-26  306  	struct {
52d43d81 Sebastian Ott       2015-10-26  307  		unsigned long rc;
52d43d81 Sebastian Ott       2015-10-26  308  		unsigned long addr;
52d43d81 Sebastian Ott       2015-10-26 @309  	} __packed data = {rc, addr};
52d43d81 Sebastian Ott       2015-10-26  310  
52d43d81 Sebastian Ott       2015-10-26  311  	zpci_err_hex(&data, sizeof(data));
52d43d81 Sebastian Ott       2015-10-26  312  }
52d43d81 Sebastian Ott       2015-10-26  313  
828b35f6 Jan Glauber         2012-11-29  314  static dma_addr_t s390_dma_map_pages(struct device *dev, struct page *page,
828b35f6 Jan Glauber         2012-11-29  315  				     unsigned long offset, size_t size,
828b35f6 Jan Glauber         2012-11-29  316  				     enum dma_data_direction direction,
00085f1e Krzysztof Kozlowski 2016-08-03  317  				     unsigned long attrs)

:::::: The code at line 309 was first introduced by commit
:::::: 52d43d8184b1840c7cf6136724223585f51a1074 s390/pci_dma: improve debugging of errors during dma map

:::::: TO: Sebastian Ott <sebott@linux.vnet.ibm.com>
:::::: CC: Martin Schwidefsky <schwidefsky@de.ibm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 16396 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161013/6b7b2883/attachment-0001.gz>

^ permalink raw reply

* [arm-soc:to-build 4/4] drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:4586:10: warning: 'is_11d' may be used uninitialized in this function
From: kbuild test robot @ 2016-10-13 13:17 UTC (permalink / raw)
  To: linux-arm-kernel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git to-build
head:   346c22ea9cb8d0ee331c73529b205414e43a4655
commit: 346c22ea9cb8d0ee331c73529b205414e43a4655 [4/4] Revert "Disable "maybe-uninitialized" warning globally"
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 346c22ea9cb8d0ee331c73529b205414e43a4655
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c: In function 'brcmf_cfg80211_start_ap':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:4586:10: warning: 'is_11d' may be used uninitialized in this function [-Wmaybe-uninitialized]
       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
              is_11d);
              ~~~~~~~

vim +/is_11d +4586 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c

a44aa400 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Hante Meuleman   2014-12-03  4570  
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4571  			/* RSN IE */
a44aa400 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Hante Meuleman   2014-12-03  4572  			err = brcmf_configure_wpaie(ifp, tmp_ie, true);
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4573  			if (err < 0)
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4574  				goto exit;
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4575  		}
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4576  	} else {
d96b801f drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Arend van Spriel 2012-12-05  4577  		brcmf_dbg(TRACE, "No WPA(2) IEs found\n");
1f170110 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2013-02-06  4578  		brcmf_configure_opensecurity(ifp);
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4579  	}
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4580  
a0f07959 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2013-02-08  4581  	brcmf_config_ap_mgmt_ie(ifp->vif, &settings->beacon);
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4582  
8707e08d drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c Rafa? Mi?ecki    2016-05-27  4583  	/* Parameters shared by all radio interfaces */
a44aa400 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Hante Meuleman   2014-12-03  4584  	if (!mbss) {
b3589dfe drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c Hante Meuleman   2016-09-19  4585  		if ((supports_11d) && (is_11d != ifp->vif->is_11d)) {
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21 @4586  			err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21  4587  						    is_11d);
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21  4588  			if (err < 0) {
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21  4589  				brcmf_err("Regulatory Set Error, %d\n", err);
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21  4590  				goto exit;
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21  4591  			}
98027769 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c          Arend van Spriel 2014-12-21  4592  		}
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Hante Meuleman   2012-09-27  4593  		if (settings->beacon_interval) {
ac24be6f drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c       Arend van Spriel 2012-10-22  4594  			err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_BCNPRD,

:::::: The code at line 4586 was first introduced by commit
:::::: 98027769828f772c7ce69b6e58d37b78ebe8ab28 brcmfmac: enable 802.11d support in firmware

:::::: TO: Arend van Spriel <arend@broadcom.com>
:::::: CC: Kalle Valo <kvalo@codeaurora.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 50804 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161013/18fc4945/attachment-0001.gz>

^ permalink raw reply

* [PATCH V3 10/10] arm64: KVM: add guest SEA support
From: Punit Agrawal @ 2016-10-13 13:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475875882-2604-11-git-send-email-tbaicar@codeaurora.org>

Hi Tyler,

I know I've had my last comment already ;), but I thought I'd rather
raise the question than stay confused...

Tyler Baicar <tbaicar@codeaurora.org> writes:

> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.
>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> ---
>  arch/arm/include/asm/kvm_arm.h       |  1 +
>  arch/arm/include/asm/system_misc.h   |  5 +++++
>  arch/arm/kvm/mmu.c                   | 15 +++++++++++++--
>  arch/arm64/include/asm/kvm_arm.h     |  1 +
>  arch/arm64/include/asm/system_misc.h |  2 ++
>  arch/arm64/mm/fault.c                | 13 +++++++++++++
>  6 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..33a77509 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,7 @@
>  #define FSC_FAULT	(0x04)
>  #define FSC_ACCESS	(0x08)
>  #define FSC_PERM	(0x0c)
> +#define FSC_EXTABT	(0x10)
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~0xf)
> diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
> index a3d61ad..86e1faa 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -24,4 +24,9 @@ extern unsigned int user_debug;
>  
>  #endif /* !__ASSEMBLY__ */
>  
> +inline int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> +	return -1;
> +}
> +
>  #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index e9a5c0e..24cde84 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -29,6 +29,7 @@
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/virt.h>
> +#include <asm/system_misc.h>
>  
>  #include "trace.h"
>  
> @@ -1441,8 +1442,18 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  	/* Check the stage-2 fault is trans. fault or write fault */
>  	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> -	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> -	    fault_status != FSC_ACCESS) {
> +
> +	if (fault_status == FSC_EXTABT) {
> +		if(handle_guest_sea((unsigned long)fault_ipa,
> +				    kvm_vcpu_get_hsr(vcpu))) {
> +			kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
> +				kvm_vcpu_trap_get_class(vcpu),
> +				(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> +				(unsigned long)kvm_vcpu_get_hsr(vcpu));
> +			return -EFAULT;
> +		}
> +	} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +		   fault_status != FSC_ACCESS) {
>  		kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>  			kvm_vcpu_trap_get_class(vcpu),
>  			(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 4b5c977..be0efb6 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -201,6 +201,7 @@
>  #define FSC_FAULT	ESR_ELx_FSC_FAULT
>  #define FSC_ACCESS	ESR_ELx_FSC_ACCESS
>  #define FSC_PERM	ESR_ELx_FSC_PERM
> +#define FSC_EXTABT	ESR_ELx_FSC_EXTABT
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~UL(0xf))
> diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
> index 90daf4a..8522fff 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -77,4 +77,6 @@ extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
>  int sea_register_handler_chain(struct notifier_block *nb);
>  void sea_unregister_handler_chain(struct notifier_block *nb);
>  
> +int handle_guest_sea(unsigned long addr, unsigned int esr);
> +
>  #endif	/* __ASM_SYSTEM_MISC_H */
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 81cb7ad..d714432 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -597,6 +597,19 @@ static const char *fault_name(unsigned int esr)
>  }
>  
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> +	atomic_notifier_call_chain(&sea_handler_chain, 0, NULL);
> +
> +	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
> +		fault_name(esr), esr, addr);
> +
> +	return 0;
> +}

Don't we need to pass the abort to the guest?

Thanks,
Punit

> +
> +/*
>   * Dispatch a data abort to the relevant handler.
>   */
>  asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,

^ permalink raw reply

* [PATCH V3 06/10] acpi: apei: panic OS with fatal error status block
From: Suzuki K Poulose @ 2016-10-13 13:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475875882-2604-7-git-send-email-tbaicar@codeaurora.org>

On 07/10/16 22:31, Tyler Baicar wrote:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
>
> Even if an error status block's severity is fatal, the kernel does not
> honor the severity level and panic.
>
> With the firmware first model, the platform could inform the OS about a
> fatal hardware error through the non-NMI GHES notification type. The OS
> should panic when a hardware error record is received with this
> severity.
>
> Call panic() after CPER data in error status block is printed if
> severity is fatal, before each error section is handled.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> ---
>  drivers/acpi/apei/ghes.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 28d5a09..36894c8 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -141,6 +141,8 @@ static unsigned long ghes_estatus_pool_size_request;
>  static struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
>  static atomic_t ghes_estatus_cache_alloced;
>
> +static int ghes_panic_timeout __read_mostly = 30;
> +
>  static int ghes_ioremap_init(void)
>  {
>  	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
> @@ -715,6 +717,12 @@ static int ghes_proc(struct ghes *ghes)
>  		if (ghes_print_estatus(NULL, ghes->generic, ghes->estatus))
>  			ghes_estatus_cache_add(ghes->generic, ghes->estatus);
>  	}
> +	if (ghes_severity(ghes->estatus->error_severity) >= GHES_SEV_PANIC) {
> +		if (panic_timeout == 0)
> +			panic_timeout = ghes_panic_timeout;
> +		panic("Fatal hardware error!");

I think there is a chance that we might miss the o/p of ghes_print_estatus() as we use
no pfx, and it could default to the normal loglevel and would never get printed
if panic() is encountered before it. On the other hand, there is already a
__ghes_panic() which does similar stuff. Is there a way we could reuse
(may be even parts of) it ? Or at least use KERN_EMERG for the ghes_print_estatus(),
if the severity could result in panic() ?

Cheers
Suzuki

^ permalink raw reply

* [PATCH 1/3] binding: irqchip: mtk-cirq: Add binding document
From: Matthias Brugger @ 2016-10-13 13:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476335194-26604-2-git-send-email-youlin.pei@mediatek.com>



On 10/13/2016 07:06 AM, Youlin Pei wrote:
> This commit adds the device tree binding document for
> the mediatek cirq.
>
> Signed-off-by: Youlin Pei <youlin.pei@mediatek.com>
>
> ---
> base on v4.8-rc1
> ---
>  .../interrupt-controller/mediatek,cirq.txt         |   30 ++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> new file mode 100644
> index 0000000..ad16953
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> @@ -0,0 +1,30 @@
> +* Mediatek 27xx cirq
> +
> +In Mediatek SOCs, the CIRQ is a low power interrupt controller designed to
> +works outside MCUSYS which comprises with Cortex-Ax cores,CCI and GIC.
> +The external interrupts (outside MCUSYS) will feed through CIRQ and connect
> +to GIC in MCUSYS. When CIRQ is enabled, it will record the edge-sensitive
> +interrupts and generated a pulse signal to parent interrupt controller when
> +flush command is executed. With CIRQ, MCUSYS can be completely turned off
> +to improve the system power consumption without losing interrupts.
> +
> +Required properties:
> +- compatible: should be: "mediatek,mt2701-cirq".

The cirq is present in several SoCs. I suppose it is the same core in 
all of them. So we can name it mediatek,mtk-cirq and add a 
mediatek,mtXXXX-cirq for every SoC, just in case we will need it.

Thanks,
Matthias

^ permalink raw reply

* [PATCH 1/1 v8] ARM: imx: Added perf functionality to mmdc driver
From: Shawn Guo @ 2016-10-13 12:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHrpEqQ2H7FZVbn_A3VPCs3aA7AekT7k+Z++js_iCTw346z9Sw@mail.gmail.com>

On Mon, Oct 10, 2016 at 03:17:36PM -0500, Zhi Li wrote:
> On Mon, Sep 26, 2016 at 11:40 AM, Zhi Li <lznuaa@gmail.com> wrote:
> > On Mon, Sep 19, 2016 at 12:57 PM, Frank Li <Frank.Li@nxp.com> wrote:
> >> From: Zhengyu Shen <zhengyu.shen@nxp.com>
> >>
> >> MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64
> >> and LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high
> >> performance, and optimized. MMDC is present on i.MX6 Quad and i.MX6
> >> QuadPlus devices, but this driver only supports i.MX6 Quad at the moment.
> >> MMDC provides registers for performance counters which read via this
> >> driver to help debug memory throughput and similar issues.
> >>
> >> $ perf stat -a -e mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/ dd if=/dev/zero of=/dev/null bs=1M count=5000
> >> Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':
> >>
> >>          898021787      mmdc/busy-cycles/
> >>           14819600      mmdc/read-accesses/
> >>             471.30 MB   mmdc/read-bytes/
> >>         2815419216      mmdc/total-cycles/
> >>           13367354      mmdc/write-accesses/
> >>             427.76 MB   mmdc/write-bytes/
> >>
> >>        5.334757334 seconds time elapsed
> >>
> >> Signed-off-by: Zhengyu Shen <zhengyu.shen@nxp.com>
> >> Signed-off-by: Frank Li <frank.li@nxp.com>
> >> ---
> >
> > Mark:
> >         Any additional comments for this version?
> >
> 
> Shawn:
>            No any new comment for more than 2 weeks.
>            Did you plan accept this patch?

We normally do not apply patches in the middle of a merge window.  I
plan to apply it when 4.9-rc1 comes out.  But I still would like to get
a Reviewed-by tag from Mark before doing that.

@Mark,

Are you happy with this version?

Shawn

^ permalink raw reply

* [PATCH 2/2] power/reset: at91-poweroff: timely shitdown LPDDR memories
From: Richard Genoud @ 2016-10-13 12:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACh+v5PgwXsCoMROO+Vftx8bbhTXP1V-JdZmdwWJ0tgOUW1Axw@mail.gmail.com>

2016-10-13 14:27 GMT+02:00 Jean-Jacques Hiblot <jjhiblot@traphandler.com>:
> 2016-10-13 13:03 GMT+02:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
>> On 12/10/2016 at 14:48:27 +0200, Jean-Jacques Hiblot wrote :
>>> > +static void at91_lpddr_poweroff(void)
>>> > +{
>>> > +       asm volatile(
>>> > +               /* Align to cache lines */
>>> > +               ".balign 32\n\t"
>>> > +
>>> > +               "       ldr     r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
>>> At first sight, it looks useless. I assume it's used to preload the
>>> TLB before the LPDDR is turned off.
>>> A comment to explain why this line is useful would prevent its removal.
>>
>> Yes, this is the case. I can add a comment.
>>
>> Anyway, I would prefer the whole thing to run from SRAM, as a PIE
>> instead of relying on the cache.
>
> Instead of copying into the SRAM, you can make the cache reliable by
> preloading it, much like the TLB.
> LDI is probably not available for most of atmel's SOC, so the only way
> I can think of, is to execute code from the targeted area. here is an
> example:
> +               /*
> +                * Jump to the end of the sequence to preload instruction cache
> +                * It only works because the sequence is short enough not to
> +                * sit accross more than 2 cache lines
> +                */
> +               "       b end_of_sequence\n\t"
> +               "start_of_sequence:\n\t"
> +
>                 /* Power down SDRAM0 */
>                 "       str     %1, [%0, #"
> __stringify(AT91_DDRSDRC_LPR) "]\n\t"
>                 /* Shutdown CPU */
>                 "       str     %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
>
>                 "       b       .\n\t"
> +
> +               /*
> +                * we're now 100% sure that the code to shutdown the LPDDR and
> +                * the CPU is in cache, go back to do the actual job
> +                */
> +               "end_of_sequence:\n\t"
> +               "       b start_of_sequence\n\t"
>                 :

My 2c: I think you may want to change your subject :)

Richard.

^ permalink raw reply

* [PATCH v2 2/2] arm64: dts: zx: Add clock controller nodes
From: Jun Nie @ 2016-10-13 12:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476361881-19685-1-git-send-email-jun.nie@linaro.org>

Add clock controller nodes, including one top controller
two low speed controllers and one audio controller.

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 arch/arm64/boot/dts/zte/zx296718.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index 6b239a3..e419eff 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -279,9 +279,33 @@
 			dma-requests = <32>;
 		};
 
+		lsp0crm: clock-controller at 1420000 {
+			compatible = "zte,zx296718-lsp0crm";
+			reg = <0x01420000 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		lsp1crm: clock-controller at 1430000 {
+			compatible = "zte,zx296718-lsp1crm";
+			reg = <0x01430000 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		topcrm: clock-controller at 1461000 {
+			compatible = "zte,zx296718-topcrm";
+			reg = <0x01461000 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		sysctrl: sysctrl at 1463000 {
 			compatible = "zte,zx296718-sysctrl", "syscon";
 			reg = <0x1463000 0x1000>;
 		};
+
+		audiocrm: clock-controller at 1480000 {
+			compatible = "zte,zx296718-audiocrm";
+			reg = <0x01480000 0x1000>;
+			#clock-cells = <1>;
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 1/2] arm64: dts: zx: Fix gic GICR property
From: Jun Nie @ 2016-10-13 12:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476361881-19685-1-git-send-email-jun.nie@linaro.org>

GICR for multiple CPU can be described with start address and stride,
or with multiple address. Current multiple address and stride are
both used. Fix it.

vmalloc patch 727a7f5a9 triggered this bug:
[    0.097146] Unable to handle kernel paging request at virtual address ffff000008060008
[    0.097150] pgd = ffff000008602000
[    0.097160] [ffff000008060008] *pgd=000000007fffe003, *pud=000000007fffd003, *pmd=000000007fffc003, *pte=0000000000000000
[    0.097165] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[    0.097170] Modules linked in:
[    0.097177] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #1474
[    0.097179] Hardware name: ZTE zx296718 evaluation board (DT)
[    0.097183] task: ffff80003e8c8b80 task.stack: ffff80003e8d0000
[    0.097197] PC is at gic_populate_rdist+0x74/0x15c
[    0.097202] LR is at gic_starting_cpu+0xc/0x20
[    0.097206] pc : [<ffff0000082b1b18>] lr : [<ffff0000082b26e0>] pstate: 600001c5

Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 arch/arm64/boot/dts/zte/zx296718.dtsi | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index a223066..6b239a3 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -239,16 +239,11 @@
 		compatible = "arm,gic-v3";
 		#interrupt-cells = <3>;
 		#address-cells = <0>;
-		#redistributor-regions = <6>;
-		redistributor-stride = <0x0 0x40000>;
+		#redistributor-regions = <1>;
+		redistributor-stride = <0x20000>;
 		interrupt-controller;
 		reg = <0x02a00000 0x10000>,
-		      <0x02b00000 0x20000>,
-		      <0x02b20000 0x20000>,
-		      <0x02b40000 0x20000>,
-		      <0x02b60000 0x20000>,
-		      <0x02b80000 0x20000>,
-		      <0x02ba0000 0x20000>;
+		      <0x02b00000 0xc0000>;
 		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 0/2] Fix and add more zx dt nodes
From: Jun Nie @ 2016-10-13 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

Changes vs V1:
  - Add more background information for the GIC fix.
  - Fix minor coding style issue.

Jun Nie (2):
  arm64: dts: zx: Fix gic GICR property
  arm64: dts: zx: Add clock controller nodes

 arch/arm64/boot/dts/zte/zx296718.dtsi | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 2/2] power/reset: at91-poweroff: timely shitdown LPDDR memories
From: Jean-Jacques Hiblot @ 2016-10-13 12:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161013110317.ee6by6njrvs53ztx@piout.net>

2016-10-13 13:03 GMT+02:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> On 12/10/2016 at 14:48:27 +0200, Jean-Jacques Hiblot wrote :
>> > +static void at91_lpddr_poweroff(void)
>> > +{
>> > +       asm volatile(
>> > +               /* Align to cache lines */
>> > +               ".balign 32\n\t"
>> > +
>> > +               "       ldr     r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
>> At first sight, it looks useless. I assume it's used to preload the
>> TLB before the LPDDR is turned off.
>> A comment to explain why this line is useful would prevent its removal.
>
> Yes, this is the case. I can add a comment.
>
> Anyway, I would prefer the whole thing to run from SRAM, as a PIE
> instead of relying on the cache.

Instead of copying into the SRAM, you can make the cache reliable by
preloading it, much like the TLB.
LDI is probably not available for most of atmel's SOC, so the only way
I can think of, is to execute code from the targeted area. here is an
example:
+               /*
+                * Jump to the end of the sequence to preload instruction cache
+                * It only works because the sequence is short enough not to
+                * sit accross more than 2 cache lines
+                */
+               "       b end_of_sequence\n\t"
+               "start_of_sequence:\n\t"
+
                /* Power down SDRAM0 */
                "       str     %1, [%0, #"
__stringify(AT91_DDRSDRC_LPR) "]\n\t"
                /* Shutdown CPU */
                "       str     %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"

                "       b       .\n\t"
+
+               /*
+                * we're now 100% sure that the code to shutdown the LPDDR and
+                * the CPU is in cache, go back to do the actual job
+                */
+               "end_of_sequence:\n\t"
+               "       b start_of_sequence\n\t"
                :


>
>> > +       ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
>> > +       if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
>> > +           (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
>> Souldn't there be something like "pm_power_off = at91_lpddr_poweroff;" here ?
>>
>
> Indeed
>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply

* [PATCH v3 1/5] arm64: mm: BUG on unsupported manipulations of live kernel mappings
From: Ard Biesheuvel @ 2016-10-13 12:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161012150428.s4gajqlnnivo6bld@localhost>

On 12 October 2016 at 16:04, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Wed, Oct 12, 2016 at 12:23:41PM +0100, Ard Biesheuvel wrote:
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -28,8 +28,6 @@
>>  #include <linux/memblock.h>
>>  #include <linux/fs.h>
>>  #include <linux/io.h>
>> -#include <linux/slab.h>
>> -#include <linux/stop_machine.h>
>>
>>  #include <asm/barrier.h>
>>  #include <asm/cputype.h>
>> @@ -95,6 +93,12 @@ static phys_addr_t __init early_pgtable_alloc(void)
>>       return phys;
>>  }
>>
>> +/*
>> + * The following mapping attributes may be updated in live
>> + * kernel mappings without the need for break-before-make.
>> + */
>> +static const pteval_t modifiable_attr_mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
>> +
>>  static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>>                                 unsigned long end, unsigned long pfn,
>>                                 pgprot_t prot,
>> @@ -115,8 +119,18 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>>
>>       pte = pte_set_fixmap_offset(pmd, addr);
>>       do {
>> +             pte_t old_pte = *pte;
>> +
>>               set_pte(pte, pfn_pte(pfn, prot));
>>               pfn++;
>> +
>> +             /*
>> +              * After the PTE entry has been populated once, we
>> +              * only allow updates to the permission attributes.
>> +              */
>> +             BUG_ON(pte_val(old_pte) != 0 &&
>> +                    ((pte_val(old_pte) ^ pte_val(*pte)) &
>> +                     ~modifiable_attr_mask) != 0);
>
> Please turn this check into a single macro. You have it in three places
> already (though with different types but a macro would do). Something
> like below (feel free to come up with a better macro name):
>
>                 BUG_ON(!safe_pgattr_change(old_pte, *pte));
>

Something like below? With that, I can also fold the PMD and PUD
versions of the BUG() together.

"""
/*
 * Returns whether updating a live page table entry is safe:
 * - if the old and new values are identical,
 * - if an invalid mapping is turned into a valid one (or vice versa),
 * - if the entry is a block or page mapping, and the old and new values
 *   only differ in the PXN/RDONLY/WRITE bits.
 *
 * NOTE: 'safe' does not imply that no TLB maintenance is required, it only
 *       means that no TLB conflicts should occur as a result of the update.
 */
#define __set_pgattr_is_safe(type, old, new, blocktype) \
(type ## _val(old) == type ## _val(new) || \
((type ## _val(old) ^ type ## _val(new)) & PTE_VALID) != 0 || \
(((type ## _val(old) & PTE_TYPE_MASK) == blocktype) && \
 (((type ## _val(old) ^ type ## _val(new)) & \
   ~(PTE_PXN | PTE_RDONLY | PTE_WRITE)) == 0)))

static inline bool set_live_pte_is_safe(pte_t old, pte_t new)
{
return __set_pgattr_is_safe(pte, old, new, PTE_TYPE_PAGE);
}

static inline bool set_live_pmd_is_safe(pmd_t old, pmd_t new)
{
return __set_pgattr_is_safe(pmd, old, new, PMD_TYPE_SECT);
}

static inline bool set_live_pud_is_safe(pud_t old, pud_t new)
{
return __set_pgattr_is_safe(pud, old, new, PUD_TYPE_SECT);
}
"""

^ permalink raw reply

* [PATCH v2 1/2] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Daniel Lezcano @ 2016-10-13 12:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475604730-140264-1-git-send-email-briannorris@chromium.org>

On 04/10/2016 20:12, Brian Norris wrote:
> The ARM ARM specifies that the system counter "must be implemented in an
> always-on power domain," and so we try to use the counter as a source of
> timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
> Rockchip's RK3399) do not keep the counter ticking properly when
> switched from their high-power clock to the lower-power clock used in
> system suspend. Support this quirk by adding a new device tree property.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---

Both patches applied for 4.10.

Thanks !

  -- Daniel

-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply

* [PATCH RESEND] ARM: dts: keystone-k2*: Increase SPI Flash partition size for U-Boot
From: Vignesh R @ 2016-10-13 12:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161012115703.GE1041@n2100.armlinux.org.uk>



On Wednesday 12 October 2016 05:27 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 12, 2016 at 04:30:28PM +0530, Vignesh R wrote:
[...]
>>>
>>> - What is this "misc" partition?
>>>
>>
>> This partition seems to exists from the very beginning.  I believe, this
>> is just a spare area of flash that can be used as per end-user
>> requirement. Either to store a small filesystem or kernel. Copying
>> Murali who added above partition if he has any input here.
>>
>>> - Why is it safe to move the "misc" partition in this way?
>>>
>>> - Do users need to do anything with data stored in the "misc" partition
>>>   when changing kernels?
>>>
>>
>> MTD layer will take care of most abstractions (like start address etc).
>> Will add a note in commit message informing about the reduction in size
>> of the partition.
>>
>>> If the "misc" partition is simply unused space on the flash device, why
>>> list it in DT?
>>>
>>
>> If the unused space is not listed in the DT, then there is no /dev/mtdX
>> node created for the unused section. User will then have to manually
>> edit DT, in order to get the node and mount it. Instead, lets make it
>> available by default.
> 
> So, taken all together, your argument is:
> 
> - We want a user partition
> - It's okay to destroy the data in the user's partition by moving it
>   around randomly between kernel versions.
> 
> The two do not naturally go together at all.  You're messing with user
> expectations in ways you should not be.  This really is not an acceptable
> approach.
> 

Ok, I understand that if the user just updates to new kernel(w/o
updating bootloader) then this patch will end up setting "misc"
partition at wrong offset. At this point, I don't see how to provide a
way to upgrade boot loader at the same time support old and new layouts
simultaneously.
Could you please suggest an alternative approach that would enable users
to update U-Boot partition?
If not, then I guess, will have to drop this patch. Note that, this
means there won't be a straight forward way of updating SPI U-Boot
partition from kernel for K2 devices.

-- 
Regards
Vignesh

^ permalink raw reply

* [PATCH 2/2] power/reset: at91-poweroff: timely shitdown LPDDR memories
From: Alexandre Belloni @ 2016-10-13 11:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACh+v5NCxPb_U8HOeczHicCD-Wp8BqBzvc1LtCzmEXZ74Cgshw@mail.gmail.com>

On 12/10/2016 at 14:48:27 +0200, Jean-Jacques Hiblot wrote :
> > +static void at91_lpddr_poweroff(void)
> > +{
> > +       asm volatile(
> > +               /* Align to cache lines */
> > +               ".balign 32\n\t"
> > +
> > +               "       ldr     r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
> At first sight, it looks useless. I assume it's used to preload the
> TLB before the LPDDR is turned off.
> A comment to explain why this line is useful would prevent its removal.

Yes, this is the case. I can add a comment.

Anyway, I would prefer the whole thing to run from SRAM, as a PIE
instead of relying on the cache.

> > +       ddr_type = readl(mpddrc_base + AT91_DDRSDRC_MDR) & AT91_DDRSDRC_MD;
> > +       if ((ddr_type == AT91_DDRSDRC_MD_LPDDR2) ||
> > +           (ddr_type == AT91_DDRSDRC_MD_LPDDR3))
> Souldn't there be something like "pm_power_off = at91_lpddr_poweroff;" here ?
> 

Indeed


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [RFC] arm64: Enforce observed order for spinlock and data
From: Will Deacon @ 2016-10-13 11:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <781b6bb229ed566e1b948bfcea18bb61@codeaurora.org>

Brent,

On Wed, Oct 12, 2016 at 04:01:06PM -0400, bdegraaf at codeaurora.org wrote:
> I am still working through some additional analyses for mixed accesses,
> but I thought I'd send along some sample commit text for the fix as it
> currently stands.  Please feel free to comment if you see something that
> needs clarification.

Everything from this point down needs clarification.

> All arm64 lockref accesses that occur without taking the spinlock must
> behave like true atomics, ensuring successive operations are all done
> sequentially.

What is a "true atomic"? What do you mean by "successive"? What do you
mean by "done sequentially"?

The guarantee provided by lockref is that, if you hold the spinlock, then
you don't need to use atomics to inspect the reference count, as it is
guaranteed to be stable. You can't just go around replacing spin_lock
calls with lockref_get -- that's not what this is about.

> Currently
> the lockref accesses, when decompiled, look like the following sequence:
> 
>                     <Lockref "unlocked" Access [A]>
> 
>                     // Lockref "unlocked" (B)
>                 1:  ldxr   x0, [B]         // Exclusive load
>                      <change lock_count B>
>                     stxr   w1, x0, [B]
>                     cbnz   w1, 1b
> 
>                      <Lockref "unlocked" Access [C]>
> 
> Even though access to the lock_count is protected by exclusives, this is not
> enough
> to guarantee order: The lock_count must change atomically, in order, so the
> only
> permitted ordering would be:
>                               A -> B -> C

Says who? Please point me at a piece of code that relies on this. I'm
willing to believe that are bugs in this area, but waving your hands around
and saying certain properties "must" hold is not helpful unless you can
say *why* they must hold and *where* that is required.

> Unfortunately, this is not the case by the letter of the architecture and,
> in fact,
> the accesses to A and C are not protected by any sort of barrier, and hence
> are
> permitted to reorder freely, resulting in orderings such as
> 
>                            Bl -> A -> C -> Bs

Again, why is this a problem? It's exactly the same as if you did:

	spin_lock(lock);
	inc_ref_cnt();
	spin_unlock(lock);

Accesses outside of the critical section can still be reordered. Big deal.

> In this specific scenario, since "change lock_count" could be an
> increment, a decrement or even a set to a specific value, there could be
> trouble. 

What trouble?

> With more agents accessing the lockref without taking the lock, even
> scenarios where the cmpxchg passes falsely can be encountered, as there is
> no guarantee that the the "old" value will not match exactly a newer value
> due to out-of-order access by a combination of agents that increment and
> decrement the lock_count by the same amount.

This is the A-B-A problem, but I don't see why it affects us here. We're
dealing with a single reference count.

> Since multiple agents are accessing this without locking the spinlock,
> this access must have the same protections in place as atomics do in the
> arch's atomic.h.

Why? I don't think that it does. Have a look at how lockref is used by
the dcache code: it's really about keeping a reference to a dentry,
which may be in the process of being unhashed and removed. The
interaction with concurrent updaters to the dentry itself is handled
using a seqlock, which does have the necessary barriers. Yes, the code
is extremely complicated, but given that you're reporting issues based
on code inspection, then you'll need to understand what you're changing.

> Fortunately, the fix is not complicated: merely removing the errant
> _relaxed option on the cmpxchg64 is enough to introduce exactly the same
> code sequence justified in commit 8e86f0b409a44193f1587e87b69c5dcf8f65be67
> to fix arm64 atomics.

I introduced cmpxchg64_relaxed precisely for the lockref case. I still
don't see a compelling reason to strengthen it. If you think there's a bug,
please spend the effort to describe how it manifests and what can actually
go wrong in the existing codebase. Your previous patches fixing so-called
bugs found by inspection have both turned out to be bogus, so I'm sorry,
but I'm not exactly leaping on your contributions to this.

Will

^ permalink raw reply

* [PATCH V3 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section
From: Punit Agrawal @ 2016-10-13 10:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475875882-2604-9-git-send-email-tbaicar@codeaurora.org>

Hi Tyler,

One last comment...

Tyler Baicar <tbaicar@codeaurora.org> writes:

> UEFI spec allows for non-standard section in Common Platform Error
> Record. This is defined in section N.2.3 of UEFI version 2.5.
>
> Currently if the CPER section's type (UUID) does not match with
> any section type that the kernel knows how to parse, trace event
> is not generated for such section. And thus user is not able to know
> happening of such hardware error, including error record of
> non-standard section.
>
> This commit generates a trace event which contains raw error data
> for unrecognized CPER section.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> ---
>  drivers/acpi/apei/ghes.c | 18 +++++++++++++++++-
>  drivers/ras/ras.c        |  1 +
>  include/ras/ras_event.h  | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 36894c8..cb4c7f4 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -49,6 +49,7 @@
>  #include <acpi/ghes.h>
>  #include <acpi/apei.h>
>  #include <asm/tlbflush.h>
> +#include <ras/ras_event.h>
>  
>  #ifdef CONFIG_HAVE_ACPI_APEI_SEA
>  #include <asm/system_misc.h>
> @@ -468,12 +469,21 @@ static void ghes_do_proc(struct ghes *ghes,
>  	int sev, sec_sev;
>  	struct acpi_hest_generic_data *gdata;
>  	uuid_le sec_type;
> +	uuid_le *fru_id;
> +	char *fru_text = "";
>  
>  	sev = ghes_severity(estatus->error_severity);
>  	apei_estatus_for_each_section(estatus, gdata) {
>  		sec_sev = ghes_severity(gdata->error_severity);
>  		sec_type = *(uuid_le *)gdata->section_type;
>  
> +		if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
> +			fru_id = (uuid_le *)gdata->fru_id;
> +		else
> +			fru_id = &NULL_UUID_LE;

fru_id can be initialised at declaration and drop the else here. The
same is already being done for fru_text.

Thanks,
Punit

> +		if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
> +			fru_text = gdata->fru_text;
> +
>  		if (!uuid_le_cmp(sec_type,
>  				 CPER_SEC_PLATFORM_MEM)) {
>  			struct cper_sec_mem_err *mem_err;

[...]

^ permalink raw reply

* [PATCH] perf: xgene: Remove bogus IS_ERR() check
From: Mark Rutland @ 2016-10-13 10:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1476290367-8092-1-git-send-email-ttnguyen@apm.com>

On Wed, Oct 12, 2016 at 09:39:27AM -0700, Tai Nguyen wrote:
> This patch fixes the warning issue with static checker.
> The bug is reported in [1]
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg535957.html
> 
> Signed-off-by: Tai Nguyen <ttnguyen@apm.com>

Please put the problem description in the commit message:

  In acpi_get_pmu_hw_inf we pass the address of a local variable to IS_ERR(),
  which doesn't make sense, as the pointer must be a real, valid pointer. This
  doesn't cause a functional problem, as IS_ERR() will evaluate as false, but
  the check is bogus and causes static checkers to complain.

  Remove the bogus check.

Please also add a Reported-by for Dan.

The patch itself is fine, so with the above, you can add:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Please Cc Will Deacon when sending that, I expect that he will pick it up
(though perhaps not until rc1 given this is not a critical issue).

Thanks,
Mark.

> ---
>  drivers/perf/xgene_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
> index c2ac764..a8ac4bc 100644
> --- a/drivers/perf/xgene_pmu.c
> +++ b/drivers/perf/xgene_pmu.c
> @@ -1011,7 +1011,7 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
>  	rc = acpi_dev_get_resources(adev, &resource_list,
>  				    acpi_pmu_dev_add_resource, &res);
>  	acpi_dev_free_resource_list(&resource_list);
> -	if (rc < 0 || IS_ERR(&res)) {
> +	if (rc < 0) {
>  		dev_err(dev, "PMU type %d: No resource address found\n", type);
>  		goto err;
>  	}
> -- 
> 1.9.1
> 

^ permalink raw reply

* [PATCH 3/4] arm64: Allow hw watchpoint of length 3,5,6 and 7
From: Pratyush Anand @ 2016-10-13 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAH=s-PNFESLA6TTem-49g0CKsjpKOsj1XYfPUXz6YbEfMWXqXg@mail.gmail.com>



On Wednesday 12 October 2016 04:46 PM, Yao Qi wrote:
> On Wed, Oct 12, 2016 at 6:58 AM, Pratyush Anand <panand@redhat.com> wrote:
>> Since, arm64 can support all offset within a double word limit. Therefore,
>> now support other lengths within that range as well.
>
> How does ptracer (like GDB) detect kernel has already supported all byte
> address select values?  I suppose ptrace(NT_ARM_HW_WATCH, ) with
> len is 3 or 5 fail on current kernel but is of success after your patches
> applied.
>

Thanks for testing these patches.

I do not know if we can know that other than the failure of 
ptrace(PTRACE_SETREGSET, .., NT_ARM_HW_WATCH, ..). I do not see any such 
option in `man ptrace`.


> GDB is aware of the byte address select limitation in kernel, so it always
> sets 1,2,4,8 in len in ctrl.  GDB needs to know whether the limitation is still
> there or not.
>

Not sure if other than "kernel version" anything will help here.

~Pratyush

^ permalink raw reply

* [PATCH v2 1/2] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Marc Zyngier @ 2016-10-13 10:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1475604730-140264-1-git-send-email-briannorris@chromium.org>

On Tue,  4 Oct 2016 11:12:09 -0700
Brian Norris <briannorris@chromium.org> wrote:

> The ARM ARM specifies that the system counter "must be implemented in an
> always-on power domain," and so we try to use the counter as a source of
> timekeeping across suspend/resume. Unfortunately, some SoCs (e.g.,
> Rockchip's RK3399) do not keep the counter ticking properly when
> switched from their high-power clock to the lower-power clock used in
> system suspend. Support this quirk by adding a new device tree property.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply

* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pratyush Anand @ 2016-10-13  9:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJt8pk-HJYFUCxRuY5EV6KnM+kNP6mVBrFgPwiGfEkMKqGyjXw@mail.gmail.com>

Hi Pavel,

Thanks a lot for your testing.

On Wed, Oct 12, 2016 at 7:20 PM, Pavel Labath <labath@google.com> wrote:
> Hello Pratyush,
>
> I am sorry about the delay. I have finally had a chance to try out
> your changes today. Response inline.
>
> On 8 October 2016 at 06:10, Pratyush Anand <panand@redhat.com> wrote:
>> On Fri, Oct 7, 2016 at 10:54 PM, Pavel Labath <labath@google.com> wrote:
>>> The thing is, I have observed different behavior here depending on the
>>> exact hardware used. I don't have the exact parameters with me now,
>>> but I can look it up next week.
>>>
>>> The thing is that the spec is imprecise about what exact address the
>>> hardware can report for the watchpoint hit. I presume that is
>>> deliberate to give some leeway to implementers. The spec says the
>>> address can be anywhere in the range from the lowest memory address
>>> accessed by the instruction to the highest address watched by the
>>> watchpoint,
>>
>> I think, my patches should  be able to take care of the above condition.
> Unfortunately, the patch does not solve the problem for my hardware,
> because of the leeway you give in watchpoint_handler is not big
> enough. It does work however, if I change the line
>> if (addr + 7 < val + lens || addr > val + lene)
> to
>> if (addr + 15 < val + lens || addr > val + lene)

Yes, I missed that floating point register will be of size 16.

> I do not think we can assume that address reported by the hardware
> will be at most 7 bytes off from the address we put the watchpoint at.
> There is nothing in the spec that guarantees that, and it does not
> seem to be enough for some hardware. In fact, I am not sure we can
> assume 15 is enough either, but maybe it can do for now, until we can

Right. It might even be bigger, in case of cache maintenance instructions.

> find hardware that does not work with that (I haven't yet tried what
> happens it the watchpoint is triggered by cache management
> instructions, which can access much larger blocks of memory).

Not, sure, may be it can lie in cache line size range.

>
> For reference, the hardware in question is:
>> Processor : AArch64 Processor rev 0 (aarch64)
>> processor : 0
>> min_vddcx : 400000
>> min_vddmx : 490000
>> BogoMIPS : 38.00
>> Features : fp asimd evtstrm aes pmull sha1 sha2 crc32
>> CPU implementer : 0x51
>> CPU architecture: 8
>> CPU variant : 0x2
>> CPU part : 0x201
>> CPU revision : 0
>> CPU param : 300 468 468 621 939 299 445 445 621 1077
>> Hardware : Qualcomm Technologies, Inc MSM8996pro
>
> And this is how it behaves:
> The output of the test app triggering the watchpoint (I have set a
> single-byte watchpoint at 555556705f)
>>
>> Writing to: 555556705f, size: 1
>> Writing to: 555556705e, size: 2
>> Writing to: 555556705c, size: 4
>> Writing to: 5555567058, size: 8
>> Writing to: 5555567050, size: 16
>> Writing to: 5555567040, size: 32
>
> The addresses received by the kernel:
> [  251.812166] c1   3780 hw-breakpoint: watchpoint_handler: addr:
> 555556705f, val+lens: 555556705f, val+lene: 555556705f
> [  251.820341] c1   3781 hw-breakpoint: watchpoint_handler: addr:
> 555556705e, val+lens: 555556705f, val+lene: 555556705f
> [  251.825572] c0   3782 hw-breakpoint: watchpoint_handler: addr:
> 555556705c, val+lens: 555556705f, val+lene: 555556705f
> [  251.831085] c0   3783 hw-breakpoint: watchpoint_handler: addr:
> 5555567058, val+lens: 555556705f, val+lene: 555556705f
> [  251.835804] c0   3784 hw-breakpoint: watchpoint_handler: addr:
> 5555567050, val+lens: 555556705f, val+lene: 555556705f
> [  251.841350] c0   3785 hw-breakpoint: watchpoint_handler: addr:
> 5555567050, val+lens: 555556705f, val+lene: 555556705f
>
> Note that for the case of 16 and 32-byte access it returns the address
> 5555567050 -- this is why the "+15" is sufficient for me.
>
>
> The other thing I am not so sure about in your patch is that it has
> potential to mis-attribute the watchpoint hit if we have two
> watchpoints close together. For example, if I have first watchpoint on
> 0x1008-0x100f and a second one on 0x1000-0x1007, *and* the application
> writes one byte to 0x1004, then your code will still attribute the hit
> to the first watchpoint, even though it was not really triggered. This

Hummm..yes, thanks for pointing it out.
There could be only two solutions for it:
(1) We read instruction at the location regs->pc and analyse it and
find the size of read/write.
or(2) What you have suggested in your patch.

I think, its easier to go with your implementation. So, I have taken
your patch and updated my perf/upstream_arm64_devel branch. May be you
can give it a test for your test cases.

> is the reason I implemented my fix as a two-stage process, first
> looking for exact hits, and then falling back to the nearest one. That
> said, I don't know enough about the codebase to say if this is a real
> problem.
>
> On the plus side, I like the fact that we can watch arbitrary memory
> regions now, and the feature is working perfectly. :)
>

Thanks :-)

>
> My proposal would be to combine the two patches - take the byte mask
> handling code from yours, and the hit-attribution code from my patch.
> What do you think?

I am ok with merging them together as well as sending them as
different patch in my v2 series.

~Pratyush

^ permalink raw reply

* [PATCH 0/2] arm64 kexec-tools fixups
From: Simon Horman @ 2016-10-13  9:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ebe32436-6069-06c5-702f-d435d0dacd48@suse.com>

On Tue, Oct 11, 2016 at 12:20:41PM +0200, Matthias Brugger wrote:
> 
> 
> On 10/10/2016 11:22 PM, Geoff Levand wrote:
> >Hi Simon,
> >
> >Fixes for 'make tarball'.  Please apply.
> >
> >-Geoff
> >
> >The following changes since commit 6e8804f9ca67bee65aedd5e1cdf504737ab5c7ed:
> >
> >  multiboot: Use the "reserved" type for non-ram zones (2016-10-07 11:54:44 +0900)
> >
> >are available in the git repository at:
> >
> >  git://git.kernel.org/pub/scm/linux/kernel/git/geoff/kexec-tools.git for-merge-arm64-dist
> >
> >for you to fetch changes up to 3552a3e85a910688d932535549b35354a54927ad:
> >
> >  arm64: Add missing kexec dist files (2016-10-10 14:15:21 -0700)
> >
> >----------------------------------------------------------------
> >Geoff Levand (2):
> >      arm64: Cleanup kexec Makefile
> >      arm64: Add missing kexec dist files
> >
> > kexec/arch/arm64/Makefile | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> 
> Reviewed-by: Matthias Brugger <mbrugger@suse.com>

Thanks, applied.

^ permalink raw reply

* [PATCH v7 2/8] power: add power sequence library
From: Peter Chen @ 2016-10-13  8:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <10186696.bBGEe05eC7@phil>

On Thu, Oct 13, 2016 at 09:04:42AM +0200, Heiko Stuebner wrote:
> > > > +static int __init pwrseq_generic_register(void)
> > > > +{
> > > > +	struct pwrseq_generic *pwrseq_gen;
> > > > +	int i;
> > > > +
> > > > +	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
> > > > +		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> > > > +		if (!pwrseq_gen)
> > > > +			return -ENOMEM;
> > > > +
> > > > +		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
> > > > +		pwrseq_gen->pwrseq.get = pwrseq_generic_get;
> > > > +		pwrseq_gen->pwrseq.on = pwrseq_generic_on;
> > > > +		pwrseq_gen->pwrseq.off = pwrseq_generic_off;
> > > > +		pwrseq_gen->pwrseq.put = pwrseq_generic_put;
> > > > +		pwrseq_gen->pwrseq.free = pwrseq_generic_free;
> > > > +
> > > > +		pwrseq_register(&pwrseq_gen->pwrseq);
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +postcore_initcall(pwrseq_generic_register)
> > > 
> > > I see that you need to have it preallocated for the compatible matching,
> > > but wouldn't it also work to either just register the type and allocate
> > > dynamically or otherwise just allocate a new spare everytime
> > > pwrseq_generic_get() picks up the previous spare?
> > 
> > Before compatible matching, the host driver doesn't know which pwrseq type
> > for its child node, then doesn't know which pwrseq instance needs to be
> > allocated. From dts, we don't know which pwrseq type for the node.
> 
> yes, that is why I was suggesting allocating one (or two) here in 
> pwrseq_generic_register() and every time pwrseq_generic_get() grabs the last 
> free sequence instance, you allocate a new free spare one in that function.

Good idea.

-- 

Best Regards,
Peter Chen

^ permalink raw reply

* [PATCH] ARM: dts: rockchip: Reserve unusable memory region on rk3066
From: Paweł Jarosz @ 2016-10-13  8:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e3746f9a-084e-163e-a412-1a1fc66926d5@rock-chips.com>

Hi


W dniu 13.10.2016 o 09:12, Huang, Tao pisze:
> Hi, Pawe?:
> On 2016?10?10? 17:11, Pawe? Jarosz wrote:
>> W dniu 10.10.2016 o 09:18, Huang, Tao pisze:
>>> Our IC guy need us tell them which master can not access such area, DMA
>>> or EMMC Controller or GPU, etc? Could you tell me how to reproduce such
>>> issue?
>>> And we can confirm CPU core can access this memory through /dev/mem and
>>> the test board is 1GB too. Personally, I don't think RK3066 has such
>>> limit because when we verify this chip, we don't found such limit at all.
>>>
>>> Thanks,
>>> Huang, Tao
>> I'm getting this on Ubuntu 16.04 with mainline kernel.
>> My board always freezes when i type: "memtester 800M"
>>
> We try run memtest 800M with Linux kernel 4.8, which killed by OOM.
> But if we run:
> # memtester -p 0x9F000000 16K 1
> memtester version 4.3.0 (32-bit)
> Copyright (C) 2001-2012 Charles Cazabon.
> Licensed under the GNU General Public License version 2 (only).
>
> pagesize is 4096
> pagesizemask is 0xfffff000
> want 0MB (16384 bytes)
> Loop 1/1:
>     Stuck Address       : ok
>     Random Value        : ok
>     Compare XOR         : ok
>     Compare SUB         : ok
>     Compare MUL         : ok
>     Compare DIV         : ok
>     Compare OR          : ok
>     Compare AND         : ok
>     Sequential Increment: ok
>     Solid Bits          : ok
>     Block Sequential    : ok
>     Checkerboard        : ok
>     Bit Spread          : ok
>     Bit Flip            : ok
>     Walking Ones        : ok
>     Walking Zeroes      : ok
>     8-bit Writes        : ok
>     16-bit Writes       : ok
>
> So these memory should be fine to CPU core. Maybe your system just
> freeze because out of memory.
>

weird ... memtester -p 0x9F000000 16K 1 gave me the same result.

Could you try one last command:

	memtester 400M

Values > 200M causing freeze.

If this won't do it, than maybe there is something wrong with my board.

Thanks for your time.

^ permalink raw reply

* [PATCH V3 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Suzuki K Poulose @ 2016-10-13  8:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c17a0c03-5037-d88d-f096-4cc72dbfcb99@codeaurora.org>

On 12/10/16 23:10, Baicar, Tyler wrote:
> Hello Suzuki,
>
> Thank you for the feedback! Responses below.
>
>
> On 10/11/2016 11:28 AM, Suzuki K Poulose wrote:
>> On 07/10/16 22:31, Tyler Baicar wrote:
>>> Currently when a RAS error is reported it is not timestamped.
>>> The ACPI 6.1 spec adds the timestamp field to the generic error
>>> data entry v3 structure. The timestamp of when the firmware
>>> generated the error is now being reported.
>>>
>>> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>>> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
>>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>>> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
>>
>> Please could you keep the people who reviewed/commented on your series in the past,
>> whenever you post a new version ?
> Do you mean to just send the new version to their e-mail directly in addition to the lists? If so, I will do that next time.

If you send a new version of a series to the list, it is a good idea to keep
the people who commented (significantly) on your previous version in Cc, especially
when you have addressed their feedback. That will help them to keep track of the
series. People can always see the new version in the list, but then it is so easy
to miss something in the 100s of emails you get each day. I am sure people have
special filters for the emails based on if they are in Cc/To etc.

>
> I know you provided good feedback on the previous patchset, but I did not have anyone specifically respond to add "reviewed-by:...". I don't think I should add reviewed-by for someone unless they specifically add it in a response :)

No, I haven't yet "Reviewed-by" your patches. I had some comments on it, which means
I expected it to be addressed as you committed in your response.

>>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>>> index 3021f0e..c8488f1 100644
>>> --- a/drivers/acpi/apei/ghes.c
>>> +++ b/drivers/acpi/apei/ghes.c
>>> @@ -80,6 +80,10 @@

> I think that should work to avoid duplication. I will move them to a header file in the next patchset.
>>> +
>>> +static void cper_estatus_print_section_v300(const char *pfx,
>>> +    const struct acpi_hest_generic_data_v300 *gdata)
>>> +{
>>> +    __u8 hour, min, sec, day, mon, year, century, *timestamp;
>>> +
>>> +    if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
>>> +        timestamp = (__u8 *)&(gdata->time_stamp);
>>> +        memcpy(&sec, timestamp, 1);
>>> +        memcpy(&min, timestamp + 1, 1);
>>> +        memcpy(&hour, timestamp + 2, 1);
>>> +        memcpy(&day, timestamp + 4, 1);
>>> +        memcpy(&mon, timestamp + 5, 1);
>>> +        memcpy(&year, timestamp + 6, 1);
>>> +        memcpy(&century, timestamp + 7, 1);
>>> +        printk("%stime: ", pfx);
>>> +        printk("%7s", 0x01 & *(timestamp + 3) ? "precise" : "");
>>
>> What format is the (timestamp + 3) stored in ? Does it need conversion ?
> The third byte of the timestamp is currently only used to determine if the time is precise or not. Bit 0 is used to specify that and the other bits in this byte are marked as reserved. This is shown in table 247 of the UEFI spec 2.6:
>
> Byte 3:
>   Bit 0 ? Timestamp is precise if this bit is set and correlates to the time of the error event.
>   Bit 7:1 ? Reserved

Is it always the same endianness as that of the CPU ?

Cheers
Suzuki

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox