Re: [RFC PATCH v2 0/7] objtool: Enable and implement --mcount option on powerpc

2022-06-17 Thread Sathvika Vasireddy

Hi Christophe,

On 15/06/22 21:33, Christophe Leroy wrote:

Do you have any idea when you plan to send next revision ?

I'm really looking forward to submitting the inline static calls on top
of your series.


I'm planning to send RFC v3 next week.

- Sathvika



[PATCH v2] powerpc: embedded6xx: Fix refcount leak bugs

2022-06-17 Thread Liang He
In xx_init_xx(), of_find_node_by_type() will return a node pointer
with refcount incremented. We should use of_node_put() when it is
not used anymore.

Signed-off-by: Liang He 
---
 v2: we merge all embedded6xx related bugs into one commit
 v1: we only report the bug in holly_init_pci() of holly.c

 arch/powerpc/platforms/embedded6xx/holly.c| 6 ++
 arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/platforms/embedded6xx/holly.c 
b/arch/powerpc/platforms/embedded6xx/holly.c
index 78f2378d9223..bebc5a972694 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -123,6 +123,8 @@ static void __init holly_init_pci(void)
if (np)
tsi108_setup_pci(np, HOLLY_PCI_CFG_PHYS, 1);
 
+   of_node_put(np);
+
ppc_md.pci_exclude_device = holly_exclude_device;
if (ppc_md.progress)
ppc_md.progress("tsi108: resources set", 0x100);
@@ -184,6 +186,9 @@ static void __init holly_init_IRQ(void)
tsi108_pci_int_init(cascade_node);
irq_set_handler_data(cascade_pci_irq, mpic);
irq_set_chained_handler(cascade_pci_irq, tsi108_irq_cascade);
+
+   of_node_put(tsi_pci);
+   of_node_put(cascade_node);
 #endif
/* Configure MPIC outputs to CPU0 */
tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
@@ -210,6 +215,7 @@ static void __noreturn holly_restart(char *cmd)
if (bridge) {
prop = of_get_property(bridge, "reg", );
addr = of_translate_address(bridge, prop);
+   of_node_put(bridge);
}
addr += (TSI108_PB_OFFSET + 0x414);
 
diff --git a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c 
b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
index 8b2b42210356..ddf0c652af80 100644
--- a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
+++ b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
@@ -135,6 +135,9 @@ static void __init mpc7448_hpc2_init_IRQ(void)
tsi108_pci_int_init(cascade_node);
irq_set_handler_data(cascade_pci_irq, mpic);
irq_set_chained_handler(cascade_pci_irq, tsi108_irq_cascade);
+
+   of_node_put(tsi_pci);
+   of_node_put(cascade_node);
 #endif
/* Configure MPIC outputs to CPU0 */
tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
-- 
2.25.1



[PATCH] powerpc: embedded6xx: Fix refcount leak bug in holly.c

2022-06-17 Thread Liang He
In holly_init_pci(), of_find_node_by_type() will return a node
pointer with refcount incremented. We should use of_node_put()
when it is not used anymore.

Signed-off-by: Liang He 
---
 arch/powerpc/platforms/embedded6xx/holly.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/embedded6xx/holly.c 
b/arch/powerpc/platforms/embedded6xx/holly.c
index 78f2378d9223..a4938505e89a 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -122,6 +122,7 @@ static void __init holly_init_pci(void)
np = of_find_node_by_type(NULL, "pci");
if (np)
tsi108_setup_pci(np, HOLLY_PCI_CFG_PHYS, 1);
+   of_node_put(np);
 
ppc_md.pci_exclude_device = holly_exclude_device;
if (ppc_md.progress)
-- 
2.25.1



Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread Baolin Wang




On 6/18/2022 1:17 AM, Mike Kravetz wrote:

On 06/17/22 10:15, Peter Xu wrote:

Hi, Mike,

On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote:

@@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
return (pte_t *)pmd;
  }
  
+/*

+ * Return a mask that can be used to update an address to the last huge
+ * page in a page table page mapping size.  Used to skip non-present
+ * page table entries when linearly scanning address ranges.  Architectures
+ * with unique huge page to page table relationships can define their own
+ * version of this routine.
+ */
+unsigned long hugetlb_mask_last_page(struct hstate *h)
+{
+   unsigned long hp_size = huge_page_size(h);
+
+   switch (hp_size) {
+   case P4D_SIZE:
+   return PGDIR_SIZE - P4D_SIZE;
+   case PUD_SIZE:
+   return P4D_SIZE - PUD_SIZE;
+   case PMD_SIZE:
+   return PUD_SIZE - PMD_SIZE;
+   default:


Should we add a WARN_ON_ONCE() if it should never trigger?



Sure.  I will add this.


+   break; /* Should never happen */
+   }
+
+   return ~(0UL);
+}
+
+#else
+
+/* See description above.  Architectures can provide their own version. */
+__weak unsigned long hugetlb_mask_last_page(struct hstate *h)
+{
+   return ~(0UL);


I'm wondering whether it's better to return 0 rather than ~0 by default.
Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some
valid address ranges with ~0, or perhaps I misread?


Thank you, thank you, thank you Peter!

Yes, the 'default' return for hugetlb_mask_last_page() should be 0.  If
there is no 'optimization', we do not want to modify the address so we
want to OR with 0 not ~0.  My bad, I must have been thinking AND instead
of OR.

I will change here as well as in Baolin's patch.


Ah, I also overlooked this. Thanks Peter, and thanks Mike for updating.


[PATCH] powerpc: sysdev: Fix refcount leak bug in fsl_pci.c

2022-06-17 Thread Liang He
In is_kdump(), we need a of_node_put() to dec the refcount which is
incremented by of_find_node_by_type().

Signed-off-by: Liang He 
---
 arch/powerpc/sysdev/fsl_pci.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 1011cfea2e32..4c986c955951 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -180,6 +180,7 @@ static int setup_one_atmu(struct ccsr_pci __iomem *pci,
 static bool is_kdump(void)
 {
struct device_node *node;
+   bool ret;
 
node = of_find_node_by_type(NULL, "memory");
if (!node) {
@@ -187,7 +188,10 @@ static bool is_kdump(void)
return false;
}
 
-   return of_property_read_bool(node, "linux,usable-memory");
+   ret = of_property_read_bool(node, "linux,usable-memory");
+   of_node_put(node);
+
+   return ret;
 }
 
 /* atmu setup for fsl pci/pcie controller */
-- 
2.25.1



[PATCH] powerpc: 8xx: Fix refcount leak bug in tqm8xx_setup

2022-06-17 Thread Liang He
In init_ioports(), of_find_node_by_name() will return a node pointer
with refcount incremented. We should use of_node_put() when it is not
used anymore.

Signed-off-by: Liang He 
---
 arch/powerpc/platforms/8xx/tqm8xx_setup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/platforms/8xx/tqm8xx_setup.c 
b/arch/powerpc/platforms/8xx/tqm8xx_setup.c
index 3725d51248df..ffcfd17a5fa3 100644
--- a/arch/powerpc/platforms/8xx/tqm8xx_setup.c
+++ b/arch/powerpc/platforms/8xx/tqm8xx_setup.c
@@ -105,6 +105,9 @@ static void __init init_ioports(void)
if (dnode == NULL)
return;
prop = of_find_property(dnode, "ethernet1", );
+
+   of_node_put(dnode);
+
if (prop == NULL)
return;
 
-- 
2.25.1



[PATCH] cpufreq: pmac32-cpufreq: Fix refcount leak bug

2022-06-17 Thread Liang He
In pmac_cpufreq_init_MacRISC3(), we need to add corresponding
of_node_put() for the three node pointers whose refcount have
been incremented by of_find_node_by_name().

Signed-off-by: Liang He 
---
 drivers/cpufreq/pmac32-cpufreq.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c
index 20f64a8b0a35..4b8ee2014da6 100644
--- a/drivers/cpufreq/pmac32-cpufreq.c
+++ b/drivers/cpufreq/pmac32-cpufreq.c
@@ -470,6 +470,10 @@ static int pmac_cpufreq_init_MacRISC3(struct device_node 
*cpunode)
if (slew_done_gpio_np)
slew_done_gpio = read_gpio(slew_done_gpio_np);
 
+   of_node_put(volt_gpio_np);
+   of_node_put(freq_gpio_np);
+   of_node_put(slew_done_gpio_np);
+
/* If we use the frequency GPIOs, calculate the min/max speeds based
 * on the bus frequencies
 */
-- 
2.25.1



[PATCH] powerpc: powernv: opal-core: Fix refcount leak bug

2022-06-17 Thread Liang He
In create_opalcore(), of_find_node_by_name() will return a node
pointer with refcount incremented. We should use of_node_put()
when it is not used anymore.

Signed-off-by: Liang He 
---
 arch/powerpc/platforms/powernv/opal-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/opal-core.c 
b/arch/powerpc/platforms/powernv/opal-core.c
index adcb1a1a2bfe..bb7657115f1d 100644
--- a/arch/powerpc/platforms/powernv/opal-core.c
+++ b/arch/powerpc/platforms/powernv/opal-core.c
@@ -348,6 +348,8 @@ static int __init create_opalcore(void)
if (!dn || ret)
pr_warn("WARNING: Failed to read OPAL base & entry values\n");
 
+   of_node_put(dn);
+
/* Use count to keep track of the program headers */
count = 0;
 
-- 
2.25.1



Re: [PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS

2022-06-17 Thread Michael Schmitz

Arnd,

Am 18.06.2022 um 00:57 schrieb Arnd Bergmann:

From: Arnd Bergmann 

All architecture-independent users of virt_to_bus() and bus_to_virt()
have been fixed to use the dma mapping interfaces or have been
removed now.  This means the definitions on most architectures, and the
CONFIG_VIRT_TO_BUS symbol are now obsolete and can be removed.

The only exceptions to this are a few network and scsi drivers for m68k
Amiga and VME machines and ppc32 Macintosh. These drivers work correctly
with the old interfaces and are probably not worth changing.


The Amiga SCSI drivers are all old WD33C93 ones, and replacing 
virt_to_bus by virt_to_phys in the dma_setup() function there would 
cause no functional change at all.


drivers/vme/bridges/vme_ca91cx42.c hasn't been used at all on m68k (it 
is a PCI-to-VME bridge chipset driver that would be needed on 
architectures that natively use a PCI bus). I haven't found anything 
that selects that driver, so not sure it is even still in use??


That would allow you to drop the remaining virt_to_bus define from 
arch/m68k/include/asm/virtconvert.h.


I could submit a patch to convert the Amiga SCSI drivers to use 
virt_to_phys if Geert and the SCSI maintainers think it's worth the churn.


32bit powerpc is a different matter though.

Cheers,

Michael


Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread Mike Kravetz
On 06/17/22 19:26, kernel test robot wrote:
> Hi Mike,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on soc/for-next]
> [also build test ERROR on linus/master v5.19-rc2 next-20220617]
> [cannot apply to arm64/for-next/core arm/for-next kvmarm/next 
> xilinx-xlnx/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:
> https://github.com/intel-lab-lkp/linux/commits/Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
> config: i386-randconfig-a002 
> (https://download.01.org/0day-ci/archive/20220617/202206171929.ziurng6p-...@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 
> f0e608de27b3d56846eebf3712ab542979d6)
> reproduce (this is a W=1 build):
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # 
> https://github.com/intel-lab-lkp/linux/commit/4c647687607f10fece04967b8180c0dadaf765e6
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review 
> Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726
> git checkout 4c647687607f10fece04967b8180c0dadaf765e6
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 
> O=build_dir ARCH=i386 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot 
> 
> All errors (new ones prefixed by >>):

A couple of things here,

> 
> >> mm/hugetlb.c:6901:7: error: duplicate case value '4194304'
>case PUD_SIZE:
> ^
>include/asm-generic/pgtable-nopud.h:20:20: note: expanded from macro 
> 'PUD_SIZE'
>#define PUD_SIZE(1UL << PUD_SHIFT)
>^
>mm/hugetlb.c:6899:7: note: previous case defined here
>case P4D_SIZE:
> ^
>include/asm-generic/pgtable-nop4d.h:13:19: note: expanded from macro 
> 'P4D_SIZE'
>#define P4D_SIZE(1UL << P4D_SHIFT)

In the CONFIG_ARCH_WANT_GENERAL_HUGETLB case covered by this version of
hugetlb_mask_last_page, huge pages can only be PMD_SIZE or PUD_SIZE.
So, the 'case P4D_SIZE:' should not exist and can be removed.

>^
>mm/hugetlb.c:6903:7: error: duplicate case value '4194304'
>case PMD_SIZE:
> ^
>include/asm-generic/pgtable-nopmd.h:22:20: note: expanded from macro 
> 'PMD_SIZE'
>#define PMD_SIZE(1UL << PMD_SHIFT)
>^

On i386 with CONFIG_PGTABLE_LEVELS=2, PUD_SIZE == PMD_SIZE.
Originally, I coded this as a if .. else if ... statement instead of a
switch.  If coded this way, the compiler does not complain about the
duplicate values.  The only other alternative I can think of is
something like '#if CONFIG_PGTABLE_LEVELS > 2' around the PUD_SIZE case.

I would prefer the if else if, unless someone can suggest something else?
-- 
Mike Kravetz


Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread kernel test robot
Hi Mike,

I love your patch! Yet something to improve:

[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220617]
[cannot apply to arm64/for-next/core arm/for-next kvmarm/next 
xilinx-xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/intel-lab-lkp/linux/commits/Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
config: i386-debian-10.3 
(https://download.01.org/0day-ci/archive/20220618/202206180021.rcc4b1by-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# 
https://github.com/intel-lab-lkp/linux/commit/4c647687607f10fece04967b8180c0dadaf765e6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726
git checkout 4c647687607f10fece04967b8180c0dadaf765e6
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   mm/hugetlb.c: In function 'hugetlb_mask_last_page':
>> mm/hugetlb.c:6901:9: error: duplicate case value
6901 | case PUD_SIZE:
 | ^~~~
   mm/hugetlb.c:6899:9: note: previously used here
6899 | case P4D_SIZE:
 | ^~~~


vim +6901 mm/hugetlb.c

  6886  
  6887  /*
  6888   * Return a mask that can be used to update an address to the last huge
  6889   * page in a page table page mapping size.  Used to skip non-present
  6890   * page table entries when linearly scanning address ranges.  
Architectures
  6891   * with unique huge page to page table relationships can define their 
own
  6892   * version of this routine.
  6893   */
  6894  unsigned long hugetlb_mask_last_page(struct hstate *h)
  6895  {
  6896  unsigned long hp_size = huge_page_size(h);
  6897  
  6898  switch (hp_size) {
  6899  case P4D_SIZE:
  6900  return PGDIR_SIZE - P4D_SIZE;
> 6901  case PUD_SIZE:
  6902  return P4D_SIZE - PUD_SIZE;
  6903  case PMD_SIZE:
  6904  return PUD_SIZE - PMD_SIZE;
  6905  default:
  6906  break; /* Should never happen */
  6907  }
  6908  
  6909  return ~(0UL);
  6910  }
  6911  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread Mike Kravetz
On 06/17/22 10:15, Peter Xu wrote:
> Hi, Mike,
> 
> On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote:
> > @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
> > return (pte_t *)pmd;
> >  }
> >  
> > +/*
> > + * Return a mask that can be used to update an address to the last huge
> > + * page in a page table page mapping size.  Used to skip non-present
> > + * page table entries when linearly scanning address ranges.  Architectures
> > + * with unique huge page to page table relationships can define their own
> > + * version of this routine.
> > + */
> > +unsigned long hugetlb_mask_last_page(struct hstate *h)
> > +{
> > +   unsigned long hp_size = huge_page_size(h);
> > +
> > +   switch (hp_size) {
> > +   case P4D_SIZE:
> > +   return PGDIR_SIZE - P4D_SIZE;
> > +   case PUD_SIZE:
> > +   return P4D_SIZE - PUD_SIZE;
> > +   case PMD_SIZE:
> > +   return PUD_SIZE - PMD_SIZE;
> > +   default:
> 
> Should we add a WARN_ON_ONCE() if it should never trigger?
> 

Sure.  I will add this.

> > +   break; /* Should never happen */
> > +   }
> > +
> > +   return ~(0UL);
> > +}
> > +
> > +#else
> > +
> > +/* See description above.  Architectures can provide their own version. */
> > +__weak unsigned long hugetlb_mask_last_page(struct hstate *h)
> > +{
> > +   return ~(0UL);
> 
> I'm wondering whether it's better to return 0 rather than ~0 by default.
> Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some
> valid address ranges with ~0, or perhaps I misread?

Thank you, thank you, thank you Peter!

Yes, the 'default' return for hugetlb_mask_last_page() should be 0.  If
there is no 'optimization', we do not want to modify the address so we
want to OR with 0 not ~0.  My bad, I must have been thinking AND instead
of OR.

I will change here as well as in Baolin's patch.
-- 
Mike Kravetz


Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread kernel test robot
Hi Mike,

I love your patch! Yet something to improve:

[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220617]
[cannot apply to arm64/for-next/core arm/for-next kvmarm/next 
xilinx-xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/intel-lab-lkp/linux/commits/Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
config: i386-randconfig-a002 
(https://download.01.org/0day-ci/archive/20220617/202206171929.ziurng6p-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 
f0e608de27b3d56846eebf3712ab542979d6)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/intel-lab-lkp/linux/commit/4c647687607f10fece04967b8180c0dadaf765e6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726
git checkout 4c647687607f10fece04967b8180c0dadaf765e6
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 
O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

>> mm/hugetlb.c:6901:7: error: duplicate case value '4194304'
   case PUD_SIZE:
^
   include/asm-generic/pgtable-nopud.h:20:20: note: expanded from macro 
'PUD_SIZE'
   #define PUD_SIZE(1UL << PUD_SHIFT)
   ^
   mm/hugetlb.c:6899:7: note: previous case defined here
   case P4D_SIZE:
^
   include/asm-generic/pgtable-nop4d.h:13:19: note: expanded from macro 
'P4D_SIZE'
   #define P4D_SIZE(1UL << P4D_SHIFT)
   ^
   mm/hugetlb.c:6903:7: error: duplicate case value '4194304'
   case PMD_SIZE:
^
   include/asm-generic/pgtable-nopmd.h:22:20: note: expanded from macro 
'PMD_SIZE'
   #define PMD_SIZE(1UL << PMD_SHIFT)
   ^
   mm/hugetlb.c:6901:7: note: previous case defined here
   case PUD_SIZE:
^
   include/asm-generic/pgtable-nopud.h:20:20: note: expanded from macro 
'PUD_SIZE'
   #define PUD_SIZE(1UL << PUD_SHIFT)
   ^
   2 errors generated.


vim +/4194304 +6901 mm/hugetlb.c

  6886  
  6887  /*
  6888   * Return a mask that can be used to update an address to the last huge
  6889   * page in a page table page mapping size.  Used to skip non-present
  6890   * page table entries when linearly scanning address ranges.  
Architectures
  6891   * with unique huge page to page table relationships can define their 
own
  6892   * version of this routine.
  6893   */
  6894  unsigned long hugetlb_mask_last_page(struct hstate *h)
  6895  {
  6896  unsigned long hp_size = huge_page_size(h);
  6897  
  6898  switch (hp_size) {
  6899  case P4D_SIZE:
  6900  return PGDIR_SIZE - P4D_SIZE;
> 6901  case PUD_SIZE:
  6902  return P4D_SIZE - PUD_SIZE;
  6903  case PMD_SIZE:
  6904  return PUD_SIZE - PMD_SIZE;
  6905  default:
  6906  break; /* Should never happen */
  6907  }
  6908  
  6909  return ~(0UL);
  6910  }
  6911  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


[PATCH] KVM: PPC: use __func__ to get funcion's name in an output message

2022-06-17 Thread XueBing Chen

Prefer using '"%s...", __func__' to get current function's name in
an output message.

Signed-off-by: XueBing Chen 
---
 arch/powerpc/kvm/book3s_64_mmu_hv.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c 
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 0aeb51738ca9..e795ed3d1346 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1194,7 +1194,7 @@ static int resize_hpt_allocate(struct kvm_resize_hpt 
*resize)
if (rc < 0)
return rc;
 
-   resize_hpt_debug(resize, "resize_hpt_allocate(): HPT @ 0x%lx\n",
+   resize_hpt_debug(resize, "%s(): HPT @ 0x%lx\n", __func__,
 resize->hpt.virt);
 
return 0;
@@ -1435,7 +1435,7 @@ static void resize_hpt_prepare_work(struct work_struct 
*work)
 */
mutex_unlock(>arch.mmu_setup_lock);
 
-   resize_hpt_debug(resize, "resize_hpt_prepare_work(): order = 
%d\n",
+   resize_hpt_debug(resize, "%s(): order = %d\n", __func__,
 resize->order);
 
err = resize_hpt_allocate(resize);
@@ -1879,8 +1879,7 @@ static ssize_t kvm_htab_write(struct file *file, const 
char __user *buf,
ret = kvmppc_virtmode_do_h_enter(kvm, H_EXACT, i, v, r,
 tmp);
if (ret != H_SUCCESS) {
-   pr_err("kvm_htab_write ret %ld i=%ld v=%lx "
-  "r=%lx\n", ret, i, v, r);
+   pr_err("%s ret %ld i=%ld v=%lx r=%lx\n", 
__func__, ret, i, v, r);
goto out;
}
if (!mmu_ready && is_vrma_hpte(v)) {
-- 
2.36.1


Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread Geert Uytterhoeven
Hi Peter,

On Fri, Jun 17, 2022 at 4:22 PM Peter Xu  wrote:
> On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote:
> > @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
> >   return (pte_t *)pmd;
> >  }
> >
> > +/*
> > + * Return a mask that can be used to update an address to the last huge
> > + * page in a page table page mapping size.  Used to skip non-present
> > + * page table entries when linearly scanning address ranges.  Architectures
> > + * with unique huge page to page table relationships can define their own
> > + * version of this routine.
> > + */
> > +unsigned long hugetlb_mask_last_page(struct hstate *h)
> > +{
> > + unsigned long hp_size = huge_page_size(h);
> > +
> > + switch (hp_size) {
> > + case P4D_SIZE:
> > + return PGDIR_SIZE - P4D_SIZE;
> > + case PUD_SIZE:
> > + return P4D_SIZE - PUD_SIZE;
> > + case PMD_SIZE:
> > + return PUD_SIZE - PMD_SIZE;
> > + default:
>
> Should we add a WARN_ON_ONCE() if it should never trigger?

And with panic_on_warn, it'll panic only once ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] KVM: PPC: Align pt_regs in kvm_vcpu_arch structure

2022-06-17 Thread Murilo Opsfelder Araújo

Hi, Fabiano.

On 5/25/22 09:49, Fabiano Rosas wrote:

The H_ENTER_NESTED hypercall receives as second parameter the address
of a region of memory containing the values for the nested guest
privileged registers. We currently use the pt_regs structure contained
within kvm_vcpu_arch for that end.

Most hypercalls that receive a memory address expect that region to
not cross a 4k page boundary. We would want H_ENTER_NESTED to follow
the same pattern so this patch ensures the pt_regs structure sits
within a page.

Signed-off-by: Fabiano Rosas 


Is it necessary to explain in the commit message that even though the second
parameter needs to be 4k-aligned, we're aligning pt_regs to 512 bytes so it can
be placed within a 4k boundary because its size is below 512 bytes?

The natural thinking would be aligning it to 4k bytes, which would punch a huge
hole in kvm_vcpu_arch. I think having the explanation of why 512 vs. 4k is
worthwhile mentioning.


---
  arch/powerpc/include/asm/kvm_host.h | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h 
b/arch/powerpc/include/asm/kvm_host.h
index faf301d0dec0..87eba60f2920 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -519,7 +519,11 @@ struct kvm_vcpu_arch {
struct kvmppc_book3s_shadow_vcpu *shadow_vcpu;
  #endif
  
-	struct pt_regs regs;

+   /*
+* This is passed along to the HV via H_ENTER_NESTED. Align to
+* prevent it crossing a real 4K page.
+*/
+   struct pt_regs regs __aligned(512);
  
  	struct thread_fp_state fp;
  



--
Murilo


[PATCH] powerpc: sysdev: Fix refcount leak bug in mpic_msgr

2022-06-17 Thread Liang He
In mpic_msgr_number_of_blocks() and mpic_msgr_block_number(),
of_find_node_by_name() will return a node pointer with refcount
incremented. We should use of_node_put() when it is not used
anymore.

Signed-off-by: Liang He 
---
 arch/powerpc/sysdev/mpic_msgr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/sysdev/mpic_msgr.c b/arch/powerpc/sysdev/mpic_msgr.c
index 698fefaaa6dd..c2a235d9eb09 100644
--- a/arch/powerpc/sysdev/mpic_msgr.c
+++ b/arch/powerpc/sysdev/mpic_msgr.c
@@ -122,6 +122,7 @@ static unsigned int mpic_msgr_number_of_blocks(void)
count += 1;
}
}
+   of_node_put(aliases);
 
return count;
 }
@@ -150,6 +151,7 @@ static int mpic_msgr_block_number(struct device_node *node)
if (node == of_find_node_by_path(prop->value))
break;
}
+   of_node_put(aliases);
 
return index == number_of_blocks ? -1 : index;
 }
-- 
2.25.1



[PATCH powerpc] powerpc/bpf: Fix use of user_pt_regs in uapi

2022-06-17 Thread Naveen N. Rao
This is a partial revert of commit a6460b03f945ee ("powerpc/bpf: Fix
broken uapi for BPF_PROG_TYPE_PERF_EVENT").

Unlike x86, powerpc has both pt_regs and user_pt_regs structures. As
such, we still need to override perf_arch_bpf_user_pt_regs() so that the
correct user_regs structure is used.

However, unlike arm64 and s390, we expose user_pt_regs to userspace as
just 'pt_regs'. Due to this, trying to #include 
throws the below error:
  /usr/include/linux/bpf_perf_event.h:14:28: error: field ‘regs’ has incomplete 
type
 14 | bpf_user_pt_regs_t regs;
|^~~~

Note that this was not showing up with the bpf selftest build since
tools/include/uapi/asm/bpf_perf_event.h didn't include the powerpc
variant.

Fix this by removing arch/powerpc/include/uapi/asm/bpf_perf_event.h,
allowing fallback to the asm-generic version.

Fixes: a6460b03f945ee ("powerpc/bpf: Fix broken uapi for 
BPF_PROG_TYPE_PERF_EVENT")
Cc: sta...@vger.kernel.org # v4.20+
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/include/uapi/asm/bpf_perf_event.h | 9 -
 1 file changed, 9 deletions(-)
 delete mode 100644 arch/powerpc/include/uapi/asm/bpf_perf_event.h

diff --git a/arch/powerpc/include/uapi/asm/bpf_perf_event.h 
b/arch/powerpc/include/uapi/asm/bpf_perf_event.h
deleted file mode 100644
index 5e1e648aeec4c8..00
--- a/arch/powerpc/include/uapi/asm/bpf_perf_event.h
+++ /dev/null
@@ -1,9 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__
-#define _UAPI__ASM_BPF_PERF_EVENT_H__
-
-#include 
-
-typedef struct user_pt_regs bpf_user_pt_regs_t;
-
-#endif /* _UAPI__ASM_BPF_PERF_EVENT_H__ */

base-commit: bcd1c02813b8ab4ae019c65ffb716c9f579868e7
-- 
2.36.1



Re:Re: [PATCH] powerpc: powernv: Fix refcount leak bug in opal-powercap

2022-06-17 Thread Liang He



At 2022-06-17 13:01:27, "Christophe JAILLET"  
wrote:
>Le 17/06/2022 à 06:20, Liang He a écrit :
>> In opal_powercap_init(), of_find_compatible_node() will return
>> a node pointer with refcount incremented. We should use of_node_put()
>> in fail path or when it is not used anymore.
>> 
>> Besides, for_each_child_of_node() will automatically *inc* and *dec*
>> refcount during iteration. However, we should add the of_node_put()
>> if there is a break.
>
>Hi,
>
>I'm not sure that your patch is right here. Because of this *inc* and 
>*dec* things, do we still need to of_node_put(powercap) once we have 
>entered for_each_child_of_node?
>
>I think that this reference will be released on the first iteration of 
>the loop.
>
>
>Maybe of_node_put(powercap) should be duplicated everywhere it is 
>relevant and removed from the error handling path?
>Or an additional reference should be taken before the loop?
>Or adding a new label with "powercap = NULL" and branching there when 
>needed?
>
>CJ
>
>> 
>> Signed-off-by: Liang He 
>> ---
>>   arch/powerpc/platforms/powernv/opal-powercap.c | 5 -
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/platforms/powernv/opal-powercap.c 
>> b/arch/powerpc/platforms/powernv/opal-powercap.c
>> index 64506b46e77b..b102477d3f95 100644
>> --- a/arch/powerpc/platforms/powernv/opal-powercap.c
>> +++ b/arch/powerpc/platforms/powernv/opal-powercap.c
>> @@ -153,7 +153,7 @@ void __init opal_powercap_init(void)
>>  pcaps = kcalloc(of_get_child_count(powercap), sizeof(*pcaps),
>>  GFP_KERNEL);
>>  if (!pcaps)
>> -return;
>> +goto out_powercap;
>>   
>>  powercap_kobj = kobject_create_and_add("powercap", opal_kobj);
>>  if (!powercap_kobj) {
>> @@ -236,6 +236,9 @@ void __init opal_powercap_init(void)
>>  kfree(pcaps[i].pg.name);
>>  }
>>  kobject_put(powercap_kobj);
>> +of_node_put(node);
>>   out_pcaps:
>>  kfree(pcaps);
>> +out_powercap:
>> +of_node_put(powercap);
>>   }

Hi, CJ.

I think my patch is correct based on the old commit: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.19-rc2=09700c504d8e63faffd2a2235074e8c5d130cb8f

Bugs and fix solutions in this 09700c504d8e63-commit are very similar with mine.

Besides, I also find similar new bugs in other two files in the same directory 
'powernv', 
so I have merged all three files' patches into one commit.  '[PATCH v2] 
powerpc: powernv: Fix refcount leak bug'.

Thanks.

Liang

[PATCH v2] powerpc: powernv: Fix refcount leak bug

2022-06-17 Thread Liang He
In these driver init functions, there are two kinds of errors:

(1) missing of_put_node() for of_find_compatible_node()'s returned
pointer (refcount incremented)  in fail path or when it is not
used anymore.
(2) missing of_put_node() for 'for_each_xxx' loop's break

These bugs are similar with the ones reported by commit-09700c504d.

Signed-off-by: Liang He 
---
 changelog:
 
 v2: merge all powernv related bugs into one commit 
 v1: only fix bugs in opal-powercap.c


 arch/powerpc/platforms/powernv/opal-powercap.c  | 6 +-
 arch/powerpc/platforms/powernv/opal-psr.c   | 6 +-
 arch/powerpc/platforms/powernv/opal-sensor-groups.c | 6 +-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-powercap.c 
b/arch/powerpc/platforms/powernv/opal-powercap.c
index 64506b46e77b..78c359c90093 100644
--- a/arch/powerpc/platforms/powernv/opal-powercap.c
+++ b/arch/powerpc/platforms/powernv/opal-powercap.c
@@ -153,7 +153,7 @@ void __init opal_powercap_init(void)
pcaps = kcalloc(of_get_child_count(powercap), sizeof(*pcaps),
GFP_KERNEL);
if (!pcaps)
-   return;
+   goto out_powercap;
 
powercap_kobj = kobject_create_and_add("powercap", opal_kobj);
if (!powercap_kobj) {
@@ -226,6 +226,7 @@ void __init opal_powercap_init(void)
}
i++;
}
+   of_node_put(powercap);
 
return;
 
@@ -236,6 +237,9 @@ void __init opal_powercap_init(void)
kfree(pcaps[i].pg.name);
}
kobject_put(powercap_kobj);
+   of_node_put(node);
 out_pcaps:
kfree(pcaps);
+out_powercap:
+   of_node_put(powercap);
 }
diff --git a/arch/powerpc/platforms/powernv/opal-psr.c 
b/arch/powerpc/platforms/powernv/opal-psr.c
index 69d7e75950d1..ec32e0a93f08 100644
--- a/arch/powerpc/platforms/powernv/opal-psr.c
+++ b/arch/powerpc/platforms/powernv/opal-psr.c
@@ -135,7 +135,7 @@ void __init opal_psr_init(void)
psr_attrs = kcalloc(of_get_child_count(psr), sizeof(*psr_attrs),
GFP_KERNEL);
if (!psr_attrs)
-   return;
+   goto out_psr;
 
psr_kobj = kobject_create_and_add("psr", opal_kobj);
if (!psr_kobj) {
@@ -162,10 +162,14 @@ void __init opal_psr_init(void)
}
i++;
}
+   of_node_put(psr);
 
return;
 out_kobj:
+   of_node_put(node);
kobject_put(psr_kobj);
 out:
kfree(psr_attrs);
+out_psr:
+   of_node_put(psr);
 }
diff --git a/arch/powerpc/platforms/powernv/opal-sensor-groups.c 
b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
index 8fba7d25ae56..9944376b115c 100644
--- a/arch/powerpc/platforms/powernv/opal-sensor-groups.c
+++ b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
@@ -170,7 +170,7 @@ void __init opal_sensor_groups_init(void)
 
sgs = kcalloc(of_get_child_count(sg), sizeof(*sgs), GFP_KERNEL);
if (!sgs)
-   return;
+   goto out_sg_put;
 
sg_kobj = kobject_create_and_add("sensor_groups", opal_kobj);
if (!sg_kobj) {
@@ -222,6 +222,7 @@ void __init opal_sensor_groups_init(void)
}
i++;
}
+   of_node_put(sg);
 
return;
 
@@ -231,6 +232,9 @@ void __init opal_sensor_groups_init(void)
kfree(sgs[i].sg.attrs);
}
kobject_put(sg_kobj);
+   of_node_put(node);
 out_sgs:
kfree(sgs);
+out_sg_put:
+   of_node_put(sg);
 }
-- 
2.25.1



Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread Peter Xu
Hi, Mike,

On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote:
> @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
>   return (pte_t *)pmd;
>  }
>  
> +/*
> + * Return a mask that can be used to update an address to the last huge
> + * page in a page table page mapping size.  Used to skip non-present
> + * page table entries when linearly scanning address ranges.  Architectures
> + * with unique huge page to page table relationships can define their own
> + * version of this routine.
> + */
> +unsigned long hugetlb_mask_last_page(struct hstate *h)
> +{
> + unsigned long hp_size = huge_page_size(h);
> +
> + switch (hp_size) {
> + case P4D_SIZE:
> + return PGDIR_SIZE - P4D_SIZE;
> + case PUD_SIZE:
> + return P4D_SIZE - PUD_SIZE;
> + case PMD_SIZE:
> + return PUD_SIZE - PMD_SIZE;
> + default:

Should we add a WARN_ON_ONCE() if it should never trigger?

> + break; /* Should never happen */
> + }
> +
> + return ~(0UL);
> +}
> +
> +#else
> +
> +/* See description above.  Architectures can provide their own version. */
> +__weak unsigned long hugetlb_mask_last_page(struct hstate *h)
> +{
> + return ~(0UL);

I'm wondering whether it's better to return 0 rather than ~0 by default.
Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some
valid address ranges with ~0, or perhaps I misread?

Thanks,

-- 
Peter Xu



Re: [RFC PATCH 4/4] objtool/powerpc: Add --mcount specific implementation

2022-06-17 Thread Naveen N. Rao

Christophe Leroy wrote:



Le 16/06/2022 à 15:57, Peter Zijlstra a écrit :

On Thu, Jun 16, 2022 at 01:40:34PM +, Christophe Leroy wrote:

sizeof(u64) is always 8 by definition.

So if size is 8 we are working on a binary file for a 64 bits target, if
not it means we are working for a 32 bits target.


Cross-builds invalidate this I think. Best to look at something like:

   elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32




Yes that's what it does indirectly:

int size = elf_class_size(elf);


With

static inline int elf_class_size(struct elf *elf)
{
if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
return sizeof(u32);
else
return sizeof(u64);
}


Ok, those come from the below patch:
https://lore.kernel.org/all/c4b06b5b314183d85615765a5ce421a057674bd8.1653398233.git.christophe.le...@csgroup.eu/T/#u

I guess it would have been clearer if 'size' was named differently: 
'addr_size' perhaps?



- Naveen


Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt

2022-06-17 Thread Robin Murphy

On 2022-06-17 13:57, Arnd Bergmann wrote:

From: Arnd Bergmann 

The BusLogic driver is the last remaining driver that relies on the
deprecated bus_to_virt() function, which in turn only works on a few
architectures, and is incompatible with both swiotlb and iommu support.

Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
the driver had a dependency on x86-32, presumably because of this
problem. However, the change introduced another bug that made it still
impossible to use the driver on any 64-bit machine.

This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
64-bit system enumeration error for Buslogic"), 8 years later, which
shows that there are not a lot of users.

Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
that the driver works with the device emulation used in VirtualBox
and VMware. Both of those only emulate it for Windows 2000 and older
operating systems that did not ship with the better LSI logic driver.

Do a minimum fix that searches through the list of descriptors to find
one that matches the bus address. This is clearly as inefficient as
was indicated in the code comment about the lack of a bus_to_virt()
replacement. A better fix would likely involve changing out the entire
descriptor allocation for a simpler one, but that would be much
more invasive.


FWIW, if efficiency *is* a practical concern, even under the current 
allocation scheme it looks like there are only 4 actual DMA allocations 
to search through. From a quick scan (since it's too hot here not to get 
distracted...), if I wanted to optimise this in future I'd probably 
remove the semi-redundant allocgrp_* fields from struct blogic_ccb and 
hang a dedicated list of the block allocations off the adapter - at that 
point the lookup could likely already be more efficient than a 
theoretical dma_to_virt() interface would be if it had to go off and 
walk an IOMMU pagetable. Then the next question would be whether it's 
viable to make a single 32KB allocation rather 4*8KB, so it's no longer 
even a list.


For now, though, I agree with the simple change that's clear and easy to 
reason about:


Reviewed-by: Robin Murphy 

Thanks,
Robin.


Cc: Maciej W. Rozycki 
Cc: Matt Wang 
Cc: Khalid Aziz 
Signed-off-by: Arnd Bergmann 
---
  drivers/scsi/BusLogic.c | 27 ---
  drivers/scsi/Kconfig|  2 +-
  2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index a897c8f914cf..d057abfcdd5c 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter 
*adapter,
return (hoststatus << 16) | tgt_status;
  }
  
+/*

+ * turn the dma address from an inbox into a ccb pointer
+ * This is rather inefficient.
+ */
+static struct blogic_ccb *
+blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
+{
+   struct blogic_ccb *ccb;
+
+   for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
+   if (inbox->ccb == ccb->dma_handle)
+   break;
+
+   return ccb;
+}
  
  /*

blogic_scan_inbox scans the Incoming Mailboxes saving any
Incoming Mailbox entries for completion processing.
  */
-
  static void blogic_scan_inbox(struct blogic_adapter *adapter)
  {
/*
@@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter 
*adapter)
enum blogic_cmplt_code comp_code;
  
  	while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {

-   /*
-  We are only allowed to do this because we limit our
-  architectures we run on to machines where bus_to_virt(
-  actually works.  There *needs* to be a dma_addr_to_virt()
-  in the new PCI DMA mapping interface to replace
-  bus_to_virt() or else this code is going to become very
-  innefficient.
-*/
-   struct blogic_ccb *ccb =
-   (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
+   struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, 
adapter->next_inbox);
if (comp_code != BLOGIC_CMD_NOTFOUND) {
if (ccb->status == BLOGIC_CCB_ACTIVE ||
ccb->status == BLOGIC_CCB_RESET) {
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index cf75588a2587..56bdc08d0b77 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -513,7 +513,7 @@ config SCSI_HPTIOP
  
  config SCSI_BUSLOGIC

tristate "BusLogic SCSI support"
-   depends on PCI && SCSI && VIRT_TO_BUS
+   depends on PCI && SCSI
help
  This is support for BusLogic MultiMaster and FlashPoint SCSI Host
  Adapters. Consult the SCSI-HOWTO, available from


Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

2022-06-17 Thread Jacob Pan
, Arnd Bergmann , ulli.kr...@googlemail.com, vgu...@kernel.org, 
linux-...@vger.kernel.org, j...@joshtriplett.org, rost...@goodmis.org, 
r...@vger.kernel.org, b...@alien8.de, bc...@quicinc.com, 
tsbog...@alpha.franken.de, linux-par...@vger.kernel.org, sudeep.ho...@arm.com, 
shawn...@kernel.org, da...@davemloft.net, dal...@libc.org, t...@atomide.com, 
amakha...@vmware.com, bjorn.anders...@linaro.org, h...@zytor.com, 
sparcli...@vger.kernel.org, linux-hexa...@vger.kernel.org, 
linux-ri...@lists.infradead.org, anton.iva...@cambridgegreys.com, 
jo...@southpole.se, yury.no...@gmail.com, rich...@nod.at, x...@kernel.org, 
li...@armlinux.org.uk, mi...@redhat.com, a...@eecs.berkeley.edu, 
paul...@kernel.org, h...@linux.ibm.com, stefan.kristians...@saunalahti.fi, 
openr...@lists.librecores.org, paul.walms...@sifive.com, 
linux-te...@vger.kernel.org, namhy...@kernel.org, 
andriy.shevche...@linux.intel.com, jpoim...@kernel.org, jgr...@suse.com, 
mon...@monstr.eu, linux-m...@vger.kernel.org, palmer@dab
 belt.com, a...@brainfault.org, i...@jurassic.park.msu.ru, 
johan...@sipsolutions.net, linuxppc-dev@lists.ozlabs.org
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev" 


Hi Peter,

On Mon, 13 Jun 2022 10:44:22 +0200, Peter Zijlstra 
wrote:

> On Thu, Jun 09, 2022 at 04:49:21PM -0700, Jacob Pan wrote:
> > Hi Peter,
> > 
> > On Wed, 08 Jun 2022 16:27:27 +0200, Peter Zijlstra
> >  wrote:
> >   
> > > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > Xeons") wrecked intel_idle in two ways:
> > > 
> > >  - must not have tracing in idle functions
> > >  - must return with IRQs disabled
> > > 
> > > Additionally, it added a branch for no good reason.
> > > 
> > > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > Xeons") Signed-off-by: Peter Zijlstra (Intel) 
> > > ---
> > >  drivers/idle/intel_idle.c |   48
> > > +++--- 1 file changed, 37
> > > insertions(+), 11 deletions(-)
> > > 
> > > --- a/drivers/idle/intel_idle.c
> > > +++ b/drivers/idle/intel_idle.c
> > > @@ -129,21 +137,37 @@ static unsigned int mwait_substates __in
> > >   *
> > >   * Must be called under local_irq_disable().
> > >   */  
> > nit: this comment is no long true, right?  
> 
> It still is, all the idle routines are called with interrupts disabled,
> but must also exit with interrupts disabled.
> 
> If the idle method requires interrupts to be enabled, it must be sure to
> disable them again before returning. Given all the RCU/tracing concerns
> it must use raw_local_irq_*() for this though.
Makes sense, it is just little confusing when the immediate caller does
raw_local_irq_enable() which does not cancel out local_irq_disable().

Thanks,

Jacob


[PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS

2022-06-17 Thread Arnd Bergmann
From: Arnd Bergmann 

All architecture-independent users of virt_to_bus() and bus_to_virt()
have been fixed to use the dma mapping interfaces or have been
removed now.  This means the definitions on most architectures, and the
CONFIG_VIRT_TO_BUS symbol are now obsolete and can be removed.

The only exceptions to this are a few network and scsi drivers for m68k
Amiga and VME machines and ppc32 Macintosh. These drivers work correctly
with the old interfaces and are probably not worth changing.

On alpha and parisc, virt_to_bus() were still used in asm/floppy.h.
alpha can use isa_virt_to_bus() like x86 does, and parisc can just
open-code the virt_to_phys() here, as this is architecture specific
code.

I tried updating the bus-virt-phys-mapping.rst documentation, which
started as an email from Linus to explain some details of the Linux-2.0
driver interfaces. The bits about virt_to_bus() were declared obsolete
backin 2000, and the rest is not all that relevant any more, so in the
end I just decided to remove the file completely.

Reviewed-by: Geert Uytterhoeven 
Acked-by: Geert Uytterhoeven 
Acked-by: Michael Ellerman  (powerpc)
Signed-off-by: Arnd Bergmann 
---
 .../core-api/bus-virt-phys-mapping.rst| 220 --
 Documentation/core-api/dma-api-howto.rst  |  14 --
 Documentation/core-api/index.rst  |   1 -
 .../translations/zh_CN/core-api/index.rst |   1 -
 arch/alpha/Kconfig|   1 -
 arch/alpha/include/asm/floppy.h   |   2 +-
 arch/alpha/include/asm/io.h   |   8 +-
 arch/ia64/Kconfig |   1 -
 arch/ia64/include/asm/io.h|   8 -
 arch/m68k/Kconfig |   1 -
 arch/m68k/include/asm/virtconvert.h   |   4 +-
 arch/microblaze/Kconfig   |   1 -
 arch/microblaze/include/asm/io.h  |   2 -
 arch/mips/Kconfig |   1 -
 arch/mips/include/asm/io.h|   9 -
 arch/parisc/Kconfig   |   1 -
 arch/parisc/include/asm/floppy.h  |   4 +-
 arch/parisc/include/asm/io.h  |   2 -
 arch/powerpc/Kconfig  |   1 -
 arch/powerpc/include/asm/io.h |   2 -
 arch/riscv/include/asm/page.h |   1 -
 arch/x86/Kconfig  |   1 -
 arch/x86/include/asm/io.h |   9 -
 arch/xtensa/Kconfig   |   1 -
 arch/xtensa/include/asm/io.h  |   3 -
 include/asm-generic/io.h  |  14 --
 mm/Kconfig|   8 -
 27 files changed, 10 insertions(+), 311 deletions(-)
 delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst

diff --git a/Documentation/core-api/bus-virt-phys-mapping.rst 
b/Documentation/core-api/bus-virt-phys-mapping.rst
deleted file mode 100644
index c72b24a7d52c..
--- a/Documentation/core-api/bus-virt-phys-mapping.rst
+++ /dev/null
@@ -1,220 +0,0 @@
-==
-How to access I/O mapped memory from within device drivers
-==
-
-:Author: Linus
-
-.. warning::
-
-   The virt_to_bus() and bus_to_virt() functions have been
-   superseded by the functionality provided by the PCI DMA interface
-   (see Documentation/core-api/dma-api-howto.rst).  They continue
-   to be documented below for historical purposes, but new code
-   must not use them. --davidm 00/12/12
-
-::
-
-  [ This is a mail message in response to a query on IO mapping, thus the
-strange format for a "document" ]
-
-The AHA-1542 is a bus-master device, and your patch makes the driver give the
-controller the physical address of the buffers, which is correct on x86
-(because all bus master devices see the physical memory mappings directly). 
-
-However, on many setups, there are actually **three** different ways of looking
-at memory addresses, and in this case we actually want the third, the
-so-called "bus address". 
-
-Essentially, the three ways of addressing memory are (this is "real memory",
-that is, normal RAM--see later about other details): 
-
- - CPU untranslated.  This is the "physical" address.  Physical address 
-   0 is what the CPU sees when it drives zeroes on the memory bus.
-
- - CPU translated address. This is the "virtual" address, and is 
-   completely internal to the CPU itself with the CPU doing the appropriate
-   translations into "CPU untranslated". 
-
- - bus address. This is the address of memory as seen by OTHER devices, 
-   not the CPU. Now, in theory there could be many different bus 
-   addresses, with each device seeing memory in some device-specific way, but
-   happily most hardware designers aren't actually actively trying to make
-   things any more complex than necessary, so you can assume that all 
-   external 

[PATCH v2 2/3] scsi: BusLogic remove bus_to_virt

2022-06-17 Thread Arnd Bergmann
From: Arnd Bergmann 

The BusLogic driver is the last remaining driver that relies on the
deprecated bus_to_virt() function, which in turn only works on a few
architectures, and is incompatible with both swiotlb and iommu support.

Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."),
the driver had a dependency on x86-32, presumably because of this
problem. However, the change introduced another bug that made it still
impossible to use the driver on any 64-bit machine.

This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix
64-bit system enumeration error for Buslogic"), 8 years later, which
shows that there are not a lot of users.

Maciej is still using the driver on 32-bit hardware, and Khalid mentioned
that the driver works with the device emulation used in VirtualBox
and VMware. Both of those only emulate it for Windows 2000 and older
operating systems that did not ship with the better LSI logic driver.

Do a minimum fix that searches through the list of descriptors to find
one that matches the bus address. This is clearly as inefficient as
was indicated in the code comment about the lack of a bus_to_virt()
replacement. A better fix would likely involve changing out the entire
descriptor allocation for a simpler one, but that would be much
more invasive.

Cc: Maciej W. Rozycki 
Cc: Matt Wang 
Cc: Khalid Aziz 
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/BusLogic.c | 27 ---
 drivers/scsi/Kconfig|  2 +-
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index a897c8f914cf..d057abfcdd5c 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter 
*adapter,
return (hoststatus << 16) | tgt_status;
 }
 
+/*
+ * turn the dma address from an inbox into a ccb pointer
+ * This is rather inefficient.
+ */
+static struct blogic_ccb *
+blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox)
+{
+   struct blogic_ccb *ccb;
+
+   for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all)
+   if (inbox->ccb == ccb->dma_handle)
+   break;
+
+   return ccb;
+}
 
 /*
   blogic_scan_inbox scans the Incoming Mailboxes saving any
   Incoming Mailbox entries for completion processing.
 */
-
 static void blogic_scan_inbox(struct blogic_adapter *adapter)
 {
/*
@@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter 
*adapter)
enum blogic_cmplt_code comp_code;
 
while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) {
-   /*
-  We are only allowed to do this because we limit our
-  architectures we run on to machines where bus_to_virt(
-  actually works.  There *needs* to be a dma_addr_to_virt()
-  in the new PCI DMA mapping interface to replace
-  bus_to_virt() or else this code is going to become very
-  innefficient.
-*/
-   struct blogic_ccb *ccb =
-   (struct blogic_ccb *) bus_to_virt(next_inbox->ccb);
+   struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, 
adapter->next_inbox);
if (comp_code != BLOGIC_CMD_NOTFOUND) {
if (ccb->status == BLOGIC_CCB_ACTIVE ||
ccb->status == BLOGIC_CCB_RESET) {
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index cf75588a2587..56bdc08d0b77 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -513,7 +513,7 @@ config SCSI_HPTIOP
 
 config SCSI_BUSLOGIC
tristate "BusLogic SCSI support"
-   depends on PCI && SCSI && VIRT_TO_BUS
+   depends on PCI && SCSI
help
  This is support for BusLogic MultiMaster and FlashPoint SCSI Host
  Adapters. Consult the SCSI-HOWTO, available from
-- 
2.29.2



[PATCH v2 1/3] scsi: dpt_i2o: drop stale VIRT_TO_BUS dependency

2022-06-17 Thread Arnd Bergmann
From: Arnd Bergmann 

The dpt_i2o driver was fixed to stop using virt_to_bus() in 2008, but
it still has a stale reference in an error handling code path that could
never work.

Fix it up to build without VIRT_TO_BUS and remove the Kconfig dependency.

The alternative to this would be to just remove the driver, as it is
clearly obsolete. The i2o driver layer was removed in 2015 with commit
4a72a7af462d ("staging: remove i2o subsystem"), but the even older
dpt_i2o scsi driver stayed around.

The last non-cleanup patches I could find were from Miquel van Smoorenburg
and Mark Salyzyn back in 2008, they might know if there is any chance
of the hardware still being used anywhere.

Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt to 
dma_alloc_coherent")
Cc: Miquel van Smoorenburg 
Cc: Mark Salyzyn 
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/Kconfig   | 2 +-
 drivers/scsi/dpt_i2o.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index a9fe5152addd..cf75588a2587 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -460,7 +460,7 @@ config SCSI_MVUMI
 
 config SCSI_DPT_I2O
tristate "Adaptec I2O RAID support "
-   depends on SCSI && PCI && VIRT_TO_BUS
+   depends on SCSI && PCI
help
  This driver supports all of Adaptec's I2O based RAID controllers as 
  well as the DPT SmartRaid V cards.  This is an Adaptec maintained
diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
index 2e9155ba7408..55dfe7011912 100644
--- a/drivers/scsi/dpt_i2o.c
+++ b/drivers/scsi/dpt_i2o.c
@@ -52,11 +52,11 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver");
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include  /* for boot_cpu_data */
-#include /* for virt_to_bus, etc. */
 
 #include 
 #include 
@@ -2112,7 +2112,7 @@ static irqreturn_t adpt_isr(int irq, void *dev_id)
} else {
/* Ick, we should *never* be here */
printk(KERN_ERR "dpti: reply frame not from pool\n");
-   reply = (u8 *)bus_to_virt(m);
+   goto out;
}
 
if (readl(reply) & MSG_FAIL) {
-- 
2.29.2



[PATCH v2 0/3] phase out CONFIG_VIRT_TO_BUS

2022-06-17 Thread Arnd Bergmann
From: Arnd Bergmann 

The virt_to_bus/bus_to_virt interface has been deprecated for
decades. After Jakub Kicinski put a lot of work into cleaning out the
network drivers using them, there are only a couple of other drivers
left, which can all be removed or otherwise cleaned up, to remove the
old interface for good.

Any out of tree drivers using virt_to_bus() should be converted to
using the dma-mapping interfaces, typically dma_alloc_coherent()
or dma_map_single()).

There are a few m68k and ppc32 specific drivers that keep using the
interfaces, but these are all guarded with architecture-specific
Kconfig dependencies, and are not actually broken.

There are still a number of drivers that are using virt_to_phys()
and phys_to_virt() in place of dma-mapping operations, and these
are often broken, but they are out of scope for this series.

I would like the first two patches to either get merged through
the SCSI tree, or get an Ack from the SCSI maintainers so I can
merge them through the asm-generic tree

  Arnd

---
Changes since v1:
 - dropped VME patches that are already in staging-next
 - dropped media patch that gets merged independently
 - added a networking patch and dropped it again after it got merged
 - replace BusLogic removal with a workaround

Cc: Jakub Kicinski 
Cc: Christoph Hellwig  # dma-mapping
Cc: Marek Szyprowski  # dma-mapping
Cc: Robin Murphy  # dma-mapping
Cc: io...@lists.linux-foundation.org
Cc: Khalid Aziz  # buslogic
Cc: Maciej W. Rozycki  # buslogic
Cc: Matt Wang  # buslogic
Cc: Miquel van Smoorenburg  # dpt_i2o
Cc: Mark Salyzyn  # dpt_i2o
Cc: linux-s...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-a...@vger.kernel.org
Cc: linux-al...@vger.kernel.org
Cc: linux-m...@lists.linux-m68k.org
Cc: linux-par...@vger.kernel.org
Cc: Denis Efremov  # floppy

Arnd Bergmann (3):
  scsi: dpt_i2o: drop stale VIRT_TO_BUS dependency
  scsi: BusLogic remove bus_to_virt
  arch/*/: remove CONFIG_VIRT_TO_BUS

 .../core-api/bus-virt-phys-mapping.rst| 220 --
 Documentation/core-api/dma-api-howto.rst  |  14 --
 Documentation/core-api/index.rst  |   1 -
 .../translations/zh_CN/core-api/index.rst |   1 -
 arch/alpha/Kconfig|   1 -
 arch/alpha/include/asm/floppy.h   |   2 +-
 arch/alpha/include/asm/io.h   |   8 +-
 arch/ia64/Kconfig |   1 -
 arch/ia64/include/asm/io.h|   8 -
 arch/m68k/Kconfig |   1 -
 arch/m68k/include/asm/virtconvert.h   |   4 +-
 arch/microblaze/Kconfig   |   1 -
 arch/microblaze/include/asm/io.h  |   2 -
 arch/mips/Kconfig |   1 -
 arch/mips/include/asm/io.h|   9 -
 arch/parisc/Kconfig   |   1 -
 arch/parisc/include/asm/floppy.h  |   4 +-
 arch/parisc/include/asm/io.h  |   2 -
 arch/powerpc/Kconfig  |   1 -
 arch/powerpc/include/asm/io.h |   2 -
 arch/riscv/include/asm/page.h |   1 -
 arch/x86/Kconfig  |   1 -
 arch/x86/include/asm/io.h |   9 -
 arch/xtensa/Kconfig   |   1 -
 arch/xtensa/include/asm/io.h  |   3 -
 drivers/scsi/BusLogic.c   |  27 ++-
 drivers/scsi/Kconfig  |   4 +-
 drivers/scsi/dpt_i2o.c|   4 +-
 include/asm-generic/io.h  |  14 --
 mm/Kconfig|   8 -
 30 files changed, 30 insertions(+), 326 deletions(-)
 delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst

-- 
2.29.2



Re: [PATCH v2 0/4] pseries-wdt: initial support for H_WATCHDOG-based watchdog timers

2022-06-17 Thread Daniel Henrique Barboza




On 6/2/22 14:53, Scott Cheloha wrote:

PAPR v2.12 defines a new hypercall, H_WATCHDOG.  This patch series
adds support for this hypercall to powerpc/pseries kernels and
introduces a new watchdog driver, "pseries-wdt", for the virtual
timers exposed by the hypercall.

This series is preceded by the following:

RFC v1: 
https://lore.kernel.org/linux-watchdog/20220413165104.179144-1-chel...@linux.ibm.com/
RFC v2: 
https://lore.kernel.org/linux-watchdog/20220509174357.5448-1-chel...@linux.ibm.com/
PATCH v1: 
https://lore.kernel.org/linux-watchdog/20220520183552.33426-1-chel...@linux.ibm.com/


Tested successfully with mainline QEMU plus Alexey's new h_watchdog patches in
https://patchwork.ozlabs.org/project/qemu-ppc/list/?series=305226.


All patches of this series:

Tested-by: Daniel Henrique Barboza 


Thanks,


Daniel



Changes of note from PATCH v1:

- Trim down the large comment documenting the H_WATCHDOG hypercall.
   The comment is likely to rot, so remove anything we aren't using
   and anything overly obvious.

- Remove any preprocessor definitions not actually used in the module
   right now.  If we want to use other features offered by the hypercall
   we can add them in later.  They're just clutter until then.

- Simplify the "action" module parameter.  The value is now an index
   into an array of possible timeoutAction values.  This design removes
   the need for the custom get/set methods used in PATCH v1.

   Now we merely need to check that the "action" value is a valid
   index during pseries_wdt_probe().  Easy.

- Make the timeoutAction a member of pseries_wdt, "action".  This
   eliminates the use of a global variable during pseries_wdt_start().

- Use watchdog_init_timeout() idiomatically.  Check its return value
   and error out of pseries_wdt_probe() if it fails.




[PATCH] powerpc: maple: Fix refcount leak bug in time.c

2022-06-17 Thread Liang He
In maple_get_boot_time(), of_find_compatible_node() will return
a node pointer with refcount incremented. We should use of_node_put()
in fail path or when it is not used anymore.

Signed-off-by: Liang He 
---
 arch/powerpc/platforms/maple/time.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/maple/time.c 
b/arch/powerpc/platforms/maple/time.c
index 823e219ef8ee..91606411d2e0 100644
--- a/arch/powerpc/platforms/maple/time.c
+++ b/arch/powerpc/platforms/maple/time.c
@@ -153,6 +153,7 @@ time64_t __init maple_get_boot_time(void)
   maple_rtc_addr);
}
  bail:
+   of_node_put(rtcs);
if (maple_rtc_addr == 0) {
maple_rtc_addr = RTC_PORT(0); /* legacy address */
printk(KERN_INFO "Maple: No device node for RTC, assuming "
-- 
2.25.1



Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He



在 2022-06-17 18:57:36,"Christophe Leroy"  写道:
>
>
>Le 17/06/2022 à 12:47, Liang He a écrit :
>> 
>> 
>> 
>> At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote:
>>> On 17/06/2022 09:17, Liang He wrote:



 At 2022-06-17 14:53:13, "Christophe Leroy"  
 wrote:
>
>
> Le 17/06/2022 à 08:45, Liang He a écrit :
>>
>>
>>
>> At 2022-06-17 14:28:56, "Christophe Leroy"  
>> wrote:
>>>
>>>
>>> Le 17/06/2022 à 08:08, Liang He a écrit :
 In gpio_halt_probe(), of_find_matching_node() will return a node
 pointer with refcount incremented. We should use of_node_put() in
 fail path or when it is not used anymore.

 Signed-off-by: Liang He 
 ---
  changelog:
  v4: reuse exist 'err' and use a simple code style, advised by CJ
  v3: use local 'child_node' advised by Michael.
  v2: use goto-label patch style advised by Christophe Leroy.
  v1: add of_node_put() before each exit.

  arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 
 ++-
  1 file changed, 22 insertions(+), 13 deletions(-)

 diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
 b/arch/powerpc/platforms/85xx/sgy_cts1000.c
 index 98ae64075193..e4588943fe7e 100644
 --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
 +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
 @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device 
 *pdev)
  {
enum of_gpio_flags flags;
struct device_node *node = pdev->dev.of_node;
 +  struct device_node *child_node;
int gpio, err, irq;
int trigger;
  
 @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device 
 *pdev)
return -ENODEV;
  
/* If there's no matching child, this isn't really an error */
 -  halt_node = of_find_matching_node(node, child_match);
 -  if (!halt_node)
 +  child_node = of_find_matching_node(node, child_match);
 +  if (!child_node)
return 0;
  
/* Technically we could just read the first one, but punish
 * DT writers for invalid form. */
 -  if (of_gpio_count(halt_node) != 1)
 -  return -EINVAL;
 +  if (of_gpio_count(child_node) != 1) {
 +  err = -EINVAL;
 +  goto err_put;
 +  }
  
/* Get the gpio number relative to the dynamic base. */
 -  gpio = of_get_gpio_flags(halt_node, 0, );
 -  if (!gpio_is_valid(gpio))
 -  return -EINVAL;
 +  gpio = of_get_gpio_flags(child_node, 0, );
 +  if (!gpio_is_valid(gpio)) {
 +  err = -EINVAL;
 +  gotot err_put;
>>>
>>> Did you test the build ?
>>
>> Sorry for this fault.
>>
>> In fact, I am still finding an efficient way to building different arch 
>> source code as I only have x86-64.
>>
>> Now I am try using QEMU.
>>
>> Anyway, sorry for this fault.
>
> You can find cross compilers for most architectures for x86-64 here :
> https://mirrors.edge.kernel.org/pub/tools/crosstool/
>
> Christophe

 Hi, Christophe and Conor.

 Sorry to trouble you again.

 Now I only know how to quickly identify the refcounting bugs, but I cannot 
 efficiently give a build test.

 For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 
 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag.
 But I meet too many header file missing errors. Even if I add some 
 'include' pathes, e.g., ./arch/powerpc/include, ./include,
 there are still too many other errors.

 So if there is any efficient way to check my patch code to avoid 'gotot' 
 error again.
>>>
>>> idk anything about powerpc, but what I find is a nice way to get a compiler
>>> for an arch I don't use is to search on lore.kernel.org for a 0day robot
>>> build error since it gives instructions for building on that arch.
>>> For example:
>>> https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/
>>>
>>>
>>> In this case, your bug seems obvious? You typed "gotot" instead of "goto".
>>>
>>> Hope that helps,
>>> Conor.
>>>

 Thanks again, Christophe and Conor.

 Liang
>> 
>> Thanks, Conor and Christophe.
>> 
>> I finally figure out an efficient way in which I can use cross-compiler to 
>> check my single patched file as follow:
>> 
>> powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d  -nostdinc 
>> -I./arch/powerpc/include 

[PATCH] powerpc: kernel: Change the order of of_node_put()

2022-06-17 Thread Liang He
In add_pcspkr(), it is better to call of_node_put() after the
'if(!np)' check.

Signed-off-by: Liang He 
---
 arch/powerpc/kernel/setup-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/setup-common.c 
b/arch/powerpc/kernel/setup-common.c
index eb0077b302e2..761817d1f4db 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -563,9 +563,9 @@ static __init int add_pcspkr(void)
int ret;
 
np = of_find_compatible_node(NULL, NULL, "pnpPNP,100");
-   of_node_put(np);
if (!np)
return -ENODEV;
+   of_node_put(np);
 
pd = platform_device_alloc("pcspkr", -1);
if (!pd)
-- 
2.25.1



Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Christophe Leroy


Le 17/06/2022 à 12:47, Liang He a écrit :
> 
> 
> 
> At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote:
>> On 17/06/2022 09:17, Liang He wrote:
>>>
>>>
>>>
>>> At 2022-06-17 14:53:13, "Christophe Leroy"  
>>> wrote:


 Le 17/06/2022 à 08:45, Liang He a écrit :
>
>
>
> At 2022-06-17 14:28:56, "Christophe Leroy"  
> wrote:
>>
>>
>> Le 17/06/2022 à 08:08, Liang He a écrit :
>>> In gpio_halt_probe(), of_find_matching_node() will return a node
>>> pointer with refcount incremented. We should use of_node_put() in
>>> fail path or when it is not used anymore.
>>>
>>> Signed-off-by: Liang He 
>>> ---
>>>  changelog:
>>>  v4: reuse exist 'err' and use a simple code style, advised by CJ
>>>  v3: use local 'child_node' advised by Michael.
>>>  v2: use goto-label patch style advised by Christophe Leroy.
>>>  v1: add of_node_put() before each exit.
>>>
>>>  arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 
>>> ++-
>>>  1 file changed, 22 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
>>> b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>> index 98ae64075193..e4588943fe7e 100644
>>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device 
>>> *pdev)
>>>  {
>>> enum of_gpio_flags flags;
>>> struct device_node *node = pdev->dev.of_node;
>>> +   struct device_node *child_node;
>>> int gpio, err, irq;
>>> int trigger;
>>>  
>>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device 
>>> *pdev)
>>> return -ENODEV;
>>>  
>>> /* If there's no matching child, this isn't really an error */
>>> -   halt_node = of_find_matching_node(node, child_match);
>>> -   if (!halt_node)
>>> +   child_node = of_find_matching_node(node, child_match);
>>> +   if (!child_node)
>>> return 0;
>>>  
>>> /* Technically we could just read the first one, but punish
>>>  * DT writers for invalid form. */
>>> -   if (of_gpio_count(halt_node) != 1)
>>> -   return -EINVAL;
>>> +   if (of_gpio_count(child_node) != 1) {
>>> +   err = -EINVAL;
>>> +   goto err_put;
>>> +   }
>>>  
>>> /* Get the gpio number relative to the dynamic base. */
>>> -   gpio = of_get_gpio_flags(halt_node, 0, );
>>> -   if (!gpio_is_valid(gpio))
>>> -   return -EINVAL;
>>> +   gpio = of_get_gpio_flags(child_node, 0, );
>>> +   if (!gpio_is_valid(gpio)) {
>>> +   err = -EINVAL;
>>> +   gotot err_put;
>>
>> Did you test the build ?
>
> Sorry for this fault.
>
> In fact, I am still finding an efficient way to building different arch 
> source code as I only have x86-64.
>
> Now I am try using QEMU.
>
> Anyway, sorry for this fault.

 You can find cross compilers for most architectures for x86-64 here :
 https://mirrors.edge.kernel.org/pub/tools/crosstool/

 Christophe
>>>
>>> Hi, Christophe and Conor.
>>>
>>> Sorry to trouble you again.
>>>
>>> Now I only know how to quickly identify the refcounting bugs, but I cannot 
>>> efficiently give a build test.
>>>
>>> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 
>>> 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag.
>>> But I meet too many header file missing errors. Even if I add some 
>>> 'include' pathes, e.g., ./arch/powerpc/include, ./include,
>>> there are still too many other errors.
>>>
>>> So if there is any efficient way to check my patch code to avoid 'gotot' 
>>> error again.
>>
>> idk anything about powerpc, but what I find is a nice way to get a compiler
>> for an arch I don't use is to search on lore.kernel.org for a 0day robot
>> build error since it gives instructions for building on that arch.
>> For example:
>> https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/
>>
>>
>> In this case, your bug seems obvious? You typed "gotot" instead of "goto".
>>
>> Hope that helps,
>> Conor.
>>
>>>
>>> Thanks again, Christophe and Conor.
>>>
>>> Liang
> 
> Thanks, Conor and Christophe.
> 
> I finally figure out an efficient way in which I can use cross-compiler to 
> check my single patched file as follow:
> 
> powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d  -nostdinc 
> -I./arch/powerpc/include -I./arch/powerpc/include/generated  -I./include 
> -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi 
> -I./include/uapi -I./include/generated/uapi 

[PATCH v5] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He
In gpio_halt_probe(), of_find_matching_node() will return a node
pointer with refcount incremented. We should use of_node_put() in
fail path or when it is not used anymore.

Signed-off-by: Liang He 
---
 changelog:
 v5: fix 'gotot' error introduced by v4 and use cross-compiler to test 
 v4: reuse exist 'err' and use a simple code style, advised by CJ
 v3: use local 'child_node' advised by Michael.
 v2: use goto-label patch style advised by Christophe Leroy.
 v1: add of_node_put() before each exit.

 arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++-
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
b/arch/powerpc/platforms/85xx/sgy_cts1000.c
index 98ae64075193..e14d1b74d4e4 100644
--- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
+++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
@@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
 {
enum of_gpio_flags flags;
struct device_node *node = pdev->dev.of_node;
+   struct device_node *child_node;
int gpio, err, irq;
int trigger;
 
@@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev)
return -ENODEV;
 
/* If there's no matching child, this isn't really an error */
-   halt_node = of_find_matching_node(node, child_match);
-   if (!halt_node)
+   child_node = of_find_matching_node(node, child_match);
+   if (!child_node)
return 0;
 
/* Technically we could just read the first one, but punish
 * DT writers for invalid form. */
-   if (of_gpio_count(halt_node) != 1)
-   return -EINVAL;
+   if (of_gpio_count(child_node) != 1) {
+   err = -EINVAL;
+   goto err_put;
+   }
 
/* Get the gpio number relative to the dynamic base. */
-   gpio = of_get_gpio_flags(halt_node, 0, );
-   if (!gpio_is_valid(gpio))
-   return -EINVAL;
+   gpio = of_get_gpio_flags(child_node, 0, );
+   if (!gpio_is_valid(gpio)) {
+   err = -EINVAL;
+   goto err_put;
+   }
 
err = gpio_request(gpio, "gpio-halt");
if (err) {
printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n",
   gpio);
-   halt_node = NULL;
-   return err;
+   goto err_put;
}
 
trigger = (flags == OF_GPIO_ACTIVE_LOW);
@@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev)
gpio_direction_output(gpio, !trigger);
 
/* Now get the IRQ which tells us when the power button is hit */
-   irq = irq_of_parse_and_map(halt_node, 0);
+   irq = irq_of_parse_and_map(child_node, 0);
err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING, "gpio-halt", halt_node);
+ IRQF_TRIGGER_FALLING, "gpio-halt", child_node);
if (err) {
printk(KERN_ERR "gpio-halt: error requesting IRQ %d for "
   "GPIO %d.\n", irq, gpio);
gpio_free(gpio);
-   halt_node = NULL;
-   return err;
+   goto err_put;
}
 
/* Register our halt function */
@@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev)
printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d"
   " irq).\n", gpio, trigger, irq);
 
+   halt_node = child_node;
return 0;
+
+err_put:
+   of_node_put(child_node);
+   return err;
 }
 
 static int gpio_halt_remove(struct platform_device *pdev)
@@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev)
 
gpio_free(gpio);
 
+   of_node_put(halt_node);
halt_node = NULL;
}
 
-- 
2.25.1



Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He



At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote:
>On 17/06/2022 09:17, Liang He wrote:
>> 
>> 
>> 
>> At 2022-06-17 14:53:13, "Christophe Leroy"  
>> wrote:
>>>
>>>
>>> Le 17/06/2022 à 08:45, Liang He a écrit :



 At 2022-06-17 14:28:56, "Christophe Leroy"  
 wrote:
>
>
> Le 17/06/2022 à 08:08, Liang He a écrit :
>> In gpio_halt_probe(), of_find_matching_node() will return a node
>> pointer with refcount incremented. We should use of_node_put() in
>> fail path or when it is not used anymore.
>>
>> Signed-off-by: Liang He 
>> ---
>> changelog:
>> v4: reuse exist 'err' and use a simple code style, advised by CJ
>> v3: use local 'child_node' advised by Michael.
>> v2: use goto-label patch style advised by Christophe Leroy.
>> v1: add of_node_put() before each exit.
>>
>> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 
>> ++-
>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
>> b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> index 98ae64075193..e4588943fe7e 100644
>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device 
>> *pdev)
>> {
>>  enum of_gpio_flags flags;
>>  struct device_node *node = pdev->dev.of_node;
>> +struct device_node *child_node;
>>  int gpio, err, irq;
>>  int trigger;
>> 
>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device 
>> *pdev)
>>  return -ENODEV;
>> 
>>  /* If there's no matching child, this isn't really an error */
>> -halt_node = of_find_matching_node(node, child_match);
>> -if (!halt_node)
>> +child_node = of_find_matching_node(node, child_match);
>> +if (!child_node)
>>  return 0;
>> 
>>  /* Technically we could just read the first one, but punish
>>   * DT writers for invalid form. */
>> -if (of_gpio_count(halt_node) != 1)
>> -return -EINVAL;
>> +if (of_gpio_count(child_node) != 1) {
>> +err = -EINVAL;
>> +goto err_put;
>> +}
>> 
>>  /* Get the gpio number relative to the dynamic base. */
>> -gpio = of_get_gpio_flags(halt_node, 0, );
>> -if (!gpio_is_valid(gpio))
>> -return -EINVAL;
>> +gpio = of_get_gpio_flags(child_node, 0, );
>> +if (!gpio_is_valid(gpio)) {
>> +err = -EINVAL;
>> +gotot err_put;
>
> Did you test the build ?

 Sorry for this fault.

 In fact, I am still finding an efficient way to building different arch 
 source code as I only have x86-64.

 Now I am try using QEMU.

 Anyway, sorry for this fault.
>>>
>>> You can find cross compilers for most architectures for x86-64 here :
>>> https://mirrors.edge.kernel.org/pub/tools/crosstool/
>>>
>>> Christophe
>> 
>> Hi, Christophe and Conor.
>> 
>> Sorry to trouble you again.
>> 
>> Now I only know how to quickly identify the refcounting bugs, but I cannot 
>> efficiently give a build test.
>> 
>> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 
>> 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag.
>> But I meet too many header file missing errors. Even if I add some 'include' 
>> pathes, e.g., ./arch/powerpc/include, ./include,
>> there are still too many other errors.
>> 
>> So if there is any efficient way to check my patch code to avoid 'gotot' 
>> error again.
>
>idk anything about powerpc, but what I find is a nice way to get a compiler
>for an arch I don't use is to search on lore.kernel.org for a 0day robot
>build error since it gives instructions for building on that arch.
>For example:
>https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/
>
>
>In this case, your bug seems obvious? You typed "gotot" instead of "goto".
>
>Hope that helps,
>Conor.
>
>> 
>> Thanks again, Christophe and Conor.
>> 
>> Liang

Thanks, Conor and Christophe.

I finally figure out an efficient way in which I can use cross-compiler to 
check my single patched file as follow:

powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d  -nostdinc 
-I./arch/powerpc/include -I./arch/powerpc/include/generated  -I./include 
-I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi 
-I./include/uapi -I./include/generated/uapi -include 
./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include 
./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc 
-fmacro-prefix-map=./= 

Re: [PATCH] arch/*: Disable softirq stacks on PREEMPT_RT.

2022-06-17 Thread Sebastian Andrzej Siewior
On 2022-06-15 17:41:45 [+0200], Arnd Bergmann wrote:
> Applied to the asm-generic tree with the above fixup, thanks!

Thank you Arnd.

>   Arnd

Sebastian


Re: [PATCH -next v5 7/8] arm64: add uaccess to machine check safe

2022-06-17 Thread Mark Rutland
On Sat, May 28, 2022 at 06:50:55AM +, Tong Tiangen wrote:
> If user access fail due to hardware memory error, only the relevant
> processes are affected, so killing the user process and isolate the
> error page with hardware memory errors is a more reasonable choice
> than kernel panic.
> 
> Signed-off-by: Tong Tiangen 

> ---
>  arch/arm64/lib/copy_from_user.S | 8 
>  arch/arm64/lib/copy_to_user.S   | 8 

All of these changes are to the *kernel* accesses performed as part of copy
to/from user, and have nothing to do with userspace, so it does not make sense
to mark these as UACCESS.

Do we *actually* need to recover from failues on these accesses? Looking at
_copy_from_user(), the kernel will immediately follow this up with a memset()
to the same address which will be fatal anyway, so this is only punting the
failure for a few instructions.

If we really need to recover from certain accesses to kernel memory we should
add a new EX_TYPE_KACCESS_ERR_ZERO_MC or similar, but we need a strong
rationale as to why that's useful. As things stand I do not beleive it makes
sense for copy to/from user specifically.

>  arch/arm64/mm/extable.c | 8 
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
> index 34e317907524..402dd48a4f93 100644
> --- a/arch/arm64/lib/copy_from_user.S
> +++ b/arch/arm64/lib/copy_from_user.S
> @@ -25,7 +25,7 @@
>   .endm
>  
>   .macro strb1 reg, ptr, val
> - strb \reg, [\ptr], \val
> + USER(9998f, strb \reg, [\ptr], \val)
>   .endm
>  
>   .macro ldrh1 reg, ptr, val
> @@ -33,7 +33,7 @@
>   .endm
>  
>   .macro strh1 reg, ptr, val
> - strh \reg, [\ptr], \val
> + USER(9998f, strh \reg, [\ptr], \val)
>   .endm
>  
>   .macro ldr1 reg, ptr, val
> @@ -41,7 +41,7 @@
>   .endm
>  
>   .macro str1 reg, ptr, val
> - str \reg, [\ptr], \val
> + USER(9998f, str \reg, [\ptr], \val)
>   .endm
>  
>   .macro ldp1 reg1, reg2, ptr, val
> @@ -49,7 +49,7 @@
>   .endm
>  
>   .macro stp1 reg1, reg2, ptr, val
> - stp \reg1, \reg2, [\ptr], \val
> + USER(9998f, stp \reg1, \reg2, [\ptr], \val)
>   .endm
>  
>  end  .reqx5
> diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
> index 802231772608..4134bdb3a8b0 100644
> --- a/arch/arm64/lib/copy_to_user.S
> +++ b/arch/arm64/lib/copy_to_user.S
> @@ -20,7 +20,7 @@
>   *   x0 - bytes not copied
>   */
>   .macro ldrb1 reg, ptr, val
> - ldrb  \reg, [\ptr], \val
> + USER(9998f, ldrb  \reg, [\ptr], \val)
>   .endm
>  
>   .macro strb1 reg, ptr, val
> @@ -28,7 +28,7 @@
>   .endm
>  
>   .macro ldrh1 reg, ptr, val
> - ldrh  \reg, [\ptr], \val
> + USER(9998f, ldrh  \reg, [\ptr], \val)
>   .endm
>  
>   .macro strh1 reg, ptr, val
> @@ -36,7 +36,7 @@
>   .endm
>  
>   .macro ldr1 reg, ptr, val
> - ldr \reg, [\ptr], \val
> + USER(9998f, ldr \reg, [\ptr], \val)
>   .endm
>  
>   .macro str1 reg, ptr, val
> @@ -44,7 +44,7 @@
>   .endm
>  
>   .macro ldp1 reg1, reg2, ptr, val
> - ldp \reg1, \reg2, [\ptr], \val
> + USER(9998f, ldp \reg1, \reg2, [\ptr], \val)
>   .endm
>  
>   .macro stp1 reg1, reg2, ptr, val
> diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
> index c301dcf6335f..8ca8d9639f9f 100644
> --- a/arch/arm64/mm/extable.c
> +++ b/arch/arm64/mm/extable.c
> @@ -86,10 +86,10 @@ bool fixup_exception_mc(struct pt_regs *regs)
>   if (!ex)
>   return false;
>  
> - /*
> -  * This is not complete, More Machine check safe extable type can
> -  * be processed here.
> -  */
> + switch (ex->type) {
> + case EX_TYPE_UACCESS_ERR_ZERO:
> + return ex_handler_uaccess_err_zero(ex, regs);
> + }

This addition specifically makes sense to me, so can you split this into a 
separate patch?

Thanks,
Mark.


Re: [PATCH -next v5 6/8] arm64: add support for machine check error safe

2022-06-17 Thread Mark Rutland
On Sat, May 28, 2022 at 06:50:54AM +, Tong Tiangen wrote:
> During the processing of arm64 kernel hardware memory errors(do_sea()), if
> the errors is consumed in the kernel, the current processing is panic.
> However, it is not optimal.
> 
> Take uaccess for example, if the uaccess operation fails due to memory
> error, only the user process will be affected, kill the user process
> and isolate the user page with hardware memory errors is a better choice.
> 
> This patch only enable machine error check framework, it add exception
> fixup before kernel panic in do_sea() and only limit the consumption of
> hardware memory errors in kernel mode triggered by user mode processes.
> If fixup successful, panic can be avoided.
> 
> Signed-off-by: Tong Tiangen 
> ---
>  arch/arm64/Kconfig   |  1 +
>  arch/arm64/include/asm/extable.h |  1 +
>  arch/arm64/mm/extable.c  | 17 +
>  arch/arm64/mm/fault.c| 27 ++-
>  4 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index aaeb70358979..a3b12ff0cd7f 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -19,6 +19,7 @@ config ARM64
>   select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
>   select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
>   select ARCH_HAS_CACHE_LINE_SIZE
> + select ARCH_HAS_COPY_MC if ACPI_APEI_GHES
>   select ARCH_HAS_CURRENT_STACK_POINTER
>   select ARCH_HAS_DEBUG_VIRTUAL
>   select ARCH_HAS_DEBUG_VM_PGTABLE
> diff --git a/arch/arm64/include/asm/extable.h 
> b/arch/arm64/include/asm/extable.h
> index 72b0e71cc3de..f80ebd0addfd 100644
> --- a/arch/arm64/include/asm/extable.h
> +++ b/arch/arm64/include/asm/extable.h
> @@ -46,4 +46,5 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
>  #endif /* !CONFIG_BPF_JIT */
>  
>  bool fixup_exception(struct pt_regs *regs);
> +bool fixup_exception_mc(struct pt_regs *regs);
>  #endif
> diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
> index 228d681a8715..c301dcf6335f 100644
> --- a/arch/arm64/mm/extable.c
> +++ b/arch/arm64/mm/extable.c
> @@ -9,6 +9,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  static inline unsigned long
>  get_ex_fixup(const struct exception_table_entry *ex)
> @@ -76,3 +77,19 @@ bool fixup_exception(struct pt_regs *regs)
>  
>   BUG();
>  }
> +
> +bool fixup_exception_mc(struct pt_regs *regs)
> +{
> + const struct exception_table_entry *ex;
> +
> + ex = search_exception_tables(instruction_pointer(regs));
> + if (!ex)
> + return false;
> +
> + /*
> +  * This is not complete, More Machine check safe extable type can
> +  * be processed here.
> +  */
> +
> + return false;
> +}
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index c5e11768e5c1..b262bd282a89 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -696,6 +696,29 @@ static int do_bad(unsigned long far, unsigned long esr, 
> struct pt_regs *regs)
>   return 1; /* "fault" */
>  }
>  
> +static bool arm64_do_kernel_sea(unsigned long addr, unsigned int esr,
> +  struct pt_regs *regs, int sig, int code)
> +{
> + if (!IS_ENABLED(CONFIG_ARCH_HAS_COPY_MC))
> + return false;
> +
> + if (user_mode(regs) || !current->mm)
> + return false;

What's the `!current->mm` check for?

> +
> + if (apei_claim_sea(regs) < 0)
> + return false;
> +
> + if (!fixup_exception_mc(regs))
> + return false;

I thought we still wanted to signal the task in this case? Or do you expect to
add that into `fixup_exception_mc()` ?

> +
> + set_thread_esr(0, esr);

Why are we not setting the address? Is that deliberate, or an oversight?

> +
> + arm64_force_sig_fault(sig, code, addr,
> + "Uncorrected hardware memory error in kernel-access\n");

I think the wording here is misleading since we don't expect to recover from
accesses to kernel memory, and would be better as something like:

"Uncorrected memory error on access to user memory\n"

Thanks,
Mark.

> +
> + return true;
> +}
> +
>  static int do_sea(unsigned long far, unsigned long esr, struct pt_regs *regs)
>  {
>   const struct fault_info *inf;
> @@ -721,7 +744,9 @@ static int do_sea(unsigned long far, unsigned long esr, 
> struct pt_regs *regs)
>*/
>   siaddr  = untagged_addr(far);
>   }
> - arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr);
> +
> + if (!arm64_do_kernel_sea(siaddr, esr, regs, inf->sig, inf->code))
> + arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, 
> esr);
>  
>   return 0;
>  }
> -- 
> 2.25.1
> 


Re: [PATCH -next v5 4/8] arm64: extable: cleanup redundant extable type EX_TYPE_FIXUP

2022-06-17 Thread Mark Rutland
On Sat, May 28, 2022 at 06:50:52AM +, Tong Tiangen wrote:
> Currently, extable type EX_TYPE_FIXUP is no place to use, We can safely
> remove it.
> 
> Suggested-by: Mark Rutland 
> Signed-off-by: Tong Tiangen 

Acked-by: Mark Rutland 

Mark.

> ---
>  arch/arm64/include/asm/asm-extable.h | 20 
>  arch/arm64/mm/extable.c  |  9 -
>  2 files changed, 4 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm-extable.h 
> b/arch/arm64/include/asm/asm-extable.h
> index d01bd94cc4c2..1f2974467273 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -3,11 +3,10 @@
>  #define __ASM_ASM_EXTABLE_H
>  
>  #define EX_TYPE_NONE 0
> -#define EX_TYPE_FIXUP1
> -#define EX_TYPE_BPF  2
> -#define EX_TYPE_UACCESS_ERR_ZERO 3
> -#define EX_TYPE_LOAD_UNALIGNED_ZEROPAD   4
> -#define EX_TYPE_KACCESS_ERR_ZERO 5
> +#define EX_TYPE_BPF  1
> +#define EX_TYPE_UACCESS_ERR_ZERO 2
> +#define EX_TYPE_LOAD_UNALIGNED_ZEROPAD   3
> +#define EX_TYPE_KACCESS_ERR_ZERO 4
>  
>  #ifdef __ASSEMBLY__
>  
> @@ -20,14 +19,6 @@
>   .short  (data); \
>   .popsection;
>  
> -/*
> - * Create an exception table entry for `insn`, which will branch to `fixup`
> - * when an unhandled fault is taken.
> - */
> - .macro  _asm_extable, insn, fixup
> - __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0)
> - .endm
> -
>  /*
>   * Create an exception table entry for uaccess `insn`, which will branch to 
> `fixup`
>   * when an unhandled fault is taken.
> @@ -62,9 +53,6 @@
>   ".short (" data ")\n"   \
>   ".popsection\n"
>  
> -#define _ASM_EXTABLE(insn, fixup) \
> - __ASM_EXTABLE_RAW(#insn, #fixup, __stringify(EX_TYPE_FIXUP), "0")
> -
>  #define EX_DATA_REG_ERR_SHIFT0
>  #define EX_DATA_REG_ERR  GENMASK(4, 0)
>  #define EX_DATA_REG_ZERO_SHIFT   5
> diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
> index 056591e5ca80..228d681a8715 100644
> --- a/arch/arm64/mm/extable.c
> +++ b/arch/arm64/mm/extable.c
> @@ -16,13 +16,6 @@ get_ex_fixup(const struct exception_table_entry *ex)
>   return ((unsigned long)>fixup + ex->fixup);
>  }
>  
> -static bool ex_handler_fixup(const struct exception_table_entry *ex,
> -  struct pt_regs *regs)
> -{
> - regs->pc = get_ex_fixup(ex);
> - return true;
> -}
> -
>  static bool ex_handler_uaccess_err_zero(const struct exception_table_entry 
> *ex,
>   struct pt_regs *regs)
>  {
> @@ -72,8 +65,6 @@ bool fixup_exception(struct pt_regs *regs)
>   return false;
>  
>   switch (ex->type) {
> - case EX_TYPE_FIXUP:
> - return ex_handler_fixup(ex, regs);
>   case EX_TYPE_BPF:
>   return ex_handler_bpf(ex, regs);
>   case EX_TYPE_UACCESS_ERR_ZERO:
> -- 
> 2.25.1
> 


Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He

At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote:
>On 17/06/2022 09:17, Liang He wrote:
>> 
>> 
>> 
>> At 2022-06-17 14:53:13, "Christophe Leroy"  
>> wrote:
>>>
>>>
>>> Le 17/06/2022 à 08:45, Liang He a écrit :



 At 2022-06-17 14:28:56, "Christophe Leroy"  
 wrote:
>
>
> Le 17/06/2022 à 08:08, Liang He a écrit :
>> In gpio_halt_probe(), of_find_matching_node() will return a node
>> pointer with refcount incremented. We should use of_node_put() in
>> fail path or when it is not used anymore.
>>
>> Signed-off-by: Liang He 
>> ---
>> changelog:
>> v4: reuse exist 'err' and use a simple code style, advised by CJ
>> v3: use local 'child_node' advised by Michael.
>> v2: use goto-label patch style advised by Christophe Leroy.
>> v1: add of_node_put() before each exit.
>>
>> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 
>> ++-
>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
>> b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> index 98ae64075193..e4588943fe7e 100644
>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device 
>> *pdev)
>> {
>>  enum of_gpio_flags flags;
>>  struct device_node *node = pdev->dev.of_node;
>> +struct device_node *child_node;
>>  int gpio, err, irq;
>>  int trigger;
>> 
>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device 
>> *pdev)
>>  return -ENODEV;
>> 
>>  /* If there's no matching child, this isn't really an error */
>> -halt_node = of_find_matching_node(node, child_match);
>> -if (!halt_node)
>> +child_node = of_find_matching_node(node, child_match);
>> +if (!child_node)
>>  return 0;
>> 
>>  /* Technically we could just read the first one, but punish
>>   * DT writers for invalid form. */
>> -if (of_gpio_count(halt_node) != 1)
>> -return -EINVAL;
>> +if (of_gpio_count(child_node) != 1) {
>> +err = -EINVAL;
>> +goto err_put;
>> +}
>> 
>>  /* Get the gpio number relative to the dynamic base. */
>> -gpio = of_get_gpio_flags(halt_node, 0, );
>> -if (!gpio_is_valid(gpio))
>> -return -EINVAL;
>> +gpio = of_get_gpio_flags(child_node, 0, );
>> +if (!gpio_is_valid(gpio)) {
>> +err = -EINVAL;
>> +gotot err_put;
>
> Did you test the build ?

 Sorry for this fault.

 In fact, I am still finding an efficient way to building different arch 
 source code as I only have x86-64.

 Now I am try using QEMU.

 Anyway, sorry for this fault.
>>>
>>> You can find cross compilers for most architectures for x86-64 here :
>>> https://mirrors.edge.kernel.org/pub/tools/crosstool/
>>>
>>> Christophe
>> 
>> Hi, Christophe and Conor.
>> 
>> Sorry to trouble you again.
>> 
>> Now I only know how to quickly identify the refcounting bugs, but I cannot 
>> efficiently give a build test.
>> 
>> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 
>> 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag.
>> But I meet too many header file missing errors. Even if I add some 'include' 
>> pathes, e.g., ./arch/powerpc/include, ./include,
>> there are still too many other errors.
>> 
>> So if there is any efficient way to check my patch code to avoid 'gotot' 
>> error again.
>
>idk anything about powerpc, but what I find is a nice way to get a compiler
>for an arch I don't use is to search on lore.kernel.org for a 0day robot
>build error since it gives instructions for building on that arch.
>For example:
>https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/
>
>
>In this case, your bug seems obvious? You typed "gotot" instead of "goto".
>
>Hope that helps,
>Conor.
>
>> 
>> Thanks again, Christophe and Conor.
>> 
>> Liang


Thanks so much, I will try it.

Re: [PATCH -next v5 3/8] arm64: extable: move _cond_extable to _cond_uaccess_extable

2022-06-17 Thread Mark Rutland
On Sat, May 28, 2022 at 06:50:51AM +, Tong Tiangen wrote:
> Currently, We use _cond_extable for cache maintenance uaccess helper
> caches_clean_inval_user_pou(), so this should be moved over to
> EX_TYPE_UACCESS_ERR_ZERO and rename _cond_extable to _cond_uaccess_extable
> for clarity.
> 
> Suggested-by: Mark Rutland 
> Signed-off-by: Tong Tiangen 

Acked-by: Mark Rutland 

Mark.

> ---
>  arch/arm64/include/asm/asm-extable.h | 6 +++---
>  arch/arm64/include/asm/assembler.h   | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm-extable.h 
> b/arch/arm64/include/asm/asm-extable.h
> index 9c94ac1f082c..d01bd94cc4c2 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -40,9 +40,9 @@
>   * Create an exception table entry for `insn` if `fixup` is provided. 
> Otherwise
>   * do nothing.
>   */
> - .macro  _cond_extable, insn, fixup
> - .ifnc   \fixup,
> - _asm_extable\insn, \fixup
> + .macro  _cond_uaccess_extable, insn, fixup
> + .ifnc   \fixup,
> + _asm_extable_uaccess\insn, \fixup
>   .endif
>   .endm
>  
> diff --git a/arch/arm64/include/asm/assembler.h 
> b/arch/arm64/include/asm/assembler.h
> index 8c5a61aeaf8e..dc422fa437c2 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -423,7 +423,7 @@ alternative_endif
>   b.lo.Ldcache_op\@
>   dsb \domain
>  
> - _cond_extable .Ldcache_op\@, \fixup
> + _cond_uaccess_extable .Ldcache_op\@, \fixup
>   .endm
>  
>  /*
> @@ -462,7 +462,7 @@ alternative_endif
>   dsb ish
>   isb
>  
> - _cond_extable .Licache_op\@, \fixup
> + _cond_uaccess_extable .Licache_op\@, \fixup
>   .endm
>  
>  /*
> -- 
> 2.25.1
> 


Re: [PATCH 2/4] arm64/hugetlb: Implement arm64 specific hugetlb_mask_last_page

2022-06-17 Thread Muchun Song
On Thu, Jun 16, 2022 at 02:05:16PM -0700, Mike Kravetz wrote:
> From: Baolin Wang 
> 
> The HugeTLB address ranges are linearly scanned during fork, unmap and
> remap operations, and the linear scan can skip to the end of range mapped
> by the page table page if hitting a non-present entry, which can help
> to speed linear scanning of the HugeTLB address ranges.
> 
> So hugetlb_mask_last_page() is introduced to help to update the address in
> the loop of HugeTLB linear scanning with getting the last huge page mapped
> by the associated page table page[1], when a non-present entry is encountered.
> 
> Considering ARM64 specific cont-pte/pmd size HugeTLB, this patch implemented
> an ARM64 specific hugetlb_mask_last_page() to help this case.
> 
> [1] 
> https://lore.kernel.org/linux-mm/20220527225849.284839-1-mike.krav...@oracle.com/
> 
> Signed-off-by: Baolin Wang 
> Signed-off-by: Mike Kravetz 

Acked-by: Muchun Song 

Thanks.


Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO

2022-06-17 Thread Mark Rutland
On Sat, May 28, 2022 at 06:50:50AM +, Tong Tiangen wrote:
> Currnetly, the extable type used by __arch_copy_from/to_user() is
> EX_TYPE_FIXUP. In fact, It is more clearly to use meaningful
> EX_TYPE_UACCESS_*.
> 
> Suggested-by: Mark Rutland 
> Signed-off-by: Tong Tiangen 
> ---
>  arch/arm64/include/asm/asm-extable.h |  8 
>  arch/arm64/include/asm/asm-uaccess.h | 12 ++--
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm-extable.h 
> b/arch/arm64/include/asm/asm-extable.h
> index 56ebe183e78b..9c94ac1f082c 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -28,6 +28,14 @@
>   __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0)
>   .endm
>  
> +/*
> + * Create an exception table entry for uaccess `insn`, which will branch to 
> `fixup`
> + * when an unhandled fault is taken.
> + * ex->data = ~0 means both reg_err and reg_zero is set to wzr(x31).
> + */
> + .macro  _asm_extable_uaccess, insn, fixup
> + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_ERR_ZERO, ~0)
> + .endm

I'm not too keen on using `~0` here, since that also sets other bits in the
data field, and its somewhat opaque.

How painful is it to generate the data fields as with the C version of this
macro, so that we can pass in wzr explciitly for the two sub-fields?

Other than that, this looks good to me.

Thanks,
Mark.

>  /*
>   * Create an exception table entry for `insn` if `fixup` is provided. 
> Otherwise
>   * do nothing.
> diff --git a/arch/arm64/include/asm/asm-uaccess.h 
> b/arch/arm64/include/asm/asm-uaccess.h
> index 0557af834e03..75b211c98dea 100644
> --- a/arch/arm64/include/asm/asm-uaccess.h
> +++ b/arch/arm64/include/asm/asm-uaccess.h
> @@ -61,7 +61,7 @@ alternative_else_nop_endif
>  
>  #define USER(l, x...)\
>  :x;  \
> - _asm_extableb, l
> + _asm_extable_uaccessb, l
>  
>  /*
>   * Generate the assembly for LDTR/STTR with exception table entries.
> @@ -73,8 +73,8 @@ alternative_else_nop_endif
>  8889:ldtr\reg2, [\addr, #8];
>   add \addr, \addr, \post_inc;
>  
> - _asm_extableb,\l;
> - _asm_extable8889b,\l;
> + _asm_extable_uaccessb, \l;
> + _asm_extable_uaccess8889b, \l;
>   .endm
>  
>   .macro user_stp l, reg1, reg2, addr, post_inc
> @@ -82,14 +82,14 @@ alternative_else_nop_endif
>  8889:sttr\reg2, [\addr, #8];
>   add \addr, \addr, \post_inc;
>  
> - _asm_extableb,\l;
> - _asm_extable8889b,\l;
> + _asm_extable_uaccessb,\l;
> + _asm_extable_uaccess8889b,\l;
>   .endm
>  
>   .macro user_ldst l, inst, reg, addr, post_inc
>  :\inst   \reg, [\addr];
>   add \addr, \addr, \post_inc;
>  
> - _asm_extableb,\l;
> + _asm_extable_uaccessb, \l;
>   .endm
>  #endif
> -- 
> 2.25.1
> 


Re: [PATCH -next v5 1/8] arm64: extable: add new extable type EX_TYPE_KACCESS_ERR_ZERO support

2022-06-17 Thread Mark Rutland
On Sat, May 28, 2022 at 06:50:49AM +, Tong Tiangen wrote:
> Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by
> __get/put_kernel_nofault(), but those helpers are not uaccess type, so we
> add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by
> __get/put_kernel_no_fault().
> 
> This is also to prepare for distinguishing the two types in machine check
> safe process.
> 
> Suggested-by: Mark Rutland 
> Signed-off-by: Tong Tiangen 

This looks good to me, so modulo one nit below:

Acked-by: Mark Rutland 

> ---
>  arch/arm64/include/asm/asm-extable.h | 13 
>  arch/arm64/include/asm/uaccess.h | 94 ++--
>  arch/arm64/mm/extable.c  |  1 +
>  3 files changed, 61 insertions(+), 47 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm-extable.h 
> b/arch/arm64/include/asm/asm-extable.h
> index c39f2437e08e..56ebe183e78b 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -7,6 +7,7 @@
>  #define EX_TYPE_BPF  2
>  #define EX_TYPE_UACCESS_ERR_ZERO 3
>  #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD   4
> +#define EX_TYPE_KACCESS_ERR_ZERO 5

Could we please renumber this so the UACCESS and KACCESS definitions are next
to one another, i.e.

#define EX_TYPE_BPF 2
#define EX_TYPE_UACCESS_ERR_ZERO3
#define EX_TYPE_KACCESS_ERR_ZERO4
#define EX_TYPE_LOAD_UNALIGNED_ZEROPAD  5

Thanks,
Mark.

>  
>  #ifdef __ASSEMBLY__
>  
> @@ -73,9 +74,21 @@
>   EX_DATA_REG(ZERO, zero) \
> ")")
>  
> +#define _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, zero)
> \
> + __DEFINE_ASM_GPR_NUMS   \
> + __ASM_EXTABLE_RAW(#insn, #fixup,\
> +   __stringify(EX_TYPE_KACCESS_ERR_ZERO),\
> +   "("   \
> + EX_DATA_REG(ERR, err) " | " \
> + EX_DATA_REG(ZERO, zero) \
> +   ")")
> +
>  #define _ASM_EXTABLE_UACCESS_ERR(insn, fixup, err)   \
>   _ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, wzr)
>  
> +#define _ASM_EXTABLE_KACCESS_ERR(insn, fixup, err)   \
> + _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, wzr)
> +
>  #define EX_DATA_REG_DATA_SHIFT   0
>  #define EX_DATA_REG_DATA GENMASK(4, 0)
>  #define EX_DATA_REG_ADDR_SHIFT   5
> diff --git a/arch/arm64/include/asm/uaccess.h 
> b/arch/arm64/include/asm/uaccess.h
> index 63f9c828f1a7..2fc9f0861769 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -232,34 +232,34 @@ static inline void __user *__uaccess_mask_ptr(const 
> void __user *ptr)
>   * The "__xxx_error" versions set the third argument to -EFAULT if an error
>   * occurs, and leave it unchanged on success.
>   */
> -#define __get_mem_asm(load, reg, x, addr, err)   
> \
> +#define __get_mem_asm(load, reg, x, addr, err, type) \
>   asm volatile(   \
>   "1: " load "" reg "1, [%2]\n"   \
>   "2:\n"  \
> - _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %w0, %w1) \
> + _ASM_EXTABLE_##type##ACCESS_ERR_ZERO(1b, 2b, %w0, %w1)  \
>   : "+r" (err), "=" (x) \
>   : "r" (addr))
>  
> -#define __raw_get_mem(ldr, x, ptr, err)  
> \
> -do { \
> - unsigned long __gu_val; \
> - switch (sizeof(*(ptr))) {   \
> - case 1: \
> - __get_mem_asm(ldr "b", "%w", __gu_val, (ptr), (err));   \
> - break;  \
> - case 2: \
> - __get_mem_asm(ldr "h", "%w", __gu_val, (ptr), (err));   \
> - break;  \
> - case 4: \
> - __get_mem_asm(ldr, "%w", __gu_val, (ptr), (err));   \
> - break;  \
> - case 8: \
> - __get_mem_asm(ldr, "%x",  __gu_val, (ptr), (err));  \
> - break;  \
> - default:\
> - BUILD_BUG(); 

Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He



At 2022-06-17 14:53:13, "Christophe Leroy"  wrote:
>
>
>Le 17/06/2022 à 08:45, Liang He a écrit :
>> 
>> 
>> 
>> At 2022-06-17 14:28:56, "Christophe Leroy"  
>> wrote:
>>>
>>>
>>> Le 17/06/2022 à 08:08, Liang He a écrit :
 In gpio_halt_probe(), of_find_matching_node() will return a node
 pointer with refcount incremented. We should use of_node_put() in
 fail path or when it is not used anymore.

 Signed-off-by: Liang He 
 ---
changelog:
v4: reuse exist 'err' and use a simple code style, advised by CJ
v3: use local 'child_node' advised by Michael.
v2: use goto-label patch style advised by Christophe Leroy.
v1: add of_node_put() before each exit.

arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)

 diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
 b/arch/powerpc/platforms/85xx/sgy_cts1000.c
 index 98ae64075193..e4588943fe7e 100644
 --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
 +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
 @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
{
enum of_gpio_flags flags;
struct device_node *node = pdev->dev.of_node;
 +  struct device_node *child_node;
int gpio, err, irq;
int trigger;

 @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device 
 *pdev)
return -ENODEV;

/* If there's no matching child, this isn't really an error */
 -  halt_node = of_find_matching_node(node, child_match);
 -  if (!halt_node)
 +  child_node = of_find_matching_node(node, child_match);
 +  if (!child_node)
return 0;

/* Technically we could just read the first one, but punish
 * DT writers for invalid form. */
 -  if (of_gpio_count(halt_node) != 1)
 -  return -EINVAL;
 +  if (of_gpio_count(child_node) != 1) {
 +  err = -EINVAL;
 +  goto err_put;
 +  }

/* Get the gpio number relative to the dynamic base. */
 -  gpio = of_get_gpio_flags(halt_node, 0, );
 -  if (!gpio_is_valid(gpio))
 -  return -EINVAL;
 +  gpio = of_get_gpio_flags(child_node, 0, );
 +  if (!gpio_is_valid(gpio)) {
 +  err = -EINVAL;
 +  gotot err_put;
>>>
>>> Did you test the build ?
>> 
>> Sorry for this fault.
>> 
>> In fact, I am still finding an efficient way to building different arch 
>> source code as I only have x86-64.
>> 
>> Now I am try using QEMU.
>> 
>> Anyway, sorry for this fault.
>
>You can find cross compilers for most architectures for x86-64 here : 
>https://mirrors.edge.kernel.org/pub/tools/crosstool/
>
>Christophe

Hi, Christophe and Conor.

Sorry to trouble you again.

Now I only know how to quickly identify the refcounting bugs, but I cannot 
efficiently give a build test.

For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 
'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag.
But I meet too many header file missing errors. Even if I add some 'include' 
pathes, e.g., ./arch/powerpc/include, ./include, 
there are still too many other errors.

So if there is any efficient way to check my patch code to avoid 'gotot' error 
again.

Thanks again, Christophe and Conor.

Liang

Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present

2022-06-17 Thread Muchun Song
On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote:
> HugeTLB address ranges are linearly scanned during fork, unmap and
> remap operations.  If a non-present entry is encountered, the code
> currently continues to the next huge page aligned address.  However,
> a non-present entry implies that the page table page for that entry
> is not present.  Therefore, the linear scan can skip to the end of
> range mapped by the page table page.  This can speed operations on
> large sparsely populated hugetlb mappings.
> 
> Create a new routine hugetlb_mask_last_page() that will return an
> address mask.  When the mask is ORed with an address, the result
> will be the address of the last huge page mapped by the associated
> page table page.  Use this mask to update addresses in routines which
> linearly scan hugetlb address ranges when a non-present pte is
> encountered.
> 
> hugetlb_mask_last_page is related to the implementation of
> huge_pte_offset as hugetlb_mask_last_page is called when huge_pte_offset
> returns NULL.  This patch only provides a complete hugetlb_mask_last_page
> implementation when CONFIG_ARCH_WANT_GENERAL_HUGETLB is defined.
> Architectures which provide their own versions of huge_pte_offset can also
> provide their own version of hugetlb_mask_last_page.
> 
> Signed-off-by: Mike Kravetz 
> Tested-by: Baolin Wang 
> Reviewed-by: Baolin Wang 

It'll be more efficient, Thanks.

Acked-by: Muchun Song 



[PATCH] powerpc: powernv: Fix refcount leak in opal

2022-06-17 Thread Liang He
In opal_imc_init_dev(), of_find_compatible_node() will return a
node pointer with refcount incremented. We should use of_node_put()
when it is not used anymore.

Signed-off-by: Liang He 
--- 

 arch/powerpc/platforms/powernv/opal.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/powernv/opal.c 
b/arch/powerpc/platforms/powernv/opal.c
index 55a8fbfdb5b2..d86cc48a10aa 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -952,6 +952,7 @@ static void __init opal_imc_init_dev(void)
np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);
if (np)
of_platform_device_create(np, NULL, NULL);
+   of_node_put(np);
 }
 
 static int kopald(void *unused)
-- 
2.25.1



[PATCH] powerc: Update asm-prototypes.h comment

2022-06-17 Thread Michael Ellerman
This header was recently cleaned up in commit 76222808fc25 ("powerpc:
Move C prototypes out of asm-prototypes.h"), update the comment to
reflect it's proper purpose.

Signed-off-by: Michael Ellerman 
---
 arch/powerpc/include/asm/asm-prototypes.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h 
b/arch/powerpc/include/asm/asm-prototypes.h
index d995c65d18ab..eda6dba9378f 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -2,8 +2,9 @@
 #ifndef _ASM_POWERPC_ASM_PROTOTYPES_H
 #define _ASM_POWERPC_ASM_PROTOTYPES_H
 /*
- * This file is for prototypes of C functions that are only called
- * from asm, and any associated variables.
+ * This file is for C prototypes of asm symbols that are EXPORTed.
+ * It allows the modversions logic to see their prototype and
+ * generate proper CRCs for them.
  *
  * Copyright 2016, Daniel Axtens, IBM Corporation.
  */
-- 
2.35.3



Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Christophe Leroy


Le 17/06/2022 à 08:45, Liang He a écrit :
> 
> 
> 
> At 2022-06-17 14:28:56, "Christophe Leroy"  
> wrote:
>>
>>
>> Le 17/06/2022 à 08:08, Liang He a écrit :
>>> In gpio_halt_probe(), of_find_matching_node() will return a node
>>> pointer with refcount incremented. We should use of_node_put() in
>>> fail path or when it is not used anymore.
>>>
>>> Signed-off-by: Liang He 
>>> ---
>>>changelog:
>>>v4: reuse exist 'err' and use a simple code style, advised by CJ
>>>v3: use local 'child_node' advised by Michael.
>>>v2: use goto-label patch style advised by Christophe Leroy.
>>>v1: add of_node_put() before each exit.
>>>
>>>arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++-
>>>1 file changed, 22 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
>>> b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>> index 98ae64075193..e4588943fe7e 100644
>>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>>{
>>> enum of_gpio_flags flags;
>>> struct device_node *node = pdev->dev.of_node;
>>> +   struct device_node *child_node;
>>> int gpio, err, irq;
>>> int trigger;
>>>
>>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>> return -ENODEV;
>>>
>>> /* If there's no matching child, this isn't really an error */
>>> -   halt_node = of_find_matching_node(node, child_match);
>>> -   if (!halt_node)
>>> +   child_node = of_find_matching_node(node, child_match);
>>> +   if (!child_node)
>>> return 0;
>>>
>>> /* Technically we could just read the first one, but punish
>>>  * DT writers for invalid form. */
>>> -   if (of_gpio_count(halt_node) != 1)
>>> -   return -EINVAL;
>>> +   if (of_gpio_count(child_node) != 1) {
>>> +   err = -EINVAL;
>>> +   goto err_put;
>>> +   }
>>>
>>> /* Get the gpio number relative to the dynamic base. */
>>> -   gpio = of_get_gpio_flags(halt_node, 0, );
>>> -   if (!gpio_is_valid(gpio))
>>> -   return -EINVAL;
>>> +   gpio = of_get_gpio_flags(child_node, 0, );
>>> +   if (!gpio_is_valid(gpio)) {
>>> +   err = -EINVAL;
>>> +   gotot err_put;
>>
>> Did you test the build ?
> 
> Sorry for this fault.
> 
> In fact, I am still finding an efficient way to building different arch 
> source code as I only have x86-64.
> 
> Now I am try using QEMU.
> 
> Anyway, sorry for this fault.

You can find cross compilers for most architectures for x86-64 here : 
https://mirrors.edge.kernel.org/pub/tools/crosstool/

Christophe

Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He



At 2022-06-17 14:28:56, "Christophe Leroy"  wrote:
>
>
>Le 17/06/2022 à 08:08, Liang He a écrit :
>> In gpio_halt_probe(), of_find_matching_node() will return a node
>> pointer with refcount incremented. We should use of_node_put() in
>> fail path or when it is not used anymore.
>> 
>> Signed-off-by: Liang He 
>> ---
>>   changelog:
>>   v4: reuse exist 'err' and use a simple code style, advised by CJ
>>   v3: use local 'child_node' advised by Michael.
>>   v2: use goto-label patch style advised by Christophe Leroy.
>>   v1: add of_node_put() before each exit.
>> 
>>   arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++-
>>   1 file changed, 22 insertions(+), 13 deletions(-)
>> 
>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
>> b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> index 98ae64075193..e4588943fe7e 100644
>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>   {
>>  enum of_gpio_flags flags;
>>  struct device_node *node = pdev->dev.of_node;
>> +struct device_node *child_node;
>>  int gpio, err, irq;
>>  int trigger;
>>   
>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>  return -ENODEV;
>>   
>>  /* If there's no matching child, this isn't really an error */
>> -halt_node = of_find_matching_node(node, child_match);
>> -if (!halt_node)
>> +child_node = of_find_matching_node(node, child_match);
>> +if (!child_node)
>>  return 0;
>>   
>>  /* Technically we could just read the first one, but punish
>>   * DT writers for invalid form. */
>> -if (of_gpio_count(halt_node) != 1)
>> -return -EINVAL;
>> +if (of_gpio_count(child_node) != 1) {
>> +err = -EINVAL;
>> +goto err_put;
>> +}
>>   
>>  /* Get the gpio number relative to the dynamic base. */
>> -gpio = of_get_gpio_flags(halt_node, 0, );
>> -if (!gpio_is_valid(gpio))
>> -return -EINVAL;
>> +gpio = of_get_gpio_flags(child_node, 0, );
>> +if (!gpio_is_valid(gpio)) {
>> +err = -EINVAL;
>> +gotot err_put;
>
>Did you test the build ?

Sorry for this fault. 

In fact, I am still finding an efficient way to building different arch source 
code as I only have x86-64.

Now I am try using QEMU.

Anyway, sorry for this fault.


>
>> +}
>>   
>>  err = gpio_request(gpio, "gpio-halt");
>>  if (err) {
>>  printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n",
>> gpio);
>> -halt_node = NULL;
>> -return err;
>> +goto err_put;
>>  }
>>   
>>  trigger = (flags == OF_GPIO_ACTIVE_LOW);
>> @@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device 
>> *pdev)
>>  gpio_direction_output(gpio, !trigger);
>>   
>>  /* Now get the IRQ which tells us when the power button is hit */
>> -irq = irq_of_parse_and_map(halt_node, 0);
>> +irq = irq_of_parse_and_map(child_node, 0);
>>  err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING |
>> -  IRQF_TRIGGER_FALLING, "gpio-halt", halt_node);
>> +  IRQF_TRIGGER_FALLING, "gpio-halt", child_node);
>>  if (err) {
>>  printk(KERN_ERR "gpio-halt: error requesting IRQ %d for "
>> "GPIO %d.\n", irq, gpio);
>>  gpio_free(gpio);
>> -halt_node = NULL;
>> -return err;
>> +goto err_put;
>>  }
>>   
>>  /* Register our halt function */
>> @@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>  printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d"
>> " irq).\n", gpio, trigger, irq);
>>   
>> +halt_node = child_node;
>>  return 0;
>> +
>> +err_put:
>> +of_node_put(child_node);
>> +return err;
>>   }
>>   
>>   static int gpio_halt_remove(struct platform_device *pdev)
>> @@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev)
>>   
>>  gpio_free(gpio);
>>   
>> +of_node_put(halt_node);
>>  halt_node = NULL;
>>  }
>>   


Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Christophe Leroy


Le 17/06/2022 à 08:08, Liang He a écrit :
> In gpio_halt_probe(), of_find_matching_node() will return a node
> pointer with refcount incremented. We should use of_node_put() in
> fail path or when it is not used anymore.
> 
> Signed-off-by: Liang He 
> ---
>   changelog:
>   v4: reuse exist 'err' and use a simple code style, advised by CJ
>   v3: use local 'child_node' advised by Michael.
>   v2: use goto-label patch style advised by Christophe Leroy.
>   v1: add of_node_put() before each exit.
> 
>   arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++-
>   1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
> b/arch/powerpc/platforms/85xx/sgy_cts1000.c
> index 98ae64075193..e4588943fe7e 100644
> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
>   {
>   enum of_gpio_flags flags;
>   struct device_node *node = pdev->dev.of_node;
> + struct device_node *child_node;
>   int gpio, err, irq;
>   int trigger;
>   
> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev)
>   return -ENODEV;
>   
>   /* If there's no matching child, this isn't really an error */
> - halt_node = of_find_matching_node(node, child_match);
> - if (!halt_node)
> + child_node = of_find_matching_node(node, child_match);
> + if (!child_node)
>   return 0;
>   
>   /* Technically we could just read the first one, but punish
>* DT writers for invalid form. */
> - if (of_gpio_count(halt_node) != 1)
> - return -EINVAL;
> + if (of_gpio_count(child_node) != 1) {
> + err = -EINVAL;
> + goto err_put;
> + }
>   
>   /* Get the gpio number relative to the dynamic base. */
> - gpio = of_get_gpio_flags(halt_node, 0, );
> - if (!gpio_is_valid(gpio))
> - return -EINVAL;
> + gpio = of_get_gpio_flags(child_node, 0, );
> + if (!gpio_is_valid(gpio)) {
> + err = -EINVAL;
> + gotot err_put;

Did you test the build ?

> + }
>   
>   err = gpio_request(gpio, "gpio-halt");
>   if (err) {
>   printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n",
>  gpio);
> - halt_node = NULL;
> - return err;
> + goto err_put;
>   }
>   
>   trigger = (flags == OF_GPIO_ACTIVE_LOW);
> @@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev)
>   gpio_direction_output(gpio, !trigger);
>   
>   /* Now get the IRQ which tells us when the power button is hit */
> - irq = irq_of_parse_and_map(halt_node, 0);
> + irq = irq_of_parse_and_map(child_node, 0);
>   err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING |
> -   IRQF_TRIGGER_FALLING, "gpio-halt", halt_node);
> +   IRQF_TRIGGER_FALLING, "gpio-halt", child_node);
>   if (err) {
>   printk(KERN_ERR "gpio-halt: error requesting IRQ %d for "
>  "GPIO %d.\n", irq, gpio);
>   gpio_free(gpio);
> - halt_node = NULL;
> - return err;
> + goto err_put;
>   }
>   
>   /* Register our halt function */
> @@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev)
>   printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d"
>  " irq).\n", gpio, trigger, irq);
>   
> + halt_node = child_node;
>   return 0;
> +
> +err_put:
> + of_node_put(child_node);
> + return err;
>   }
>   
>   static int gpio_halt_remove(struct platform_device *pdev)
> @@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev)
>   
>   gpio_free(gpio);
>   
> + of_node_put(halt_node);
>   halt_node = NULL;
>   }
>   

[PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000

2022-06-17 Thread Liang He
In gpio_halt_probe(), of_find_matching_node() will return a node
pointer with refcount incremented. We should use of_node_put() in
fail path or when it is not used anymore.

Signed-off-by: Liang He 
---
 changelog:
 v4: reuse exist 'err' and use a simple code style, advised by CJ
 v3: use local 'child_node' advised by Michael.
 v2: use goto-label patch style advised by Christophe Leroy.
 v1: add of_node_put() before each exit.

 arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++-
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
b/arch/powerpc/platforms/85xx/sgy_cts1000.c
index 98ae64075193..e4588943fe7e 100644
--- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
+++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
@@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
 {
enum of_gpio_flags flags;
struct device_node *node = pdev->dev.of_node;
+   struct device_node *child_node;
int gpio, err, irq;
int trigger;
 
@@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev)
return -ENODEV;
 
/* If there's no matching child, this isn't really an error */
-   halt_node = of_find_matching_node(node, child_match);
-   if (!halt_node)
+   child_node = of_find_matching_node(node, child_match);
+   if (!child_node)
return 0;
 
/* Technically we could just read the first one, but punish
 * DT writers for invalid form. */
-   if (of_gpio_count(halt_node) != 1)
-   return -EINVAL;
+   if (of_gpio_count(child_node) != 1) {
+   err = -EINVAL;
+   goto err_put;
+   }
 
/* Get the gpio number relative to the dynamic base. */
-   gpio = of_get_gpio_flags(halt_node, 0, );
-   if (!gpio_is_valid(gpio))
-   return -EINVAL;
+   gpio = of_get_gpio_flags(child_node, 0, );
+   if (!gpio_is_valid(gpio)) {
+   err = -EINVAL;
+   gotot err_put;
+   }
 
err = gpio_request(gpio, "gpio-halt");
if (err) {
printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n",
   gpio);
-   halt_node = NULL;
-   return err;
+   goto err_put;
}
 
trigger = (flags == OF_GPIO_ACTIVE_LOW);
@@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev)
gpio_direction_output(gpio, !trigger);
 
/* Now get the IRQ which tells us when the power button is hit */
-   irq = irq_of_parse_and_map(halt_node, 0);
+   irq = irq_of_parse_and_map(child_node, 0);
err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING, "gpio-halt", halt_node);
+ IRQF_TRIGGER_FALLING, "gpio-halt", child_node);
if (err) {
printk(KERN_ERR "gpio-halt: error requesting IRQ %d for "
   "GPIO %d.\n", irq, gpio);
gpio_free(gpio);
-   halt_node = NULL;
-   return err;
+   goto err_put;
}
 
/* Register our halt function */
@@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev)
printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d"
   " irq).\n", gpio, trigger, irq);
 
+   halt_node = child_node;
return 0;
+
+err_put:
+   of_node_put(child_node);
+   return err;
 }
 
 static int gpio_halt_remove(struct platform_device *pdev)
@@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev)
 
gpio_free(gpio);
 
+   of_node_put(halt_node);
halt_node = NULL;
}
 
-- 
2.25.1