* [PATCH] powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT
From: Aneesh Kumar K.V @ 2021-03-26 7:07 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, Murilo Opsfelder Araujo
H_PROTECT expect the flag value to include
flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags
This patch updates hpte_updatepp() to fetch the storage key value from the linux page
table and use the same in H_PROTECT hcall.
native_hpte_updatepp() is not updated because the kernel doesn't clear the existing
storage key value there. The kernel also doesn't use hpte_updatepp() callback for
updating storage keys.
This fixes the below kernel crash observed with KUAP enabled.
BUG: Unable to handle kernel data access on write at 0xc009fffffc440000
Faulting instruction address: 0xc0000000000b7030
Key fault AMR: 0xfcffffffffffffff IAMR: 0xc0000077bc498100
Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
.........
CFAR: c000000000010100 DAR: c009fffffc440000 DSISR: 02200000 IRQMASK: 0
..........
NIP [c0000000000b7030] memset+0x68/0x104
LR [c0000000003ef00c] pcpu_alloc+0x54c/0xb50
Call Trace:
[c00000001c7534f0] [c0000000003ef01c] pcpu_alloc+0x55c/0xb50 (unreliable)
[c00000001c753600] [c0000000008bb214] blk_stat_alloc_callback+0x94/0x150
[c00000001c753650] [c0000000008b7a04] blk_mq_init_allocated_queue+0x64/0x560
[c00000001c7536b0] [c0000000008b8024] blk_mq_init_queue+0x54/0xb0
[c00000001c7536e0] [c000000000b87650] scsi_mq_alloc_queue+0x30/0xa0
[c00000001c753710] [c000000000b88b2c] scsi_alloc_sdev+0x1cc/0x300
[c00000001c7537b0] [c000000000b897b0] scsi_probe_and_add_lun+0xb50/0x1020
[c00000001c753950] [c000000000b8a35c] __scsi_scan_target+0x17c/0x790
[c00000001c753a80] [c000000000b8ab90] scsi_scan_channel+0x90/0xe0
[c00000001c753ad0] [c000000000b8ae48] scsi_scan_host_selected+0x148/0x1f0
[c00000001c753b60] [c000000000b8b31c] do_scan_async+0x2c/0x2a0
[c00000001c753be0] [c000000000187a18] async_run_entry_fn+0x78/0x220
[c00000001c753c70] [c000000000176a74] process_one_work+0x264/0x540
[c00000001c753d10] [c000000000177338] worker_thread+0xa8/0x600
[c00000001c753da0] [c0000000001807b0] kthread+0x190/0x1a0
[c00000001c753e10] [c00000000000d8f0] ret_from_kernel_thread+0x5c/0x6c
With KUAP enabled the kernel uses storage key 3 for all its translations. But as
shown by the debug print, in this specific case we have the hash page table
entry created with key value 0.
[ 2.249497] Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194
and DSISR indicates a key fault.
This can happen due to parallel fault on the same EA by different CPUs
CPU 0 CPU 1
fault on X
H_PAGE_BUSY set
fault on X
finish fault handling and
clear H_PAGE_BUSY
check for H_PAGE_BUSY
continue with fault handling.
This implies CPU1 will end up calling hpte_updatepp for address X
and the kernel updated the hash pte entry with key 0
Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash translation")
Debugged-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
Similar change for bolted hash pte entries was done by
https://lore.kernel.org/linuxppc-dev/20210211135130.3474832-2-mpe@ellerman.id.au/
arch/powerpc/platforms/pseries/lpar.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 764170fdb0f7..3805519a6469 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -887,7 +887,8 @@ static long pSeries_lpar_hpte_updatepp(unsigned long slot,
want_v = hpte_encode_avpn(vpn, psize, ssize);
- flags = (newpp & 7) | H_AVPN;
+ flags = (newpp & (HPTE_R_PP | HPTE_R_N | HPTE_R_KEY_LO)) | H_AVPN;
+ flags |= (newpp & HPTE_R_KEY_HI) >> 48;
if (mmu_has_feature(MMU_FTR_KERNEL_RO))
/* Move pp0 into bit 8 (IBM 55) */
flags |= (newpp & HPTE_R_PP0) >> 55;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH -next] treewide: Remove duplicated include from tm-vmx-unavail.c
From: Christophe Leroy @ 2021-03-26 7:04 UTC (permalink / raw)
To: Zheng Yongjun, Michael Ellerman, Shuah Khan
Cc: linuxppc-dev, kernel-janitors, linux-kselftest, Hulk Robot
In-Reply-To: <20210326064812.3262659-1-zhengyongjun3@huawei.com>
Le 26/03/2021 à 07:48, Zheng Yongjun a écrit :
> Remove duplicated include.
>
Why is that flagged "treewide" ?
Can you please combine all similar changes to tools/testing/selftests/powerpc/ into a single patch
please ?
Thanks
Christophe
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com>
> ---
> tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> index e2a0c07e8362..9ef37a9836ac 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> @@ -17,7 +17,6 @@
> #include <pthread.h>
> #include <sys/mman.h>
> #include <unistd.h>
> -#include <pthread.h>
>
> #include "tm.h"
> #include "utils.h"
>
^ permalink raw reply
* [PATCH -next] treewide: Remove duplicated include from tm-vmx-unavail.c
From: Zheng Yongjun @ 2021-03-26 6:48 UTC (permalink / raw)
To: zhengyongjun3, Michael Ellerman, Shuah Khan
Cc: kernel-janitors, linuxppc-dev, linux-kselftest, Hulk Robot
Remove duplicated include.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com>
---
tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
index e2a0c07e8362..9ef37a9836ac 100644
--- a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
+++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
@@ -17,7 +17,6 @@
#include <pthread.h>
#include <sys/mman.h>
#include <unistd.h>
-#include <pthread.h>
#include "tm.h"
#include "utils.h"
^ permalink raw reply related
* [PATCH -next] selftests/powerpc: Remove duplicated include from tm-poison.c
From: Zheng Yongjun @ 2021-03-26 6:48 UTC (permalink / raw)
To: zhengyongjun3, Michael Ellerman, Shuah Khan
Cc: kernel-janitors, linuxppc-dev, linux-kselftest, Hulk Robot
Remove duplicated include.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zheng Yongjun <zhengyongjun3@huawei.com>
---
tools/testing/selftests/powerpc/tm/tm-poison.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/tm/tm-poison.c b/tools/testing/selftests/powerpc/tm/tm-poison.c
index 29e5f26af7b9..27c083a03d1f 100644
--- a/tools/testing/selftests/powerpc/tm/tm-poison.c
+++ b/tools/testing/selftests/powerpc/tm/tm-poison.c
@@ -20,7 +20,6 @@
#include <sched.h>
#include <sys/types.h>
#include <signal.h>
-#include <inttypes.h>
#include "tm.h"
^ permalink raw reply related
* Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
From: Christophe Leroy @ 2021-03-26 6:44 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arch, robh, daniel, devicetree, linux-kernel,
Paul Mackerras, linuxppc-dev, danielwa
In-Reply-To: <20210303175747.GD19713@willie-the-truck>
Le 03/03/2021 à 18:57, Will Deacon a écrit :
> On Tue, Mar 02, 2021 at 05:25:22PM +0000, Christophe Leroy wrote:
>> Most architectures have similar boot command line manipulation
>> options. This patchs adds the definition in init/Kconfig, gated by
>> CONFIG_HAVE_CMDLINE that the architectures can select to use them.
>>
>> In order to use this, a few architectures will have to change their
>> CONFIG options:
>> - riscv has to replace CMDLINE_FALLBACK by CMDLINE_FROM_BOOTLOADER
>> - architectures using CONFIG_CMDLINE_OVERRIDE or
>> CONFIG_CMDLINE_OVERWRITE have to replace them by CONFIG_CMDLINE_FORCE.
>>
>> Architectures also have to define CONFIG_DEFAULT_CMDLINE.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> init/Kconfig | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 56 insertions(+)
>>
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 22946fe5ded9..a0f2ad9467df 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -117,6 +117,62 @@ config INIT_ENV_ARG_LIMIT
>> Maximum of each of the number of arguments and environment
>> variables passed to init from the kernel command line.
>>
>> +config HAVE_CMDLINE
>> + bool
>> +
>> +config CMDLINE_BOOL
>> + bool "Default bootloader kernel arguments"
>> + depends on HAVE_CMDLINE
>> + help
>> + On some platforms, there is currently no way for the boot loader to
>> + pass arguments to the kernel. For these platforms, you can supply
>> + some command-line options at build time by entering them here. In
>> + most cases you will need to specify the root device here.
>
> Why is this needed as well as CMDLINE_FROM_BOOTLOADER? IIUC, the latter
> will use CONFIG_CMDLINE if it fails to get anything from the bootloader,
> which sounds like the same scenario.
>
>> +config CMDLINE
>> + string "Initial kernel command string"
>
> s/Initial/Default
>
> which is then consistent with the rest of the text here.
>
>> + depends on CMDLINE_BOOL
>
> Ah, so this is a bit different and I don't think lines-up with the
> CMDLINE_BOOL help text.
You are right, the help text is duplicated, I will change the text for the CMDLINE_BOOL
>
>> + default DEFAULT_CMDLINE
>> + help
>> + On some platforms, there is currently no way for the boot loader to
>> + pass arguments to the kernel. For these platforms, you can supply
>> + some command-line options at build time by entering them here. In
>> + most cases you will need to specify the root device here.
>
> (same stale text)
>
>> +choice
>> + prompt "Kernel command line type" if CMDLINE != ""
>> + default CMDLINE_FROM_BOOTLOADER
>> + help
>> + Selects the way you want to use the default kernel arguments.
>
> How about:
>
> "Determines how the default kernel arguments are combined with any
> arguments passed by the bootloader"
>
>> +config CMDLINE_FROM_BOOTLOADER
>> + bool "Use bootloader kernel arguments if available"
>> + help
>> + Uses the command-line options passed by the boot loader. If
>> + the boot loader doesn't provide any, the default kernel command
>> + string provided in CMDLINE will be used.
>> +
>> +config CMDLINE_EXTEND
>
> Can we rename this to CMDLINE_APPEND, please? There is code in the tree
> which disagrees about what CMDLINE_EXTEND means, so that will need be
> to be updated to be consistent (e.g. the EFI stub parsing order). Having
> the generic option with a different name means we won't accidentally end
> up with the same inconsistent behaviours.
>
>> + bool "Extend bootloader kernel arguments"
>
> "Append to the bootloader kernel arguments"
>
>> + help
>> + The default kernel command string will be appended to the
>> + command-line arguments provided during boot.
>
> s/provided during boot/provided by the bootloader/
>
>> +
>> +config CMDLINE_PREPEND
>> + bool "Prepend bootloader kernel arguments"
>
> "Prepend to the bootloader kernel arguments"
>
>> + help
>> + The default kernel command string will be prepend to the
>> + command-line arguments provided during boot.
>
> s/prepend/prepended/
> s/provided during boot/provided by the bootloader/
>
>> +
>> +config CMDLINE_FORCE
>> + bool "Always use the default kernel command string"
>> + help
>> + Always use the default kernel command string, even if the boot
>> + loader passes other arguments to the kernel.
>> + This is useful if you cannot or don't want to change the
>> + command-line options your boot loader passes to the kernel.
>
> I find the "This is useful if ..." sentence really confusing, perhaps just
> remove it? I'd then tweak it to be:
>
> "Always use the default kernel command string, ignoring any arguments
> provided by the bootloader."
>
Taken all your suggested text.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
From: Christophe Leroy @ 2021-03-26 6:18 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arch, robh, daniel, devicetree, linux-kernel,
Paul Mackerras, linuxppc-dev, danielwa
In-Reply-To: <20210325193216.GC16123@willie-the-truck>
Le 25/03/2021 à 20:32, Will Deacon a écrit :
> On Thu, Mar 25, 2021 at 12:18:38PM +0100, Christophe Leroy wrote:
>>
>> - For ARM it means to append the bootloader arguments to the CONFIG_CMDLINE
>> - For Powerpc it means to append the CONFIG_CMDLINE to the bootloader arguments
>> - For SH it means to append the CONFIG_CMDLINE to the bootloader arguments
>> - For EFI it means to append the bootloader arguments to the CONFIG_CMDLINE
>> - For OF it means to append the CONFIG_CMDLINE to the bootloader arguments
>>
>> So what happens on ARM for instance when it selects CONFIG_OF for instance ?
>
> I think ARM gets different behaviour depending on whether it uses ATAGs or
> FDT.
As far as I can see, ARM uses either ATAGs only or both ATAGs and FDT. ATAGs is forced to 'y' when
USE_OF is set. Do I miss something ?
>
>> Or should we consider that EXTEND means APPEND or PREPEND, no matter which ?
>> Because EXTEND is for instance used for:
>>
>> config INITRAMFS_FORCE
>> bool "Ignore the initramfs passed by the bootloader"
>> depends on CMDLINE_EXTEND || CMDLINE_FORCE
>
> Oh man, I didn't spot that one :(
>
> I think I would make the generic options explicit: either APPEND or PREPEND.
> Then architectures which choose to define CMDLINE_EXTEND in their Kconfigs
> can select the generic option that matches their behaviour.
>
> INITRAMFS_FORCE sounds like it should depend on APPEND (assuming that means
> CONFIG_CMDLINE is appended to the bootloader arguments).
>
Christophe
^ permalink raw reply
* [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2
From: Christoph Hellwig @ 2021-03-26 6:13 UTC (permalink / raw)
To: Michael Ellerman, Alex Williamson
Cc: Jason Gunthorpe, kvm, David Airlie, linux-kernel, dri-devel,
Paul Mackerras, Daniel Vetter, Greg Kroah-Hartman, linux-api,
linuxppc-dev
In-Reply-To: <20210326061311.1497642-1-hch@lst.de>
This driver never had any open userspace (which for VFIO would include
VM kernel drivers) that use it, and thus should never have been added
by our normal userspace ABI rules.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/vfio/pci/Kconfig | 6 -
drivers/vfio/pci/Makefile | 1 -
drivers/vfio/pci/vfio_pci.c | 18 -
drivers/vfio/pci/vfio_pci_nvlink2.c | 490 ----------------------------
drivers/vfio/pci/vfio_pci_private.h | 14 -
include/uapi/linux/vfio.h | 38 +--
6 files changed, 4 insertions(+), 563 deletions(-)
delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index ac3c1dd3edeff1..53ce78d7d07be0 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -39,9 +39,3 @@ config VFIO_PCI_IGD
and LPC bridge config space.
To enable Intel IGD assignment through vfio-pci, say Y.
-
-config VFIO_PCI_NVLINK2
- def_bool y
- depends on VFIO_PCI && PPC_POWERNV
- help
- VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index eff97a7cd9f139..3ff42093962f6f 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -2,7 +2,6 @@
vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
-vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
vfio-pci-$(CONFIG_S390) += vfio_pci_zdev.o
obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 65e7e6b44578c2..d691006b642839 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -389,24 +389,6 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
}
}
- if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
- IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2)) {
- ret = vfio_pci_nvdia_v100_nvlink2_init(vdev);
- if (ret && ret != -ENODEV) {
- pci_warn(pdev, "Failed to setup NVIDIA NV2 RAM region\n");
- goto disable_exit;
- }
- }
-
- if (pdev->vendor == PCI_VENDOR_ID_IBM &&
- IS_ENABLED(CONFIG_VFIO_PCI_NVLINK2)) {
- ret = vfio_pci_ibm_npu2_init(vdev);
- if (ret && ret != -ENODEV) {
- pci_warn(pdev, "Failed to setup NVIDIA NV2 ATSD region\n");
- goto disable_exit;
- }
- }
-
vfio_pci_probe_mmaps(vdev);
return 0;
diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
deleted file mode 100644
index 9adcf6a8f88857..00000000000000
--- a/drivers/vfio/pci/vfio_pci_nvlink2.c
+++ /dev/null
@@ -1,490 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * VFIO PCI NVIDIA Whitherspoon GPU support a.k.a. NVLink2.
- *
- * Copyright (C) 2018 IBM Corp. All rights reserved.
- * Author: Alexey Kardashevskiy <aik@ozlabs.ru>
- *
- * Register an on-GPU RAM region for cacheable access.
- *
- * Derived from original vfio_pci_igd.c:
- * Copyright (C) 2016 Red Hat, Inc. All rights reserved.
- * Author: Alex Williamson <alex.williamson@redhat.com>
- */
-
-#include <linux/io.h>
-#include <linux/pci.h>
-#include <linux/uaccess.h>
-#include <linux/vfio.h>
-#include <linux/sched/mm.h>
-#include <linux/mmu_context.h>
-#include <asm/kvm_ppc.h>
-#include "vfio_pci_private.h"
-
-#define CREATE_TRACE_POINTS
-#include "trace.h"
-
-EXPORT_TRACEPOINT_SYMBOL_GPL(vfio_pci_nvgpu_mmap_fault);
-EXPORT_TRACEPOINT_SYMBOL_GPL(vfio_pci_nvgpu_mmap);
-EXPORT_TRACEPOINT_SYMBOL_GPL(vfio_pci_npu2_mmap);
-
-struct vfio_pci_nvgpu_data {
- unsigned long gpu_hpa; /* GPU RAM physical address */
- unsigned long gpu_tgt; /* TGT address of corresponding GPU RAM */
- unsigned long useraddr; /* GPU RAM userspace address */
- unsigned long size; /* Size of the GPU RAM window (usually 128GB) */
- struct mm_struct *mm;
- struct mm_iommu_table_group_mem_t *mem; /* Pre-registered RAM descr. */
- struct pci_dev *gpdev;
- struct notifier_block group_notifier;
-};
-
-static size_t vfio_pci_nvgpu_rw(struct vfio_pci_device *vdev,
- char __user *buf, size_t count, loff_t *ppos, bool iswrite)
-{
- unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS;
- struct vfio_pci_nvgpu_data *data = vdev->region[i].data;
- loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
- loff_t posaligned = pos & PAGE_MASK, posoff = pos & ~PAGE_MASK;
- size_t sizealigned;
- void __iomem *ptr;
-
- if (pos >= vdev->region[i].size)
- return -EINVAL;
-
- count = min(count, (size_t)(vdev->region[i].size - pos));
-
- /*
- * We map only a bit of GPU RAM for a short time instead of mapping it
- * for the guest lifetime as:
- *
- * 1) we do not know GPU RAM size, only aperture which is 4-8 times
- * bigger than actual RAM size (16/32GB RAM vs. 128GB aperture);
- * 2) mapping GPU RAM allows CPU to prefetch and if this happens
- * before NVLink bridge is reset (which fences GPU RAM),
- * hardware management interrupts (HMI) might happen, this
- * will freeze NVLink bridge.
- *
- * This is not fast path anyway.
- */
- sizealigned = ALIGN(posoff + count, PAGE_SIZE);
- ptr = ioremap_cache(data->gpu_hpa + posaligned, sizealigned);
- if (!ptr)
- return -EFAULT;
-
- if (iswrite) {
- if (copy_from_user(ptr + posoff, buf, count))
- count = -EFAULT;
- else
- *ppos += count;
- } else {
- if (copy_to_user(buf, ptr + posoff, count))
- count = -EFAULT;
- else
- *ppos += count;
- }
-
- iounmap(ptr);
-
- return count;
-}
-
-static void vfio_pci_nvgpu_release(struct vfio_pci_device *vdev,
- struct vfio_pci_region *region)
-{
- struct vfio_pci_nvgpu_data *data = region->data;
- long ret;
-
- /* If there were any mappings at all... */
- if (data->mm) {
- if (data->mem) {
- ret = mm_iommu_put(data->mm, data->mem);
- WARN_ON(ret);
- }
-
- mmdrop(data->mm);
- }
-
- vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
- &data->group_notifier);
-
- pnv_npu2_unmap_lpar_dev(data->gpdev);
-
- kfree(data);
-}
-
-static vm_fault_t vfio_pci_nvgpu_mmap_fault(struct vm_fault *vmf)
-{
- vm_fault_t ret;
- struct vm_area_struct *vma = vmf->vma;
- struct vfio_pci_region *region = vma->vm_private_data;
- struct vfio_pci_nvgpu_data *data = region->data;
- unsigned long vmf_off = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
- unsigned long nv2pg = data->gpu_hpa >> PAGE_SHIFT;
- unsigned long vm_pgoff = vma->vm_pgoff &
- ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
- unsigned long pfn = nv2pg + vm_pgoff + vmf_off;
-
- ret = vmf_insert_pfn(vma, vmf->address, pfn);
- trace_vfio_pci_nvgpu_mmap_fault(data->gpdev, pfn << PAGE_SHIFT,
- vmf->address, ret);
-
- return ret;
-}
-
-static const struct vm_operations_struct vfio_pci_nvgpu_mmap_vmops = {
- .fault = vfio_pci_nvgpu_mmap_fault,
-};
-
-static int vfio_pci_nvgpu_mmap(struct vfio_pci_device *vdev,
- struct vfio_pci_region *region, struct vm_area_struct *vma)
-{
- int ret;
- struct vfio_pci_nvgpu_data *data = region->data;
-
- if (data->useraddr)
- return -EPERM;
-
- if (vma->vm_end - vma->vm_start > data->size)
- return -EINVAL;
-
- vma->vm_private_data = region;
- vma->vm_flags |= VM_PFNMAP;
- vma->vm_ops = &vfio_pci_nvgpu_mmap_vmops;
-
- /*
- * Calling mm_iommu_newdev() here once as the region is not
- * registered yet and therefore right initialization will happen now.
- * Other places will use mm_iommu_find() which returns
- * registered @mem and does not go gup().
- */
- data->useraddr = vma->vm_start;
- data->mm = current->mm;
-
- mmgrab(data->mm);
- ret = (int) mm_iommu_newdev(data->mm, data->useraddr,
- vma_pages(vma), data->gpu_hpa, &data->mem);
-
- trace_vfio_pci_nvgpu_mmap(vdev->pdev, data->gpu_hpa, data->useraddr,
- vma->vm_end - vma->vm_start, ret);
-
- return ret;
-}
-
-static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev,
- struct vfio_pci_region *region, struct vfio_info_cap *caps)
-{
- struct vfio_pci_nvgpu_data *data = region->data;
- struct vfio_region_info_cap_nvlink2_ssatgt cap = {
- .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
- .header.version = 1,
- .tgt = data->gpu_tgt
- };
-
- return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
-}
-
-static const struct vfio_pci_regops vfio_pci_nvgpu_regops = {
- .rw = vfio_pci_nvgpu_rw,
- .release = vfio_pci_nvgpu_release,
- .mmap = vfio_pci_nvgpu_mmap,
- .add_capability = vfio_pci_nvgpu_add_capability,
-};
-
-static int vfio_pci_nvgpu_group_notifier(struct notifier_block *nb,
- unsigned long action, void *opaque)
-{
- struct kvm *kvm = opaque;
- struct vfio_pci_nvgpu_data *data = container_of(nb,
- struct vfio_pci_nvgpu_data,
- group_notifier);
-
- if (action == VFIO_GROUP_NOTIFY_SET_KVM && kvm &&
- pnv_npu2_map_lpar_dev(data->gpdev,
- kvm->arch.lpid, MSR_DR | MSR_PR))
- return NOTIFY_BAD;
-
- return NOTIFY_OK;
-}
-
-int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
-{
- int ret;
- u64 reg[2];
- u64 tgt = 0;
- struct device_node *npu_node, *mem_node;
- struct pci_dev *npu_dev;
- struct vfio_pci_nvgpu_data *data;
- uint32_t mem_phandle = 0;
- unsigned long events = VFIO_GROUP_NOTIFY_SET_KVM;
-
- /*
- * PCI config space does not tell us about NVLink presense but
- * platform does, use this.
- */
- npu_dev = pnv_pci_get_npu_dev(vdev->pdev, 0);
- if (!npu_dev)
- return -ENODEV;
-
- npu_node = pci_device_to_OF_node(npu_dev);
- if (!npu_node)
- return -EINVAL;
-
- if (of_property_read_u32(npu_node, "memory-region", &mem_phandle))
- return -ENODEV;
-
- mem_node = of_find_node_by_phandle(mem_phandle);
- if (!mem_node)
- return -EINVAL;
-
- if (of_property_read_variable_u64_array(mem_node, "reg", reg,
- ARRAY_SIZE(reg), ARRAY_SIZE(reg)) !=
- ARRAY_SIZE(reg))
- return -EINVAL;
-
- if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) {
- dev_warn(&vdev->pdev->dev, "No ibm,device-tgt-addr found\n");
- return -EFAULT;
- }
-
- data = kzalloc(sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->gpu_hpa = reg[0];
- data->gpu_tgt = tgt;
- data->size = reg[1];
-
- dev_dbg(&vdev->pdev->dev, "%lx..%lx\n", data->gpu_hpa,
- data->gpu_hpa + data->size - 1);
-
- data->gpdev = vdev->pdev;
- data->group_notifier.notifier_call = vfio_pci_nvgpu_group_notifier;
-
- ret = vfio_register_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
- &events, &data->group_notifier);
- if (ret)
- goto free_exit;
-
- /*
- * We have just set KVM, we do not need the listener anymore.
- * Also, keeping it registered means that if more than one GPU is
- * assigned, we will get several similar notifiers notifying about
- * the same device again which does not help with anything.
- */
- vfio_unregister_notifier(&data->gpdev->dev, VFIO_GROUP_NOTIFY,
- &data->group_notifier);
-
- ret = vfio_pci_register_dev_region(vdev,
- PCI_VENDOR_ID_NVIDIA | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
- VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM,
- &vfio_pci_nvgpu_regops,
- data->size,
- VFIO_REGION_INFO_FLAG_READ |
- VFIO_REGION_INFO_FLAG_WRITE |
- VFIO_REGION_INFO_FLAG_MMAP,
- data);
- if (ret)
- goto free_exit;
-
- return 0;
-free_exit:
- kfree(data);
-
- return ret;
-}
-
-/*
- * IBM NPU2 bridge
- */
-struct vfio_pci_npu2_data {
- void *base; /* ATSD register virtual address, for emulated access */
- unsigned long mmio_atsd; /* ATSD physical address */
- unsigned long gpu_tgt; /* TGT address of corresponding GPU RAM */
- unsigned int link_speed; /* The link speed from DT's ibm,nvlink-speed */
-};
-
-static size_t vfio_pci_npu2_rw(struct vfio_pci_device *vdev,
- char __user *buf, size_t count, loff_t *ppos, bool iswrite)
-{
- unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - VFIO_PCI_NUM_REGIONS;
- struct vfio_pci_npu2_data *data = vdev->region[i].data;
- loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
-
- if (pos >= vdev->region[i].size)
- return -EINVAL;
-
- count = min(count, (size_t)(vdev->region[i].size - pos));
-
- if (iswrite) {
- if (copy_from_user(data->base + pos, buf, count))
- return -EFAULT;
- } else {
- if (copy_to_user(buf, data->base + pos, count))
- return -EFAULT;
- }
- *ppos += count;
-
- return count;
-}
-
-static int vfio_pci_npu2_mmap(struct vfio_pci_device *vdev,
- struct vfio_pci_region *region, struct vm_area_struct *vma)
-{
- int ret;
- struct vfio_pci_npu2_data *data = region->data;
- unsigned long req_len = vma->vm_end - vma->vm_start;
-
- if (req_len != PAGE_SIZE)
- return -EINVAL;
-
- vma->vm_flags |= VM_PFNMAP;
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-
- ret = remap_pfn_range(vma, vma->vm_start, data->mmio_atsd >> PAGE_SHIFT,
- req_len, vma->vm_page_prot);
- trace_vfio_pci_npu2_mmap(vdev->pdev, data->mmio_atsd, vma->vm_start,
- vma->vm_end - vma->vm_start, ret);
-
- return ret;
-}
-
-static void vfio_pci_npu2_release(struct vfio_pci_device *vdev,
- struct vfio_pci_region *region)
-{
- struct vfio_pci_npu2_data *data = region->data;
-
- memunmap(data->base);
- kfree(data);
-}
-
-static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev,
- struct vfio_pci_region *region, struct vfio_info_cap *caps)
-{
- struct vfio_pci_npu2_data *data = region->data;
- struct vfio_region_info_cap_nvlink2_ssatgt captgt = {
- .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
- .header.version = 1,
- .tgt = data->gpu_tgt
- };
- struct vfio_region_info_cap_nvlink2_lnkspd capspd = {
- .header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD,
- .header.version = 1,
- .link_speed = data->link_speed
- };
- int ret;
-
- ret = vfio_info_add_capability(caps, &captgt.header, sizeof(captgt));
- if (ret)
- return ret;
-
- return vfio_info_add_capability(caps, &capspd.header, sizeof(capspd));
-}
-
-static const struct vfio_pci_regops vfio_pci_npu2_regops = {
- .rw = vfio_pci_npu2_rw,
- .mmap = vfio_pci_npu2_mmap,
- .release = vfio_pci_npu2_release,
- .add_capability = vfio_pci_npu2_add_capability,
-};
-
-int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
-{
- int ret;
- struct vfio_pci_npu2_data *data;
- struct device_node *nvlink_dn;
- u32 nvlink_index = 0, mem_phandle = 0;
- struct pci_dev *npdev = vdev->pdev;
- struct device_node *npu_node = pci_device_to_OF_node(npdev);
- struct pci_controller *hose = pci_bus_to_host(npdev->bus);
- u64 mmio_atsd = 0;
- u64 tgt = 0;
- u32 link_speed = 0xff;
-
- /*
- * PCI config space does not tell us about NVLink presense but
- * platform does, use this.
- */
- if (!pnv_pci_get_gpu_dev(vdev->pdev))
- return -ENODEV;
-
- if (of_property_read_u32(npu_node, "memory-region", &mem_phandle))
- return -ENODEV;
-
- /*
- * NPU2 normally has 8 ATSD registers (for concurrency) and 6 links
- * so we can allocate one register per link, using nvlink index as
- * a key.
- * There is always at least one ATSD register so as long as at least
- * NVLink bridge #0 is passed to the guest, ATSD will be available.
- */
- nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
- if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
- &nvlink_index)))
- return -ENODEV;
-
- if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", nvlink_index,
- &mmio_atsd)) {
- if (of_property_read_u64_index(hose->dn, "ibm,mmio-atsd", 0,
- &mmio_atsd)) {
- dev_warn(&vdev->pdev->dev, "No available ATSD found\n");
- mmio_atsd = 0;
- } else {
- dev_warn(&vdev->pdev->dev,
- "Using fallback ibm,mmio-atsd[0] for ATSD.\n");
- }
- }
-
- if (of_property_read_u64(npu_node, "ibm,device-tgt-addr", &tgt)) {
- dev_warn(&vdev->pdev->dev, "No ibm,device-tgt-addr found\n");
- return -EFAULT;
- }
-
- if (of_property_read_u32(npu_node, "ibm,nvlink-speed", &link_speed)) {
- dev_warn(&vdev->pdev->dev, "No ibm,nvlink-speed found\n");
- return -EFAULT;
- }
-
- data = kzalloc(sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->mmio_atsd = mmio_atsd;
- data->gpu_tgt = tgt;
- data->link_speed = link_speed;
- if (data->mmio_atsd) {
- data->base = memremap(data->mmio_atsd, SZ_64K, MEMREMAP_WT);
- if (!data->base) {
- ret = -ENOMEM;
- goto free_exit;
- }
- }
-
- /*
- * We want to expose the capability even if this specific NVLink
- * did not get its own ATSD register because capabilities
- * belong to VFIO regions and normally there will be ATSD register
- * assigned to the NVLink bridge.
- */
- ret = vfio_pci_register_dev_region(vdev,
- PCI_VENDOR_ID_IBM |
- VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
- VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD,
- &vfio_pci_npu2_regops,
- data->mmio_atsd ? PAGE_SIZE : 0,
- VFIO_REGION_INFO_FLAG_READ |
- VFIO_REGION_INFO_FLAG_WRITE |
- VFIO_REGION_INFO_FLAG_MMAP,
- data);
- if (ret)
- goto free_exit;
-
- return 0;
-
-free_exit:
- if (data->base)
- memunmap(data->base);
- kfree(data);
-
- return ret;
-}
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index 9cd1882a05af69..cdae2e4cf11cca 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -199,20 +199,6 @@ static inline int vfio_pci_igd_init(struct vfio_pci_device *vdev)
return -ENODEV;
}
#endif
-#ifdef CONFIG_VFIO_PCI_NVLINK2
-extern int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev);
-extern int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev);
-#else
-static inline int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
-{
- return -ENODEV;
-}
-
-static inline int vfio_pci_ibm_npu2_init(struct vfio_pci_device *vdev)
-{
- return -ENODEV;
-}
-#endif
#ifdef CONFIG_S390
extern int vfio_pci_info_zdev_add_caps(struct vfio_pci_device *vdev,
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 8ce36c1d53ca11..34b1f53a3901cb 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -333,17 +333,10 @@ struct vfio_region_info_cap_type {
#define VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG (3)
/* 10de vendor PCI sub-types */
-/*
- * NVIDIA GPU NVlink2 RAM is coherent RAM mapped onto the host address space.
- */
-#define VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM (1)
+/* subtype 1 was VFIO_REGION_SUBTYPE_NVIDIA_NVLINK2_RAM, don't use */
/* 1014 vendor PCI sub-types */
-/*
- * IBM NPU NVlink2 ATSD (Address Translation Shootdown) register of NPU
- * to do TLB invalidation on a GPU.
- */
-#define VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD (1)
+/* subtype 1 was VFIO_REGION_SUBTYPE_IBM_NVLINK2_ATSD, don't use */
/* sub-types for VFIO_REGION_TYPE_GFX */
#define VFIO_REGION_SUBTYPE_GFX_EDID (1)
@@ -637,32 +630,9 @@ struct vfio_device_migration_info {
*/
#define VFIO_REGION_INFO_CAP_MSIX_MAPPABLE 3
-/*
- * Capability with compressed real address (aka SSA - small system address)
- * where GPU RAM is mapped on a system bus. Used by a GPU for DMA routing
- * and by the userspace to associate a NVLink bridge with a GPU.
- */
-#define VFIO_REGION_INFO_CAP_NVLINK2_SSATGT 4
-
-struct vfio_region_info_cap_nvlink2_ssatgt {
- struct vfio_info_cap_header header;
- __u64 tgt;
-};
+/* subtype 4 was VFIO_REGION_INFO_CAP_NVLINK2_SSATGT, don't use */
-/*
- * Capability with an NVLink link speed. The value is read by
- * the NVlink2 bridge driver from the bridge's "ibm,nvlink-speed"
- * property in the device tree. The value is fixed in the hardware
- * and failing to provide the correct value results in the link
- * not working with no indication from the driver why.
- */
-#define VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD 5
-
-struct vfio_region_info_cap_nvlink2_lnkspd {
- struct vfio_info_cap_header header;
- __u32 link_speed;
- __u32 __pad;
-};
+/* subtype 5 was VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD, don't use */
/**
* VFIO_DEVICE_GET_IRQ_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 9,
--
2.30.1
^ permalink raw reply related
* [PATCH 2/2] powerpc/powernv: remove the nvlink support
From: Christoph Hellwig @ 2021-03-26 6:13 UTC (permalink / raw)
To: Michael Ellerman, Alex Williamson
Cc: Jason Gunthorpe, kvm, David Airlie, linux-kernel, dri-devel,
Paul Mackerras, Daniel Vetter, Greg Kroah-Hartman, linux-api,
linuxppc-dev
In-Reply-To: <20210326061311.1497642-1-hch@lst.de>
This code was only used by the vfio-nvlink2 code, which itself had no
proper use. Drop this huge chunk of code build into every powernv
or generic build.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/opal.h | 3 -
arch/powerpc/include/asm/pci-bridge.h | 1 -
arch/powerpc/include/asm/pci.h | 7 -
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/npu-dma.c | 705 ---------------------
arch/powerpc/platforms/powernv/opal-call.c | 2 -
arch/powerpc/platforms/powernv/pci-ioda.c | 185 +-----
arch/powerpc/platforms/powernv/pci.c | 11 -
arch/powerpc/platforms/powernv/pci.h | 17 +-
arch/powerpc/platforms/pseries/pci.c | 23 -
10 files changed, 8 insertions(+), 948 deletions(-)
delete mode 100644 arch/powerpc/platforms/powernv/npu-dma.c
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9986ac34b8e224..06eaa231697344 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -28,9 +28,6 @@ extern struct device_node *opal_node;
/* API functions */
int64_t opal_invalid_call(void);
-int64_t opal_npu_destroy_context(uint64_t phb_id, uint64_t pid, uint64_t bdf);
-int64_t opal_npu_init_context(uint64_t phb_id, int pasid, uint64_t msr,
- uint64_t bdf);
int64_t opal_npu_map_lpar(uint64_t phb_id, uint64_t bdf, uint64_t lparid,
uint64_t lpcr);
int64_t opal_npu_spa_setup(uint64_t phb_id, uint32_t bdfn,
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index d2a2a14e56f91e..74424c14515ce0 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -126,7 +126,6 @@ struct pci_controller {
#endif /* CONFIG_PPC64 */
void *private_data;
- struct npu *npu;
};
/* These are used for config access before all the PCI probing
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 6436f0b41539e3..d1f53260725ca7 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -119,11 +119,4 @@ extern void pcibios_scan_phb(struct pci_controller *hose);
#endif /* __KERNEL__ */
-extern struct pci_dev *pnv_pci_get_gpu_dev(struct pci_dev *npdev);
-extern struct pci_dev *pnv_pci_get_npu_dev(struct pci_dev *gpdev, int index);
-extern int pnv_npu2_init(struct pci_controller *hose);
-extern int pnv_npu2_map_lpar_dev(struct pci_dev *gpdev, unsigned int lparid,
- unsigned long msr);
-extern int pnv_npu2_unmap_lpar_dev(struct pci_dev *gpdev);
-
#endif /* __ASM_POWERPC_PCI_H */
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 2eb6ae150d1fd5..be2546b968165e 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_SMP) += smp.o subcore.o subcore-asm.o
obj-$(CONFIG_FA_DUMP) += opal-fadump.o
obj-$(CONFIG_PRESERVE_FA_DUMP) += opal-fadump.o
obj-$(CONFIG_OPAL_CORE) += opal-core.o
-obj-$(CONFIG_PCI) += pci.o pci-ioda.o npu-dma.o pci-ioda-tce.o
+obj-$(CONFIG_PCI) += pci.o pci-ioda.o pci-ioda-tce.o
obj-$(CONFIG_PCI_IOV) += pci-sriov.o
obj-$(CONFIG_CXL_BASE) += pci-cxl.o
obj-$(CONFIG_EEH) += eeh-powernv.o
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
deleted file mode 100644
index b711dc3262a308..00000000000000
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ /dev/null
@@ -1,705 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * This file implements the DMA operations for NVLink devices. The NPU
- * devices all point to the same iommu table as the parent PCI device.
- *
- * Copyright Alistair Popple, IBM Corporation 2015.
- */
-
-#include <linux/mmu_notifier.h>
-#include <linux/mmu_context.h>
-#include <linux/of.h>
-#include <linux/pci.h>
-#include <linux/memblock.h>
-#include <linux/sizes.h>
-
-#include <asm/debugfs.h>
-#include <asm/powernv.h>
-#include <asm/ppc-pci.h>
-#include <asm/opal.h>
-
-#include "pci.h"
-
-static struct pci_dev *get_pci_dev(struct device_node *dn)
-{
- struct pci_dn *pdn = PCI_DN(dn);
- struct pci_dev *pdev;
-
- pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdn->phb->bus),
- pdn->busno, pdn->devfn);
-
- /*
- * pci_get_domain_bus_and_slot() increased the reference count of
- * the PCI device, but callers don't need that actually as the PE
- * already holds a reference to the device. Since callers aren't
- * aware of the reference count change, call pci_dev_put() now to
- * avoid leaks.
- */
- if (pdev)
- pci_dev_put(pdev);
-
- return pdev;
-}
-
-/* Given a NPU device get the associated PCI device. */
-struct pci_dev *pnv_pci_get_gpu_dev(struct pci_dev *npdev)
-{
- struct device_node *dn;
- struct pci_dev *gpdev;
-
- if (WARN_ON(!npdev))
- return NULL;
-
- if (WARN_ON(!npdev->dev.of_node))
- return NULL;
-
- /* Get assoicated PCI device */
- dn = of_parse_phandle(npdev->dev.of_node, "ibm,gpu", 0);
- if (!dn)
- return NULL;
-
- gpdev = get_pci_dev(dn);
- of_node_put(dn);
-
- return gpdev;
-}
-EXPORT_SYMBOL(pnv_pci_get_gpu_dev);
-
-/* Given the real PCI device get a linked NPU device. */
-struct pci_dev *pnv_pci_get_npu_dev(struct pci_dev *gpdev, int index)
-{
- struct device_node *dn;
- struct pci_dev *npdev;
-
- if (WARN_ON(!gpdev))
- return NULL;
-
- /* Not all PCI devices have device-tree nodes */
- if (!gpdev->dev.of_node)
- return NULL;
-
- /* Get assoicated PCI device */
- dn = of_parse_phandle(gpdev->dev.of_node, "ibm,npu", index);
- if (!dn)
- return NULL;
-
- npdev = get_pci_dev(dn);
- of_node_put(dn);
-
- return npdev;
-}
-EXPORT_SYMBOL(pnv_pci_get_npu_dev);
-
-#ifdef CONFIG_IOMMU_API
-/*
- * Returns the PE assoicated with the PCI device of the given
- * NPU. Returns the linked pci device if pci_dev != NULL.
- */
-static struct pnv_ioda_pe *get_gpu_pci_dev_and_pe(struct pnv_ioda_pe *npe,
- struct pci_dev **gpdev)
-{
- struct pnv_phb *phb;
- struct pci_controller *hose;
- struct pci_dev *pdev;
- struct pnv_ioda_pe *pe;
- struct pci_dn *pdn;
-
- pdev = pnv_pci_get_gpu_dev(npe->pdev);
- if (!pdev)
- return NULL;
-
- pdn = pci_get_pdn(pdev);
- if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
- return NULL;
-
- hose = pci_bus_to_host(pdev->bus);
- phb = hose->private_data;
- pe = &phb->ioda.pe_array[pdn->pe_number];
-
- if (gpdev)
- *gpdev = pdev;
-
- return pe;
-}
-
-static long pnv_npu_unset_window(struct iommu_table_group *table_group,
- int num);
-
-static long pnv_npu_set_window(struct iommu_table_group *table_group, int num,
- struct iommu_table *tbl)
-{
- struct pnv_ioda_pe *npe = container_of(table_group, struct pnv_ioda_pe,
- table_group);
- struct pnv_phb *phb = npe->phb;
- int64_t rc;
- const unsigned long size = tbl->it_indirect_levels ?
- tbl->it_level_size : tbl->it_size;
- const __u64 start_addr = tbl->it_offset << tbl->it_page_shift;
- const __u64 win_size = tbl->it_size << tbl->it_page_shift;
- int num2 = (num == 0) ? 1 : 0;
-
- /* NPU has just one TVE so if there is another table, remove it first */
- if (npe->table_group.tables[num2])
- pnv_npu_unset_window(&npe->table_group, num2);
-
- pe_info(npe, "Setting up window %llx..%llx pg=%lx\n",
- start_addr, start_addr + win_size - 1,
- IOMMU_PAGE_SIZE(tbl));
-
- rc = opal_pci_map_pe_dma_window(phb->opal_id,
- npe->pe_number,
- npe->pe_number,
- tbl->it_indirect_levels + 1,
- __pa(tbl->it_base),
- size << 3,
- IOMMU_PAGE_SIZE(tbl));
- if (rc) {
- pe_err(npe, "Failed to configure TCE table, err %lld\n", rc);
- return rc;
- }
- pnv_pci_ioda2_tce_invalidate_entire(phb, false);
-
- /* Add the table to the list so its TCE cache will get invalidated */
- pnv_pci_link_table_and_group(phb->hose->node, num,
- tbl, &npe->table_group);
-
- return 0;
-}
-
-static long pnv_npu_unset_window(struct iommu_table_group *table_group, int num)
-{
- struct pnv_ioda_pe *npe = container_of(table_group, struct pnv_ioda_pe,
- table_group);
- struct pnv_phb *phb = npe->phb;
- int64_t rc;
-
- if (!npe->table_group.tables[num])
- return 0;
-
- pe_info(npe, "Removing DMA window\n");
-
- rc = opal_pci_map_pe_dma_window(phb->opal_id, npe->pe_number,
- npe->pe_number,
- 0/* levels */, 0/* table address */,
- 0/* table size */, 0/* page size */);
- if (rc) {
- pe_err(npe, "Unmapping failed, ret = %lld\n", rc);
- return rc;
- }
- pnv_pci_ioda2_tce_invalidate_entire(phb, false);
-
- pnv_pci_unlink_table_and_group(npe->table_group.tables[num],
- &npe->table_group);
-
- return 0;
-}
-
-/* Switch ownership from platform code to external user (e.g. VFIO) */
-static void pnv_npu_take_ownership(struct iommu_table_group *table_group)
-{
- struct pnv_ioda_pe *npe = container_of(table_group, struct pnv_ioda_pe,
- table_group);
- struct pnv_phb *phb = npe->phb;
- int64_t rc;
- struct pci_dev *gpdev = NULL;
-
- /*
- * Note: NPU has just a single TVE in the hardware which means that
- * while used by the kernel, it can have either 32bit window or
- * DMA bypass but never both. So we deconfigure 32bit window only
- * if it was enabled at the moment of ownership change.
- */
- if (npe->table_group.tables[0]) {
- pnv_npu_unset_window(&npe->table_group, 0);
- return;
- }
-
- /* Disable bypass */
- rc = opal_pci_map_pe_dma_window_real(phb->opal_id,
- npe->pe_number, npe->pe_number,
- 0 /* bypass base */, 0);
- if (rc) {
- pe_err(npe, "Failed to disable bypass, err %lld\n", rc);
- return;
- }
- pnv_pci_ioda2_tce_invalidate_entire(npe->phb, false);
-
- get_gpu_pci_dev_and_pe(npe, &gpdev);
- if (gpdev)
- pnv_npu2_unmap_lpar_dev(gpdev);
-}
-
-static void pnv_npu_release_ownership(struct iommu_table_group *table_group)
-{
- struct pnv_ioda_pe *npe = container_of(table_group, struct pnv_ioda_pe,
- table_group);
- struct pci_dev *gpdev = NULL;
-
- get_gpu_pci_dev_and_pe(npe, &gpdev);
- if (gpdev)
- pnv_npu2_map_lpar_dev(gpdev, 0, MSR_DR | MSR_PR | MSR_HV);
-}
-
-static struct iommu_table_group_ops pnv_pci_npu_ops = {
- .set_window = pnv_npu_set_window,
- .unset_window = pnv_npu_unset_window,
- .take_ownership = pnv_npu_take_ownership,
- .release_ownership = pnv_npu_release_ownership,
-};
-#endif /* !CONFIG_IOMMU_API */
-
-/*
- * NPU2 ATS
- */
-/* Maximum possible number of ATSD MMIO registers per NPU */
-#define NV_NMMU_ATSD_REGS 8
-#define NV_NPU_MAX_PE_NUM 16
-
-/*
- * A compound NPU IOMMU group which might consist of 1 GPU + 2xNPUs (POWER8) or
- * up to 3 x (GPU + 2xNPUs) (POWER9).
- */
-struct npu_comp {
- struct iommu_table_group table_group;
- int pe_num;
- struct pnv_ioda_pe *pe[NV_NPU_MAX_PE_NUM];
-};
-
-/* An NPU descriptor, valid for POWER9 only */
-struct npu {
- int index;
- struct npu_comp npucomp;
-};
-
-#ifdef CONFIG_IOMMU_API
-static long pnv_npu_peers_create_table_userspace(
- struct iommu_table_group *table_group,
- int num, __u32 page_shift, __u64 window_size, __u32 levels,
- struct iommu_table **ptbl)
-{
- struct npu_comp *npucomp = container_of(table_group, struct npu_comp,
- table_group);
-
- if (!npucomp->pe_num || !npucomp->pe[0] ||
- !npucomp->pe[0]->table_group.ops ||
- !npucomp->pe[0]->table_group.ops->create_table)
- return -EFAULT;
-
- return npucomp->pe[0]->table_group.ops->create_table(
- &npucomp->pe[0]->table_group, num, page_shift,
- window_size, levels, ptbl);
-}
-
-static long pnv_npu_peers_set_window(struct iommu_table_group *table_group,
- int num, struct iommu_table *tbl)
-{
- int i, j;
- long ret = 0;
- struct npu_comp *npucomp = container_of(table_group, struct npu_comp,
- table_group);
-
- for (i = 0; i < npucomp->pe_num; ++i) {
- struct pnv_ioda_pe *pe = npucomp->pe[i];
-
- if (!pe->table_group.ops->set_window)
- continue;
-
- ret = pe->table_group.ops->set_window(&pe->table_group,
- num, tbl);
- if (ret)
- break;
- }
-
- if (ret) {
- for (j = 0; j < i; ++j) {
- struct pnv_ioda_pe *pe = npucomp->pe[j];
-
- if (!pe->table_group.ops->unset_window)
- continue;
-
- ret = pe->table_group.ops->unset_window(
- &pe->table_group, num);
- if (ret)
- break;
- }
- } else {
- table_group->tables[num] = iommu_tce_table_get(tbl);
- }
-
- return ret;
-}
-
-static long pnv_npu_peers_unset_window(struct iommu_table_group *table_group,
- int num)
-{
- int i, j;
- long ret = 0;
- struct npu_comp *npucomp = container_of(table_group, struct npu_comp,
- table_group);
-
- for (i = 0; i < npucomp->pe_num; ++i) {
- struct pnv_ioda_pe *pe = npucomp->pe[i];
-
- WARN_ON(npucomp->table_group.tables[num] !=
- table_group->tables[num]);
- if (!npucomp->table_group.tables[num])
- continue;
-
- if (!pe->table_group.ops->unset_window)
- continue;
-
- ret = pe->table_group.ops->unset_window(&pe->table_group, num);
- if (ret)
- break;
- }
-
- if (ret) {
- for (j = 0; j < i; ++j) {
- struct pnv_ioda_pe *pe = npucomp->pe[j];
-
- if (!npucomp->table_group.tables[num])
- continue;
-
- if (!pe->table_group.ops->set_window)
- continue;
-
- ret = pe->table_group.ops->set_window(&pe->table_group,
- num, table_group->tables[num]);
- if (ret)
- break;
- }
- } else if (table_group->tables[num]) {
- iommu_tce_table_put(table_group->tables[num]);
- table_group->tables[num] = NULL;
- }
-
- return ret;
-}
-
-static void pnv_npu_peers_take_ownership(struct iommu_table_group *table_group)
-{
- int i;
- struct npu_comp *npucomp = container_of(table_group, struct npu_comp,
- table_group);
-
- for (i = 0; i < npucomp->pe_num; ++i) {
- struct pnv_ioda_pe *pe = npucomp->pe[i];
-
- if (!pe->table_group.ops ||
- !pe->table_group.ops->take_ownership)
- continue;
- pe->table_group.ops->take_ownership(&pe->table_group);
- }
-}
-
-static void pnv_npu_peers_release_ownership(
- struct iommu_table_group *table_group)
-{
- int i;
- struct npu_comp *npucomp = container_of(table_group, struct npu_comp,
- table_group);
-
- for (i = 0; i < npucomp->pe_num; ++i) {
- struct pnv_ioda_pe *pe = npucomp->pe[i];
-
- if (!pe->table_group.ops ||
- !pe->table_group.ops->release_ownership)
- continue;
- pe->table_group.ops->release_ownership(&pe->table_group);
- }
-}
-
-static struct iommu_table_group_ops pnv_npu_peers_ops = {
- .get_table_size = pnv_pci_ioda2_get_table_size,
- .create_table = pnv_npu_peers_create_table_userspace,
- .set_window = pnv_npu_peers_set_window,
- .unset_window = pnv_npu_peers_unset_window,
- .take_ownership = pnv_npu_peers_take_ownership,
- .release_ownership = pnv_npu_peers_release_ownership,
-};
-
-static void pnv_comp_attach_table_group(struct npu_comp *npucomp,
- struct pnv_ioda_pe *pe)
-{
- if (WARN_ON(npucomp->pe_num == NV_NPU_MAX_PE_NUM))
- return;
-
- npucomp->pe[npucomp->pe_num] = pe;
- ++npucomp->pe_num;
-}
-
-static struct iommu_table_group *
- pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
-{
- struct iommu_table_group *compound_group;
- struct npu_comp *npucomp;
- struct pci_dev *gpdev = NULL;
- struct pci_controller *hose;
- struct pci_dev *npdev = NULL;
-
- list_for_each_entry(gpdev, &pe->pbus->devices, bus_list) {
- npdev = pnv_pci_get_npu_dev(gpdev, 0);
- if (npdev)
- break;
- }
-
- if (!npdev)
- /* It is not an NPU attached device, skip */
- return NULL;
-
- hose = pci_bus_to_host(npdev->bus);
-
- if (hose->npu) {
- /* P9 case: compound group is per-NPU (all gpus, all links) */
- npucomp = &hose->npu->npucomp;
- } else {
- /* P8 case: Compound group is per-GPU (1 gpu, 2 links) */
- npucomp = pe->npucomp = kzalloc(sizeof(*npucomp), GFP_KERNEL);
- }
-
- compound_group = &npucomp->table_group;
- if (!compound_group->group) {
- compound_group->ops = &pnv_npu_peers_ops;
- iommu_register_group(compound_group, hose->global_number,
- pe->pe_number);
-
- /* Steal capabilities from a GPU PE */
- compound_group->max_dynamic_windows_supported =
- pe->table_group.max_dynamic_windows_supported;
- compound_group->tce32_start = pe->table_group.tce32_start;
- compound_group->tce32_size = pe->table_group.tce32_size;
- compound_group->max_levels = pe->table_group.max_levels;
- if (!compound_group->pgsizes)
- compound_group->pgsizes = pe->table_group.pgsizes;
- }
-
- /*
- * The gpu would have been added to the iommu group that's created
- * for the PE. Pull it out now.
- */
- iommu_del_device(&gpdev->dev);
-
- /*
- * I'm not sure this is strictly required, but it's probably a good idea
- * since the table_group for the PE is going to be attached to the
- * compound table group. If we leave the PE's iommu group active then
- * we might have the same table_group being modifiable via two sepeate
- * iommu groups.
- */
- iommu_group_put(pe->table_group.group);
-
- /* now put the GPU into the compound group */
- pnv_comp_attach_table_group(npucomp, pe);
- iommu_add_device(compound_group, &gpdev->dev);
-
- return compound_group;
-}
-
-static struct iommu_table_group *pnv_npu_compound_attach(struct pnv_ioda_pe *pe)
-{
- struct iommu_table_group *table_group;
- struct npu_comp *npucomp;
- struct pci_dev *gpdev = NULL;
- struct pci_dev *npdev;
- struct pnv_ioda_pe *gpe = get_gpu_pci_dev_and_pe(pe, &gpdev);
-
- WARN_ON(!(pe->flags & PNV_IODA_PE_DEV));
- if (!gpe)
- return NULL;
-
- /*
- * IODA2 bridges get this set up from pci_controller_ops::setup_bridge
- * but NPU bridges do not have this hook defined so we do it here.
- * We do not setup other table group parameters as they won't be used
- * anyway - NVLink bridges are subordinate PEs.
- */
- pe->table_group.ops = &pnv_pci_npu_ops;
-
- table_group = iommu_group_get_iommudata(
- iommu_group_get(&gpdev->dev));
-
- /*
- * On P9 NPU PHB and PCI PHB support different page sizes,
- * keep only matching. We expect here that NVLink bridge PE pgsizes is
- * initialized by the caller.
- */
- table_group->pgsizes &= pe->table_group.pgsizes;
- npucomp = container_of(table_group, struct npu_comp, table_group);
- pnv_comp_attach_table_group(npucomp, pe);
-
- list_for_each_entry(npdev, &pe->phb->hose->bus->devices, bus_list) {
- struct pci_dev *gpdevtmp = pnv_pci_get_gpu_dev(npdev);
-
- if (gpdevtmp != gpdev)
- continue;
-
- iommu_add_device(table_group, &npdev->dev);
- }
-
- return table_group;
-}
-
-void pnv_pci_npu_setup_iommu_groups(void)
-{
- struct pci_controller *hose;
- struct pnv_phb *phb;
- struct pnv_ioda_pe *pe;
-
- /*
- * For non-nvlink devices the IOMMU group is registered when the PE is
- * configured and devices are added to the group when the per-device
- * DMA setup is run. That's done in hose->ops.dma_dev_setup() which is
- * only initialise for "normal" IODA PHBs.
- *
- * For NVLink devices we need to ensure the NVLinks and the GPU end up
- * in the same IOMMU group, so that's handled here.
- */
- list_for_each_entry(hose, &hose_list, list_node) {
- phb = hose->private_data;
-
- if (phb->type == PNV_PHB_IODA2)
- list_for_each_entry(pe, &phb->ioda.pe_list, list)
- pnv_try_setup_npu_table_group(pe);
- }
-
- /*
- * Now we have all PHBs discovered, time to add NPU devices to
- * the corresponding IOMMU groups.
- */
- list_for_each_entry(hose, &hose_list, list_node) {
- unsigned long pgsizes;
-
- phb = hose->private_data;
-
- if (phb->type != PNV_PHB_NPU_NVLINK)
- continue;
-
- pgsizes = pnv_ioda_parse_tce_sizes(phb);
- list_for_each_entry(pe, &phb->ioda.pe_list, list) {
- /*
- * IODA2 bridges get this set up from
- * pci_controller_ops::setup_bridge but NPU bridges
- * do not have this hook defined so we do it here.
- */
- pe->table_group.pgsizes = pgsizes;
- pnv_npu_compound_attach(pe);
- }
- }
-}
-#endif /* CONFIG_IOMMU_API */
-
-int pnv_npu2_init(struct pci_controller *hose)
-{
- static int npu_index;
- struct npu *npu;
- int ret;
-
- npu = kzalloc(sizeof(*npu), GFP_KERNEL);
- if (!npu)
- return -ENOMEM;
-
- npu_index++;
- if (WARN_ON(npu_index >= NV_MAX_NPUS)) {
- ret = -ENOSPC;
- goto fail_exit;
- }
- npu->index = npu_index;
- hose->npu = npu;
-
- return 0;
-
-fail_exit:
- kfree(npu);
- return ret;
-}
-
-int pnv_npu2_map_lpar_dev(struct pci_dev *gpdev, unsigned int lparid,
- unsigned long msr)
-{
- int ret;
- struct pci_dev *npdev = pnv_pci_get_npu_dev(gpdev, 0);
- struct pci_controller *hose;
- struct pnv_phb *nphb;
-
- if (!npdev)
- return -ENODEV;
-
- hose = pci_bus_to_host(npdev->bus);
- if (hose->npu == NULL) {
- dev_info_once(&npdev->dev, "Nvlink1 does not support contexts");
- return 0;
- }
-
- nphb = hose->private_data;
-
- dev_dbg(&gpdev->dev, "Map LPAR opalid=%llu lparid=%u\n",
- nphb->opal_id, lparid);
- /*
- * Currently we only support radix and non-zero LPCR only makes sense
- * for hash tables so skiboot expects the LPCR parameter to be a zero.
- */
- ret = opal_npu_map_lpar(nphb->opal_id, pci_dev_id(gpdev), lparid,
- 0 /* LPCR bits */);
- if (ret) {
- dev_err(&gpdev->dev, "Error %d mapping device to LPAR\n", ret);
- return ret;
- }
-
- dev_dbg(&gpdev->dev, "init context opalid=%llu msr=%lx\n",
- nphb->opal_id, msr);
- ret = opal_npu_init_context(nphb->opal_id, 0/*__unused*/, msr,
- pci_dev_id(gpdev));
- if (ret < 0)
- dev_err(&gpdev->dev, "Failed to init context: %d\n", ret);
- else
- ret = 0;
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(pnv_npu2_map_lpar_dev);
-
-void pnv_npu2_map_lpar(struct pnv_ioda_pe *gpe, unsigned long msr)
-{
- struct pci_dev *gpdev;
-
- list_for_each_entry(gpdev, &gpe->pbus->devices, bus_list)
- pnv_npu2_map_lpar_dev(gpdev, 0, msr);
-}
-
-int pnv_npu2_unmap_lpar_dev(struct pci_dev *gpdev)
-{
- int ret;
- struct pci_dev *npdev = pnv_pci_get_npu_dev(gpdev, 0);
- struct pci_controller *hose;
- struct pnv_phb *nphb;
-
- if (!npdev)
- return -ENODEV;
-
- hose = pci_bus_to_host(npdev->bus);
- if (hose->npu == NULL) {
- dev_info_once(&npdev->dev, "Nvlink1 does not support contexts");
- return 0;
- }
-
- nphb = hose->private_data;
-
- dev_dbg(&gpdev->dev, "destroy context opalid=%llu\n",
- nphb->opal_id);
- ret = opal_npu_destroy_context(nphb->opal_id, 0/*__unused*/,
- pci_dev_id(gpdev));
- if (ret < 0) {
- dev_err(&gpdev->dev, "Failed to destroy context: %d\n", ret);
- return ret;
- }
-
- /* Set LPID to 0 anyway, just to be safe */
- dev_dbg(&gpdev->dev, "Map LPAR opalid=%llu lparid=0\n", nphb->opal_id);
- ret = opal_npu_map_lpar(nphb->opal_id, pci_dev_id(gpdev), 0 /*LPID*/,
- 0 /* LPCR bits */);
- if (ret)
- dev_err(&gpdev->dev, "Error %d mapping device to LPAR\n", ret);
-
- return ret;
-}
-EXPORT_SYMBOL_GPL(pnv_npu2_unmap_lpar_dev);
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 5cd0f52d258f64..01401e3da7ca14 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -267,8 +267,6 @@ OPAL_CALL(opal_xive_get_queue_state, OPAL_XIVE_GET_QUEUE_STATE);
OPAL_CALL(opal_xive_set_queue_state, OPAL_XIVE_SET_QUEUE_STATE);
OPAL_CALL(opal_xive_get_vp_state, OPAL_XIVE_GET_VP_STATE);
OPAL_CALL(opal_signal_system_reset, OPAL_SIGNAL_SYSTEM_RESET);
-OPAL_CALL(opal_npu_init_context, OPAL_NPU_INIT_CONTEXT);
-OPAL_CALL(opal_npu_destroy_context, OPAL_NPU_DESTROY_CONTEXT);
OPAL_CALL(opal_npu_map_lpar, OPAL_NPU_MAP_LPAR);
OPAL_CALL(opal_imc_counters_init, OPAL_IMC_COUNTERS_INIT);
OPAL_CALL(opal_imc_counters_start, OPAL_IMC_COUNTERS_START);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index f0f901683a2fe1..5c88d7145a23e0 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -47,8 +47,7 @@
#define PNV_IODA1_M64_SEGS 8 /* Segments per M64 BAR */
#define PNV_IODA1_DMA32_SEGSIZE 0x10000000
-static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK",
- "NPU_OCAPI" };
+static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_OCAPI" };
static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable);
static void pnv_pci_configure_bus(struct pci_bus *bus);
@@ -192,8 +191,6 @@ void pnv_ioda_free_pe(struct pnv_ioda_pe *pe)
unsigned int pe_num = pe->pe_number;
WARN_ON(pe->pdev);
- 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);
@@ -875,7 +872,7 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
* Release from all parents PELT-V. NPUs don't have a PELTV
* table
*/
- if (phb->type != PNV_PHB_NPU_NVLINK && phb->type != PNV_PHB_NPU_OCAPI)
+ if (phb->type != PNV_PHB_NPU_OCAPI)
pnv_ioda_unset_peltv(phb, pe, parent);
rc = opal_pci_set_pe(phb->opal_id, pe->pe_number, pe->rid,
@@ -946,7 +943,7 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
* Configure PELTV. NPUs don't have a PELTV table so skip
* configuration on them.
*/
- if (phb->type != PNV_PHB_NPU_NVLINK && phb->type != PNV_PHB_NPU_OCAPI)
+ if (phb->type != PNV_PHB_NPU_OCAPI)
pnv_ioda_set_peltv(phb, pe, true);
/* Setup reverse map */
@@ -1002,8 +999,7 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
/* NOTE: We don't get a reference for the pointer in the PE
* data structure, both the device and PE structures should be
- * destroyed at the same time. However, removing nvlink
- * devices will need some work.
+ * destroyed at the same time.
*
* At some point we want to remove the PDN completely anyways
*/
@@ -1099,113 +1095,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
return pe;
}
-static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
-{
- int pe_num, found_pe = false, rc;
- long rid;
- struct pnv_ioda_pe *pe;
- struct pci_dev *gpu_pdev;
- struct pci_dn *npu_pdn;
- struct pnv_phb *phb = pci_bus_to_pnvhb(npu_pdev->bus);
-
- /*
- * Intentionally leak a reference on the npu device (for
- * nvlink only; this is not an opencapi path) to make sure it
- * never goes away, as it's been the case all along and some
- * work is needed otherwise.
- */
- pci_dev_get(npu_pdev);
-
- /*
- * Due to a hardware errata PE#0 on the NPU is reserved for
- * error handling. This means we only have three PEs remaining
- * which need to be assigned to four links, implying some
- * links must share PEs.
- *
- * To achieve this we assign PEs such that NPUs linking the
- * same GPU get assigned the same PE.
- */
- gpu_pdev = pnv_pci_get_gpu_dev(npu_pdev);
- for (pe_num = 0; pe_num < phb->ioda.total_pe_num; pe_num++) {
- pe = &phb->ioda.pe_array[pe_num];
- if (!pe->pdev)
- continue;
-
- if (pnv_pci_get_gpu_dev(pe->pdev) == gpu_pdev) {
- /*
- * This device has the same peer GPU so should
- * be assigned the same PE as the existing
- * peer NPU.
- */
- dev_info(&npu_pdev->dev,
- "Associating to existing PE %x\n", pe_num);
- npu_pdn = pci_get_pdn(npu_pdev);
- rid = npu_pdev->bus->number << 8 | npu_pdn->devfn;
- npu_pdn->pe_number = pe_num;
- phb->ioda.pe_rmap[rid] = pe->pe_number;
- pe->device_count++;
-
- /* Map the PE to this link */
- rc = opal_pci_set_pe(phb->opal_id, pe_num, rid,
- OpalPciBusAll,
- OPAL_COMPARE_RID_DEVICE_NUMBER,
- OPAL_COMPARE_RID_FUNCTION_NUMBER,
- OPAL_MAP_PE);
- WARN_ON(rc != OPAL_SUCCESS);
- found_pe = true;
- break;
- }
- }
-
- if (!found_pe)
- /*
- * Could not find an existing PE so allocate a new
- * one.
- */
- return pnv_ioda_setup_dev_PE(npu_pdev);
- else
- return pe;
-}
-
-static void pnv_ioda_setup_npu_PEs(struct pci_bus *bus)
-{
- struct pci_dev *pdev;
-
- list_for_each_entry(pdev, &bus->devices, bus_list)
- pnv_ioda_setup_npu_PE(pdev);
-}
-
-static void pnv_pci_ioda_setup_nvlink(void)
-{
- struct pci_controller *hose;
- struct pnv_phb *phb;
- struct pnv_ioda_pe *pe;
-
- list_for_each_entry(hose, &hose_list, list_node) {
- phb = hose->private_data;
- if (phb->type == PNV_PHB_NPU_NVLINK) {
- /* PE#0 is needed for error reporting */
- pnv_ioda_reserve_pe(phb, 0);
- pnv_ioda_setup_npu_PEs(hose->bus);
- if (phb->model == PNV_PHB_MODEL_NPU2)
- WARN_ON_ONCE(pnv_npu2_init(hose));
- }
- }
- list_for_each_entry(hose, &hose_list, list_node) {
- phb = hose->private_data;
- if (phb->type != PNV_PHB_IODA2)
- continue;
-
- list_for_each_entry(pe, &phb->ioda.pe_list, list)
- pnv_npu2_map_lpar(pe, MSR_DR | MSR_PR | MSR_HV);
- }
-
-#ifdef CONFIG_IOMMU_API
- /* setup iommu groups so we can do nvlink pass-thru */
- pnv_pci_npu_setup_iommu_groups();
-#endif
-}
-
static void pnv_pci_ioda1_setup_dma_pe(struct pnv_phb *phb,
struct pnv_ioda_pe *pe);
@@ -1468,18 +1357,6 @@ static struct iommu_table_ops pnv_ioda1_iommu_ops = {
#define PHB3_TCE_KILL_INVAL_PE PPC_BIT(1)
#define PHB3_TCE_KILL_INVAL_ONE PPC_BIT(2)
-static void pnv_pci_phb3_tce_invalidate_entire(struct pnv_phb *phb, bool rm)
-{
- __be64 __iomem *invalidate = pnv_ioda_get_inval_reg(phb, rm);
- const unsigned long val = PHB3_TCE_KILL_INVAL_ALL;
-
- mb(); /* Ensure previous TCE table stores are visible */
- if (rm)
- __raw_rm_writeq_be(val, invalidate);
- else
- __raw_writeq_be(val, invalidate);
-}
-
static inline void pnv_pci_phb3_tce_invalidate_pe(struct pnv_ioda_pe *pe)
{
/* 01xb - invalidate TCEs that match the specified PE# */
@@ -1539,20 +1416,6 @@ static void pnv_pci_ioda2_tce_invalidate(struct iommu_table *tbl,
struct pnv_phb *phb = pe->phb;
unsigned int shift = tbl->it_page_shift;
- /*
- * NVLink1 can use the TCE kill register directly as
- * it's the same as PHB3. NVLink2 is different and
- * should go via the OPAL call.
- */
- if (phb->model == PNV_PHB_MODEL_NPU) {
- /*
- * The NVLink hardware does not support TCE kill
- * per TCE entry so we have to invalidate
- * the entire cache for it.
- */
- pnv_pci_phb3_tce_invalidate_entire(phb, rm);
- continue;
- }
if (phb->model == PNV_PHB_MODEL_PHB3 && phb->regs)
pnv_pci_phb3_tce_invalidate(pe, rm, shift,
index, npages);
@@ -1564,14 +1427,6 @@ static void pnv_pci_ioda2_tce_invalidate(struct iommu_table *tbl,
}
}
-void pnv_pci_ioda2_tce_invalidate_entire(struct pnv_phb *phb, bool rm)
-{
- if (phb->model == PNV_PHB_MODEL_NPU || phb->model == PNV_PHB_MODEL_PHB3)
- pnv_pci_phb3_tce_invalidate_entire(phb, rm);
- else
- opal_pci_tce_kill(phb->opal_id, OPAL_PCI_TCE_KILL, 0, 0, 0, 0);
-}
-
static int pnv_ioda2_tce_build(struct iommu_table *tbl, long index,
long npages, unsigned long uaddr,
enum dma_data_direction direction,
@@ -2450,7 +2305,6 @@ static void pnv_pci_enable_bridges(void)
static void pnv_pci_ioda_fixup(void)
{
- pnv_pci_ioda_setup_nvlink();
pnv_pci_ioda_create_dbgfs();
pnv_pci_enable_bridges();
@@ -2823,15 +2677,6 @@ static void pnv_pci_release_device(struct pci_dev *pdev)
pnv_ioda_release_pe(pe);
}
-static void pnv_npu_disable_device(struct pci_dev *pdev)
-{
- struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
- struct eeh_pe *eehpe = edev ? edev->pe : NULL;
-
- if (eehpe && eeh_ops && eeh_ops->reset)
- eeh_ops->reset(eehpe, EEH_RESET_HOT);
-}
-
static void pnv_pci_ioda_shutdown(struct pci_controller *hose)
{
struct pnv_phb *phb = hose->private_data;
@@ -2873,16 +2718,6 @@ static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
.shutdown = pnv_pci_ioda_shutdown,
};
-static const struct pci_controller_ops pnv_npu_ioda_controller_ops = {
- .setup_msi_irqs = pnv_setup_msi_irqs,
- .teardown_msi_irqs = pnv_teardown_msi_irqs,
- .enable_device_hook = pnv_pci_enable_device_hook,
- .window_alignment = pnv_pci_window_alignment,
- .reset_secondary_bus = pnv_pci_reset_secondary_bus,
- .shutdown = pnv_pci_ioda_shutdown,
- .disable_device = pnv_npu_disable_device,
-};
-
static const struct pci_controller_ops pnv_npu_ocapi_ioda_controller_ops = {
.enable_device_hook = pnv_ocapi_enable_device_hook,
.release_device = pnv_pci_release_device,
@@ -2956,10 +2791,6 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
phb->model = PNV_PHB_MODEL_P7IOC;
else if (of_device_is_compatible(np, "ibm,power8-pciex"))
phb->model = PNV_PHB_MODEL_PHB3;
- else if (of_device_is_compatible(np, "ibm,power8-npu-pciex"))
- phb->model = PNV_PHB_MODEL_NPU;
- else if (of_device_is_compatible(np, "ibm,power9-npu-pciex"))
- phb->model = PNV_PHB_MODEL_NPU2;
else
phb->model = PNV_PHB_MODEL_UNKNOWN;
@@ -3117,9 +2948,6 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
ppc_md.pcibios_fixup = pnv_pci_ioda_fixup;
switch (phb->type) {
- case PNV_PHB_NPU_NVLINK:
- hose->controller_ops = pnv_npu_ioda_controller_ops;
- break;
case PNV_PHB_NPU_OCAPI:
hose->controller_ops = pnv_npu_ocapi_ioda_controller_ops;
break;
@@ -3172,11 +3000,6 @@ void __init pnv_pci_init_ioda2_phb(struct device_node *np)
pnv_pci_init_ioda_phb(np, 0, PNV_PHB_IODA2);
}
-void __init pnv_pci_init_npu_phb(struct device_node *np)
-{
- pnv_pci_init_ioda_phb(np, 0, PNV_PHB_NPU_NVLINK);
-}
-
void __init pnv_pci_init_npu2_opencapi_phb(struct device_node *np)
{
pnv_pci_init_ioda_phb(np, 0, PNV_PHB_NPU_OCAPI);
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 9b9bca169275a8..b18468dc31ff5b 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -926,17 +926,6 @@ void __init pnv_pci_init(void)
for_each_compatible_node(np, NULL, "ibm,ioda3-phb")
pnv_pci_init_ioda2_phb(np);
- /* Look for NPU PHBs */
- for_each_compatible_node(np, NULL, "ibm,ioda2-npu-phb")
- pnv_pci_init_npu_phb(np);
-
- /*
- * Look for NPU2 PHBs which we treat mostly as NPU PHBs with
- * the exception of TCE kill which requires an OPAL call.
- */
- for_each_compatible_node(np, NULL, "ibm,ioda2-npu2-phb")
- pnv_pci_init_npu_phb(np);
-
/* Look for NPU2 OpenCAPI PHBs */
for_each_compatible_node(np, NULL, "ibm,ioda2-npu2-opencapi-phb")
pnv_pci_init_npu2_opencapi_phb(np);
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 36d22920f5a3cb..c8d4f222a86fe3 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -10,10 +10,9 @@
struct pci_dn;
enum pnv_phb_type {
- PNV_PHB_IODA1 = 0,
- PNV_PHB_IODA2 = 1,
- PNV_PHB_NPU_NVLINK = 2,
- PNV_PHB_NPU_OCAPI = 3,
+ PNV_PHB_IODA1,
+ PNV_PHB_IODA2,
+ PNV_PHB_NPU_OCAPI,
};
/* Precise PHB model for error management */
@@ -21,8 +20,6 @@ enum pnv_phb_model {
PNV_PHB_MODEL_UNKNOWN,
PNV_PHB_MODEL_P7IOC,
PNV_PHB_MODEL_PHB3,
- PNV_PHB_MODEL_NPU,
- PNV_PHB_MODEL_NPU2,
};
#define PNV_PCI_DIAG_BUF_SIZE 8192
@@ -81,7 +78,6 @@ struct pnv_ioda_pe {
/* "Base" iommu table, ie, 4K TCEs, 32-bit DMA */
struct iommu_table_group table_group;
- struct npu_comp *npucomp;
/* 64-bit TCE bypass region */
bool tce_bypass_enabled;
@@ -289,9 +285,7 @@ extern struct iommu_table *pnv_pci_table_alloc(int nid);
extern void pnv_pci_init_ioda_hub(struct device_node *np);
extern void pnv_pci_init_ioda2_phb(struct device_node *np);
-extern void pnv_pci_init_npu_phb(struct device_node *np);
extern void pnv_pci_init_npu2_opencapi_phb(struct device_node *np);
-extern void pnv_npu2_map_lpar(struct pnv_ioda_pe *gpe, unsigned long msr);
extern void pnv_pci_reset_secondary_bus(struct pci_dev *dev);
extern int pnv_eeh_phb_reset(struct pci_controller *hose, int option);
@@ -314,11 +308,6 @@ extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
#define pe_info(pe, fmt, ...) \
pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__)
-/* Nvlink functions */
-extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
-extern void pnv_pci_ioda2_tce_invalidate_entire(struct pnv_phb *phb, bool rm);
-extern void pnv_pci_npu_setup_iommu_groups(void);
-
/* pci-ioda-tce.c */
#define POWERNV_IOMMU_DEFAULT_LEVELS 2
#define POWERNV_IOMMU_MAX_LEVELS 5
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 1bffbd1c9a94b1..3b6800f774c241 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -224,8 +224,6 @@ static void __init pSeries_request_regions(void)
void __init pSeries_final_fixup(void)
{
- struct pci_controller *hose;
-
pSeries_request_regions();
eeh_show_enabled();
@@ -234,27 +232,6 @@ void __init pSeries_final_fixup(void)
ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
ppc_md.pcibios_sriov_disable = pseries_pcibios_sriov_disable;
#endif
- list_for_each_entry(hose, &hose_list, list_node) {
- struct device_node *dn = hose->dn, *nvdn;
-
- while (1) {
- dn = of_find_all_nodes(dn);
- if (!dn)
- break;
- nvdn = of_parse_phandle(dn, "ibm,nvlink", 0);
- if (!nvdn)
- continue;
- if (!of_device_is_compatible(nvdn, "ibm,npu-link"))
- continue;
- if (!of_device_is_compatible(nvdn->parent,
- "ibm,power9-npu"))
- continue;
-#ifdef CONFIG_PPC_POWERNV
- WARN_ON_ONCE(pnv_npu2_init(hose));
-#endif
- break;
- }
- }
}
/*
--
2.30.1
^ permalink raw reply related
* remove the nvlink2 pci_vfio subdriver v2
From: Christoph Hellwig @ 2021-03-26 6:13 UTC (permalink / raw)
To: Michael Ellerman, Alex Williamson
Cc: Jason Gunthorpe, kvm, David Airlie, linux-kernel, dri-devel,
Paul Mackerras, Daniel Vetter, Greg Kroah-Hartman, linux-api,
linuxppc-dev
Hi all,
the nvlink2 vfio subdriver is a weird beast. It supports a hardware
feature without any open source component - what would normally be
the normal open source userspace that we require for kernel drivers,
although in this particular case user space could of course be a
kernel driver in a VM. It also happens to be a complete mess that
does not properly bind to PCI IDs, is hacked into the vfio_pci driver
and also pulles in over 1000 lines of code always build into powerpc
kernels that have Power NV support enabled. Because of all these
issues and the lack of breaking userspace when it is removed I think
the best idea is to simply kill.
Changes since v1:
- document the removed subtypes as reserved
- add the ACK from Greg
Diffstat:
arch/powerpc/platforms/powernv/npu-dma.c | 705 ---------------------------
b/arch/powerpc/include/asm/opal.h | 3
b/arch/powerpc/include/asm/pci-bridge.h | 1
b/arch/powerpc/include/asm/pci.h | 7
b/arch/powerpc/platforms/powernv/Makefile | 2
b/arch/powerpc/platforms/powernv/opal-call.c | 2
b/arch/powerpc/platforms/powernv/pci-ioda.c | 185 -------
b/arch/powerpc/platforms/powernv/pci.c | 11
b/arch/powerpc/platforms/powernv/pci.h | 17
b/arch/powerpc/platforms/pseries/pci.c | 23
b/drivers/vfio/pci/Kconfig | 6
b/drivers/vfio/pci/Makefile | 1
b/drivers/vfio/pci/vfio_pci.c | 18
b/drivers/vfio/pci/vfio_pci_private.h | 14
b/include/uapi/linux/vfio.h | 38 -
drivers/vfio/pci/vfio_pci_nvlink2.c | 490 ------------------
16 files changed, 12 insertions(+), 1511 deletions(-)
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/debug: Remove redundant NULL check
From: Daniel Axtens @ 2021-03-26 6:12 UTC (permalink / raw)
To: Jiapeng Chong, mpe; +Cc: Jiapeng Chong, paulus, linuxppc-dev, linux-kernel
In-Reply-To: <87r1k2795x.fsf@linkitivity.dja.id.au>
Daniel Axtens <dja@axtens.net> writes:
It looks like the kernel test robot also reported this:
"[PATCH] powerpc/iommu/debug: fix ifnullfree.cocci warnings"
Weirdly I don't see it in patchwork.
I'm not sure which one mpe will want to take but either would do.
>> Fix the following coccicheck warnings:
>>
>> ./fs/io_uring.c:5989:4-9: WARNING: NULL check before some freeing
>> functions is not needed.
(Also, while unimportant, that's technically not the error you fix here
as it's for a different file!)
>
> This looks correct to me, and matches the description of debugfs_remove
> in Documentation/filesystems/debugfs.rst.
>
> If you have a number of similar fixes it might be helpful to do them in
> a single bigger patch, but I'm not sure if coccicheck reports much else
> as I don't have coccinelle installed at the moment.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
>
> Kind regards,
> Daniel
^ permalink raw reply
* Re: [PATCH] crypto: vmx: fix incorrect kernel-doc comment syntax in files
From: Daniel Axtens @ 2021-03-26 6:02 UTC (permalink / raw)
To: Aditya Srivastava, leitao
Cc: rdunlap, herbert, corbet, linux-kernel-mentees, nayna,
linux-kernel, yashsri421, pfsmorigo, linux-crypto, lukas.bulwahn,
linuxppc-dev, davem, paulus
In-Reply-To: <20210320202525.12562-1-yashsri421@gmail.com>
Hi Aditya,
Thanks for your patch!
> The opening comment mark '/**' is used for highlighting the beginning of
> kernel-doc comments.
> There are certain files in drivers/crypto/vmx, which follow this syntax,
> but the content inside does not comply with kernel-doc.
> Such lines were probably not meant for kernel-doc parsing, but are parsed
> due to the presence of kernel-doc like comment syntax(i.e, '/**'), which
> causes unexpected warnings from kernel-doc.
>
> E.g., presence of kernel-doc like comment in the header line for
> drivers/crypto/vmx/vmx.c causes this warning by kernel-doc:
>
> "warning: expecting prototype for Routines supporting VMX instructions on the Power 8(). Prototype was for p8_init() instead"
checkpatch (scripts/checkpatch.pl --strict -g HEAD) complains about this line:
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
but checkpatch should be ignored here, as you did the right thing by not
breaking an error message across multiple lines.
> Similarly for other files too.
>
> Provide a simple fix by replacing such occurrences with general comment
> format, i.e. '/*', to prevent kernel-doc from parsing it.
This makes sense.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> * Applies perfectly on next-20210319
>
> drivers/crypto/vmx/aes.c | 2 +-
> drivers/crypto/vmx/aes_cbc.c | 2 +-
> drivers/crypto/vmx/aes_ctr.c | 2 +-
> drivers/crypto/vmx/aes_xts.c | 2 +-
> drivers/crypto/vmx/ghash.c | 2 +-
> drivers/crypto/vmx/vmx.c | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
> index d05c02baebcf..ec06189fbf99 100644
> --- a/drivers/crypto/vmx/aes.c
> +++ b/drivers/crypto/vmx/aes.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
> index d88084447f1c..ed0debc7acb5 100644
> --- a/drivers/crypto/vmx/aes_cbc.c
> +++ b/drivers/crypto/vmx/aes_cbc.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES CBC routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c
> index 79ba062ee1c1..9a3da8cd62f3 100644
> --- a/drivers/crypto/vmx/aes_ctr.c
> +++ b/drivers/crypto/vmx/aes_ctr.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES CTR routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
> index 9fee1b1532a4..dabbccb41550 100644
> --- a/drivers/crypto/vmx/aes_xts.c
> +++ b/drivers/crypto/vmx/aes_xts.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES XTS routines supporting VMX In-core instructions on Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
> index 14807ac2e3b9..5bc5710a6de0 100644
> --- a/drivers/crypto/vmx/ghash.c
> +++ b/drivers/crypto/vmx/ghash.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0
> -/**
> +/*
> * GHASH routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015, 2019 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c
> index a40d08e75fc0..7eb713cc87c8 100644
> --- a/drivers/crypto/vmx/vmx.c
> +++ b/drivers/crypto/vmx/vmx.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * Routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/debug: Remove redundant NULL check
From: Daniel Axtens @ 2021-03-26 5:53 UTC (permalink / raw)
To: Jiapeng Chong, mpe; +Cc: Jiapeng Chong, paulus, linuxppc-dev, linux-kernel
In-Reply-To: <1616406217-94635-1-git-send-email-jiapeng.chong@linux.alibaba.com>
Hi Jiapeng Chong, <jiapeng.chong@linux.alibaba.com> writes:
> Fix the following coccicheck warnings:
>
> ./fs/io_uring.c:5989:4-9: WARNING: NULL check before some freeing
> functions is not needed.
This looks correct to me, and matches the description of debugfs_remove
in Documentation/filesystems/debugfs.rst.
If you have a number of similar fixes it might be helpful to do them in
a single bigger patch, but I'm not sure if coccicheck reports much else
as I don't have coccinelle installed at the moment.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH] [v2] tools: testing: Remove duplicate includes
From: Daniel Axtens @ 2021-03-26 5:45 UTC (permalink / raw)
To: Wan Jiabing, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Shuah Khan, Wan Jiabing, linuxppc-dev,
linux-kselftest, linux-kernel
Cc: kael_w
In-Reply-To: <20210323061604.289958-1-wanjiabing@vivo.com>
Wan Jiabing <wanjiabing@vivo.com> writes:
> sched.h has been included at line 33, so remove the
> duplicate one at line 36.
> pthread.h has been included at line 17,so remove the
> duplicate one at line 20.
I can see that both of these are correct from the diff.
> inttypes.h has been included at line 19, so remove the
> duplicate one at line 23.
For this one I checked the file. Indeed there is another inttypes.h, so
this is also correct.
Again, all the automated checks pass. (although I don't think any of the
automated builds include selftests.)
So:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
> ---
> tools/testing/selftests/powerpc/mm/tlbie_test.c | 1 -
> tools/testing/selftests/powerpc/tm/tm-poison.c | 1 -
> tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/mm/tlbie_test.c b/tools/testing/selftests/powerpc/mm/tlbie_test.c
> index f85a0938ab25..48344a74b212 100644
> --- a/tools/testing/selftests/powerpc/mm/tlbie_test.c
> +++ b/tools/testing/selftests/powerpc/mm/tlbie_test.c
> @@ -33,7 +33,6 @@
> #include <sched.h>
> #include <time.h>
> #include <stdarg.h>
> -#include <sched.h>
> #include <pthread.h>
> #include <signal.h>
> #include <sys/prctl.h>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-poison.c b/tools/testing/selftests/powerpc/tm/tm-poison.c
> index 29e5f26af7b9..27c083a03d1f 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-poison.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-poison.c
> @@ -20,7 +20,6 @@
> #include <sched.h>
> #include <sys/types.h>
> #include <signal.h>
> -#include <inttypes.h>
>
> #include "tm.h"
>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> index e2a0c07e8362..9ef37a9836ac 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> @@ -17,7 +17,6 @@
> #include <pthread.h>
> #include <sys/mman.h>
> #include <unistd.h>
> -#include <pthread.h>
>
> #include "tm.h"
> #include "utils.h"
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH] [v2] arch: powerpc: Remove duplicate includes
From: Daniel Axtens @ 2021-03-26 5:38 UTC (permalink / raw)
To: Wan Jiabing, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Nicholas Piggin, Aneesh Kumar K.V,
Cédric Le Goater, Randy Dunlap, Wan Jiabing, Ganesh Goudar,
Christophe Leroy, Michal Suchanek, Geert Uytterhoeven,
Pingfan Liu, Frederic Weisbecker, linuxppc-dev, linux-kernel
Cc: kael_w
In-Reply-To: <20210323062916.295346-1-wanjiabing@vivo.com>
Wan Jiabing <wanjiabing@vivo.com> writes:
> mmu-hash.h: asm/bug.h has been included at line 12, so remove
> the duplicate one at line 21.
Looking at the file I had wondered if this was due to a #ifdef being
removed, but no, the second one was just added in commit 891121e6c02c
("powerpc/mm: Differentiate between hugetlb and THP during page
walk"). How odd!
Anyway, all of these look good to me, and the automated checks at
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210323062916.295346-1-wanjiabing@vivo.com/
have all passed.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH v2 1/1] hotplug-cpu.c: show 'last online CPU' error in dlpar_cpu_offline()
From: Daniel Axtens @ 2021-03-26 5:24 UTC (permalink / raw)
To: Daniel Henrique Barboza, linuxppc-dev; +Cc: Daniel Henrique Barboza
In-Reply-To: <20210323205056.52768-2-danielhb413@gmail.com>
Hi Daniel,
Two small nitpicks:
> This patch adds a 'last online' check in dlpar_cpu_offline() to catch
> the 'last online CPU' offline error, eturning a more informative error
^--- s/eturning/returning/;
> + /* device_offline() will return -EBUSY (via cpu_down())
> + * if there is only one CPU left. Check it here to fail
> + * earlier and with a more informative error message,
> + * while also retaining the cpu_add_remove_lock to be sure
> + * that no CPUs are being online/offlined during this
> + * check. */
Checkpatch has a small issue with this comment:
WARNING: Block comments use a trailing */ on a separate line
#50: FILE: arch/powerpc/platforms/pseries/hotplug-cpu.c:279:
+ * check. */
Apart from that, this patch seems sane to me, but I haven't been able to
test it.
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH -next] powerpc/eeh: Remove unused inline function eeh_dev_phb_init_dynamic()
From: Daniel Axtens @ 2021-03-26 5:08 UTC (permalink / raw)
To: YueHaibing, ruscur, oohall, mpe, benh, paulus
Cc: YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <20210324140714.19612-1-yuehaibing@huawei.com>
Hi,
> commit 475028efc708 ("powerpc/eeh: Remove eeh_dev_phb_init_dynamic()")
> left behind this, so can remove it.
I had a look: the inline that you are removing here is for the
!CONFIG_EEH case, which explains why it was missed the first time.
This looks like a good change. Out of interest, what tool are you using
to find these unused inlines? If there are many more, it might make
sense to combine future patches removing them into a single patch, but
I'm not sure.
checkpatch likes this patch, so that's also good :)
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH -next] powerpc/smp: Remove unused inline functions
From: Daniel Axtens @ 2021-03-26 5:01 UTC (permalink / raw)
To: YueHaibing, mpe, benh, paulus; +Cc: YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <20210324140752.11320-1-yuehaibing@huawei.com>
Hi,
> commit 441c19c8a290 ("powerpc/kvm/book3s_hv: Rework the secondary inhibit code")
> left behind this, so can remove it.
>
Interesting: that commit removed some instances of
(un)inhibit_secondary_onlining, but it seems to have missed the ones for
the uni-processor case, which your patch removes. This seems like a good
change.
Checkpatch does have one small complaint about your commit message:
| WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
| #6:
| commit 441c19c8a290 ("powerpc/kvm/book3s_hv: Rework the secondary inhibit code")
I don't think this warrants another revision, I think leaving the commit
name on one line makes sense.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> 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 7a13bc20f0a0..ad7129a19e8f 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -189,8 +189,6 @@ extern void __cpu_die(unsigned int cpu);
> #define hard_smp_processor_id() get_hard_smp_processor_id(0)
> #define smp_setup_cpu_maps()
> #define thread_group_shares_l2 0
> -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
* Re: [PATCH] selftests: powerpc: unmark non-kernel-doc comments
From: Daniel Axtens @ 2021-03-26 4:50 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel; +Cc: Randy Dunlap, linuxppc-dev
In-Reply-To: <20210325200820.16594-1-rdunlap@infradead.org>
Randy Dunlap <rdunlap@infradead.org> writes:
> Drop the 'beginning of kernel-doc' notation markers (/**)
> in places that are not in kernel-doc format.
This looks good to me. Arguably we don't need the comments at all, but
it doesn't seem to hurt to keep them.
checkpatch is OK with the entire file, so there's nothing else we'd
really want to clean up while you're doing cleanups.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> tools/testing/selftests/powerpc/tm/tm-trap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- linux-next-20210323.orig/tools/testing/selftests/powerpc/tm/tm-trap.c
> +++ linux-next-20210323/tools/testing/selftests/powerpc/tm/tm-trap.c
> @@ -66,7 +66,7 @@ void trap_signal_handler(int signo, sigi
> /* Get thread endianness: extract bit LE from MSR */
> thread_endianness = MSR_LE & ucp->uc_mcontext.gp_regs[PT_MSR];
>
> - /***
> + /*
> * Little-Endian Machine
> */
>
> @@ -126,7 +126,7 @@ void trap_signal_handler(int signo, sigi
> }
> }
>
> - /***
> + /*
> * Big-Endian Machine
> */
>
^ permalink raw reply
* Re: [PATCH] soc/fsl: qbman: fix conflicting alignment attributes
From: Li Yang @ 2021-03-26 2:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, Madalin-cristian Bucur, soc, Lee Jones, Roy Pledge,
YueHaibing, lkml, Scott Wood, Claudiu Manoil, Jakub Kicinski,
linuxppc-dev, Sebastian Andrzej Siewior,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20210323131530.2619900-1-arnd@kernel.org>
On Tue, Mar 23, 2021 at 8:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> When building with W=1, gcc points out that the __packed attribute
> on struct qm_eqcr_entry conflicts with the 8-byte alignment
> attribute on struct qm_fd inside it:
>
> drivers/soc/fsl/qbman/qman.c:189:1: error: alignment 1 of 'struct qm_eqcr_entry' is less than 8 [-Werror=packed-not-aligned]
>
> I assume that the alignment attribute is the correct one, and
> that qm_eqcr_entry cannot actually be unaligned in memory,
> so add the same alignment on the outer struct.
>
> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/soc/fsl/qbman/qman.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index a1b9be1d105a..fde4edd83c14 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -186,7 +186,7 @@ struct qm_eqcr_entry {
> __be32 tag;
> struct qm_fd fd;
> u8 __reserved3[32];
> -} __packed;
> +} __packed __aligned(8);
The EQCR structure is actually aligned on 64-byte from the manual.
But probably 8 is enough to let the compiler not complain.
> #define QM_EQCR_VERB_VBIT 0x80
> #define QM_EQCR_VERB_CMD_MASK 0x61 /* but only one value; */
> #define QM_EQCR_VERB_CMD_ENQUEUE 0x01
> --
> 2.29.2
>
^ permalink raw reply
* Re: [PATCH v4 24/46] KVM: PPC: Book3S HV P9: Use large decrementer for HDEC
From: Alexey Kardashevskiy @ 2021-03-26 2:05 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210323010305.1045293-25-npiggin@gmail.com>
On 23/03/2021 12:02, Nicholas Piggin wrote:
> On processors that don't suppress the HDEC exceptions when LPCR[HDICE]=0,
> this could help reduce needless guest exits due to leftover exceptions on
> entering the guest.
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
ERROR: modpost: "decrementer_max" [arch/powerpc/kvm/kvm-hv.ko] undefined!
need this:
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -89,6 +89,7 @@ static struct clocksource clocksource_timebase = {
#define DECREMENTER_DEFAULT_MAX 0x7FFFFFFF
u64 decrementer_max = DECREMENTER_DEFAULT_MAX;
+EXPORT_SYMBOL_GPL(decrementer_max);
> ---
> arch/powerpc/include/asm/time.h | 2 ++
> arch/powerpc/kvm/book3s_hv.c | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..68d94711811e 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -18,6 +18,8 @@
> #include <asm/vdso/timebase.h>
>
> /* time.c */
> +extern u64 decrementer_max;
> +
> extern unsigned long tb_ticks_per_jiffy;
> extern unsigned long tb_ticks_per_usec;
> extern unsigned long tb_ticks_per_sec;
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8215430e6d5e..bb30c5ab53d1 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3658,7 +3658,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> vc->tb_offset_applied = 0;
> }
>
> - mtspr(SPRN_HDEC, 0x7fffffff);
> + /* HDEC must be at least as large as DEC, so decrementer_max fits */
> + mtspr(SPRN_HDEC, decrementer_max);
>
> switch_mmu_to_host_radix(kvm, host_pidr);
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Rob Herring @ 2021-03-25 23:29 UTC (permalink / raw)
To: Daniel Walker
Cc: Daniel Gimpelevich, linuxppc-dev, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, xe-linux-external,
Andrew Morton, Will Deacon
In-Reply-To: <20210325195956.GM109100@zorba>
On Thu, Mar 25, 2021 at 2:00 PM Daniel Walker <danielwa@cisco.com> wrote:
>
> On Thu, Mar 25, 2021 at 01:03:55PM +0100, Christophe Leroy wrote:
> >
> > Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
> >
> > Let's only provide once CMDLINE as of today, and ask the user to select
> > whether he wants it appended or prepended or replacee. Then no need to
> > change all existing config to rename CONFIG_CMDLINE into either of the new
> > ones.
> >
> > That's the main difference between my series and Daniel's series. So I'll
> > finish taking Will's comment into account and we'll send out a v3 soon.
>
> It doesn't solve the needs of Cisco, I've stated many times your changes have
> little value. Please stop submitting them.
Can you please outline what those needs are which aren't met?
Rob
^ permalink raw reply
* Re: [PATCH v2 07/15] powerpc/uaccess: Call might_fault() inconditionaly
From: Daniel Axtens @ 2021-03-25 22:44 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <874kgykgfk.fsf@dja-thinkpad.axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> Hi Christophe,
>
>> Commit 6bfd93c32a50 ("powerpc: Fix incorrect might_sleep in
>> __get_user/__put_user on kernel addresses") added a check to not call
>> might_sleep() on kernel addresses. This was to enable the use of
>> __get_user() in the alignment exception handler for any address.
>>
>> Then commit 95156f0051cb ("lockdep, mm: fix might_fault() annotation")
>> added a check of the address space in might_fault(), based on
>> set_fs() logic. But this didn't solve the powerpc alignment exception
>> case as it didn't call set_fs(KERNEL_DS).
>>
>> Nowadays, set_fs() is gone, previous patch fixed the alignment
>> exception handler and __get_user/__put_user are not supposed to be
>> used anymore to read kernel memory.
>>
>> Therefore the is_kernel_addr() check has become useless and can be
>> removed.
>
> While I agree that __get_user/__put_user should not be used on kernel
> memory, I'm not sure that we have covered every case where they might be
> used on kernel memory yet. I did a git grep for __get_user - there are
> several callers in arch/powerpc and it looks like at least lib/sstep.c
> might be using __get_user to read kernel memory while single-stepping.
>
> I am not sure if might_sleep has got logic to cover the powerpc case -
> it uses uaccess_kernel, but we don't supply a definition for that on
> powerpc, so if we do end up calling __get_user on a kernel address, I
> think we might now throw a warning. (Unless we are saved by
> pagefault_disabled()?)
Ah, I just re-read some of my earlier emails and was reminded that yes,
if we are calling __get/put, we must have disabled page faults.
So yes, this is good.
>
> (But I haven't tested this yet, so it's possible I misunderstood
> something.)
>
> Do you expect any consequences if we've missed a case where
> __(get|put)_user is called on a kernel address because it hasn't been
> converted to use better helpers yet?
>
> Kind regards,
> Daniel
>
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/include/asm/uaccess.h | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
>> index eaa828a6a419..c4bbc64758a0 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -77,8 +77,7 @@ __pu_failed: \
>> __typeof__(*(ptr)) __pu_val = (x); \
>> __typeof__(size) __pu_size = (size); \
>> \
>> - if (!is_kernel_addr((unsigned long)__pu_addr)) \
>> - might_fault(); \
>> + might_fault(); \
>> __chk_user_ptr(__pu_addr); \
>> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
>> \
>> @@ -238,12 +237,12 @@ do { \
>> __typeof__(size) __gu_size = (size); \
>> \
>> __chk_user_ptr(__gu_addr); \
>> - if (do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
>> + if (do_allow) { \
>> might_fault(); \
>> - if (do_allow) \
>> __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
>> - else \
>> + } else { \
>> __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
>> + } \
One microscopic nit: these changes throw the '\'s further out of
alignment.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>> (x) = (__typeof__(*(ptr)))__gu_val; \
>> \
>> __gu_err; \
>> --
>> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 07/15] powerpc/uaccess: Call might_fault() inconditionaly
From: Daniel Axtens @ 2021-03-25 22:38 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e0a980a4dc7a2551183dd5cb30f46eafdbee390c.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Commit 6bfd93c32a50 ("powerpc: Fix incorrect might_sleep in
> __get_user/__put_user on kernel addresses") added a check to not call
> might_sleep() on kernel addresses. This was to enable the use of
> __get_user() in the alignment exception handler for any address.
>
> Then commit 95156f0051cb ("lockdep, mm: fix might_fault() annotation")
> added a check of the address space in might_fault(), based on
> set_fs() logic. But this didn't solve the powerpc alignment exception
> case as it didn't call set_fs(KERNEL_DS).
>
> Nowadays, set_fs() is gone, previous patch fixed the alignment
> exception handler and __get_user/__put_user are not supposed to be
> used anymore to read kernel memory.
>
> Therefore the is_kernel_addr() check has become useless and can be
> removed.
While I agree that __get_user/__put_user should not be used on kernel
memory, I'm not sure that we have covered every case where they might be
used on kernel memory yet. I did a git grep for __get_user - there are
several callers in arch/powerpc and it looks like at least lib/sstep.c
might be using __get_user to read kernel memory while single-stepping.
I am not sure if might_sleep has got logic to cover the powerpc case -
it uses uaccess_kernel, but we don't supply a definition for that on
powerpc, so if we do end up calling __get_user on a kernel address, I
think we might now throw a warning. (Unless we are saved by
pagefault_disabled()?)
(But I haven't tested this yet, so it's possible I misunderstood
something.)
Do you expect any consequences if we've missed a case where
__(get|put)_user is called on a kernel address because it hasn't been
converted to use better helpers yet?
Kind regards,
Daniel
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/uaccess.h | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index eaa828a6a419..c4bbc64758a0 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -77,8 +77,7 @@ __pu_failed: \
> __typeof__(*(ptr)) __pu_val = (x); \
> __typeof__(size) __pu_size = (size); \
> \
> - if (!is_kernel_addr((unsigned long)__pu_addr)) \
> - might_fault(); \
> + might_fault(); \
> __chk_user_ptr(__pu_addr); \
> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
> \
> @@ -238,12 +237,12 @@ do { \
> __typeof__(size) __gu_size = (size); \
> \
> __chk_user_ptr(__gu_addr); \
> - if (do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
> + if (do_allow) { \
> might_fault(); \
> - if (do_allow) \
> __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
> - else \
> + } else { \
> __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
> + } \
> (x) = (__typeof__(*(ptr)))__gu_val; \
> \
> __gu_err; \
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 06/15] powerpc/align: Don't use __get_user_instr() on kernel addresses
From: Daniel Axtens @ 2021-03-25 22:12 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d9ecbce00178484e66ca7adec2ff210058037704.1615398265.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> In the old days, when we didn't have kernel userspace access
> protection and had set_fs(), it was wise to use __get_user()
> and friends to read kernel memory.
>
> Nowadays, get_user() is granting userspace access and is exclusively
> for userspace access.
>
> In alignment exception handler, use probe_kernel_read_inst()
> instead of __get_user_instr() for reading instructions in kernel.
>
> This will allow to remove the is_kernel_addr() check in
> __get/put_user() in a following patch.
>
Looks good to me!
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/align.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index c4d7b445b459..8d4c7af262e2 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -310,7 +310,11 @@ int fix_alignment(struct pt_regs *regs)
> */
> CHECK_FULL_REGS(regs);
>
> - if (unlikely(__get_user_instr(instr, (void __user *)regs->nip)))
> + if (is_kernel_addr(regs->nip))
> + r = probe_kernel_read_inst(&instr, (void *)regs->nip);
> + else
> + r = __get_user_instr(instr, (void __user *)regs->nip);
> + if (unlikely(r))
> return -EFAULT;
> if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
> /* We don't handle PPC little-endian any more... */
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 05/15] powerpc/uaccess: Move get_user_instr helpers in asm/inst.h
From: Daniel Axtens @ 2021-03-25 21:59 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <2c6e83581b4fa434aa7cf2fa7714c41e98f57007.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Those helpers use get_user helpers but they don't participate
> in their implementation, so they do not belong to asm/uaccess.h
>
> Move them in asm/inst.h
Hmm, is asm/inst.h the right place for this?
asm/inst.h seems to be entirely concerned with the ppc_inst type:
converting things to and from ppc_inst, print ppc_inst as a string,
dealing with prefixed instructs, etc., etc. The only things currently
that look at memory are the probe_user_read_inst and
probe_kernel_read_inst prototypes...
Having said that, I'm not sure quite where else to put it, and none of
the other places in arch/powerpc/include that currently reference
ppc_inst seem any better...
If we do use asm/inst.h, I think maybe it makes sense to put the
code towards the end rather than at the start, as uses structs and calls
macros that are defined later on in the function.
Kind regards,
Daniel
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/inst.h | 34 ++++++++++++++++++++++++++++++
> arch/powerpc/include/asm/uaccess.h | 34 ------------------------------
> 2 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index cc73c1267572..19e18af2fac9 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -4,6 +4,40 @@
>
> #include <asm/ppc-opcode.h>
>
> +#ifdef CONFIG_PPC64
> +
> +#define ___get_user_instr(gu_op, dest, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + unsigned long __gui_ptr = (unsigned long)ptr; \
> + struct ppc_inst __gui_inst; \
> + unsigned int __prefix, __suffix; \
> + __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
> + if (__gui_ret == 0) { \
> + if ((__prefix >> 26) == OP_PREFIX) { \
> + __gui_ret = gu_op(__suffix, \
> + (unsigned int __user *)__gui_ptr + 1); \
> + __gui_inst = ppc_inst_prefix(__prefix, \
> + __suffix); \
> + } else { \
> + __gui_inst = ppc_inst(__prefix); \
> + } \
> + if (__gui_ret == 0) \
> + (dest) = __gui_inst; \
> + } \
> + __gui_ret; \
> +})
> +#else /* !CONFIG_PPC64 */
> +#define ___get_user_instr(gu_op, dest, ptr) \
> + gu_op((dest).val, (u32 __user *)(ptr))
> +#endif /* CONFIG_PPC64 */
> +
> +#define get_user_instr(x, ptr) \
> + ___get_user_instr(get_user, x, ptr)
> +
> +#define __get_user_instr(x, ptr) \
> + ___get_user_instr(__get_user, x, ptr)
> +
> /*
> * Instruction data type for POWER
> */
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 01aea0df4dd0..eaa828a6a419 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -53,40 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
> #define __put_user(x, ptr) \
> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> -#ifdef CONFIG_PPC64
> -
> -#define ___get_user_instr(gu_op, dest, ptr) \
> -({ \
> - long __gui_ret = 0; \
> - unsigned long __gui_ptr = (unsigned long)ptr; \
> - struct ppc_inst __gui_inst; \
> - unsigned int __prefix, __suffix; \
> - __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
> - if (__gui_ret == 0) { \
> - if ((__prefix >> 26) == OP_PREFIX) { \
> - __gui_ret = gu_op(__suffix, \
> - (unsigned int __user *)__gui_ptr + 1); \
> - __gui_inst = ppc_inst_prefix(__prefix, \
> - __suffix); \
> - } else { \
> - __gui_inst = ppc_inst(__prefix); \
> - } \
> - if (__gui_ret == 0) \
> - (dest) = __gui_inst; \
> - } \
> - __gui_ret; \
> -})
> -#else /* !CONFIG_PPC64 */
> -#define ___get_user_instr(gu_op, dest, ptr) \
> - gu_op((dest).val, (u32 __user *)(ptr))
> -#endif /* CONFIG_PPC64 */
> -
> -#define get_user_instr(x, ptr) \
> - ___get_user_instr(get_user, x, ptr)
> -
> -#define __get_user_instr(x, ptr) \
> - ___get_user_instr(__get_user, x, ptr)
> -
> extern long __put_user_bad(void);
>
> #define __put_user_size(x, ptr, size, retval) \
> --
> 2.25.0
^ 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