* Re: [PATCH 10/15] powerpc/powernv/pci: Refactor pnv_ioda_alloc_pe()
From: Alexey Kardashevskiy @ 2020-07-15 2:29 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200710052340.737567-11-oohall@gmail.com>
On 10/07/2020 15:23, Oliver O'Halloran wrote:
> Rework the PE allocation logic to allow allocating blocks of PEs rather
> than individually. We'll use this to allocate contigious blocks of PEs for
> the SR-IOVs.
The patch does not do just this, it also adds missing mutexes (which is
good) but still misses them in pnv_pci_sriov_disable() and
pnv_pci_ioda_pe_dump().
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda.c | 41 ++++++++++++++++++-----
> arch/powerpc/platforms/powernv/pci.h | 2 +-
> 2 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 2d36a9ebf0e9..c9c25fb0783c 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -145,23 +145,45 @@ static void pnv_ioda_reserve_pe(struct pnv_phb *phb, int pe_no)
> return;
> }
>
> + mutex_lock(&phb->ioda.pe_alloc_mutex);
> if (test_and_set_bit(pe_no, phb->ioda.pe_alloc))
> pr_debug("%s: PE %x was reserved on PHB#%x\n",
> __func__, pe_no, phb->hose->global_number);
> + mutex_unlock(&phb->ioda.pe_alloc_mutex);
>
> pnv_ioda_init_pe(phb, pe_no);
> }
>
> -struct pnv_ioda_pe *pnv_ioda_alloc_pe(struct pnv_phb *phb)
> +struct pnv_ioda_pe *pnv_ioda_alloc_pe(struct pnv_phb *phb, int count)
> {
> - long pe;
> + struct pnv_ioda_pe *ret = NULL;
> + int run = 0, pe, i;
>
> + mutex_lock(&phb->ioda.pe_alloc_mutex);
> +
> + /* scan backwards for a run of @count cleared bits */
> for (pe = phb->ioda.total_pe_num - 1; pe >= 0; pe--) {
> - if (!test_and_set_bit(pe, phb->ioda.pe_alloc))
> - return pnv_ioda_init_pe(phb, pe);
> + if (test_bit(pe, phb->ioda.pe_alloc)) {
> + run = 0;
> + continue;
> + }
> +
> + run++;
> + if (run == count)
> + break;
> }
> + if (run != count)
> + goto out;
>
> - return NULL;
> + for (i = pe; i < pe + count; i++) {
> + set_bit(i, phb->ioda.pe_alloc);
> + pnv_ioda_init_pe(phb, i);
> + }
> + ret = &phb->ioda.pe_array[pe];
> +
> +out:
> + mutex_unlock(&phb->ioda.pe_alloc_mutex);
> + return ret;
> }
>
> void pnv_ioda_free_pe(struct pnv_ioda_pe *pe)
> @@ -173,7 +195,10 @@ void pnv_ioda_free_pe(struct pnv_ioda_pe *pe)
> WARN_ON(pe->npucomp); /* NPUs for nvlink are not supposed to be freed */
> kfree(pe->npucomp);
> memset(pe, 0, sizeof(struct pnv_ioda_pe));
> +
> + mutex_lock(&phb->ioda.pe_alloc_mutex);
> clear_bit(pe_num, phb->ioda.pe_alloc);
> + mutex_unlock(&phb->ioda.pe_alloc_mutex);
> }
>
> /* The default M64 BAR is shared by all PEs */
> @@ -976,7 +1001,7 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
> if (pdn->pe_number != IODA_INVALID_PE)
> return NULL;
>
> - pe = pnv_ioda_alloc_pe(phb);
> + pe = pnv_ioda_alloc_pe(phb, 1);
> if (!pe) {
> pr_warn("%s: Not enough PE# available, disabling device\n",
> pci_name(dev));
> @@ -1047,7 +1072,7 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
>
> /* The PE number isn't pinned by M64 */
> if (!pe)
> - pe = pnv_ioda_alloc_pe(phb);
> + pe = pnv_ioda_alloc_pe(phb, 1);
>
> if (!pe) {
> pr_warn("%s: Not enough PE# available for PCI bus %04x:%02x\n",
> @@ -3065,7 +3090,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
> pnv_ioda_reserve_pe(phb, phb->ioda.root_pe_idx);
> } else {
> /* otherwise just allocate one */
> - root_pe = pnv_ioda_alloc_pe(phb);
> + root_pe = pnv_ioda_alloc_pe(phb, 1);
> phb->ioda.root_pe_idx = root_pe->pe_number;
> }
>
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 58c97e60c3db..b4c9bdba7217 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -223,7 +223,7 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe);
> void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe);
> void pnv_pci_ioda2_release_pe_dma(struct pnv_ioda_pe *pe);
>
> -struct pnv_ioda_pe *pnv_ioda_alloc_pe(struct pnv_phb *phb);
> +struct pnv_ioda_pe *pnv_ioda_alloc_pe(struct pnv_phb *phb, int count);
> void pnv_ioda_free_pe(struct pnv_ioda_pe *pe);
>
> #ifdef CONFIG_PCI_IOV
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v3 04/12] ppc64/kexec_file: avoid stomping memory used by special regions
From: Thiago Jung Bauermann @ 2020-07-15 2:39 UTC (permalink / raw)
To: Hari Bathini
Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159466088775.24747.1248185448154277951.stgit@hbathini.in.ibm.com>
Hari Bathini <hbathini@linux.ibm.com> writes:
> diff --git a/arch/powerpc/include/asm/crashdump-ppc64.h b/arch/powerpc/include/asm/crashdump-ppc64.h
> new file mode 100644
> index 0000000..90deb46
> --- /dev/null
> +++ b/arch/powerpc/include/asm/crashdump-ppc64.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _ASM_POWERPC_CRASHDUMP_PPC64_H
> +#define _ASM_POWERPC_CRASHDUMP_PPC64_H
> +
> +/* min & max addresses for kdump load segments */
> +#define KDUMP_BUF_MIN (crashk_res.start)
> +#define KDUMP_BUF_MAX ((crashk_res.end < ppc64_rma_size) ? \
> + crashk_res.end : (ppc64_rma_size - 1))
> +
> +#endif /* __ASM_POWERPC_CRASHDUMP_PPC64_H */
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 7008ea1..bf47a01 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -100,14 +100,16 @@ void relocate_new_kernel(unsigned long indirection_page, unsigned long reboot_co
> #ifdef CONFIG_KEXEC_FILE
> extern const struct kexec_file_ops kexec_elf64_ops;
>
> -#ifdef CONFIG_IMA_KEXEC
> #define ARCH_HAS_KIMAGE_ARCH
>
> struct kimage_arch {
> + struct crash_mem *exclude_ranges;
> +
> +#ifdef CONFIG_IMA_KEXEC
> phys_addr_t ima_buffer_addr;
> size_t ima_buffer_size;
> -};
> #endif
> +};
>
> int setup_purgatory(struct kimage *image, const void *slave_code,
> const void *fdt, unsigned long kernel_load_addr,
> @@ -125,6 +127,7 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
> unsigned long initrd_load_addr,
> unsigned long initrd_len, const char *cmdline);
> #endif /* CONFIG_PPC64 */
> +
> #endif /* CONFIG_KEXEC_FILE */
>
> #else /* !CONFIG_KEXEC_CORE */
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 23ad04c..c695f94 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -22,6 +22,7 @@
> #include <linux/of_fdt.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> +#include <asm/crashdump-ppc64.h>
>
> static void *elf64_load(struct kimage *image, char *kernel_buf,
> unsigned long kernel_len, char *initrd,
> @@ -46,6 +47,12 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> if (ret)
> goto out;
>
> + if (image->type == KEXEC_TYPE_CRASH) {
> + /* min & max buffer values for kdump case */
> + kbuf.buf_min = pbuf.buf_min = KDUMP_BUF_MIN;
> + kbuf.buf_max = pbuf.buf_max = KDUMP_BUF_MAX;
This is only my personal opinion and an actual maintainer may disagree,
but just looking at the lines above, I would assume that KDUMP_BUF_MIN
and KDUMP_BUF_MAX were constants, when in fact they aren't.
I suggest using static inline macros in <asm/crashdump-ppc64.h>, for
example:
static inline resource_size_t get_kdump_buf_min(void)
{
return crashk_res.start;
}
static inline resource_size_t get_kdump_buf_max(void)
{
return (crashk_res.end < ppc64_rma_size) ? \
crashk_res.end : (ppc64_rma_size - 1)
}
> + }
> +
> ret = kexec_elf_load(image, &ehdr, &elf_info, &kbuf, &kernel_load_addr);
> if (ret)
> goto out;
<snip>
> +/**
> + * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
> + * in the memory regions between buf_min & buf_max
> + * for the buffer. If found, sets kbuf->mem.
> + * @kbuf: Buffer contents and memory parameters.
> + * @buf_min: Minimum address for the buffer.
> + * @buf_max: Maximum address for the buffer.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
> + u64 buf_min, u64 buf_max)
> +{
> + int ret = -EADDRNOTAVAIL;
> + phys_addr_t start, end;
> + u64 i;
> +
> + for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
> + MEMBLOCK_NONE, &start, &end, NULL) {
> + if (start > buf_max)
> + continue;
> +
> + /* Memory hole not found */
> + if (end < buf_min)
> + break;
> +
> + /* Adjust memory region based on the given range */
> + if (start < buf_min)
> + start = buf_min;
> + if (end > buf_max)
> + end = buf_max;
> +
> + start = ALIGN(start, kbuf->buf_align);
> + if (start < end && (end - start + 1) >= kbuf->memsz) {
This is why I dislike using start and end to express address ranges:
While struct resource seems to use the [address, end] convention, my
reading of memblock code is that it uses [addres, end). This is
guaranteed to lead to bugs. So the above has an off-by-one error. To
calculate the size of the current range, you need to use `end - start`.
> + /* Suitable memory range found. Set kbuf->mem */
> + kbuf->mem = ALIGN_DOWN(end - kbuf->memsz + 1,
Similarly, I believe the `+ 1` here is wrong.
> + kbuf->buf_align);
> + ret = 0;
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * locate_mem_hole_top_down_ppc64 - Skip special memory regions to find a
> + * suitable buffer with top down approach.
> + * @kbuf: Buffer contents and memory parameters.
> + * @buf_min: Minimum address for the buffer.
> + * @buf_max: Maximum address for the buffer.
> + * @emem: Exclude memory ranges.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int locate_mem_hole_top_down_ppc64(struct kexec_buf *kbuf,
> + u64 buf_min, u64 buf_max,
> + const struct crash_mem *emem)
> +{
> + int i, ret = 0, err = -EADDRNOTAVAIL;
> + u64 start, end, tmin, tmax;
> +
> + tmax = buf_max;
> + for (i = (emem->nr_ranges - 1); i >= 0; i--) {
> + start = emem->ranges[i].start;
> + end = emem->ranges[i].end;
> +
> + if (start > tmax)
> + continue;
> +
> + if (end < tmax) {
> + tmin = (end < buf_min ? buf_min : end + 1);
> + ret = __locate_mem_hole_top_down(kbuf, tmin, tmax);
> + if (!ret)
> + return 0;
> + }
> +
> + tmax = start - 1;
> +
> + if (tmax < buf_min) {
> + ret = err;
> + break;
> + }
> + ret = 0;
> + }
> +
> + if (!ret) {
> + tmin = buf_min;
> + ret = __locate_mem_hole_top_down(kbuf, tmin, tmax);
> + }
> + return ret;
> +}
> +
> +/**
> + * __locate_mem_hole_bottom_up - Looks bottom up for a large enough memory hole
> + * in the memory regions between buf_min & buf_max
> + * for the buffer. If found, sets kbuf->mem.
> + * @kbuf: Buffer contents and memory parameters.
> + * @buf_min: Minimum address for the buffer.
> + * @buf_max: Maximum address for the buffer.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int __locate_mem_hole_bottom_up(struct kexec_buf *kbuf,
> + u64 buf_min, u64 buf_max)
> +{
> + int ret = -EADDRNOTAVAIL;
> + phys_addr_t start, end;
> + u64 i;
> +
> + for_each_mem_range(i, &memblock.memory, NULL, NUMA_NO_NODE,
> + MEMBLOCK_NONE, &start, &end, NULL) {
> + if (end < buf_min)
> + continue;
> +
> + /* Memory hole not found */
> + if (start > buf_max)
> + break;
> +
> + /* Adjust memory region based on the given range */
> + if (start < buf_min)
> + start = buf_min;
> + if (end > buf_max)
> + end = buf_max;
buf_max is an inclusive end address, right? Then this should read
`end = buf_max + 1`. Same thing in the top-down version above.
> +
> + start = ALIGN(start, kbuf->buf_align);
> + if (start < end && (end - start + 1) >= kbuf->memsz) {
Same off-by-one problem. There shouldn't be a `+ 1` here.
> + /* Suitable memory range found. Set kbuf->mem */
> + kbuf->mem = start;
> + ret = 0;
> + break;
> + }
> + }
> +
> + return ret;
> +}
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 17babc2734a55cdf4a678d572f316280b820202b
From: kernel test robot @ 2020-07-15 2:52 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 17babc2734a55cdf4a678d572f316280b820202b powerpc/fadump: fix race between pstore write and fadump crash trigger
elapsed time: 720m
configs tested: 104
configs skipped: 8
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm allyesconfig
arm allmodconfig
arm allnoconfig
arm64 allyesconfig
arm64 defconfig
arm64 allmodconfig
arm64 allnoconfig
sh se7724_defconfig
sh sdk7786_defconfig
arm tango4_defconfig
sh migor_defconfig
sh se7206_defconfig
mips malta_qemu_32r6_defconfig
sparc64 defconfig
mips pistachio_defconfig
sh alldefconfig
powerpc64 defconfig
arm u300_defconfig
i386 allyesconfig
arm s3c6400_defconfig
m68k amcore_defconfig
c6x allnoconfig
xtensa generic_kc705_defconfig
mips ip27_defconfig
mips mips_paravirt_defconfig
c6x evmc6474_defconfig
powerpc pasemi_defconfig
sh dreamcast_defconfig
openrisc alldefconfig
m68k m5407c3_defconfig
arm ep93xx_defconfig
arm moxart_defconfig
sparc defconfig
mips maltaup_defconfig
mips rb532_defconfig
powerpc mpc7448_hpc2_defconfig
mips nlm_xlp_defconfig
sh kfr2r09-romimage_defconfig
openrisc defconfig
xtensa iss_defconfig
arm colibri_pxa270_defconfig
ia64 gensparse_defconfig
i386 allnoconfig
i386 defconfig
i386 debian-10.3
ia64 allmodconfig
ia64 defconfig
ia64 allnoconfig
ia64 allyesconfig
m68k allmodconfig
m68k allnoconfig
m68k sun3_defconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nios2 allyesconfig
c6x allyesconfig
openrisc allyesconfig
nds32 defconfig
nds32 allnoconfig
csky allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
h8300 allmodconfig
xtensa defconfig
arc defconfig
arc allyesconfig
sh allmodconfig
sh allnoconfig
microblaze allnoconfig
mips allyesconfig
mips allnoconfig
mips allmodconfig
parisc allnoconfig
parisc defconfig
parisc allyesconfig
parisc allmodconfig
powerpc allyesconfig
powerpc rhel-kconfig
powerpc allmodconfig
powerpc allnoconfig
powerpc defconfig
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
s390 allyesconfig
s390 allnoconfig
s390 allmodconfig
s390 defconfig
sparc allyesconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 allmodconfig
x86_64 rhel-7.6-kselftests
x86_64 rhel-8.3
x86_64 kexec
x86_64 rhel
x86_64 lkp
x86_64 fedora-25
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS b710d27bf72068b15b2f0305d825988183e2ff28
From: kernel test robot @ 2020-07-15 2:52 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: b710d27bf72068b15b2f0305d825988183e2ff28 powerpc/pseries/svm: Fix incorrect check for shared_lppaca_size
elapsed time: 721m
configs tested: 110
configs skipped: 6
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm allyesconfig
arm allmodconfig
arm allnoconfig
arm64 allyesconfig
arm64 defconfig
arm64 allmodconfig
arm64 allnoconfig
sh se7724_defconfig
sh sdk7786_defconfig
arm tango4_defconfig
sh migor_defconfig
i386 allyesconfig
arm s3c6400_defconfig
m68k amcore_defconfig
c6x allnoconfig
xtensa generic_kc705_defconfig
mips ip27_defconfig
mips mips_paravirt_defconfig
c6x evmc6474_defconfig
powerpc pasemi_defconfig
sh dreamcast_defconfig
openrisc alldefconfig
m68k m5407c3_defconfig
arm ep93xx_defconfig
arm moxart_defconfig
sparc defconfig
mips maltaup_defconfig
mips rb532_defconfig
powerpc mpc7448_hpc2_defconfig
mips nlm_xlp_defconfig
sh kfr2r09-romimage_defconfig
openrisc defconfig
xtensa iss_defconfig
arm colibri_pxa270_defconfig
ia64 gensparse_defconfig
i386 allnoconfig
i386 defconfig
i386 debian-10.3
ia64 allmodconfig
ia64 defconfig
ia64 allnoconfig
ia64 allyesconfig
m68k allmodconfig
m68k allnoconfig
m68k sun3_defconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nios2 allyesconfig
c6x allyesconfig
openrisc allyesconfig
nds32 defconfig
nds32 allnoconfig
csky allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
h8300 allmodconfig
xtensa defconfig
arc defconfig
arc allyesconfig
sh allmodconfig
sh allnoconfig
microblaze allnoconfig
mips allyesconfig
mips allnoconfig
mips allmodconfig
parisc allnoconfig
parisc defconfig
parisc allyesconfig
parisc allmodconfig
powerpc allyesconfig
powerpc rhel-kconfig
powerpc allmodconfig
powerpc allnoconfig
powerpc defconfig
i386 randconfig-a001-20200714
i386 randconfig-a005-20200714
i386 randconfig-a002-20200714
i386 randconfig-a006-20200714
i386 randconfig-a003-20200714
i386 randconfig-a004-20200714
i386 randconfig-a016-20200714
i386 randconfig-a011-20200714
i386 randconfig-a015-20200714
i386 randconfig-a012-20200714
i386 randconfig-a013-20200714
i386 randconfig-a014-20200714
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
s390 allyesconfig
s390 allnoconfig
s390 allmodconfig
s390 defconfig
sparc allyesconfig
sparc64 defconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 allmodconfig
x86_64 rhel-7.6-kselftests
x86_64 rhel-8.3
x86_64 kexec
x86_64 rhel
x86_64 lkp
x86_64 fedora-25
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 10/15] powerpc/powernv/pci: Refactor pnv_ioda_alloc_pe()
From: Oliver O'Halloran @ 2020-07-15 2:53 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev
In-Reply-To: <dd85f256-0f52-f2f5-f729-2d5b7269f895@ozlabs.ru>
On Wed, Jul 15, 2020 at 12:29 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>
>
> On 10/07/2020 15:23, Oliver O'Halloran wrote:
> > Rework the PE allocation logic to allow allocating blocks of PEs rather
> > than individually. We'll use this to allocate contigious blocks of PEs for
> > the SR-IOVs.
>
> The patch does not do just this, it also adds missing mutexes (which is
> good) but still misses them in pnv_pci_sriov_disable() and
> pnv_pci_ioda_pe_dump().
The current implementation doesn't need the mutex because alloc,
reserve and free all use atomic bit ops. The mutex has been there
forever with nothing actually using it, but with the change we need to
prevent modifications to the bitmap while alloc() is scanning it. I
probably should have mentioned that in the commit message.
^ permalink raw reply
* [PATCH -next] powerpc/xive: Remove unused inline function xive_kexec_teardown_cpu()
From: YueHaibing @ 2020-07-15 2:50 UTC (permalink / raw)
To: mpe, benh, paulus, groug; +Cc: YueHaibing, linuxppc-dev, linux-kernel
commit e27e0a94651e ("powerpc/xive: Remove xive_kexec_teardown_cpu()")
left behind this, remove it.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
arch/powerpc/include/asm/xive.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index d08ea11b271c..309b4d65b74f 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -155,7 +155,6 @@ static inline void xive_smp_probe(void) { }
static inline int xive_smp_prepare_cpu(unsigned int cpu) { return -EINVAL; }
static inline void xive_smp_setup_cpu(void) { }
static inline void xive_smp_disable_cpu(void) { }
-static inline void xive_kexec_teardown_cpu(int secondary) { }
static inline void xive_shutdown(void) { }
static inline void xive_flush_interrupt(void) { }
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header
From: Viresh Kumar @ 2020-07-15 3:07 UTC (permalink / raw)
To: Lee Jones
Cc: linux-pm, rjw, linux-kernel, Paul Mackerras, Olof Johansson,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200714145049.2496163-6-lee.jones@linaro.org>
On 14-07-20, 15:50, Lee Jones wrote:
> If function callers and providers do not share the same prototypes the
> compiler complains of missing prototypes. Fix this by moving the
> already existing prototypes out to a mutually convenient location.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/cpufreq/pasemi-cpufreq.c:109:5: warning: no previous prototype for ‘check_astate’ [-Wmissing-prototypes]
> 109 | int check_astate(void)
> | ^~~~~~~~~~~~
> drivers/cpufreq/pasemi-cpufreq.c:114:6: warning: no previous prototype for ‘restore_astate’ [-Wmissing-prototypes]
> 114 | void restore_astate(int cpu)
> | ^~~~~~~~~~~~~~
>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> arch/powerpc/platforms/pasemi/pasemi.h | 15 ------------
Is there no sane way we can include this file directly to the cpufreq
file ?
> arch/powerpc/platforms/pasemi/powersave.S | 2 ++
> drivers/cpufreq/pasemi-cpufreq.c | 1 +
> include/linux/platform_data/pasemi.h | 28 +++++++++++++++++++++++
> 4 files changed, 31 insertions(+), 15 deletions(-)
> create mode 100644 include/linux/platform_data/pasemi.h
--
viresh
^ permalink raw reply
* Re: [PATCH 06/13] cpufreq: powernv-cpufreq: Functions only used in call-backs should be static
From: Viresh Kumar @ 2020-07-15 3:07 UTC (permalink / raw)
To: Lee Jones
Cc: linux-pm, rjw, linux-kernel, Paul Mackerras, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20200714145049.2496163-7-lee.jones@linaro.org>
On 14-07-20, 15:50, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
>
> drivers/cpufreq/powernv-cpufreq.c:669:6: warning: no previous prototype for ‘gpstate_timer_handler’ [-Wmissing-prototypes]
> drivers/cpufreq/powernv-cpufreq.c:902:6: warning: no previous prototype for ‘powernv_cpufreq_work_fn’ [-Wmissing-prototypes]
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/cpufreq/powernv-cpufreq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 8646eb197cd96..068cc53abe320 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -666,7 +666,7 @@ static inline void queue_gpstate_timer(struct global_pstate_info *gpstates)
> * according quadratic equation. Queues a new timer if it is still not equal
> * to local pstate
> */
> -void gpstate_timer_handler(struct timer_list *t)
> +static void gpstate_timer_handler(struct timer_list *t)
> {
> struct global_pstate_info *gpstates = from_timer(gpstates, t, timer);
> struct cpufreq_policy *policy = gpstates->policy;
> @@ -899,7 +899,7 @@ static struct notifier_block powernv_cpufreq_reboot_nb = {
> .notifier_call = powernv_cpufreq_reboot_notifier,
> };
>
> -void powernv_cpufreq_work_fn(struct work_struct *work)
> +static void powernv_cpufreq_work_fn(struct work_struct *work)
> {
> struct chip *chip = container_of(work, struct chip, throttle);
> struct cpufreq_policy *policy;
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH 07/13] cpufreq: powernv-cpufreq: Fix a bunch of kerneldoc related issues
From: Viresh Kumar @ 2020-07-15 3:09 UTC (permalink / raw)
To: Lee Jones
Cc: linux-pm, rjw, linux-kernel, Paul Mackerras, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20200714145049.2496163-8-lee.jones@linaro.org>
On 14-07-20, 15:50, Lee Jones wrote:
> Repair problems with formatting and missing attributes/parameters, and
> demote header comments which do not meet the required standards
> applicable to kerneldoc.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/cpufreq/powernv-cpufreq.c:84: warning: Function parameter or member 'last_lpstate_idx' not described in 'global_pstate_info'
> drivers/cpufreq/powernv-cpufreq.c:84: warning: Function parameter or member 'last_gpstate_idx' not described in 'global_pstate_info'
> drivers/cpufreq/powernv-cpufreq.c:84: warning: Function parameter or member 'policy' not described in 'global_pstate_info'
> drivers/cpufreq/powernv-cpufreq.c:182: warning: Function parameter or member 'i' not described in 'idx_to_pstate'
> drivers/cpufreq/powernv-cpufreq.c:201: warning: Function parameter or member 'pstate' not described in 'pstate_to_idx'
> drivers/cpufreq/powernv-cpufreq.c:670: warning: Function parameter or member 't' not described in 'gpstate_timer_handler'
> drivers/cpufreq/powernv-cpufreq.c:670: warning: Excess function parameter 'data' description in 'gpstate_timer_handler'
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/cpufreq/powernv-cpufreq.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 068cc53abe320..2e5a8b8a4abaa 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -64,13 +64,14 @@
> * highest_lpstate_idx
> * @last_sampled_time: Time from boot in ms when global pstates were
> * last set
> - * @last_lpstate_idx, Last set value of local pstate and global
> - * last_gpstate_idx pstate in terms of cpufreq table index
> + * @last_lpstate_idx: Last set value of local pstate and global
> + * @last_gpstate_idx: pstate in terms of cpufreq table index
> * @timer: Is used for ramping down if cpu goes idle for
> * a long time with global pstate held high
> * @gpstate_lock: A spinlock to maintain synchronization between
> * routines called by the timer handler and
> * governer's target_index calls
> + * @policy: Associated CPUFreq policy
> */
> struct global_pstate_info {
> int highest_lpstate_idx;
> @@ -170,7 +171,7 @@ static inline u8 extract_pstate(u64 pmsr_val, unsigned int shift)
>
> /* Use following functions for conversions between pstate_id and index */
>
> -/**
> +/*
> * idx_to_pstate : Returns the pstate id corresponding to the
> * frequency in the cpufreq frequency table
> * powernv_freqs indexed by @i.
> @@ -188,7 +189,7 @@ static inline u8 idx_to_pstate(unsigned int i)
> return powernv_freqs[i].driver_data;
> }
>
> -/**
> +/*
> * pstate_to_idx : Returns the index in the cpufreq frequencytable
> * powernv_freqs for the frequency whose corresponding
> * pstate id is @pstate.
> @@ -660,7 +661,7 @@ static inline void queue_gpstate_timer(struct global_pstate_info *gpstates)
> /**
> * gpstate_timer_handler
> *
> - * @data: pointer to cpufreq_policy on which timer was queued
> + * @t: Timer context used to fetch global pstate info struct
> *
> * This handler brings down the global pstate closer to the local pstate
> * according quadratic equation. Queues a new timer if it is still not equal
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* [PATCH -next] powerpc/smp: Remove unused inline functions
From: YueHaibing @ 2020-07-15 3:09 UTC (permalink / raw)
To: mpe, benh, paulus; +Cc: YueHaibing, linuxppc-dev, linux-kernel
commit 441c19c8a290 ("powerpc/kvm/book3s_hv: Rework the secondary inhibit code")
left behind this, remove it.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
arch/powerpc/include/asm/smp.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 49a25e2400f2..5d5a7596d144 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -176,8 +176,6 @@ extern void __cpu_die(unsigned int cpu);
/* for UP */
#define hard_smp_processor_id() get_hard_smp_processor_id(0)
#define smp_setup_cpu_maps()
-static inline void inhibit_secondary_onlining(void) {}
-static inline void uninhibit_secondary_onlining(void) {}
static inline const struct cpumask *cpu_sibling_mask(int cpu)
{
return cpumask_of(cpu);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 10/15] powerpc/powernv/pci: Refactor pnv_ioda_alloc_pe()
From: Alexey Kardashevskiy @ 2020-07-15 3:15 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <CAOSf1CGkQBL5caG7tgCUzC7OP55qVk86_XbAx9L43+hy7YNXSw@mail.gmail.com>
On 15/07/2020 12:53, Oliver O'Halloran wrote:
> On Wed, Jul 15, 2020 at 12:29 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>
>>
>> On 10/07/2020 15:23, Oliver O'Halloran wrote:
>>> Rework the PE allocation logic to allow allocating blocks of PEs rather
>>> than individually. We'll use this to allocate contigious blocks of PEs for
>>> the SR-IOVs.
>>
>> The patch does not do just this, it also adds missing mutexes (which is
>> good) but still misses them in pnv_pci_sriov_disable() and
>> pnv_pci_ioda_pe_dump().
>
> The current implementation doesn't need the mutex because alloc,
> reserve and free all use atomic bit ops.
Ah, ok.
> The mutex has been there
> forever with nothing actually using it, but with the change we need to
> prevent modifications to the bitmap while alloc() is scanning it. I
> probably should have mentioned that in the commit message.
but bitmap_clear() (from pnv_pci_sriov_disable()) is not atomic. It
probably does not matter as the next patch gets rid of it anyway.
--
Alexey
^ permalink raw reply
* [PATCH] powerpc: Fix inconsistent function names
From: YueHaibing @ 2020-07-15 3:19 UTC (permalink / raw)
To: mpe, benh, paulus, haren, dave.hansen, npiggin
Cc: YueHaibing, linuxppc-dev, linux-kernel
The stub helpers name should be consistent with prototypes.
mm_context_add_vas_windows() --> mm_context_add_vas_window()
mm_context_remove_vas_windows() --> mm_context_remove_vas_window()
Fixes: c420644c0a8f ("powerpc: Use mm_context vas_windows counter to issue CP_ABORT")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
arch/powerpc/include/asm/mmu_context.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 1a474f6b1992..00fd1d44731a 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -218,8 +218,8 @@ static inline void inc_mm_active_cpus(struct mm_struct *mm) { }
static inline void dec_mm_active_cpus(struct mm_struct *mm) { }
static inline void mm_context_add_copro(struct mm_struct *mm) { }
static inline void mm_context_remove_copro(struct mm_struct *mm) { }
-static inline void mm_context_add_vas_windows(struct mm_struct *mm) { }
-static inline void mm_context_remove_vas_windows(struct mm_struct *mm) { }
+static inline void mm_context_add_vas_window(struct mm_struct *mm) { }
+static inline void mm_context_remove_vas_window(struct mm_struct *mm) { }
#endif
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header
From: Olof Johansson @ 2020-07-15 3:26 UTC (permalink / raw)
To: Lee Jones
Cc: linux-pm, Rafael J. Wysocki, Linux Kernel Mailing List,
Paul Mackerras, viresh kumar, linuxppc-dev,
Linux ARM Mailing List
In-Reply-To: <20200714145049.2496163-6-lee.jones@linaro.org>
On Tue, Jul 14, 2020 at 7:50 AM Lee Jones <lee.jones@linaro.org> wrote:
>
> If function callers and providers do not share the same prototypes the
> compiler complains of missing prototypes. Fix this by moving the
> already existing prototypes out to a mutually convenient location.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/cpufreq/pasemi-cpufreq.c:109:5: warning: no previous prototype for ‘check_astate’ [-Wmissing-prototypes]
> 109 | int check_astate(void)
> | ^~~~~~~~~~~~
> drivers/cpufreq/pasemi-cpufreq.c:114:6: warning: no previous prototype for ‘restore_astate’ [-Wmissing-prototypes]
> 114 | void restore_astate(int cpu)
> | ^~~~~~~~~~~~~~
>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> arch/powerpc/platforms/pasemi/pasemi.h | 15 ------------
> arch/powerpc/platforms/pasemi/powersave.S | 2 ++
> drivers/cpufreq/pasemi-cpufreq.c | 1 +
> include/linux/platform_data/pasemi.h | 28 +++++++++++++++++++++++
> 4 files changed, 31 insertions(+), 15 deletions(-)
> create mode 100644 include/linux/platform_data/pasemi.h
>
> diff --git a/arch/powerpc/platforms/pasemi/pasemi.h b/arch/powerpc/platforms/pasemi/pasemi.h
> index 70b56048ed1be..528d81ef748ad 100644
> --- a/arch/powerpc/platforms/pasemi/pasemi.h
> +++ b/arch/powerpc/platforms/pasemi/pasemi.h
> @@ -15,21 +15,6 @@ extern void __init pasemi_map_registers(void);
> extern void idle_spin(void);
> extern void idle_doze(void);
>
> -/* Restore astate to last set */
> -#ifdef CONFIG_PPC_PASEMI_CPUFREQ
> -extern int check_astate(void);
> -extern void restore_astate(int cpu);
> -#else
> -static inline int check_astate(void)
> -{
> - /* Always return >0 so we never power save */
> - return 1;
> -}
> -static inline void restore_astate(int cpu)
> -{
> -}
> -#endif
> -
> extern struct pci_controller_ops pasemi_pci_controller_ops;
>
> #endif /* _PASEMI_PASEMI_H */
> diff --git a/arch/powerpc/platforms/pasemi/powersave.S b/arch/powerpc/platforms/pasemi/powersave.S
> index d0215d5329ca7..7747b48963286 100644
> --- a/arch/powerpc/platforms/pasemi/powersave.S
> +++ b/arch/powerpc/platforms/pasemi/powersave.S
> @@ -5,6 +5,8 @@
> * Maintained by: Olof Johansson <olof@lixom.net>
> */
>
> +#include <linux/platform_data/pasemi.h>
> +
> #include <asm/processor.h>
> #include <asm/page.h>
> #include <asm/ppc_asm.h>
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> index c66f566a854cb..c6bb3ecc90ef3 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -15,6 +15,7 @@
> #include <linux/timer.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> +#include <linux/platform_data/pasemi.h>
>
> #include <asm/hw_irq.h>
> #include <asm/io.h>
> diff --git a/include/linux/platform_data/pasemi.h b/include/linux/platform_data/pasemi.h
> new file mode 100644
> index 0000000000000..3fed0687fcc9a
> --- /dev/null
> +++ b/include/linux/platform_data/pasemi.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Linaro Ltd.
> + *
> + * Author: Lee Jones <lee.jones@linaro.org>
> + */
Absolutely not. It's neither your copyright, nor your authorship.
-Olof
^ permalink raw reply
* Re: [PATCH 11/15] powerpc/powernv/sriov: Drop iov->pe_num_map[]
From: Alexey Kardashevskiy @ 2020-07-15 3:31 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200710052340.737567-12-oohall@gmail.com>
On 10/07/2020 15:23, Oliver O'Halloran wrote:
> Currently the iov->pe_num_map[] does one of two things depending on
> whether single PE mode is being used or not. When it is, this contains an
> array which maps a vf_index to the corresponding PE number. When single PE
> mode is not being used this contains a scalar which is the base PE for the
> set of enabled VFs (for for VFn is base + n).
>
> The array was necessary because when calling pnv_ioda_alloc_pe() there is
> no guarantee that the allocated PEs would be contigious. We can now
s/contigious/contiguous/ here and below.
> allocate contigious blocks of PEs so this is no longer an issue. This
> allows us to drop the if (single_mode) {} .. else {} block scattered
> through the SR-IOV code which is a nice clean up.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> arch/powerpc/platforms/powernv/pci-sriov.c | 109 +++++----------------
> arch/powerpc/platforms/powernv/pci.h | 4 +-
> 2 files changed, 25 insertions(+), 88 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> index d53a85ccb538..08f88187d65a 100644
> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> @@ -456,11 +456,13 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, u16 num_vfs)
>
>
> if (iov->m64_single_mode) {
> + int pe_num = iov->vf_pe_arr[j].pe_number;
> +
> size = pci_iov_resource_size(pdev,
> PCI_IOV_RESOURCES + i);
> start = res->start + size * j;
> rc = pnv_ioda_map_m64_single(phb, win,
> - iov->pe_num_map[j],
> + pe_num,
> start,
> size);
> } else {
> @@ -599,38 +601,24 @@ static int pnv_pci_vf_resource_shift(struct pci_dev *dev, int offset)
>
> static void pnv_pci_sriov_disable(struct pci_dev *pdev)
> {
> + u16 num_vfs, base_pe;
> struct pnv_phb *phb;
> - struct pnv_ioda_pe *pe;
> struct pnv_iov_data *iov;
> - u16 num_vfs, i;
>
> phb = pci_bus_to_pnvhb(pdev->bus);
> iov = pnv_iov_get(pdev);
> num_vfs = iov->num_vfs;
> + base_pe = iov->vf_pe_arr[0].pe_number;
>
> /* Release VF PEs */
> pnv_ioda_release_vf_PE(pdev);
>
> if (phb->type == PNV_PHB_IODA2) {
> if (!iov->m64_single_mode)
> - pnv_pci_vf_resource_shift(pdev, -*iov->pe_num_map);
> + pnv_pci_vf_resource_shift(pdev, -base_pe);
>
> /* Release M64 windows */
> pnv_pci_vf_release_m64(pdev, num_vfs);
> -
> - /* Release PE numbers */
> - if (iov->m64_single_mode) {
> - for (i = 0; i < num_vfs; i++) {
> - if (iov->pe_num_map[i] == IODA_INVALID_PE)
> - continue;
> -
> - pe = &phb->ioda.pe_array[iov->pe_num_map[i]];
> - pnv_ioda_free_pe(pe);
> - }
> - } else
> - bitmap_clear(phb->ioda.pe_alloc, *iov->pe_num_map, num_vfs);
> - /* Releasing pe_num_map */
> - kfree(iov->pe_num_map);
> }
> }
>
> @@ -656,13 +644,7 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
> int vf_bus = pci_iov_virtfn_bus(pdev, vf_index);
> struct pci_dn *vf_pdn;
>
> - if (iov->m64_single_mode)
> - pe_num = iov->pe_num_map[vf_index];
> - else
> - pe_num = *iov->pe_num_map + vf_index;
> -
> - pe = &phb->ioda.pe_array[pe_num];
> - pe->pe_number = pe_num;
> + pe = &iov->vf_pe_arr[vf_index];
> pe->phb = phb;
> pe->flags = PNV_IODA_PE_VF;
> pe->pbus = NULL;
> @@ -670,6 +652,7 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
> pe->mve_number = -1;
> pe->rid = (vf_bus << 8) | vf_devfn;
>
> + pe_num = pe->pe_number;
> pe_info(pe, "VF %04d:%02d:%02d.%d associated with PE#%x\n",
> pci_domain_nr(pdev->bus), pdev->bus->number,
> PCI_SLOT(vf_devfn), PCI_FUNC(vf_devfn), pe_num);
> @@ -701,9 +684,9 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
>
> static int pnv_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
> {
> + struct pnv_ioda_pe *base_pe;
> struct pnv_iov_data *iov;
> struct pnv_phb *phb;
> - struct pnv_ioda_pe *pe;
> int ret;
> u16 i;
>
> @@ -717,55 +700,14 @@ static int pnv_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
> return -ENOSPC;
> }
>
> - /*
> - * When M64 BARs functions in Single PE mode, the number of VFs
> - * could be enabled must be less than the number of M64 BARs.
> - */
> - if (iov->m64_single_mode && num_vfs > phb->ioda.m64_bar_idx) {
> - dev_info(&pdev->dev, "Not enough M64 BAR for VFs\n");
> + /* allocate a contigious block of PEs for our VFs */
> + base_pe = pnv_ioda_alloc_pe(phb, num_vfs);
> + if (!base_pe) {
> + pci_err(pdev, "Unable to allocate PEs for %d VFs\n", num_vfs);
> return -EBUSY;
> }
>
> - /* Allocating pe_num_map */
> - if (iov->m64_single_mode)
> - iov->pe_num_map = kmalloc_array(num_vfs,
> - sizeof(*iov->pe_num_map),
> - GFP_KERNEL);
> - else
> - iov->pe_num_map = kmalloc(sizeof(*iov->pe_num_map), GFP_KERNEL);
> -
> - if (!iov->pe_num_map)
> - return -ENOMEM;
> -
> - if (iov->m64_single_mode)
> - for (i = 0; i < num_vfs; i++)
> - iov->pe_num_map[i] = IODA_INVALID_PE;
> -
> - /* Calculate available PE for required VFs */
> - if (iov->m64_single_mode) {
> - for (i = 0; i < num_vfs; i++) {
> - pe = pnv_ioda_alloc_pe(phb);
> - if (!pe) {
> - ret = -EBUSY;
> - goto m64_failed;
> - }
> -
> - iov->pe_num_map[i] = pe->pe_number;
> - }
> - } else {
> - mutex_lock(&phb->ioda.pe_alloc_mutex);
> - *iov->pe_num_map = bitmap_find_next_zero_area(
> - phb->ioda.pe_alloc, phb->ioda.total_pe_num,
> - 0, num_vfs, 0);
> - if (*iov->pe_num_map >= phb->ioda.total_pe_num) {
> - mutex_unlock(&phb->ioda.pe_alloc_mutex);
> - dev_info(&pdev->dev, "Failed to enable VF%d\n", num_vfs);
> - kfree(iov->pe_num_map);
> - return -EBUSY;
> - }
> - bitmap_set(phb->ioda.pe_alloc, *iov->pe_num_map, num_vfs);
> - mutex_unlock(&phb->ioda.pe_alloc_mutex);
> - }
> + iov->vf_pe_arr = base_pe;
> iov->num_vfs = num_vfs;
>
> /* Assign M64 window accordingly */
> @@ -781,9 +723,10 @@ static int pnv_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
> * Otherwise, the PE# for the VF will conflict with others.
> */
> if (!iov->m64_single_mode) {
> - ret = pnv_pci_vf_resource_shift(pdev, *iov->pe_num_map);
> + ret = pnv_pci_vf_resource_shift(pdev,
> + base_pe->pe_number);
> if (ret)
> - goto m64_failed;
> + goto shift_failed;
> }
> }
>
> @@ -792,20 +735,12 @@ static int pnv_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>
> return 0;
>
> -m64_failed:
> - if (iov->m64_single_mode) {
> - for (i = 0; i < num_vfs; i++) {
> - if (iov->pe_num_map[i] == IODA_INVALID_PE)
> - continue;
> -
> - pe = &phb->ioda.pe_array[iov->pe_num_map[i]];
> - pnv_ioda_free_pe(pe);
> - }
> - } else
> - bitmap_clear(phb->ioda.pe_alloc, *iov->pe_num_map, num_vfs);
> +shift_failed:
> + pnv_pci_vf_release_m64(pdev, num_vfs);
>
> - /* Releasing pe_num_map */
> - kfree(iov->pe_num_map);
> +m64_failed:
> + for (i = 0; i < num_vfs; i++)
> + pnv_ioda_free_pe(&iov->vf_pe_arr[i]);
>
> return ret;
> }
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index b4c9bdba7217..13555bc549f4 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -238,7 +238,9 @@ struct pnv_iov_data {
>
> /* number of VFs enabled */
> u16 num_vfs;
> - unsigned int *pe_num_map; /* PE# for the first VF PE or array */
> +
> + /* pointer to the array of VF PEs. num_vfs long*/
I read the comment and for a second I thought that now you are storing
pnv_ioda_pe structs in pnv_iov_data which is not true: vf_pe_arr
actually points inside phb->ioda.pe_array[]. May be add this to the
comment please.
Otherwise good,
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> + struct pnv_ioda_pe *vf_pe_arr;
>
> /* Did we map the VF BARs with single-PE IODA BARs? */
> bool m64_single_mode;
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 03/15] powerpc/powernv/pci: Add explicit tracking of the DMA setup state
From: Alexey Kardashevskiy @ 2020-07-15 3:33 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev, Cédric Le Goater
In-Reply-To: <CAOSf1CGhc1i64_DS18evMhwoXvMWyafOk3r2czUz49F6gERtfw@mail.gmail.com>
On 15/07/2020 11:38, Oliver O'Halloran wrote:
> On Tue, Jul 14, 2020 at 5:21 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> On 14/07/2020 15:58, Oliver O'Halloran wrote:
>>> On Tue, Jul 14, 2020 at 3:37 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>
>>>> On 10/07/2020 15:23, Oliver O'Halloran wrote:
>>>>> This also means the only remaining user of the old "DMA Weight" code is
>>>>> the IODA1 DMA setup code that it was originally added for, which is good.
>>>>
>>>>
>>>> Is ditching IODA1 in the plan? :)
>>>
>>> That or separating out the pci_controller_ops for IODA1 and IODA2 so
>>> we can stop any IODA2 specific changes from breaking it.
>>
>> Is IODA1 tested at all these days? Or, is anyone running upstream
>> kernels anywhere and keeps shouting when it does not work on IODA1? Thanks,
>
> Cedric has a P7 with OPAL. That's probably the one left though.
Has he tried these patches on that box? Or we hope for the best here? :)
--
Alexey
^ permalink raw reply
* Re: [PATCH -next] cpufreq: powernv: Make some symbols static
From: Viresh Kumar @ 2020-07-15 3:41 UTC (permalink / raw)
To: Wei Yongjun, lee.jones
Cc: Hulk Robot, linux-pm, Rafael J. Wysocki, linuxppc-dev
In-Reply-To: <20200714142355.29819-1-weiyongjun1@huawei.com>
On 14-07-20, 22:23, Wei Yongjun wrote:
> The sparse tool complains as follows:
>
> drivers/cpufreq/powernv-cpufreq.c:88:1: warning:
> symbol 'pstate_revmap' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:383:18: warning:
> symbol 'cpufreq_freq_attr_cpuinfo_nominal_freq' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:669:6: warning:
> symbol 'gpstate_timer_handler' was not declared. Should it be static?
> drivers/cpufreq/powernv-cpufreq.c:902:6: warning:
> symbol 'powernv_cpufreq_work_fn' was not declared. Should it be static?
>
> Those symbols are not used outside of this file, so mark
> them static.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> drivers/cpufreq/powernv-cpufreq.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Lee also sent a fix for this, but yours look complete :)
https://lore.kernel.org/lkml/20200714145049.2496163-7-lee.jones@linaro.org/
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header
From: Olof Johansson @ 2020-07-15 3:49 UTC (permalink / raw)
To: Viresh Kumar
Cc: linux-pm, linuxppc-dev, Rafael J. Wysocki,
Linux Kernel Mailing List, Paul Mackerras, Lee Jones,
Linux ARM Mailing List
In-Reply-To: <20200715030706.prxya7fyylscoy25@vireshk-i7>
On Tue, Jul 14, 2020 at 8:07 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 14-07-20, 15:50, Lee Jones wrote:
> > If function callers and providers do not share the same prototypes the
> > compiler complains of missing prototypes. Fix this by moving the
> > already existing prototypes out to a mutually convenient location.
> >
> > Fixes the following W=1 kernel build warning(s):
> >
> > drivers/cpufreq/pasemi-cpufreq.c:109:5: warning: no previous prototype for ‘check_astate’ [-Wmissing-prototypes]
> > 109 | int check_astate(void)
> > | ^~~~~~~~~~~~
> > drivers/cpufreq/pasemi-cpufreq.c:114:6: warning: no previous prototype for ‘restore_astate’ [-Wmissing-prototypes]
> > 114 | void restore_astate(int cpu)
> > | ^~~~~~~~~~~~~~
> >
> > Cc: Olof Johansson <olof@lixom.net>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > arch/powerpc/platforms/pasemi/pasemi.h | 15 ------------
>
> Is there no sane way we can include this file directly to the cpufreq
> file ?
Yep. arch/powerpc seems to be in the search path for modules on powerpc, so:
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index c66f566a854cb..815645170c4de 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -22,6 +22,8 @@
#include <asm/time.h>
#include <asm/smp.h>
+#include <platforms/pasemi/pasemi.h>
+
#define SDCASR_REG 0x0100
#define SDCASR_REG_STRIDE 0x1000
#define SDCPWR_CFGA0_REG 0x0100
-Olof
^ permalink raw reply related
* Re: [PATCH v3 05/12] powerpc/drmem: make lmb walk a bit more flexible
From: Thiago Jung Bauermann @ 2020-07-15 3:50 UTC (permalink / raw)
To: Hari Bathini
Cc: Pingfan Liu, Nayna Jain, Kexec-ml, Mahesh J Salgaonkar,
Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain, Petr Tesarik,
Andrew Morton, Dave Young, Vivek Goyal, Eric Biederman
In-Reply-To: <159466090332.24747.9255471295044653085.stgit@hbathini.in.ibm.com>
Hari Bathini <hbathini@linux.ibm.com> writes:
> @@ -534,7 +537,7 @@ static int __init early_init_dt_scan_memory_ppc(unsigned long node,
> #ifdef CONFIG_PPC_PSERIES
> if (depth == 1 &&
> strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
> - walk_drmem_lmbs_early(node, early_init_drmem_lmb);
> + walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
walk_drmem_lmbs_early() can now fail. Should this failure be propagated
as a return value of early_init_dt_scan_memory_ppc()?
> return 0;
> }
> #endif
<snip>
> @@ -787,7 +790,7 @@ static int __init parse_numa_properties(void)
> */
> memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> if (memory) {
> - walk_drmem_lmbs(memory, numa_setup_drmem_lmb);
> + walk_drmem_lmbs(memory, NULL, numa_setup_drmem_lmb);
Similarly here. Now that this call can fail, should
parse_numa_properties() handle or propagate the failure?
> of_node_put(memory);
> }
>
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH 05/13] cpufreq/arch: powerpc: pasemi: Move prototypes to shared header
From: Viresh Kumar @ 2020-07-15 3:51 UTC (permalink / raw)
To: Olof Johansson
Cc: linux-pm, linuxppc-dev, Rafael J. Wysocki,
Linux Kernel Mailing List, Paul Mackerras, Lee Jones,
Linux ARM Mailing List
In-Reply-To: <CAOesGMi1dfqPbFJ8YoUoJ75NdU1=XiNoYx+6+JLu44a4LuuYGA@mail.gmail.com>
On 14-07-20, 20:49, Olof Johansson wrote:
> On Tue, Jul 14, 2020 at 8:07 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 14-07-20, 15:50, Lee Jones wrote:
> > > If function callers and providers do not share the same prototypes the
> > > compiler complains of missing prototypes. Fix this by moving the
> > > already existing prototypes out to a mutually convenient location.
> > >
> > > Fixes the following W=1 kernel build warning(s):
> > >
> > > drivers/cpufreq/pasemi-cpufreq.c:109:5: warning: no previous prototype for ‘check_astate’ [-Wmissing-prototypes]
> > > 109 | int check_astate(void)
> > > | ^~~~~~~~~~~~
> > > drivers/cpufreq/pasemi-cpufreq.c:114:6: warning: no previous prototype for ‘restore_astate’ [-Wmissing-prototypes]
> > > 114 | void restore_astate(int cpu)
> > > | ^~~~~~~~~~~~~~
> > >
> > > Cc: Olof Johansson <olof@lixom.net>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > > arch/powerpc/platforms/pasemi/pasemi.h | 15 ------------
> >
> > Is there no sane way we can include this file directly to the cpufreq
> > file ?
>
> Yep. arch/powerpc seems to be in the search path for modules on powerpc, so:
>
> diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> index c66f566a854cb..815645170c4de 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -22,6 +22,8 @@
> #include <asm/time.h>
> #include <asm/smp.h>
>
> +#include <platforms/pasemi/pasemi.h>
> +
> #define SDCASR_REG 0x0100
> #define SDCASR_REG_STRIDE 0x1000
> #define SDCPWR_CFGA0_REG 0x0100
Fantastic. Thanks.
--
viresh
^ permalink raw reply
* Re: [PATCH] powerpc: Fix inconsistent function names
From: Michael Ellerman @ 2020-07-15 3:54 UTC (permalink / raw)
To: YueHaibing, benh, paulus, haren, dave.hansen, npiggin
Cc: YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <20200715031939.31968-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> writes:
> The stub helpers name should be consistent with prototypes.
>
> mm_context_add_vas_windows() --> mm_context_add_vas_window()
> mm_context_remove_vas_windows() --> mm_context_remove_vas_window()
>
> Fixes: c420644c0a8f ("powerpc: Use mm_context vas_windows counter to issue CP_ABORT")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> arch/powerpc/include/asm/mmu_context.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 1a474f6b1992..00fd1d44731a 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -218,8 +218,8 @@ static inline void inc_mm_active_cpus(struct mm_struct *mm) { }
> static inline void dec_mm_active_cpus(struct mm_struct *mm) { }
> static inline void mm_context_add_copro(struct mm_struct *mm) { }
> static inline void mm_context_remove_copro(struct mm_struct *mm) { }
> -static inline void mm_context_add_vas_windows(struct mm_struct *mm) { }
> -static inline void mm_context_remove_vas_windows(struct mm_struct *mm) { }
> +static inline void mm_context_add_vas_window(struct mm_struct *mm) { }
> +static inline void mm_context_remove_vas_window(struct mm_struct *mm) { }
> #endif
Both of those functions are only called from 64-bit only code, so the
stubs should not be needed at all. Which explains why we haven't seen a
build break.
So just dropping them would be better IMO.
cheers
^ permalink raw reply
* Re: [PATCH 12/15] powerpc/powernv/sriov: De-indent setup and teardown
From: Alexey Kardashevskiy @ 2020-07-15 4:00 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200710052340.737567-13-oohall@gmail.com>
On 10/07/2020 15:23, Oliver O'Halloran wrote:
> Remove the IODA2 PHB checks. We already assume IODA2 in several places so
> there's not much point in wrapping most of the setup and teardown process
> in an if block.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> arch/powerpc/platforms/powernv/pci-sriov.c | 86 ++++++++++++----------
> 1 file changed, 49 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> index 08f88187d65a..d5699cd2ab7a 100644
> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> @@ -610,16 +610,18 @@ static void pnv_pci_sriov_disable(struct pci_dev *pdev)
> num_vfs = iov->num_vfs;
> base_pe = iov->vf_pe_arr[0].pe_number;
>
> + if (WARN_ON(!iov))
> + return;
> +
> /* Release VF PEs */
> pnv_ioda_release_vf_PE(pdev);
>
> - if (phb->type == PNV_PHB_IODA2) {
> - if (!iov->m64_single_mode)
> - pnv_pci_vf_resource_shift(pdev, -base_pe);
> + /* Un-shift the IOV BAR resources */
> + if (!iov->m64_single_mode)
> + pnv_pci_vf_resource_shift(pdev, -base_pe);
>
> - /* Release M64 windows */
> - pnv_pci_vf_release_m64(pdev, num_vfs);
> - }
> + /* Release M64 windows */
> + pnv_pci_vf_release_m64(pdev, num_vfs);
> }
>
> static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
> @@ -693,41 +695,51 @@ static int pnv_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
> phb = pci_bus_to_pnvhb(pdev->bus);
> iov = pnv_iov_get(pdev);
>
> - if (phb->type == PNV_PHB_IODA2) {
> - if (!iov->vfs_expanded) {
> - dev_info(&pdev->dev, "don't support this SRIOV device"
> - " with non 64bit-prefetchable IOV BAR\n");
> - return -ENOSPC;
> - }
> + /*
> + * There's a calls to IODA2 PE setup code littered throughout. We could
> + * probably fix that, but we'd still have problems due to the
> + * restriction inherent on IODA1 PHBs.
> + *
> + * NB: We class IODA3 as IODA2 since they're very similar.
> + */
> + if (phb->type != PNV_PHB_IODA2) {
> + pci_err(pdev, "SR-IOV is not supported on this PHB\n");
> + return -ENXIO;
> + }
or we could just skip setting
ppc_md.pcibios_sriov_enable = pnv_pcibios_sriov_enable;
for uninteresting platforms in pnv_pci_init_ioda_phb().
>
> - /* allocate a contigious block of PEs for our VFs */
> - base_pe = pnv_ioda_alloc_pe(phb, num_vfs);
> - if (!base_pe) {
> - pci_err(pdev, "Unable to allocate PEs for %d VFs\n", num_vfs);
> - return -EBUSY;
> - }
> + if (!iov->vfs_expanded) {
> + dev_info(&pdev->dev, "don't support this SRIOV device"
> + " with non 64bit-prefetchable IOV BAR\n");
> + return -ENOSPC;
> + }
>
> - iov->vf_pe_arr = base_pe;
> - iov->num_vfs = num_vfs;
> + /* allocate a contigious block of PEs for our VFs */
> + base_pe = pnv_ioda_alloc_pe(phb, num_vfs);
> + if (!base_pe) {
> + pci_err(pdev, "Unable to allocate PEs for %d VFs\n", num_vfs);
> + return -EBUSY;
> + }
>
> - /* Assign M64 window accordingly */
> - ret = pnv_pci_vf_assign_m64(pdev, num_vfs);
> - if (ret) {
> - dev_info(&pdev->dev, "Not enough M64 window resources\n");
> - goto m64_failed;
> - }
> + iov->vf_pe_arr = base_pe;
> + iov->num_vfs = num_vfs;
>
> - /*
> - * When using one M64 BAR to map one IOV BAR, we need to shift
> - * the IOV BAR according to the PE# allocated to the VFs.
> - * Otherwise, the PE# for the VF will conflict with others.
> - */
> - if (!iov->m64_single_mode) {
> - ret = pnv_pci_vf_resource_shift(pdev,
> - base_pe->pe_number);
> - if (ret)
> - goto shift_failed;
> - }
> + /* Assign M64 window accordingly */
> + ret = pnv_pci_vf_assign_m64(pdev, num_vfs);
> + if (ret) {
> + dev_info(&pdev->dev, "Not enough M64 window resources\n");
> + goto m64_failed;
> + }
> +
> + /*
> + * When using one M64 BAR to map one IOV BAR, we need to shift
> + * the IOV BAR according to the PE# allocated to the VFs.
> + * Otherwise, the PE# for the VF will conflict with others.
> + */
> + if (!iov->m64_single_mode) {
> + ret = pnv_pci_vf_resource_shift(pdev,
> + base_pe->pe_number);
This can be a single line now. Thanks,
> + if (ret)
> + goto shift_failed;
> }
>
> /* Setup VF PEs */
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 13/15] powerpc/powernv/sriov: Move M64 BAR allocation into a helper
From: Alexey Kardashevskiy @ 2020-07-15 4:02 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200710052340.737567-14-oohall@gmail.com>
On 10/07/2020 15:23, Oliver O'Halloran wrote:
> I want to refactor the loop this code is currently inside of. Hoist it on
> out.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/platforms/powernv/pci-sriov.c | 31 ++++++++++++++--------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> index d5699cd2ab7a..2f967aa4fbf5 100644
> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> @@ -416,6 +416,23 @@ static int64_t pnv_ioda_map_m64_single(struct pnv_phb *phb,
> return rc;
> }
>
> +static int pnv_pci_alloc_m64_bar(struct pnv_phb *phb, struct pnv_iov_data *iov)
> +{
> + int win;
> +
> + do {
> + win = find_next_zero_bit(&phb->ioda.m64_bar_alloc,
> + phb->ioda.m64_bar_idx + 1, 0);
> +
> + if (win >= phb->ioda.m64_bar_idx + 1)
> + return -1;
> + } while (test_and_set_bit(win, &phb->ioda.m64_bar_alloc));
> +
> + set_bit(win, iov->used_m64_bar_mask);
> +
> + return win;
> +}
> +
> static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, u16 num_vfs)
> {
> struct pnv_iov_data *iov;
> @@ -443,17 +460,9 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, u16 num_vfs)
> continue;
>
> for (j = 0; j < m64_bars; j++) {
> -
> - /* allocate a window ID for this BAR */
> - do {
> - win = find_next_zero_bit(&phb->ioda.m64_bar_alloc,
> - phb->ioda.m64_bar_idx + 1, 0);
> -
> - if (win >= phb->ioda.m64_bar_idx + 1)
> - goto m64_failed;
> - } while (test_and_set_bit(win, &phb->ioda.m64_bar_alloc));
> - set_bit(win, iov->used_m64_bar_mask);
> -
> + win = pnv_pci_alloc_m64_bar(phb, iov);
> + if (win < 0)
> + goto m64_failed;
>
> if (iov->m64_single_mode) {
> int pe_num = iov->vf_pe_arr[j].pe_number;
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v3 2/9] powerpc/watchpoint: Fix DAWR exception constraint
From: Ravi Bangoria @ 2020-07-15 4:08 UTC (permalink / raw)
To: Jordan Niethe
Cc: Christophe Leroy, apopple, mikey, miltonm, peterz, oleg,
Nicholas Piggin, linux-kernel, Paul Mackerras, jolsa, fweisbec,
pedromfc, naveen.n.rao, linuxppc-dev, mingo, Ravi Bangoria
In-Reply-To: <CACzsE9qSjWKWGDQPGSk-c5f5pxUyWWtUFW+AzzB5M76qFcQ-Cw@mail.gmail.com>
Hi Jordan,
>> @@ -536,7 +538,12 @@ static bool check_dawrx_constraints(struct pt_regs *regs, int type,
>> if (OP_IS_LOAD(type) && !(info->type & HW_BRK_TYPE_READ))
>> return false;
>>
>> - if (OP_IS_STORE(type) && !(info->type & HW_BRK_TYPE_WRITE))
>> + /*
>> + * The Cache Management instructions other than dcbz never
>> + * cause a match. i.e. if type is CACHEOP, the instruction
>> + * is dcbz, and dcbz is treated as Store.
>> + */
>> + if ((OP_IS_STORE(type) || type == CACHEOP) && !(info->type & HW_BRK_TYPE_WRITE))
>> return false;
> This change seems seperate to this commit?
I also thought about it but was not sure. See below ...
>>
>> if (is_kernel_addr(regs->nip) && !(info->type & HW_BRK_TYPE_KERNEL))
>> @@ -553,7 +560,8 @@ static bool check_dawrx_constraints(struct pt_regs *regs, int type,
>> * including extraneous exception. Otherwise return false.
>> */
>> static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
>> - int type, int size, struct arch_hw_breakpoint *info)
>> + unsigned long ea, int type, int size,
>> + struct arch_hw_breakpoint *info)
>> {
>> bool in_user_range = dar_in_user_range(regs->dar, info);
>> bool dawrx_constraints;
>> @@ -569,11 +577,10 @@ static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
>> }
>>
>> if (unlikely(ppc_inst_equal(instr, ppc_inst(0)))) {
>> - if (in_user_range)
>> - return true;
>> -
>> - if (dar_in_hw_range(regs->dar, info)) {
>> - info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>> + if (dar_in_hw_range(regs->dar, info))
>> + return true;
>> + } else {
>> return true;
> I think this would be clearer as:
> if (cpu_has_feature(CPU_FTR_ARCH_31) &&
> !(dar_in_hw_range(regs->dar, info)))
> return false;
> else
> return true;
ok
>
>> }
>> return false;
>> @@ -581,10 +588,20 @@ static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
>>
>> dawrx_constraints = check_dawrx_constraints(regs, type, info);
>>
>> - if (dar_user_range_overlaps(regs->dar, size, info))
>> + if (type == UNKNOWN) {
>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>> + if (dar_in_hw_range(regs->dar, info))
>> + return dawrx_constraints;
>> + } else {
>> + return dawrx_constraints;
>> + }
>> + return false;
>> + }
> Similar thing here, it could be:
> if ((cpu_has_feature(CPU_FTR_ARCH_31)) &&
> !(dar_in_hw_range(regs->dar, info)))
> return false;
> else
> return dawrx_constraints;
ok
>> +
>> + if (ea_user_range_overlaps(ea, size, info))
>> return dawrx_constraints;
>>
>> - if (dar_hw_range_overlaps(regs->dar, size, info)) {
>> + if (ea_hw_range_overlaps(ea, size, info)) {
>> if (dawrx_constraints) {
>> info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
>> return true;
>> @@ -593,8 +610,17 @@ static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
>> return false;
>> }
>>
>> +static int cache_op_size(void)
>> +{
>> +#ifdef __powerpc64__
>> + return ppc64_caches.l1d.block_size;
>> +#else
>> + return L1_CACHE_BYTES;
>> +#endif
>> +}
>> +
>> static void get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
>> - int *type, int *size, bool *larx_stcx)
>> + int *type, int *size, unsigned long *ea)
>> {
>> struct instruction_op op;
>>
>> @@ -602,16 +628,23 @@ static void get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
>> return;
>>
>> analyse_instr(&op, regs, *instr);
>> -
>> - /*
>> - * Set size = 8 if analyse_instr() fails. If it's a userspace
>> - * watchpoint(valid or extraneous), we can notify user about it.
>> - * If it's a kernel watchpoint, instruction emulation will fail
>> - * in stepping_handler() and watchpoint will be disabled.
>> - */
>> *type = GETTYPE(op.type);
>> - *size = !(*type == UNKNOWN) ? GETSIZE(op.type) : 8;
>> - *larx_stcx = (*type == LARX || *type == STCX);
>> + *ea = op.ea;
>> +#ifdef __powerpc64__
>> + if (!(regs->msr & MSR_64BIT))
>> + *ea &= 0xffffffffUL;
>> +#endif
>> +
>> + *size = GETSIZE(op.type);
>> + if (*type == CACHEOP) {
>> + *size = cache_op_size();
>> + *ea &= ~(*size - 1);
>> + }
> Again related to CACHEOP, should these changes be mentioned in the
> commit message?
For CACHEOP, ea returned by analyse_instr() needs to be aligned down to cache
block size manually. Also, for CACHEOP, size returned by analyse_instr() is 0
and thus size also needs to be calculated manually. This was missed in
27985b2a640e. So it kind of relates to other changes of the patch but needs
special treatment as well. Will see if I can split it.
Thanks for the review,
Ravi
^ permalink raw reply
* Re: [PATCH 3/3] ASoC: fsl-asoc-card: Support Headphone and Microphone Jack detection
From: Shengjiu Wang @ 2020-07-15 4:14 UTC (permalink / raw)
To: Nicolin Chen
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, kuninori.morimoto.gx, Timur Tabi, samuel, katsuhiro,
Fabio Estevam, Shengjiu Wang, Xiubo Li, linux-kernel,
Liam Girdwood, Rob Herring, Mark Brown, Takashi Iwai,
linuxppc-dev
In-Reply-To: <20200714211432.GA10818@Asurada-Nvidia>
On Wed, Jul 15, 2020 at 5:16 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> Hi Shengjiu,
>
> The whole series looks good to me. Just a couple of small
> questions inline:
>
> On Tue, Jul 14, 2020 at 05:05:36PM +0800, Shengjiu Wang wrote:
> > Use asoc_simple_init_jack function from simple card to implement
> > the Headphone and Microphone detection.
> > Register notifier to disable Speaker when Headphone is plugged in
> > and enable Speaker when Headphone is unplugged.
> > Register notifier to disable Digital Microphone when Analog Microphone
> > is plugged in and enable DMIC when Analog Microphone is unplugged.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > sound/soc/fsl/Kconfig | 1 +
> > sound/soc/fsl/fsl-asoc-card.c | 69 ++++++++++++++++++++++++++++++++++-
> > 2 files changed, 68 insertions(+), 2 deletions(-)
>
> > static int fsl_asoc_card_late_probe(struct snd_soc_card *card)
> > {
> > struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card);
> > @@ -745,8 +789,29 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> > snd_soc_card_set_drvdata(&priv->card, priv);
> >
> > ret = devm_snd_soc_register_card(&pdev->dev, &priv->card);
> > - if (ret && ret != -EPROBE_DEFER)
> > - dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
> > + if (ret) {
> > + if (ret != -EPROBE_DEFER)
> > + dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
>
> I think we may move this EPROBE_DEFER to the asrc_fail label.
If we move this to asrc_fail label, then it will be hard to define the
error message.
There are many places that goto asrc_fail.
>
> > + goto asrc_fail;
> > + }
> > +
> > + if (of_property_read_bool(np, "hp-det-gpio")) {
>
> Could we move this check inside asoc_simple_init_jack? There's no
> problem with doing it here though, yet I got a bit confused by it
> as I thought it's a boolean type property, which would be against
> the DT bindings until I saw asoc_simple_init_jack() uses the same
> string to get the GPIO. Just it probably would be a bit tricky as
> we need it to be optional here.
>
> Otherwise, I think we may add a line of comments to indicate that
> the API would use the same string to get the GPIO.
In asoc_simple_init_jack, gpio_is_valid() will be invalid when there is
no "hp-det-gpio" property, and asoc_simple_init_jack will return 0.
The reason why I add a check here is mostly for
snd_soc_jack_notifier_register().
when there is no jack created, there will be a kernel dump.
or I can use this code:
- if (of_property_read_bool(np, "hp-det-gpio")) {
- ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack,
- 1, NULL, "Headphone Jack");
- if (ret)
- goto asrc_fail;
+ ret = asoc_simple_init_jack(&priv->card, &priv->hp_jack,
+ 1, NULL, "Headphone Jack");
+ if (ret)
+ goto asrc_fail;
+ if (priv->hp_jack.jack.jack)
snd_soc_jack_notifier_register(&priv->hp_jack.jack,
&hp_jack_nb);
- }
what do you think?
best regards
wang shengjiu
^ permalink raw reply
* Re: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: Oliver O'Halloran @ 2020-07-15 4:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, linux-pci, bjorn, Paul Mackerras, sparclinux,
Toan Le, Christoph Hellwig, Marek Vasut, Rob Herring,
Lorenzo Pieralisi, Sagi Grimberg, Kevin Hilman, Russell King,
Ley Foon Tan, Greg Ungerer, Geert Uytterhoeven, Jakub Kicinski,
Matt Turner, linux-kernel-mentees, Guenter Roeck, Bjorn Helgaas,
Ray Jui, linuxppc-dev, Jens Axboe, Ivan Kokshaysky, Shuah Khan,
Keith Busch, Boris Ostrovsky, Richard Henderson, Juergen Gross,
Thomas Bogendoerfer, Scott Branden, Jingoo Han,
linux-kernel@vger.kernel.org, Philipp Zabel, Saheed O. Bolarinwa,
Gustavo Pimentel, Bjorn Helgaas, David S. Miller, Heiner Kallweit
In-Reply-To: <CAK8P3a3EZX8=649R9cYF6_=ivh1Xyrgsc5mUtS=d5yvQ3doZaQ@mail.gmail.com>
On Wed, Jul 15, 2020 at 8:03 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> - Most error checking is static: PCIBIOS_BAD_REGISTER_NUMBER
> only happens if you pass an invalid register number, but most
> callers pass a compile-time constant register number that is
> known to be correct, or the driver would never work. Similarly,
> PCIBIOS_DEVICE_NOT_FOUND wouldn't normally happen
> since you pass a valid pci_device pointer that was already
> probed.
Having some feedback about obvious programming errors is still useful
when doing driver development. Reporting those via printk() would
probably be more useful to those who care though.
> - config space accesses are very rare compared to memory
> space access and on the hardware side the error handling
> would be similar, but readl/writel don't return errors, they just
> access wrong registers or return 0xffffffff.
> arch/powerpc/kernel/eeh.c has a ton extra code written to
> deal with it, but no other architectures do.
TBH the EEH MMIO hooks were probably a mistake to begin with. Errors
detected via MMIO are almost always asynchronous to the error itself
so you usually just wind up with a misleading stack trace rather than
any kind of useful synchronous error reporting. It seems like most
drivers don't bother checking for 0xFFs either and rely on the
asynchronous reporting via .error_detected() instead, so I have to
wonder what the point is. I've been thinking of removing the MMIO
hooks and using a background poller to check for errors on each PHB
periodically (assuming we don't have an EEH interrupt) instead. That
would remove the requirement for eeh_dev_check_failure() to be
interrupt safe too, so it might even let us fix all the godawful races
in EEH.
> - If we add code to detect errors in pci_read_config_*
> and do some of the stuff from powerpc's
> eeh_dev_check_failure(), we are more likely to catch
> intermittent failures when drivers don't check, or bugs
> with invalid arguments in device drivers than relying on
> drivers to get their error handling right when those code
> paths don't ever get covered in normal testing.
Adding some kind of error detection to the generic config accessors
wouldn't hurt, but detection is only half the problem. The main job of
eeh_dev_check_failure() is waking up the EEH recovery thread which
actually handles notifying drivers, device resets, etc and you'd want
something in the PCI core. Right now there's two implementations of
that reporting logic: one for EEH in arch/powerpc/eeh_driver.c and one
for AER/DPC in drivers/pci/pcie/err.c. I think the latter could be
moved into the PCI core easily enough since there's not much about it
that's really specific to PCIe. Ideally we could drop the EEH specific
one too, but I'm not sure how to implement that without it devolving
into callback spaghetti.
Oliver
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox