* [PATCH kernel v2 4/7] powerpc/powernv/phb4: Use IOMMU instead of bypassing
From: Alexey Kardashevskiy @ 2020-03-23 7:53 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Fabiano Rosas, Alexey Kardashevskiy, Alistair Popple,
kvm-ppc, David Gibson
In-Reply-To: <20200323075354.93825-1-aik@ozlabs.ru>
At the moment IODA2 systems do 64bit DMA by bypassing IOMMU which
allows mapping PCI space to system space at fixed offset (1<<59).
The bypass is controlled via the "iommu" kernel parameter.
This adds a "iommu_bypass" mode which maps PCI space to system space
using an actual TCE table with the biggest IOMMU page size available
(256MB or 1GB) and 2 levels so in a typical case about 4 to 6 system
pages per PHB are allocated.
This creates a single TCE table per PHB which is shared among devices
under the same PHB.
With this enabled, all DMA goes via IOMMU. Tests on 100GBit ethernet
did not show any regression.
The following patch allows using a special PHB4 4GB PCI hack which
moved 64bit DMA window at 4GB from 1<<59 to improve DMA support.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/powernv/pci.h | 1 +
arch/powerpc/platforms/powernv/pci-ioda.c | 128 ++++++++++++++++++----
2 files changed, 107 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index a808dd396522..ce00278185b0 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -100,6 +100,7 @@ struct pnv_phb {
int has_dbgfs;
struct dentry *dbgfs;
#endif
+ struct iommu_table *bypass_tbl; /* PNV_IOMMU_TCE_BYPASS only */
unsigned int msi_base;
unsigned int msi32_support;
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index f5f1b4e25530..9928a1618a8b 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -51,6 +51,10 @@ static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK",
"NPU_OCAPI" };
static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable);
+static unsigned long pnv_ioda_parse_tce_sizes(struct pnv_phb *phb);
+static long pnv_pci_ioda2_create_table(int nid, int num, __u64 bus_offset,
+ __u32 page_shift, __u64 window_size, __u32 levels,
+ bool alloc_userspace_copy, struct iommu_table **ptbl);
void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
const char *fmt, ...)
@@ -83,7 +87,14 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
va_end(args);
}
-static bool pnv_iommu_bypass_disabled __read_mostly;
+enum pnv_iommu_bypass_mode {
+ PNV_IOMMU_NO_TRANSLATE,
+ PNV_IOMMU_BYPASS_DISABLED,
+ PNV_IOMMU_TCE_BYPASS
+};
+
+static enum pnv_iommu_bypass_mode pnv_iommu_bypass_mode __read_mostly =
+ PNV_IOMMU_NO_TRANSLATE;
static bool pci_reset_phbs __read_mostly;
static int __init iommu_setup(char *str)
@@ -93,9 +104,13 @@ static int __init iommu_setup(char *str)
while (*str) {
if (!strncmp(str, "nobypass", 8)) {
- pnv_iommu_bypass_disabled = true;
+ pnv_iommu_bypass_mode = PNV_IOMMU_BYPASS_DISABLED;
pr_info("PowerNV: IOMMU bypass window disabled.\n");
break;
+ } else if (!strncmp(str, "iommu_bypass", 12)) {
+ pnv_iommu_bypass_mode = PNV_IOMMU_TCE_BYPASS;
+ pr_info("PowerNV: IOMMU TCE bypass window selected.\n");
+ break;
}
str += strcspn(str, ",");
if (*str == ',')
@@ -2351,28 +2366,99 @@ static long pnv_pci_ioda2_set_window(struct iommu_table_group *table_group,
return 0;
}
+static long pnv_pci_ioda2_set_bypass_iommu(struct pnv_ioda_pe *pe,
+ unsigned long bus_offset)
+{
+ struct pnv_phb *phb = pe->phb;
+ long rc;
+ struct memblock_region *r;
+ unsigned long pgsizes;
+
+
+ pgsizes = pnv_ioda_parse_tce_sizes(phb);
+ if (!pgsizes)
+ return -1;
+
+ if (!phb->bypass_tbl) {
+ struct iommu_table *tbl = NULL;
+
+ rc = pnv_pci_ioda2_create_table(phb->hose->node,
+ 1 /* window number */,
+ bus_offset,
+ __fls(pgsizes),
+ roundup_pow_of_two(memory_hotplug_max()),
+ 2 /* levels */,
+ false /* userspace cache */,
+ &tbl);
+ if (rc)
+ return -1;
+
+ for_each_memblock(memory, r)
+ pnv_ioda2_tce_build(tbl,
+ (r->base >> tbl->it_page_shift) +
+ tbl->it_offset,
+ r->size >> tbl->it_page_shift,
+ (unsigned long) __va(r->base),
+ DMA_BIDIRECTIONAL,
+ 0);
+ phb->bypass_tbl = tbl;
+ pe_info(pe, "Created 64-bit bypass TCE table\n");
+ } else {
+ iommu_tce_table_get(phb->bypass_tbl);
+ }
+
+ rc = pnv_pci_ioda2_set_window(&pe->table_group, 1, phb->bypass_tbl);
+ if (rc) {
+ iommu_tce_table_put(phb->bypass_tbl);
+ return -1;
+ }
+
+ pe->tce_bypass_enabled = true;
+
+ return 0;
+}
+
static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
{
+ struct pnv_phb *phb = pe->phb;
uint16_t window_id = (pe->pe_number << 1 ) + 1;
int64_t rc;
+ phys_addr_t top;
- pe_info(pe, "%sabling 64-bit DMA bypass\n", enable ? "En" : "Dis");
- if (enable) {
- phys_addr_t top = memblock_end_of_DRAM();
+ if (!enable) {
+ pe_info(pe, "Disabling 64-bit bypass\n");
+ rc = opal_pci_map_pe_dma_window_real(phb->opal_id,
+ pe->pe_number, window_id, 0, 0);
+ if (rc)
+ pe_err(pe, "OPAL error %lld configuring bypass window\n",
+ rc);
- top = roundup_pow_of_two(top);
- rc = opal_pci_map_pe_dma_window_real(pe->phb->opal_id,
- pe->pe_number, window_id,
- pe->table_group.tce64_start, top);
- } else {
- rc = opal_pci_map_pe_dma_window_real(pe->phb->opal_id,
- pe->pe_number, window_id,
- pe->table_group.tce64_start, 0);
+ pe->tce_bypass_enabled = false;
+ return;
+ }
+
+ if (pnv_iommu_bypass_mode == PNV_IOMMU_TCE_BYPASS) {
+ if (!pnv_pci_ioda2_set_bypass_iommu(pe,
+ pe->table_group.tce64_start)) {
+ pe->tce_bypass_enabled = true;
+ pe_info(pe, "Enabled 64-bit IOMMU bypass at %llx\n",
+ pe->table_group.tce64_start);
+ return;
+ }
+ /* IOMMU bypass failed, fallback to direct bypass */
+ pnv_iommu_bypass_mode = PNV_IOMMU_NO_TRANSLATE;
+ }
+
+ if (pnv_iommu_bypass_mode == PNV_IOMMU_NO_TRANSLATE) {
+ top = roundup_pow_of_two(memblock_end_of_DRAM());
+ if (!opal_pci_map_pe_dma_window_real(phb->opal_id,
+ pe->pe_number, window_id,
+ pe->table_group.tce64_start, top)) {
+ pe->tce_bypass_enabled = true;
+ pe_info(pe, "Enabled 64-bit direct bypass at %llx\n",
+ pe->table_group.tce64_start);
+ }
}
- if (rc)
- pe_err(pe, "OPAL error %lld configuring bypass window\n", rc);
- else
- pe->tce_bypass_enabled = enable;
}
static long pnv_pci_ioda2_create_table(int nid, int num, __u64 bus_offset,
@@ -2409,6 +2495,9 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
const unsigned int tceshift = PAGE_SHIFT;
unsigned long res_start, res_end, tces_order, tcelevel_order, levels;
+ if (pnv_iommu_bypass_mode != PNV_IOMMU_BYPASS_DISABLED)
+ pnv_pci_ioda2_set_bypass(pe, true);
+
/*
* crashkernel= specifies the kdump kernel's maximum memory at
* some offset and there is no guaranteed the result is a power
@@ -2470,9 +2559,6 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
return rc;
}
- if (!pnv_iommu_bypass_disabled)
- pnv_pci_ioda2_set_bypass(pe, true);
-
/*
* Set table base for the case of IOMMU DMA use. Usually this is done
* from dma_dev_setup() which is not called when a device is returned
@@ -2624,8 +2710,6 @@ static void pnv_ioda_setup_bus_iommu_group(struct pnv_ioda_pe *pe,
bus);
}
-static unsigned long pnv_ioda_parse_tce_sizes(struct pnv_phb *phb);
-
static void pnv_pci_ioda_setup_iommu_api(void)
{
struct pci_controller *hose;
--
2.17.1
^ permalink raw reply related
* [PATCH kernel v2 2/7] powerpc/powernv/ioda: Rework for huge DMA window at 4GB
From: Alexey Kardashevskiy @ 2020-03-23 7:53 UTC (permalink / raw)
To: linuxppc-dev
Cc: kvm, Fabiano Rosas, Alexey Kardashevskiy, Alistair Popple,
kvm-ppc, David Gibson
In-Reply-To: <20200323075354.93825-1-aik@ozlabs.ru>
This moves code to make the next patches look simpler. In particular:
1. Separate locals declaration as we will be allocating a smaller DMA
window if a TVE1_4GB option (allows a huge DMA windows at 4GB) is enabled;
2. Pass the bypass offset directly to pnv_pci_ioda2_create_table()
as it is the only information needed from @pe;
3. Use PAGE_SHIFT for it_map allocation estimate and @tceshift for
the IOMMU page size; this makes the distinction clear and allows
easy switching between different IOMMU page size.
These changes should not cause behavioral change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
I really need 1), 2) makes the code less dependent on the PE struct member
value (==easier to follow), 3) is to enable 2MB quickly for the default
DMA window for debugging/performance testing.
---
arch/powerpc/platforms/powernv/pci-ioda.c | 38 ++++++++++++-----------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 52db10ab4fef..f5f1b4e25530 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2375,15 +2375,10 @@ static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
pe->tce_bypass_enabled = enable;
}
-static long pnv_pci_ioda2_create_table(struct iommu_table_group *table_group,
- int num, __u32 page_shift, __u64 window_size, __u32 levels,
+static long pnv_pci_ioda2_create_table(int nid, int num, __u64 bus_offset,
+ __u32 page_shift, __u64 window_size, __u32 levels,
bool alloc_userspace_copy, struct iommu_table **ptbl)
{
- struct pnv_ioda_pe *pe = container_of(table_group, struct pnv_ioda_pe,
- table_group);
- int nid = pe->phb->hose->node;
- __u64 bus_offset = num ?
- pe->table_group.tce64_start : table_group->tce32_start;
long ret;
struct iommu_table *tbl;
@@ -2410,21 +2405,23 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
{
struct iommu_table *tbl = NULL;
long rc;
- unsigned long res_start, res_end;
+ u64 max_memory, maxblock, window_size;
+ const unsigned int tceshift = PAGE_SHIFT;
+ unsigned long res_start, res_end, tces_order, tcelevel_order, levels;
/*
* crashkernel= specifies the kdump kernel's maximum memory at
* some offset and there is no guaranteed the result is a power
* of 2, which will cause errors later.
*/
- const u64 max_memory = __rounddown_pow_of_two(memory_hotplug_max());
+ max_memory = __rounddown_pow_of_two(memory_hotplug_max());
/*
* In memory constrained environments, e.g. kdump kernel, the
* DMA window can be larger than available memory, which will
* cause errors later.
*/
- const u64 maxblock = 1UL << (PAGE_SHIFT + MAX_ORDER - 1);
+ maxblock = 1UL << (PAGE_SHIFT + MAX_ORDER - 1);
/*
* We create the default window as big as we can. The constraint is
@@ -2434,11 +2431,11 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
* to support crippled devices (i.e. not fully 64bit DMAble) only.
*/
/* iommu_table::it_map uses 1 bit per IOMMU page, hence 8 */
- const u64 window_size = min((maxblock * 8) << PAGE_SHIFT, max_memory);
+ window_size = min((maxblock * 8) << tceshift, max_memory);
/* Each TCE level cannot exceed maxblock so go multilevel if needed */
- unsigned long tces_order = ilog2(window_size >> PAGE_SHIFT);
- unsigned long tcelevel_order = ilog2(maxblock >> 3);
- unsigned int levels = tces_order / tcelevel_order;
+ tces_order = ilog2(window_size >> tceshift);
+ tcelevel_order = ilog2(maxblock >> 3);
+ levels = tces_order / tcelevel_order;
if (tces_order % tcelevel_order)
levels += 1;
@@ -2448,8 +2445,8 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
*/
levels = max_t(unsigned int, levels, POWERNV_IOMMU_DEFAULT_LEVELS);
- rc = pnv_pci_ioda2_create_table(&pe->table_group, 0, PAGE_SHIFT,
- window_size, levels, false, &tbl);
+ rc = pnv_pci_ioda2_create_table(pe->phb->hose->node,
+ 0, 0, tceshift, window_size, levels, false, &tbl);
if (rc) {
pe_err(pe, "Failed to create 32-bit TCE table, err %ld",
rc);
@@ -2551,8 +2548,13 @@ static long pnv_pci_ioda2_create_table_userspace(
int num, __u32 page_shift, __u64 window_size, __u32 levels,
struct iommu_table **ptbl)
{
- long ret = pnv_pci_ioda2_create_table(table_group,
- num, page_shift, window_size, levels, true, ptbl);
+ struct pnv_ioda_pe *pe = container_of(table_group, struct pnv_ioda_pe,
+ table_group);
+ __u64 bus_offset = num ?
+ pe->table_group.tce64_start : table_group->tce32_start;
+ long ret = pnv_pci_ioda2_create_table(pe->phb->hose->node,
+ num, bus_offset, page_shift, window_size, levels, true,
+ ptbl);
if (!ret)
(*ptbl)->it_allocated_size = pnv_pci_ioda2_get_table_size(
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v4 3/9] powerpc/vas: Add VAS user space API
From: Daniel Axtens @ 2020-03-23 7:43 UTC (permalink / raw)
To: Haren Myneni, herbert; +Cc: mikey, npiggin, linux-crypto, sukadev, linuxppc-dev
In-Reply-To: <1584936142.9256.15325.camel@hbabu-laptop>
Haren Myneni <haren@linux.ibm.com> writes:
> On power9, userspace can send GZIP compression requests directly to NX
> once kernel establishes NX channel / window with VAS. This patch provides
> user space API which allows user space to establish channel using open
> VAS_TX_WIN_OPEN ioctl, mmap and close operations.
>
> Each window corresponds to file descriptor and application can open
> multiple windows. After the window is opened, VAS_TX_WIN_OPEN icoctl to
> open a window on specific VAS instance, mmap() system call to map
> the hardware address of engine's request queue into the application's
> virtual address space.
>
> Then the application can then submit one or more requests to the the
> engine by using the copy/paste instructions and pasting the CRBs to
> the virtual address (aka paste_address) returned by mmap().
>
> Only NX GZIP coprocessor type is supported right now and allow GZIP
> engine access via /dev/crypto/nx-gzip device node.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> arch/powerpc/include/asm/vas.h | 11 ++
> arch/powerpc/platforms/powernv/Makefile | 2 +-
> arch/powerpc/platforms/powernv/vas-api.c | 257 ++++++++++++++++++++++++++++
> arch/powerpc/platforms/powernv/vas-window.c | 6 +-
> arch/powerpc/platforms/powernv/vas.h | 2 +
> 5 files changed, 274 insertions(+), 4 deletions(-)
> create mode 100644 arch/powerpc/platforms/powernv/vas-api.c
>
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index f93e6b0..e064953 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -163,4 +163,15 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
> */
> int vas_paste_crb(struct vas_window *win, int offset, bool re);
>
> +/*
> + * Register / unregister coprocessor type to VAS API which will be exported
> + * to user space. Applications can use this API to open / close window
> + * which can be used to send / receive requests directly to cooprcessor.
> + *
> + * Only NX GZIP coprocessor type is supported now, but this API can be
> + * used for others in future.
> + */
> +int vas_register_coproc_api(struct module *mod);
> +void vas_unregister_coproc_api(void);
> +
> #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index 395789f..fe3f0fb 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -17,7 +17,7 @@ obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o
> obj-$(CONFIG_OPAL_PRD) += opal-prd.o
> obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
> obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
> -obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o vas-fault.o
> +obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o vas-fault.o vas-api.o
> obj-$(CONFIG_OCXL_BASE) += ocxl.o
> obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
> obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
> diff --git a/arch/powerpc/platforms/powernv/vas-api.c b/arch/powerpc/platforms/powernv/vas-api.c
> new file mode 100644
> index 0000000..7d049af
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/vas-api.c
> @@ -0,0 +1,257 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * VAS user space API for its accelerators (Only NX-GZIP is supported now)
> + * Copyright (C) 2019 Haren Myneni, IBM Corp
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <asm/vas.h>
> +#include <uapi/asm/vas-api.h>
> +#include "vas.h"
> +
> +/*
> + * The driver creates the device node that can be used as follows:
> + * For NX-GZIP
> + *
> + * fd = open("/dev/crypto/nx-gzip", O_RDWR);
> + * rc = ioctl(fd, VAS_TX_WIN_OPEN, &attr);
> + * paste_addr = mmap(NULL, PAGE_SIZE, prot, MAP_SHARED, fd, 0ULL).
> + * vas_copy(&crb, 0, 1);
> + * vas_paste(paste_addr, 0, 1);
> + * close(fd) or exit process to close window.
> + *
> + * where "vas_copy" and "vas_paste" are defined in copy-paste.h.
> + * copy/paste returns to the user space directly. So refer NX hardware
> + * documententation for exact copy/paste usage and completion / error
> + * conditions.
> + */
> +
> +static char *coproc_dev_name = "nx-gzip";
> +
> +/*
> + * Wrapper object for the nx-gzip device - there is just one instance of
> + * this node for the whole system.
> + */
> +static struct coproc_dev {
> + struct cdev cdev;
> + struct device *device;
> + char *name;
> + dev_t devt;
> + struct class *class;
> +} coproc_device;
> +
> +static char *coproc_devnode(struct device *dev, umode_t *mode)
> +{
> + return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));
> +}
> +
> +static int coproc_open(struct inode *inode, struct file *fp)
> +{
> + /*
> + * vas_window is allocated and assigned to fp->private_data
> + * in ioctl. Nothing to do here for NX GZIP.
> + */
> + return 0;
> +}
> +
> +static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
> +{
> + void __user *uptr = (void __user *)arg;
> + struct vas_tx_win_attr txattr = {};
> + struct vas_tx_win_open_attr uattr;
> + struct vas_window *txwin;
> + int rc, vasid;
> +
> + /*
> + * One window for file descriptor
> + */
> + if (fp->private_data)
> + return -EEXIST;
> +
> + rc = copy_from_user(&uattr, uptr, sizeof(uattr));
> + if (rc) {
> + pr_err("%s(): copy_from_user() returns %d\n", __func__, rc);
> + return -EFAULT;
> + }
> +
> + if (uattr.version != 1) {
> + pr_err("Invalid version\n");
> + return -EINVAL;
> + }
> +
> + vasid = uattr.vas_id;
> +
> + vas_init_tx_win_attr(&txattr, VAS_COP_TYPE_GZIP);
> +
> + txattr.lpid = mfspr(SPRN_LPID);
> + txattr.pidr = mfspr(SPRN_PID);
> + txattr.user_win = true;
> + txattr.rsvd_txbuf_count = false;
> + txattr.pswid = false;
> +
> + pr_devel("Pid %d: Opening txwin, PIDR %ld\n", txattr.pidr,
> + mfspr(SPRN_PID));
> +
> + txwin = vas_tx_win_open(vasid, VAS_COP_TYPE_GZIP, &txattr);
> + if (IS_ERR(txwin)) {
> + pr_err("%s() vas_tx_win_open() failed, %ld\n", __func__,
> + PTR_ERR(txwin));
> + return PTR_ERR(txwin);
> + }
> +
> + fp->private_data = txwin;
> +
> + return 0;
> +}
> +
> +static int coproc_release(struct inode *inode, struct file *fp)
> +{
> + struct vas_window *txwin = fp->private_data;
> +
> + if (txwin) {
> + vas_win_close(txwin);
> + fp->private_data = NULL;
> + }
> +
> + /*
> + * We don't know here if user has other receive windows
> + * open, so we can't really call clear_thread_tidr().
> + * So, once the process calls set_thread_tidr(), the
> + * TIDR value sticks around until process exits, resulting
> + * in an extra copy in restore_sprs().
> + */
> +
> + return 0;
> +}
> +
> +static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
> +{
> + struct vas_window *txwin = fp->private_data;
> + unsigned long pfn;
> + u64 paste_addr;
> + pgprot_t prot;
> + int rc;
> +
> + if ((vma->vm_end - vma->vm_start) > PAGE_SIZE) {
I think you said this should be 4096 rather than 64k, regardless of what
PAGE_SIZE you are compiled with?
> + pr_debug("%s(): size 0x%zx, PAGE_SIZE 0x%zx\n", __func__,
> + (vma->vm_end - vma->vm_start), PAGE_SIZE);
> + return -EINVAL;
> + }
> +
> + /* Ensure instance has an open send window */
> + if (!txwin) {
> + pr_err("%s(): No send window open?\n", __func__);
> + return -EINVAL;
> + }
> +
> + vas_win_paste_addr(txwin, &paste_addr, NULL);
> + pfn = paste_addr >> PAGE_SHIFT;
> +
> + /* flags, page_prot from cxl_mmap(), except we want cachable */
> + vma->vm_flags |= VM_IO | VM_PFNMAP;
> + vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> + prot = __pgprot(pgprot_val(vma->vm_page_prot) | _PAGE_DIRTY);
> +
> + rc = remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
> + vma->vm_end - vma->vm_start, prot);
> +
> + pr_devel("%s(): paste addr %llx at %lx, rc %d\n", __func__,
> + paste_addr, vma->vm_start, rc);
> +
> + return rc;
> +}
> +
> +static long coproc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> + switch (cmd) {
> + case VAS_TX_WIN_OPEN:
> + return coproc_ioc_tx_win_open(fp, arg);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static struct file_operations coproc_fops = {
> + .open = coproc_open,
> + .release = coproc_release,
> + .mmap = coproc_mmap,
> + .unlocked_ioctl = coproc_ioctl,
> +};
> +
> +/*
> + * Supporting only nx-gzip coprocessor type now, but this API code
> + * extended to other coprocessor types later.
> + */
> +int vas_register_coproc_api(struct module *mod)
> +{
> + int rc = -EINVAL;
> + dev_t devno;
> +
> + rc = alloc_chrdev_region(&coproc_device.devt, 1, 1, coproc_dev_name);
> + if (rc) {
> + pr_err("Unable to allocate coproc major number: %i\n", rc);
> + return rc;
> + }
> +
> + pr_devel("%s device allocated, dev [%i,%i]\n", coproc_dev_name,
> + MAJOR(coproc_device.devt), MINOR(coproc_device.devt));
> +
> + coproc_device.class = class_create(mod, coproc_dev_name);
> + if (IS_ERR(coproc_device.class)) {
> + rc = PTR_ERR(coproc_device.class);
> + pr_err("Unable to create %s class %d\n", coproc_dev_name, rc);
> + goto err_class;
> + }
> + coproc_device.class->devnode = coproc_devnode;
> +
> + coproc_fops.owner = mod;
> + cdev_init(&coproc_device.cdev, &coproc_fops);
> +
> + devno = MKDEV(MAJOR(coproc_device.devt), 0);
> + rc = cdev_add(&coproc_device.cdev, devno, 1);
> + if (rc) {
> + pr_err("cdev_add() failed %d\n", rc);
> + goto err_cdev;
> + }
> +
> + coproc_device.device = device_create(coproc_device.class, NULL,
> + devno, NULL, coproc_dev_name, MINOR(devno));
> + if (IS_ERR(coproc_device.device)) {
> + rc = PTR_ERR(coproc_device.device);
> + pr_err("Unable to create coproc-%d %d\n", MINOR(devno), rc);
> + goto err;
> + }
> +
> + pr_devel("%s: Added dev [%d,%d]\n", __func__, MAJOR(devno),
> + MINOR(devno));
> +
> + return 0;
> +
> +err:
> + cdev_del(&coproc_device.cdev);
> +err_cdev:
> + class_destroy(coproc_device.class);
> +err_class:
> + unregister_chrdev_region(coproc_device.devt, 1);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(vas_register_coproc_api);
> +
> +void vas_unregister_coproc_api(void)
> +{
> + dev_t devno;
> +
> + cdev_del(&coproc_device.cdev);
> + devno = MKDEV(MAJOR(coproc_device.devt), 0);
> + device_destroy(coproc_device.class, devno);
> +
> + class_destroy(coproc_device.class);
> + unregister_chrdev_region(coproc_device.devt, 1);
> +}
> +EXPORT_SYMBOL_GPL(vas_unregister_coproc_api);
> diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
> index 4ce421f..7b5187b 100644
> --- a/arch/powerpc/platforms/powernv/vas-window.c
> +++ b/arch/powerpc/platforms/powernv/vas-window.c
> @@ -26,7 +26,7 @@
> * Compute the paste address region for the window @window using the
> * ->paste_base_addr and ->paste_win_id_shift we got from device tree.
> */
> -static void compute_paste_address(struct vas_window *window, u64 *addr, int *len)
> +void vas_win_paste_addr(struct vas_window *window, u64 *addr, int *len)
> {
> int winid;
> u64 base, shift;
> @@ -80,7 +80,7 @@ static void *map_paste_region(struct vas_window *txwin)
> goto free_name;
>
> txwin->paste_addr_name = name;
> - compute_paste_address(txwin, &start, &len);
> + vas_win_paste_addr(txwin, &start, &len);
>
> if (!request_mem_region(start, len, name)) {
> pr_devel("%s(): request_mem_region(0x%llx, %d) failed\n",
> @@ -138,7 +138,7 @@ static void unmap_paste_region(struct vas_window *window)
> u64 busaddr_start;
>
> if (window->paste_kaddr) {
> - compute_paste_address(window, &busaddr_start, &len);
> + vas_win_paste_addr(window, &busaddr_start, &len);
> unmap_region(window->paste_kaddr, busaddr_start, len);
> window->paste_kaddr = NULL;
> kfree(window->paste_addr_name);
> diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
> index 8c39a7d..a10abed 100644
> --- a/arch/powerpc/platforms/powernv/vas.h
> +++ b/arch/powerpc/platforms/powernv/vas.h
> @@ -431,6 +431,8 @@ struct vas_winctx {
> extern void vas_return_credit(struct vas_window *window, bool tx);
> extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
> uint32_t pswid);
> +extern void vas_win_paste_addr(struct vas_window *window, u64 *addr,
> + int *len);
>
> static inline int vas_window_pid(struct vas_window *window)
> {
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH v4 14/16] powerpc64: Add prefixed instructions to instruction data type
From: Nicholas Piggin @ 2020-03-23 7:33 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-15-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:18 pm:
> For powerpc64, redefine the ppc_inst type so both word and prefixed
> instructions can be represented. On powerpc32 the type will remain the
> same. Update places which had assumed instructions to be 4 bytes long.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/include/asm/code-patching.h | 10 +--
> arch/powerpc/include/asm/inst.h | 90 ++++++++++++++++++++++++
> arch/powerpc/include/asm/kprobes.h | 2 +-
> arch/powerpc/include/asm/sstep.h | 4 ++
> arch/powerpc/include/asm/uaccess.h | 22 ++++++
> arch/powerpc/include/asm/uprobes.h | 2 +-
> arch/powerpc/kernel/align.c | 5 +-
> arch/powerpc/kernel/hw_breakpoint.c | 2 +-
> arch/powerpc/kernel/kprobes.c | 7 +-
> arch/powerpc/kernel/optprobes.c | 42 ++++++-----
> arch/powerpc/kernel/optprobes_head.S | 3 +
> arch/powerpc/kernel/trace/ftrace.c | 19 ++++-
> arch/powerpc/kernel/uprobes.c | 2 +-
> arch/powerpc/lib/code-patching.c | 22 ++++--
> arch/powerpc/lib/sstep.c | 4 +-
> arch/powerpc/xmon/xmon.c | 38 +++++++---
> 16 files changed, 221 insertions(+), 53 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index 68bd9db334bd..bd41e1558707 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -25,11 +25,11 @@
> bool is_offset_in_branch_range(long offset);
> ppc_inst create_branch(const ppc_inst *addr,
> unsigned long target, int flags);
> -unsigned int create_cond_branch(const ppc_inst *addr,
> +ppc_inst create_cond_branch(const void *addr,
> unsigned long target, int flags);
> -int patch_branch(ppc_inst *addr, unsigned long target, int flags);
> -int patch_instruction(ppc_inst *addr, ppc_inst instr);
> -int raw_patch_instruction(ppc_inst *addr, ppc_inst instr);
> +int patch_branch(void *addr, unsigned long target, int flags);
> +int patch_instruction(void *addr, ppc_inst instr);
> +int raw_patch_instruction(void *addr, ppc_inst instr);
>
> static inline unsigned long patch_site_addr(s32 *site)
> {
> @@ -60,7 +60,7 @@ static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned
> int instr_is_relative_branch(ppc_inst instr);
> int instr_is_relative_link_branch(ppc_inst instr);
> int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr);
> -unsigned long branch_target(const ppc_inst *instr);
> +unsigned long branch_target(const void *instr);
> ppc_inst translate_branch(const ppc_inst *dest,
> const ppc_inst *src);
> extern bool is_conditional_branch(ppc_inst instr);
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 7c8596ee411e..1a40b0a71128 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -6,6 +6,95 @@
> * Instruction data type for POWER
> */
>
> +#ifdef __powerpc64__
> +
> +typedef struct ppc_inst {
> + union {
> + struct {
> + u32 word;
> + u32 pad;
> + } __packed;
> + struct {
> + u32 prefix;
> + u32 suffix;
> + } __packed;
> + };
> +} ppc_inst;
> +
> +#define PPC_INST(x) ((ppc_inst) { .word = (x), .pad = 0 })
> +#define PPC_INST_PREFIXED(x, y) ((ppc_inst) { .prefix = (x), .suffix = (y) })
> +
> +static inline int ppc_inst_opcode(ppc_inst x)
> +{
> + return x.word >> 26;
> +}
> +
> +static inline bool ppc_inst_prefixed(ppc_inst x) {
> + return ppc_inst_opcode(x) == 1;
> +}
> +
> +static inline int ppc_inst_len(ppc_inst x)
> +{
> + if (ppc_inst_prefixed(x))
> + return 8;
> + else
> + return 4;
> +}
> +
> +static inline u32 ppc_inst_word(ppc_inst x)
> +{
> + return x.word;
> +}
I guess a concern could be that code using ppc_inst_word could now get a
prefix unexpectedly and not handle it properly. The reason it should
generally be okay is that prefix won't match any existing valid
instruction words, so callers won't match or think it's an unknown
instruction. Am I right? Possibly a small comment?
> +
> +static inline u32 ppc_inst_prefix(ppc_inst x)
> +{
> + return x.prefix;
> +}
> +
> +static inline u32 ppc_inst_suffix(ppc_inst x)
> +{
> + return x.suffix;
> +}
> +
> +
> +static inline ppc_inst ppc_inst_read(const void *ptr)
> +{
> + ppc_inst inst;
> + inst.word = *(u32 *)ptr;
> + if (ppc_inst_prefixed(inst))
> + inst.suffix = *((u32 *)ptr + 1);
> + else
> + inst.pad = 0;
I'm a bit against using partially constructed opaque type for things
like this, even if it is in the code that knows about the type. We
could modify ppc_inst_prefixed() to assert that pad is equal to zero
(or some poisoned value) if it's not prefixed. Or do some validation
on the suffix if it is.
> +static inline bool ppc_inst_equal(ppc_inst x, ppc_inst y)
> +{
> + return !memcmp(&x, &y, sizeof(struct ppc_inst));
> +}
I guess a variable length memcmp will make terrible code, so you're
requiring pad to equal 0 to match non-prefixed. Fine.
> +
> +static inline bool ppc_inst_null(ppc_inst x)
> +{
> + return x.word == 0 && x.pad == 0;
> +}
In this case you shouldn't need x.pad == 0. If x.word == 0, then
WARN_ON_ONCE(x.pad != 0) ?
> if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
> /* We don't handle PPC little-endian any more... */
> if (cpu_has_feature(CPU_FTR_PPC_LE))
> return -EIO;
> - instr = PPC_INST(swab32(ppc_inst_word(instr)));
> + instr = PPC_INST_PREFIXED(swab32(ppc_inst_word(instr)),
> + swab32(ppc_inst_suffix(instr)));
Ugly, don't suppose you'd bother to do a ppc_inst_bswap function for
this one case?
[snip probes stuff]
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index fa7f32adf029..3b8277a64b8f 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -24,17 +24,27 @@ static int __patch_instruction(ppc_inst *exec_addr, ppc_inst instr,
> {
> int err = 0;
>
> - __put_user_asm(instr, patch_addr, err, "stw");
> + __put_user_asm(ppc_inst_word(instr), patch_addr, err, "stw");
> if (err)
> return err;
>
> asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
> "r" (exec_addr));
>
> + if (!ppc_inst_prefixed(instr))
> + return 0;
> +
> + __put_user_asm(ppc_inst_suffix(instr), patch_addr + 4, err, "stw");
> + if (err)
> + return err;
> +
> + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr + 4),
> + "r" (exec_addr + 4));
Although there's proably no real performance or atomicity issues here,
I'd be pleased if we could do a case for prefixed and a case for non
prefixed, and store the non-prefixed with "std". Just for the principle
of not having half-written instructions in the image.
You could skip the dcbst and icbi for the second address if you happen
to know this future CPU does not store prefix insns across a CL
boundary. But probably not necessary to make that assumption in non
perf critical code here, so I'd leave it as you have.
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index ee084411f2f5..c5536e1a3356 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -110,7 +110,7 @@ struct bpt {
> #define BP_DABR 4
>
> #define NBPTS 256
> -#define BPT_WORDS 2
> +#define BPT_WORDS 4
(2 * sizeof(ppc_inst) / sizeof(u32)) ?
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -118,12 +118,13 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> -static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS];
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS] __aligned(64);
Should have a define somewhere for this magical 64.
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> static int mwrite(unsigned long, void *, int);
> +static int mread_instr(unsigned long, ppc_inst *);
In some cases you've addd helpers like this as separate patches,
others you've bundled them together. NBD but I liked the prep patches
which then made the more important changes easier to see.
> @@ -759,8 +760,8 @@ static int xmon_bpt(struct pt_regs *regs)
>
> /* Are we at the trap at bp->instr[1] for some bp? */
> bp = in_breakpoint_table(regs->nip, &offset);
> - if (bp != NULL && offset == 4) {
> - regs->nip = bp->address + 4;
> + if (bp != NULL && (offset == 4 || offset == 8)) {
> + regs->nip = bp->address + offset;
> atomic_dec(&bp->ref_count);
> return 1;
> }
> @@ -862,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
> if (off >= sizeof(bpt_table))
> return NULL;
> bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> - if (bp_off != 0 && bp_off != 4)
> + if (bp_off != 0 && bp_off != 4 && bp_off != 8)
> return NULL;
> *offp = bp_off;
> return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
> @@ -881,7 +882,6 @@ static struct bpt *new_breakpoint(unsigned long a)
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> - patch_instruction(bp->instr + 1, PPC_INST(bpinstr));
> return bp;
> }
> }
Why is this okay to remove?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 13/16] powerpc: Support prefixed instructions in alignment handler
From: Nicholas Piggin @ 2020-03-23 7:05 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-14-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:18 pm:
> Alignment interrupts can be caused by prefixed instructions accessing
> memory. Prefixed instructions are not permitted to cross 64-byte
> boundaries. If they do the alignment interrupt is invoked with SRR1
> BOUNDARY bit set. If this occurs send a SIGBUS to the offending process
> if in user mode. If in kernel mode call bad_page_fault().
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v2: - Move __get_user_instr() and __get_user_instr_inatomic() to this
> commit (previously in "powerpc sstep: Prepare to support prefixed
> instructions").
> - Rename sufx to suffix
> - Use a macro for calculating instruction length
> v3: Move __get_user_{instr(), instr_inatomic()} up with the other
> get_user definitions and remove nested if.
> v4: Just do the things for alignment_exception(). Other changes handled
> elsewhere.
> ---
> arch/powerpc/kernel/traps.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index a4764b039749..cd8b3043c268 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -583,6 +583,10 @@ static inline int check_io_access(struct pt_regs *regs)
> #define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
> #define REASON_PRIVILEGED ESR_PPR
> #define REASON_TRAP ESR_PTR
> +#define REASON_PREFIXED 0
> +#define REASON_BOUNDARY 0
> +
> +#define inst_length(reason) 4
>
> /* single-step stuff */
> #define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)
> @@ -597,6 +601,10 @@ static inline int check_io_access(struct pt_regs *regs)
> #define REASON_ILLEGAL SRR1_PROGILL
> #define REASON_PRIVILEGED SRR1_PROGPRIV
> #define REASON_TRAP SRR1_PROGTRAP
> +#define REASON_PREFIXED SRR1_PREFIXED
> +#define REASON_BOUNDARY SRR1_BOUNDARY
> +
> +#define inst_length(reason) (((reason) & REASON_PREFIXED) ? 8 : 4)
Looks good. If you define REASON_BOUNDARY 0, then this will constant
fold away so no need to define it twice.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 12/16] powerpc: Define new SRR1 bits for a future ISA version
From: Nicholas Piggin @ 2020-03-23 7:03 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-13-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:18 pm:
> Add the BOUNDARY SRR1 bit definition for when the cause of an alignment
> exception is a prefixed instruction that crosses a 64-byte boundary.
> Add the PREFIXED SRR1 bit definition for exceptions caused by prefixed
> instructions.
>
> Bit 35 of SRR1 is called SRR1_ISI_N_OR_G. This name comes from it being
> used to indicate that an ISI was due to the access being no-exec or
> guarded. A future ISA version adds another purpose. It is also set if
> there is an access in a cache-inhibited location for prefixed
> instruction. Rename from SRR1_ISI_N_OR_G to SRR1_ISI_N_G_OR_CIP.
Seems okay.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v2: Combined all the commits concerning SRR1 bits.
> ---
> arch/powerpc/include/asm/reg.h | 4 +++-
> arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index c7758c2ccc5f..173f33df4fab 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -762,7 +762,7 @@
> #endif
>
> #define SRR1_ISI_NOPT 0x40000000 /* ISI: Not found in hash */
> -#define SRR1_ISI_N_OR_G 0x10000000 /* ISI: Access is no-exec or G */
> +#define SRR1_ISI_N_G_OR_CIP 0x10000000 /* ISI: Access is no-exec or G or CI for a prefixed instruction */
> #define SRR1_ISI_PROT 0x08000000 /* ISI: Other protection fault */
> #define SRR1_WAKEMASK 0x00380000 /* reason for wakeup */
> #define SRR1_WAKEMASK_P8 0x003c0000 /* reason for wakeup on POWER8 and 9 */
> @@ -789,6 +789,8 @@
> #define SRR1_PROGADDR 0x00010000 /* SRR0 contains subsequent addr */
>
> #define SRR1_MCE_MCP 0x00080000 /* Machine check signal caused interrupt */
> +#define SRR1_BOUNDARY 0x10000000 /* Prefixed instruction crosses 64-byte boundary */
> +#define SRR1_PREFIXED 0x20000000 /* Exception caused by prefixed instruction */
>
> #define SPRN_HSRR0 0x13A /* Save/Restore Register 0 */
> #define SPRN_HSRR1 0x13B /* Save/Restore Register 1 */
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index dc97e5be76f6..6ab685227574 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -1169,7 +1169,7 @@ static int kvmhv_translate_addr_nested(struct kvm_vcpu *vcpu,
> } else if (vcpu->arch.trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
> /* Can we execute? */
> if (!gpte_p->may_execute) {
> - flags |= SRR1_ISI_N_OR_G;
> + flags |= SRR1_ISI_N_G_OR_CIP;
> goto forward_to_l1;
> }
> } else {
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> index 220305454c23..b53a9f1c1a46 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> @@ -1260,7 +1260,7 @@ long kvmppc_hpte_hv_fault(struct kvm_vcpu *vcpu, unsigned long addr,
> status &= ~DSISR_NOHPTE; /* DSISR_NOHPTE == SRR1_ISI_NOPT */
> if (!data) {
> if (gr & (HPTE_R_N | HPTE_R_G))
> - return status | SRR1_ISI_N_OR_G;
> + return status | SRR1_ISI_N_G_OR_CIP;
> if (!hpte_read_permission(pp, slb_v & key))
> return status | SRR1_ISI_PROT;
> } else if (status & DSISR_ISSTORE) {
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH v4 11/16] powerpc: Enable Prefixed Instructions
From: Nicholas Piggin @ 2020-03-23 7:02 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-12-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:18 pm:
> From: Alistair Popple <alistair@popple.id.au>
>
> Prefix instructions have their own FSCR bit which needs to enabled via
> a CPU feature. The kernel will save the FSCR for problem state but it
> needs to be enabled initially.
>
> If prefixed instructions are made unavailable by the [H]FSCR, attempting
> to use them will cause a facility unavailable exception. Add "PREFIX" to
> the facility_strings[].
>
> Currently there are no prefixed instructions that are actually emulated
> by emulate_instruction() within facility_unavailable_exception().
> However, when caused by a prefixed instructions the SRR1 PREFIXED bit is
> set. Prepare for dealing with emulated prefixed instructions by checking
> for this bit.
>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4:
> - Squash "Check for prefixed instructions in
> facility_unavailable_exception()" here
> - Remove dt parts for now
> ---
> arch/powerpc/include/asm/reg.h | 3 +++
> arch/powerpc/kernel/traps.c | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1aa46dff0957..c7758c2ccc5f 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -397,6 +397,7 @@
> #define SPRN_RWMR 0x375 /* Region-Weighting Mode Register */
>
> /* HFSCR and FSCR bit numbers are the same */
> +#define FSCR_PREFIX_LG 13 /* Enable Prefix Instructions */
> #define FSCR_SCV_LG 12 /* Enable System Call Vectored */
> #define FSCR_MSGP_LG 10 /* Enable MSGP */
> #define FSCR_TAR_LG 8 /* Enable Target Address Register */
> @@ -408,11 +409,13 @@
> #define FSCR_VECVSX_LG 1 /* Enable VMX/VSX */
> #define FSCR_FP_LG 0 /* Enable Floating Point */
> #define SPRN_FSCR 0x099 /* Facility Status & Control Register */
> +#define FSCR_PREFIX __MASK(FSCR_PREFIX_LG)
> #define FSCR_SCV __MASK(FSCR_SCV_LG)
> #define FSCR_TAR __MASK(FSCR_TAR_LG)
> #define FSCR_EBB __MASK(FSCR_EBB_LG)
> #define FSCR_DSCR __MASK(FSCR_DSCR_LG)
> #define SPRN_HFSCR 0xbe /* HV=1 Facility Status & Control Register */
> +#define HFSCR_PREFIX __MASK(FSCR_PREFIX_LG)
> #define HFSCR_MSGP __MASK(FSCR_MSGP_LG)
> #define HFSCR_TAR __MASK(FSCR_TAR_LG)
> #define HFSCR_EBB __MASK(FSCR_EBB_LG)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 82a3438300fd..a4764b039749 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1720,6 +1720,7 @@ void facility_unavailable_exception(struct pt_regs *regs)
> [FSCR_TAR_LG] = "TAR",
> [FSCR_MSGP_LG] = "MSGP",
> [FSCR_SCV_LG] = "SCV",
> + [FSCR_PREFIX_LG] = "PREFIX",
> };
> char *facility = "unknown";
> u64 value;
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH v4 14/16] powerpc64: Add prefixed instructions to instruction data type
From: Nicholas Piggin @ 2020-03-23 6:58 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-15-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:18 pm:
> For powerpc64, redefine the ppc_inst type so both word and prefixed
> instructions can be represented. On powerpc32 the type will remain the
> same. Update places which had assumed instructions to be 4 bytes long.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/include/asm/code-patching.h | 10 +--
> arch/powerpc/include/asm/inst.h | 90 ++++++++++++++++++++++++
> arch/powerpc/include/asm/kprobes.h | 2 +-
> arch/powerpc/include/asm/sstep.h | 4 ++
> arch/powerpc/include/asm/uaccess.h | 22 ++++++
> arch/powerpc/include/asm/uprobes.h | 2 +-
> arch/powerpc/kernel/align.c | 5 +-
> arch/powerpc/kernel/hw_breakpoint.c | 2 +-
> arch/powerpc/kernel/kprobes.c | 7 +-
> arch/powerpc/kernel/optprobes.c | 42 ++++++-----
> arch/powerpc/kernel/optprobes_head.S | 3 +
> arch/powerpc/kernel/trace/ftrace.c | 19 ++++-
> arch/powerpc/kernel/uprobes.c | 2 +-
> arch/powerpc/lib/code-patching.c | 22 ++++--
> arch/powerpc/lib/sstep.c | 4 +-
> arch/powerpc/xmon/xmon.c | 38 +++++++---
> 16 files changed, 221 insertions(+), 53 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index 68bd9db334bd..bd41e1558707 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -25,11 +25,11 @@
> bool is_offset_in_branch_range(long offset);
> ppc_inst create_branch(const ppc_inst *addr,
> unsigned long target, int flags);
> -unsigned int create_cond_branch(const ppc_inst *addr,
> +ppc_inst create_cond_branch(const void *addr,
> unsigned long target, int flags);
> -int patch_branch(ppc_inst *addr, unsigned long target, int flags);
> -int patch_instruction(ppc_inst *addr, ppc_inst instr);
> -int raw_patch_instruction(ppc_inst *addr, ppc_inst instr);
> +int patch_branch(void *addr, unsigned long target, int flags);
> +int patch_instruction(void *addr, ppc_inst instr);
> +int raw_patch_instruction(void *addr, ppc_inst instr);
>
> static inline unsigned long patch_site_addr(s32 *site)
> {
> @@ -60,7 +60,7 @@ static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned
> int instr_is_relative_branch(ppc_inst instr);
> int instr_is_relative_link_branch(ppc_inst instr);
> int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr);
> -unsigned long branch_target(const ppc_inst *instr);
> +unsigned long branch_target(const void *instr);
> ppc_inst translate_branch(const ppc_inst *dest,
> const ppc_inst *src);
> extern bool is_conditional_branch(ppc_inst instr);
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 7c8596ee411e..1a40b0a71128 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -6,6 +6,95 @@
> * Instruction data type for POWER
> */
>
> +#ifdef __powerpc64__
> +
> +typedef struct ppc_inst {
> + union {
> + struct {
> + u32 word;
> + u32 pad;
> + } __packed;
> + struct {
> + u32 prefix;
> + u32 suffix;
> + } __packed;
> + };
> +} ppc_inst;
> +
> +#define PPC_INST(x) ((ppc_inst) { .word = (x), .pad = 0 })
> +#define PPC_INST_PREFIXED(x, y) ((ppc_inst) { .prefix = (x), .suffix = (y) })
> +
> +static inline int ppc_inst_opcode(ppc_inst x)
> +{
> + return x.word >> 26;
> +}
> +
> +static inline bool ppc_inst_prefixed(ppc_inst x) {
> + return ppc_inst_opcode(x) == 1;
> +}
> +
> +static inline int ppc_inst_len(ppc_inst x)
> +{
> + if (ppc_inst_prefixed(x))
> + return 8;
> + else
> + return 4;
> +}
> +
> +static inline u32 ppc_inst_word(ppc_inst x)
> +{
> + return x.word;
> +}
> +
> +static inline u32 ppc_inst_prefix(ppc_inst x)
> +{
> + return x.prefix;
> +}
> +
> +static inline u32 ppc_inst_suffix(ppc_inst x)
> +{
> + return x.suffix;
> +}
> +
> +
> +static inline ppc_inst ppc_inst_read(const void *ptr)
> +{
> + ppc_inst inst;
> + inst.word = *(u32 *)ptr;
> + if (ppc_inst_prefixed(inst))
> + inst.suffix = *((u32 *)ptr + 1);
> + else
> + inst.pad = 0;
> + return inst;
> +}
> +
> +static inline void ppc_inst_write(ppc_inst *ptr, ppc_inst x)
> +{
> + if (ppc_inst_prefixed(x)) {
> + *(u32 *)ptr = x.prefix;
> + *((u32 *)ptr + 1) = x.suffix;
> + } else {
> + *(u32 *)ptr = x.word;
> + }
> +}
> +
> +static inline bool ppc_inst_equal(ppc_inst x, ppc_inst y)
> +{
> + return !memcmp(&x, &y, sizeof(struct ppc_inst));
> +}
> +
> +static inline bool ppc_inst_null(ppc_inst x)
> +{
> + return x.word == 0 && x.pad == 0;
> +}
> +
> +static inline u32 ppc_inst_mask(ppc_inst x, u32 mask)
> +{
> + return ppc_inst_word(x) & mask;
> +}
> +
> +#else /* !__powerpc64__ */
> +
> typedef u32 ppc_inst;
>
> #define PPC_INST(x) (x)
> @@ -50,4 +139,5 @@ static inline u32 ppc_inst_mask(ppc_inst x, u32 mask)
> return ppc_inst_word(x) & mask;
> }
>
> +#endif /* __powerpc64__ */
> #endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
> index 66b3f2983b22..4fc0e15e23a5 100644
> --- a/arch/powerpc/include/asm/kprobes.h
> +++ b/arch/powerpc/include/asm/kprobes.h
> @@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
> extern kprobe_opcode_t optprobe_template_end[];
>
> /* Fixed instruction size for powerpc */
> -#define MAX_INSN_SIZE 1
> +#define MAX_INSN_SIZE 2
> #define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
> #define MAX_OPTINSN_SIZE (optprobe_template_end - optprobe_template_entry)
> #define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
> diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
> index ef5483288920..5eb825fb77cd 100644
> --- a/arch/powerpc/include/asm/sstep.h
> +++ b/arch/powerpc/include/asm/sstep.h
> @@ -90,11 +90,15 @@ enum instruction_type {
> #define VSX_LDLEFT 4 /* load VSX register from left */
> #define VSX_CHECK_VEC 8 /* check MSR_VEC not MSR_VSX for reg >= 32 */
>
> +/* Prefixed flag, ORed in with type */
> +#define PREFIXED 0x800
> +
> /* Size field in type word */
> #define SIZE(n) ((n) << 12)
> #define GETSIZE(w) ((w) >> 12)
>
> #define GETTYPE(t) ((t) & INSTR_TYPE_MASK)
> +#define GETLENGTH(t) (((t) & PREFIXED) ? 8 : 4)
>
> #define MKOP(t, f, s) ((t) | (f) | SIZE(s))
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 2f500debae21..dee4fa1cd3ec 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -105,6 +105,28 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
> #define __put_user_inatomic(x, ptr) \
> __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> +/*
> + * When reading an instruction iff it is a prefix, the suffix needs to be also
> + * loaded.
> + */
> +#define __get_user_instr(x, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + __gui_ret = __get_user(x.prefix, (unsigned int __user *)ptr); \
> + if (!__gui_ret && ppc_inst_prefixed(x)) \
> + __gui_ret = __get_user(x.suffix, (unsigned int __user *)ptr + 1); \
> + __gui_ret; \
> +})
> +
> +#define __get_user_instr_inatomic(x, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + __gui_ret = __get_user_inatomic(x.prefix, (unsigned int __user *)ptr); \
> + if (!__gui_ret && ppc_inst_prefixed(x)) \
> + __gui_ret = __get_user_inatomic(x.suffix, (unsigned int __user *)ptr + 1); \
> + __gui_ret; \
> +})
> +
> extern long __put_user_bad(void);
>
> /*
> diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h
> index fff3c5fc90b5..7896a7125fa9 100644
> --- a/arch/powerpc/include/asm/uprobes.h
> +++ b/arch/powerpc/include/asm/uprobes.h
> @@ -14,7 +14,7 @@
>
> typedef ppc_opcode_t uprobe_opcode_t;
>
> -#define MAX_UINSN_BYTES 4
> +#define MAX_UINSN_BYTES 8
> #define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
>
> /* The following alias is needed for reference from arch-agnostic code */
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index b246ca124931..110eadd85c58 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -303,13 +303,14 @@ int fix_alignment(struct pt_regs *regs)
> */
> CHECK_FULL_REGS(regs);
>
> - if (unlikely(__get_user(instr, (unsigned int __user *)regs->nip)))
> + if (unlikely(__get_user_instr(instr, (void __user *)regs->nip)))
> return -EFAULT;
> if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
> /* We don't handle PPC little-endian any more... */
> if (cpu_has_feature(CPU_FTR_PPC_LE))
> return -EIO;
> - instr = PPC_INST(swab32(ppc_inst_word(instr)));
> + instr = PPC_INST_PREFIXED(swab32(ppc_inst_word(instr)),
> + swab32(ppc_inst_suffix(instr)));
> }
>
> #ifdef CONFIG_SPE
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index f001de471b98..212e0abfda43 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -249,7 +249,7 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
> struct instruction_op op;
> unsigned long addr = info->address;
>
> - if (__get_user_inatomic(instr, (unsigned int *)regs->nip))
> + if (__get_user_instr_inatomic(instr, (void __user*)regs->nip))
> goto fail;
>
> ret = analyse_instr(&op, regs, instr);
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index f142d11d7b48..1a5370a3c7c8 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -153,7 +153,7 @@ NOKPROBE_SYMBOL(arch_arm_kprobe);
>
> void arch_disarm_kprobe(struct kprobe *p)
> {
> - patch_instruction(p->addr, PPC_INST(p->opcode));
> + patch_instruction(p->addr, ppc_inst_read(p->ainsn.insn));
> }
> NOKPROBE_SYMBOL(arch_disarm_kprobe);
>
> @@ -487,12 +487,13 @@ int kprobe_post_handler(struct pt_regs *regs)
> {
> struct kprobe *cur = kprobe_running();
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> + int len = ppc_inst_len(ppc_inst_read(cur->ainsn.insn));
>
> if (!cur || user_mode(regs))
> return 0;
>
> /* make sure we got here for instruction we have a kprobe on */
> - if (((unsigned long)cur->ainsn.insn + 4) != regs->nip)
> + if ((unsigned long)cur->ainsn.insn + len != regs->nip)
> return 0;
>
> if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
> @@ -501,7 +502,7 @@ int kprobe_post_handler(struct pt_regs *regs)
> }
>
> /* Adjust nip to after the single-stepped instruction */
> - regs->nip = (unsigned long)cur->addr + 4;
> + regs->nip = (unsigned long)cur->addr + len;
> regs->msr |= kcb->kprobe_saved_msr;
>
> /*Restore back the original saved kprobes variables and continue. */
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 5b53c373373b..af6761859fba 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -158,38 +158,38 @@ void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
>
> /*
> * Generate instructions to load provided immediate 64-bit value
> - * to register 'r3' and patch these instructions at 'addr'.
> + * to register 'reg' and patch these instructions at 'addr'.
> */
> -void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> +void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> {
> - /* lis r3,(op)@highest */
> - patch_instruction(addr, PPC_INST(PPC_INST_ADDIS | ___PPC_RT(3) |
> + /* lis reg,(op)@highest */
> + patch_instruction(addr, PPC_INST(PPC_INST_ADDIS | ___PPC_RT(reg) |
> ((val >> 48) & 0xffff)));
> addr++;
>
> - /* ori r3,r3,(op)@higher */
> - patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> + /* ori reg,reg,(op)@higher */
> + patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
> addr++;
>
> - /* rldicr r3,r3,32,31 */
> - patch_instruction(addr, PPC_INST(PPC_INST_RLDICR | ___PPC_RA(3) |
> - ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
> + /* rldicr reg,reg,32,31 */
> + patch_instruction(addr, PPC_INST(PPC_INST_RLDICR | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | __PPC_SH64(32) | __PPC_ME64(31)));
> addr++;
>
> - /* oris r3,r3,(op)@h */
> - patch_instruction(addr, PPC_INST(PPC_INST_ORIS | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> + /* oris reg,reg,(op)@h */
> + patch_instruction(addr, PPC_INST(PPC_INST_ORIS | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
> addr++;
>
> - /* ori r3,r3,(op)@l */
> - patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | (val & 0xffff)));
> + /* ori reg,reg,(op)@l */
> + patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | (val & 0xffff)));
> }
>
> int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> {
> - ppc_inst branch_op_callback, branch_emulate_step;
> + ppc_inst branch_op_callback, branch_emulate_step, temp;
> kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> long b_offset;
> unsigned long nip, size;
> @@ -239,7 +239,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> * Fixup the template with instructions to:
> * 1. load the address of the actual probepoint
> */
> - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
> + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
>
> /*
> * 2. branch to optimized_callback() and emulate_step()
> @@ -268,7 +268,11 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> /*
> * 3. load instruction to be emulated into relevant register, and
> */
> - patch_imm32_load_insns(*(unsigned int *)p->ainsn.insn, buff + TMPL_INSN_IDX);
> + temp = ppc_inst_read(p->ainsn.insn);
> + patch_imm64_load_insns(ppc_inst_word(temp) |
> + ((u64)ppc_inst_suffix(temp) << 32),
> + 4,
> + buff + TMPL_INSN_IDX);
>
> /*
> * 4. branch back from trampoline
> diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
> index cf383520843f..ff8ba4d3824d 100644
> --- a/arch/powerpc/kernel/optprobes_head.S
> +++ b/arch/powerpc/kernel/optprobes_head.S
> @@ -94,6 +94,9 @@ optprobe_template_insn:
> /* 2, Pass instruction to be emulated in r4 */
> nop
> nop
> + nop
> + nop
> + nop
>
> .global optprobe_template_call_emulate
> optprobe_template_call_emulate:
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index ad451205f268..3b8655f57b4a 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -42,9 +42,24 @@
> static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
>
> static long
> -read_inst(ppc_inst *inst, const void *src)
> +read_inst(ppc_inst *p, const void *src)
> {
> - return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> + ppc_inst inst;
> + long err;
> +
> + err = probe_kernel_read((void *)&inst.prefix,
> + src, MCOUNT_INSN_SIZE);
> + if (err)
> + return err;
> +
> + if (ppc_inst_prefixed(inst))
> + err = probe_kernel_read((void *)&inst.suffix,
> + src + 4, MCOUNT_INSN_SIZE);
> + if (err)
> + return err;
> +
> + ppc_inst_write(p, inst);
> + return 0;
> }
>
> static ppc_inst
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index d1dff1dc3a11..3e4fbb5c1b1e 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -111,7 +111,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
> * support doesn't exist and have to fix-up the next instruction
> * to be executed.
> */
> - regs->nip = utask->vaddr + MAX_UINSN_BYTES;
> + regs->nip = utask->vaddr + ppc_inst_len(ppc_inst_read(auprobe->insn));
>
> user_disable_single_step(current);
> return 0;
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index fa7f32adf029..3b8277a64b8f 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -24,17 +24,27 @@ static int __patch_instruction(ppc_inst *exec_addr, ppc_inst instr,
> {
> int err = 0;
>
> - __put_user_asm(instr, patch_addr, err, "stw");
> + __put_user_asm(ppc_inst_word(instr), patch_addr, err, "stw");
> if (err)
> return err;
>
> asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
> "r" (exec_addr));
>
> + if (!ppc_inst_prefixed(instr))
> + return 0;
> +
> + __put_user_asm(ppc_inst_suffix(instr), patch_addr + 4, err, "stw");
> + if (err)
> + return err;
> +
> + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr + 4),
> + "r" (exec_addr + 4));
> +
> return 0;
> }
>
> -int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> +int raw_patch_instruction(void *addr, ppc_inst instr)
> {
> return __patch_instruction(addr, instr, addr);
> }
> @@ -184,7 +194,7 @@ static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
>
> #endif /* CONFIG_STRICT_KERNEL_RWX */
>
> -int patch_instruction(unsigned int *addr, unsigned int instr)
> +int patch_instruction(void *addr, ppc_inst instr)
> {
> /* Make sure we aren't patching a freed init section */
> if (init_mem_is_free && init_section_contains(addr, 4)) {
> @@ -195,7 +205,7 @@ int patch_instruction(unsigned int *addr, unsigned int instr)
> }
> NOKPROBE_SYMBOL(patch_instruction);
>
> -int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> +int patch_branch(void *addr, unsigned long target, int flags)
> {
> return patch_instruction(addr, create_branch(addr, target, flags));
> }
> @@ -264,7 +274,7 @@ ppc_inst create_branch(const ppc_inst *addr,
> return instruction;
> }
>
> -unsigned int create_cond_branch(const unsigned int *addr,
> +ppc_inst create_cond_branch(const void *addr,
> unsigned long target, int flags)
> {
> ppc_inst instruction;
> @@ -344,7 +354,7 @@ static unsigned long branch_bform_target(const ppc_inst *instr)
> return (unsigned long)imm;
> }
>
> -unsigned long branch_target(const ppc_inst *instr)
> +unsigned long branch_target(const void *instr)
> {
> if (instr_is_branch_iform(ppc_inst_read(instr)))
> return branch_iform_target(instr);
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index bae878a83fa5..ab4c71c43c8c 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> unsigned long int imm;
> unsigned long int val, val2;
> unsigned int mb, me, sh;
> - unsigned int word;
> + unsigned int word, suffix;
> long ival;
>
> word = ppc_inst_word(instr);
> + suffix = ppc_inst_suffix(instr);
> +
> op->type = COMPUTE;
>
> opcode = word >> 26;
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index ee084411f2f5..c5536e1a3356 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -110,7 +110,7 @@ struct bpt {
> #define BP_DABR 4
>
> #define NBPTS 256
> -#define BPT_WORDS 2
> +#define BPT_WORDS 4
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -118,12 +118,13 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> -static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS];
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS] __aligned(64);
>
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> static int mwrite(unsigned long, void *, int);
> +static int mread_instr(unsigned long, ppc_inst *);
> static int handle_fault(struct pt_regs *);
> static void byterev(unsigned char *, int);
> static void memex(void);
> @@ -759,8 +760,8 @@ static int xmon_bpt(struct pt_regs *regs)
>
> /* Are we at the trap at bp->instr[1] for some bp? */
> bp = in_breakpoint_table(regs->nip, &offset);
> - if (bp != NULL && offset == 4) {
> - regs->nip = bp->address + 4;
> + if (bp != NULL && (offset == 4 || offset == 8)) {
> + regs->nip = bp->address + offset;
> atomic_dec(&bp->ref_count);
> return 1;
> }
> @@ -862,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
> if (off >= sizeof(bpt_table))
> return NULL;
> bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> - if (bp_off != 0 && bp_off != 4)
> + if (bp_off != 0 && bp_off != 4 && bp_off != 8)
> return NULL;
> *offp = bp_off;
> return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
> @@ -881,7 +882,6 @@ static struct bpt *new_breakpoint(unsigned long a)
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> - patch_instruction(bp->instr + 1, PPC_INST(bpinstr));
> return bp;
> }
> }
> @@ -900,7 +900,7 @@ static void insert_bpts(void)
> for (i = 0; i < NBPTS; ++i, ++bp) {
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
> continue;
> - if (mread(bp->address, &instr, 4) != 4) {
> + if (!mread_instr(bp->address, &instr)) {
> printf("Couldn't read instruction at %lx, "
> "disabling breakpoint there\n", bp->address);
> bp->enabled = 0;
> @@ -913,9 +913,10 @@ static void insert_bpts(void)
> continue;
> }
> patch_instruction(bp->instr, instr);
> + patch_instruction(bp->instr + ppc_inst_len(instr), PPC_INST(bpinstr));
> if (bp->enabled & BP_CIABR)
> continue;
> - if (patch_instruction((ppc_inst *)bp->address,
> + if (patch_instruction((void *)bp->address,
> PPC_INST(bpinstr)) != 0) {
> printf("Couldn't write instruction at %lx, "
> "disabling breakpoint there\n", bp->address);
> @@ -950,7 +951,7 @@ static void remove_bpts(void)
> for (i = 0; i < NBPTS; ++i, ++bp) {
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> continue;
> - if (mread(bp->address, &instr, 4) == 4
> + if (mread_instr(bp->address, &instr)
> && ppc_inst_equal(instr, PPC_INST(bpinstr))
> && patch_instruction(
> (ppc_inst *)bp->address, ppc_inst_read(bp->instr)) != 0)
> @@ -1166,7 +1167,7 @@ static int do_step(struct pt_regs *regs)
> force_enable_xmon();
> /* check we are in 64-bit kernel mode, translation enabled */
> if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) {
> - if (mread(regs->nip, &instr, 4) == 4) {
> + if (mread_instr(regs->nip, &instr)) {
> stepped = emulate_step(regs, instr);
> if (stepped < 0) {
> printf("Couldn't single-step %s instruction\n",
> @@ -1333,7 +1334,7 @@ static long check_bp_loc(unsigned long addr)
> printf("Breakpoints may only be placed at kernel addresses\n");
> return 0;
> }
> - if (!mread(addr, &instr, sizeof(instr))) {
> + if (!mread_instr(addr, &instr)) {
> printf("Can't read instruction at address %lx\n", addr);
> return 0;
> }
> @@ -2126,6 +2127,21 @@ mwrite(unsigned long adrs, void *buf, int size)
> return n;
> }
>
> +static int
> +mread_instr(unsigned long adrs, ppc_inst *instr)
> +{
> + if (setjmp(bus_error_jmp) == 0) {
> + catch_memory_errors = 1;
> + sync();
> + *instr = ppc_inst_read((void *)adrs);
> + sync();
> + /* wait a little while to see if we get a machine check */
> + __delay(200);
> + }
> + catch_memory_errors = 0;
> + return ppc_inst_len(*instr);
> +}
> +
> static int fault_type;
> static int fault_except;
> static char *fault_chars[] = { "--", "**", "##" };
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH v4 06/16] powerpc: Use a function for getting the instruction op code
From: Balamuruhan S @ 2020-03-23 6:54 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, npiggin
In-Reply-To: <20200320051809.24332-7-jniethe5@gmail.com>
On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> In preparation for using a data type for instructions that can not be
> directly used with the '>>' operator use a function for getting the op
> code of an instruction.
we need to adopt this in sstep.c and vecemu.c
-- Bala
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/kernel/align.c | 4 ++--
> arch/powerpc/lib/code-patching.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 38542fffa179..77c49dfdc1b4 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -313,8 +313,8 @@ int fix_alignment(struct pt_regs *regs)
> }
>
> #ifdef CONFIG_SPE
> - if ((instr >> 26) == 0x4) {
> - int reg = (instr >> 21) & 0x1f;
> + if (ppc_inst_opcode(instr) == 0x4) {
> + int reg = (ppc_inst_word(instr) >> 21) & 0x1f;
> PPC_WARN_ALIGNMENT(spe, regs);
> return emulate_spe(regs, reg, instr);
> }
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index e2ba23fd6f4d..04a303c059e2 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -228,7 +228,7 @@ bool is_offset_in_branch_range(long offset)
> */
> bool is_conditional_branch(ppc_inst instr)
> {
> - unsigned int opcode = instr >> 26;
> + unsigned int opcode = ppc_inst_opcode(instr);
>
> if (opcode == 16) /* bc, bca, bcl, bcla */
> return true;
> @@ -286,7 +286,7 @@ unsigned int create_cond_branch(const unsigned int *addr,
>
> static unsigned int branch_opcode(ppc_inst instr)
> {
> - return (instr >> 26) & 0x3F;
> + return ppc_inst_opcode(instr) & 0x3F;
> }
>
> static int instr_is_branch_iform(ppc_inst instr)
^ permalink raw reply
* Re: [PATCH v4 07/16] powerpc: Introduce functions for instruction nullity and equality
From: Nicholas Piggin @ 2020-03-23 6:43 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-8-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:18 pm:
> In preparation for an instruction data type that can not be directly
> used with the '==' operator use functions for checking equality and
> nullity.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> arch/powerpc/kernel/optprobes.c | 2 +-
> arch/powerpc/kernel/trace/ftrace.c | 33 +++++++++++++++-------------
> arch/powerpc/lib/code-patching.c | 16 +++++++-------
> arch/powerpc/lib/feature-fixups.c | 2 +-
> arch/powerpc/lib/test_emulate_step.c | 4 ++--
> arch/powerpc/xmon/xmon.c | 4 ++--
> 6 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 1025a7a3b3a8..6027425a85f2 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -259,7 +259,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> (unsigned long)emulate_step_addr,
> BRANCH_SET_LINK);
>
> - if (!branch_op_callback || !branch_emulate_step)
> + if (ppc_inst_null(branch_op_callback) || ppc_inst_null(branch_emulate_step))
Is an instruction null, or zeroes?
Oh, most of this comes from create_branch and things. Hmm, would rather
see those functions modified to take a &insn and return an int err.
> @@ -437,7 +438,7 @@ int ftrace_make_nop(struct module *mod,
> * then we had to use a trampoline to make the call.
> * Otherwise just update the call site.
> */
> - if (test_24bit_addr(ip, addr)) {
> + if (!ppc_inst_null(test_24bit_addr(ip, addr))) {
> /* within range */
> old = ftrace_call_replace(ip, addr, 1);
> new = PPC_INST(PPC_INST_NOP);
test_24bit_addr shouldn't be passing a ppc_inst back, but a bool.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 05/16] powerpc: Use a function for masking instructions
From: Nicholas Piggin @ 2020-03-23 6:37 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-6-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:17 pm:
> In preparation for using an instruction data type that can not be used
> directly with the '&' operator, use a function to mask instructions.
Hmm. ppc_inst_mask isn't such a good interface I think. It takes a
ppc_inst and a mask, you would expect it to return a ppc_inst, probably
with some part of its value anded with your mask value but not entirely
clear.
I would have a ppc_inst_val that is a more mechanical replacement and
lets you do more things with it, although I like the other helpers you
add later. Oh you've added ppc_inst_word further down. Why not use that
here intead of ppc_inst_mask()?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 8/9] crypto/nx: Remove 'pid' in vas_tx_win_attr struct
From: Herbert Xu @ 2020-03-23 6:31 UTC (permalink / raw)
To: Haren Myneni; +Cc: mikey, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1584936377.9256.15330.camel@hbabu-laptop>
On Sun, Mar 22, 2020 at 09:06:17PM -0700, Haren Myneni wrote:
>
> When window is opened, pid reference is taken for user space
> windows. Not needed for kernel windows. So remove 'pid' in
> vas_tx_win_attr struct.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> arch/powerpc/include/asm/vas.h | 1 -
> drivers/crypto/nx/nx-common-powernv.c | 1 -
> 2 files changed, 2 deletions(-)
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 7/9] crypto/nx: Enable and setup GZIP compression type
From: Herbert Xu @ 2020-03-23 6:31 UTC (permalink / raw)
To: Haren Myneni; +Cc: mikey, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1584936337.9256.15329.camel@hbabu-laptop>
On Sun, Mar 22, 2020 at 09:05:37PM -0700, Haren Myneni wrote:
>
> Changes to probe GZIP device-tree nodes, open RX windows and setup
> GZIP compression type. No plans to provide GZIP usage in kernel right
> now, but this patch enables GZIP for user space usage.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> drivers/crypto/nx/nx-common-powernv.c | 43 ++++++++++++++++++++++++++++++-----
> 1 file changed, 37 insertions(+), 6 deletions(-)
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 6/9] crypto/NX: Make enable code generic to add new GZIP compression type
From: Herbert Xu @ 2020-03-23 6:31 UTC (permalink / raw)
To: Haren Myneni; +Cc: mikey, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1584936287.9256.15328.camel@hbabu-laptop>
On Sun, Mar 22, 2020 at 09:04:47PM -0700, Haren Myneni wrote:
>
> Make setup and enable code generic to support new GZIP compression type.
> Changed nx842 reference to nx and moved some code to new functions.
> Functionality is not changed except sparse warning fix - setting NULL
> instead of 0 for per_cpu send window in nx_delete_coprocs().
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> drivers/crypto/nx/nx-common-powernv.c | 161 +++++++++++++++++++++-------------
> 1 file changed, 101 insertions(+), 60 deletions(-)
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 5/9] crypto/nx: Rename nx-842-powernv file name to nx-common-powernv
From: Herbert Xu @ 2020-03-23 6:31 UTC (permalink / raw)
To: Haren Myneni; +Cc: mikey, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1584936230.9256.15327.camel@hbabu-laptop>
On Sun, Mar 22, 2020 at 09:03:50PM -0700, Haren Myneni wrote:
>
> Rename nx-842-powernv.c to nx-common-powernv.c to add code for setup
> and enable new GZIP compression type. The actual functionality is not
> changed in this patch.
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> drivers/crypto/nx/Makefile | 2 +-
> drivers/crypto/nx/nx-842-powernv.c | 1062 ---------------------------------
> drivers/crypto/nx/nx-common-powernv.c | 1062 +++++++++++++++++++++++++++++++++
> 3 files changed, 1063 insertions(+), 1063 deletions(-)
> delete mode 100644 drivers/crypto/nx/nx-842-powernv.c
> create mode 100644 drivers/crypto/nx/nx-common-powernv.c
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 4/9] crypto/nx: Initialize coproc entry with kzalloc
From: Herbert Xu @ 2020-03-23 6:30 UTC (permalink / raw)
To: Haren Myneni; +Cc: mikey, npiggin, linux-crypto, sukadev, linuxppc-dev, dja
In-Reply-To: <1584936180.9256.15326.camel@hbabu-laptop>
On Sun, Mar 22, 2020 at 09:03:00PM -0700, Haren Myneni wrote:
>
> coproc entry is initialized during NX probe on power9, but not on P8.
> nx842_delete_coprocs() is used for both and frees receive window if it
> is allocated. Getting crash for rmmod on P8 since coproc->vas.rxwin
> is not initialized.
>
> This patch replaces kmalloc with kzalloc in nx842_powernv_probe()
>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> drivers/crypto/nx/nx-842-powernv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 04/16] powerpc: Use a macro for creating instructions from u32s
From: Nicholas Piggin @ 2020-03-23 6:26 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-5-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:17 pm:
> In preparation for instructions having a more complex data type start
> using a macro, PPC_INST(), for making an instruction out of a u32.
> Currently this does nothing, but it will allow for creating a data type
> that can represent prefixed instructions.
Where is the macro? And, can it be a static inline (and lowercase)
instead? No big deal if not.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions
From: Nicholas Piggin @ 2020-03-23 6:23 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-4-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:17 pm:
> Currently unsigned ints are used to represent instructions on powerpc.
> This has worked well as instructions have always been 4 byte words.
> However, a future ISA version will introduce some changes to
> instructions that mean this scheme will no longer work as well. This
> change is Prefixed Instructions. A prefixed instruction is made up of a
> word prefix followed by a word suffix to make an 8 byte double word
> instruction. No matter the endianess of the system the prefix always
> comes first. Prefixed instructions are only planned for powerpc64.
>
> Introduce a ppc_inst type to represent both prefixed and word
> instructions on powerpc64 while keeping it possible to exclusively have
> word instructions on powerpc32, A latter patch will expand the type to
> include prefixed instructions but for now just typedef it to a u32.
>
> Later patches will introduce helper functions and macros for
> manipulating the instructions so that powerpc64 and powerpc32 might
> maintain separate type definitions.
ppc_inst_t I would slightly prefer for a typedef like this.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
^ permalink raw reply
* Re: [PATCH v4 02/16] xmon: Move out-of-line instructions to text section
From: Nicholas Piggin @ 2020-03-23 6:22 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-3-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:17 pm:
> To execute an instruction out of line after a breakpoint, the NIP is set
> to the address of struct bpt::instr. Here a copy of the instruction that
> was replaced with a breakpoint is kept, along with a trap so normal flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked as
> no execute.
>
> Instead of each struct bpt holding the instructions to be XOL'd, make a
> new array, bpt_table[], with enough space to hold instructions for the
> number of supported breakpoints. Place this array in the text section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple mapping:
> bpts[n] -> bpt_table[n * words per breakpoint]. Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/kernel/vmlinux.lds.S | 2 +-
> arch/powerpc/xmon/xmon.c | 22 +++++++++++++---------
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..e90845b8c300 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -86,7 +86,7 @@ SECTIONS
> ALIGN_FUNCTION();
> #endif
> /* careful! __ftr_alt_* sections need to be close to .text */
> - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text);
> + *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text .text.xmon_bpts);
> #ifdef CONFIG_PPC64
> *(.tramp.ftrace.text);
> #endif
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..7875d1a37770 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> /* Breakpoint stuff */
> struct bpt {
> unsigned long address;
> - unsigned int instr[2];
> + unsigned int *instr;
> atomic_t ref_count;
> int enabled;
> unsigned long pad;
> @@ -109,6 +109,7 @@ struct bpt {
> #define BP_DABR 4
>
> #define NBPTS 256
> +#define BPT_WORDS 2
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS];
> +
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long pc)
> static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
> {
> unsigned long off;
> + unsigned long bp_off;
>
> - off = nip - (unsigned long) bpts;
> - if (off >= sizeof(bpts))
> + off = nip - (unsigned long) bpt_table;
> + if (off >= sizeof(bpt_table))
> return NULL;
> - off %= sizeof(struct bpt);
> - if (off != offsetof(struct bpt, instr[0])
> - && off != offsetof(struct bpt, instr[1]))
> + bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> + if (bp_off != 0 && bp_off != 4)
> return NULL;
> - *offp = off - offsetof(struct bpt, instr[0]);
> - return (struct bpt *) (nip - off);
> + *offp = bp_off;
> + return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
> }
>
> static struct bpt *new_breakpoint(unsigned long a)
> @@ -876,7 +879,8 @@ static struct bpt *new_breakpoint(unsigned long a)
> for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> - patch_instruction(&bp->instr[1], bpinstr);
> + bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> + patch_instruction(bp->instr + 1, bpinstr);
> return bp;
> }
> }
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH v4 01/16] powerpc/xmon: Remove store_inst() for patch_instruction()
From: Nicholas Piggin @ 2020-03-23 6:19 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-2-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:17 pm:
> For modifying instructions in xmon, patch_instruction() can serve the
> same role that store_inst() is performing with the advantage of not
> being specific to xmon. In some places patch_instruction() is already
> being using followed by store_inst(). In these cases just remove the
> store_inst(). Otherwise replace store_inst() with patch_instruction().
LGTM
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: Read into a local variable
> ---
> arch/powerpc/xmon/xmon.c | 18 +++++-------------
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index e8c84d265602..02e3bd62cab4 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -325,11 +325,6 @@ static inline void sync(void)
> asm volatile("sync; isync");
> }
>
> -static inline void store_inst(void *p)
> -{
> - asm volatile ("dcbst 0,%0; sync; icbi 0,%0; isync" : : "r" (p));
> -}
> -
> static inline void cflush(void *p)
> {
> asm volatile ("dcbf 0,%0; icbi 0,%0" : : "r" (p));
> @@ -881,8 +876,7 @@ static struct bpt *new_breakpoint(unsigned long a)
> for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> - bp->instr[1] = bpinstr;
> - store_inst(&bp->instr[1]);
> + patch_instruction(&bp->instr[1], bpinstr);
> return bp;
> }
> }
> @@ -894,25 +888,26 @@ static struct bpt *new_breakpoint(unsigned long a)
> static void insert_bpts(void)
> {
> int i;
> + unsigned int instr;
> struct bpt *bp;
>
> bp = bpts;
> for (i = 0; i < NBPTS; ++i, ++bp) {
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
> continue;
> - if (mread(bp->address, &bp->instr[0], 4) != 4) {
> + if (mread(bp->address, &instr, 4) != 4) {
> printf("Couldn't read instruction at %lx, "
> "disabling breakpoint there\n", bp->address);
> bp->enabled = 0;
> continue;
> }
> - if (IS_MTMSRD(bp->instr[0]) || IS_RFID(bp->instr[0])) {
> + if (IS_MTMSRD(instr) || IS_RFID(instr)) {
> printf("Breakpoint at %lx is on an mtmsrd or rfid "
> "instruction, disabling it\n", bp->address);
> bp->enabled = 0;
> continue;
> }
> - store_inst(&bp->instr[0]);
> + patch_instruction(bp->instr, instr);
> if (bp->enabled & BP_CIABR)
> continue;
> if (patch_instruction((unsigned int *)bp->address,
> @@ -922,7 +917,6 @@ static void insert_bpts(void)
> bp->enabled &= ~BP_TRAP;
> continue;
> }
> - store_inst((void *)bp->address);
> }
> }
>
> @@ -957,8 +951,6 @@ static void remove_bpts(void)
> (unsigned int *)bp->address, bp->instr[0]) != 0)
> printf("Couldn't remove breakpoint at %lx\n",
> bp->address);
> - else
> - store_inst((void *)bp->address);
> }
> }
>
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH v4 00/16] Initial Prefixed Instruction support
From: Nicholas Piggin @ 2020-03-23 6:18 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, bala24
In-Reply-To: <20200320051809.24332-1-jniethe5@gmail.com>
Jordan Niethe's on March 20, 2020 3:17 pm:
> A future revision of the ISA will introduce prefixed instructions. A
> prefixed instruction is composed of a 4-byte prefix followed by a
> 4-byte suffix.
>
> All prefixes have the major opcode 1. A prefix will never be a valid
> word instruction. A suffix may be an existing word instruction or a
> new instruction.
>
> This series enables prefixed instructions and extends the instruction
> emulation to support them. Then the places where prefixed instructions
> might need to be emulated are updated.
>
> The series is based on top of:
> https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> kprobes.
>
> v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
> The major changes:
> - Move xmon breakpoints from data section to text section
> - Introduce a data type for instructions on powerpc
Thanks for doing this, looks like a lot of work, I hope it works out :)
Thanks,
Nick
^ permalink raw reply
* [PATCH 2/2] Fix 2 "[v3, 28/32] powerpc/64s: interrupt implement exit logic in C"
From: Nicholas Piggin @ 2020-03-23 6:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200323054409.1932037-1-npiggin@gmail.com>
This completes the move of the stray hunk. Also fixes an irq tracer
bug, returning to irqs-disabled context should not trace_hardirqs_on().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/asm-prototypes.h | 2 ++
arch/powerpc/kernel/syscall_64.c | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index ab59a4904254..7d81e86a1e5d 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -99,6 +99,8 @@ void __init machine_init(u64 dt_ptr);
#endif
long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, unsigned long r0, struct pt_regs *regs);
notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs);
+notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr);
+notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr);
long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
u32 len_high, u32 len_low);
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index ef4ce621f862..ffd601d87065 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -355,7 +355,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
irq_soft_mask_set(IRQS_ENABLED);
} else {
/* Returning to a kernel context with local irqs disabled. */
- trace_hardirqs_on();
__hard_EE_RI_disable();
if (regs->msr & MSR_EE)
local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions
From: Balamuruhan S @ 2020-03-23 6:07 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, npiggin
In-Reply-To: <20200320051809.24332-4-jniethe5@gmail.com>
On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> Currently unsigned ints are used to represent instructions on powerpc.
> This has worked well as instructions have always been 4 byte words.
> However, a future ISA version will introduce some changes to
> instructions that mean this scheme will no longer work as well. This
> change is Prefixed Instructions. A prefixed instruction is made up of a
> word prefix followed by a word suffix to make an 8 byte double word
> instruction. No matter the endianess of the system the prefix always
> comes first. Prefixed instructions are only planned for powerpc64.
>
> Introduce a ppc_inst type to represent both prefixed and word
> instructions on powerpc64 while keeping it possible to exclusively have
> word instructions on powerpc32, A latter patch will expand the type to
> include prefixed instructions but for now just typedef it to a u32.
>
> Later patches will introduce helper functions and macros for
> manipulating the instructions so that powerpc64 and powerpc32 might
> maintain separate type definitions.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> arch/powerpc/include/asm/code-patching.h | 31 +++++------
> arch/powerpc/include/asm/inst.h | 53 +++++++++++++++++++
> arch/powerpc/include/asm/sstep.h | 5 +-
> arch/powerpc/kernel/align.c | 2 +-
> arch/powerpc/kernel/hw_breakpoint.c | 3 +-
> arch/powerpc/kernel/kprobes.c | 2 +-
> arch/powerpc/kernel/mce_power.c | 5 +-
> arch/powerpc/kernel/optprobes.c | 10 ++--
> arch/powerpc/kernel/trace/ftrace.c | 66 ++++++++++++------------
> arch/powerpc/kvm/emulate_loadstore.c | 1 +
> arch/powerpc/lib/code-patching.c | 54 +++++++++----------
> arch/powerpc/lib/sstep.c | 4 +-
> arch/powerpc/lib/test_emulate_step.c | 9 ++--
> arch/powerpc/xmon/xmon.c | 12 ++---
> 14 files changed, 160 insertions(+), 97 deletions(-)
> create mode 100644 arch/powerpc/include/asm/inst.h
>
> diff --git a/arch/powerpc/include/asm/code-patching.h
> b/arch/powerpc/include/asm/code-patching.h
> index 898b54262881..cb5106f92d67 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -11,6 +11,7 @@
> #include <linux/string.h>
> #include <linux/kallsyms.h>
> #include <asm/asm-compat.h>
> +#include <asm/inst.h>
>
> /* Flags for create_branch:
> * "b" == create_branch(addr, target, 0);
> @@ -22,27 +23,27 @@
> #define BRANCH_ABSOLUTE 0x2
>
> bool is_offset_in_branch_range(long offset);
> -unsigned int create_branch(const unsigned int *addr,
> +ppc_inst create_branch(const ppc_inst *addr,
> unsigned long target, int flags);
> -unsigned int create_cond_branch(const unsigned int *addr,
> +unsigned int create_cond_branch(const ppc_inst *addr,
> unsigned long target, int flags);
> -int patch_branch(unsigned int *addr, unsigned long target, int flags);
> -int patch_instruction(unsigned int *addr, unsigned int instr);
> -int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> +int patch_branch(ppc_inst *addr, unsigned long target, int flags);
> +int patch_instruction(ppc_inst *addr, ppc_inst instr);
> +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr);
>
> static inline unsigned long patch_site_addr(s32 *site)
> {
> return (unsigned long)site + *site;
> }
>
> -static inline int patch_instruction_site(s32 *site, unsigned int instr)
> +static inline int patch_instruction_site(s32 *site, ppc_inst instr)
> {
> - return patch_instruction((unsigned int *)patch_site_addr(site), instr);
> + return patch_instruction((ppc_inst *)patch_site_addr(site), instr);
> }
>
> static inline int patch_branch_site(s32 *site, unsigned long target, int
> flags)
> {
> - return patch_branch((unsigned int *)patch_site_addr(site), target,
> flags);
> + return patch_branch((ppc_inst *)patch_site_addr(site), target, flags);
> }
>
> static inline int modify_instruction(unsigned int *addr, unsigned int clr,
> @@ -56,13 +57,13 @@ static inline int modify_instruction_site(s32 *site,
> unsigned int clr, unsigned
> return modify_instruction((unsigned int *)patch_site_addr(site), clr,
> set);
> }
>
> -int instr_is_relative_branch(unsigned int instr);
> -int instr_is_relative_link_branch(unsigned int instr);
> -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> -unsigned long branch_target(const unsigned int *instr);
> -unsigned int translate_branch(const unsigned int *dest,
> - const unsigned int *src);
> -extern bool is_conditional_branch(unsigned int instr);
> +int instr_is_relative_branch(ppc_inst instr);
> +int instr_is_relative_link_branch(ppc_inst instr);
> +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr);
> +unsigned long branch_target(const ppc_inst *instr);
> +ppc_inst translate_branch(const ppc_inst *dest,
> + const ppc_inst *src);
> +extern bool is_conditional_branch(ppc_inst instr);
> #ifdef CONFIG_PPC_BOOK3E_64
> void __patch_exception(int exc, unsigned long addr);
> #define patch_exception(exc, name) do { \
> diff --git a/arch/powerpc/include/asm/inst.h
> b/arch/powerpc/include/asm/inst.h
> new file mode 100644
> index 000000000000..7c8596ee411e
> --- /dev/null
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _ASM_INST_H
> +#define _ASM_INST_H
> +
> +/*
> + * Instruction data type for POWER
> + */
> +
> +typedef u32 ppc_inst;
> +
> +#define PPC_INST(x) (x)
> +
> +static inline int ppc_inst_len(ppc_inst x)
should we consider `size_t` which is of type unsigned int ?
-- Bala
> +{
> + return sizeof(ppc_inst);
> +}
> +
> +static inline int ppc_inst_opcode(ppc_inst x)
> +{
> + return x >> 26;
> +}
> +
> +static inline u32 ppc_inst_word(ppc_inst x)
> +{
> + return x;
> +}
> +
> +static inline ppc_inst ppc_inst_read(const ppc_inst *ptr)
> +{
> + return *(ppc_inst *)ptr;
> +}
> +
> +static inline void ppc_inst_write(void *ptr, ppc_inst x)
> +{
> + *(ppc_inst *)ptr = x;
> +}
> +
> +static inline bool ppc_inst_equal(ppc_inst x, ppc_inst y)
> +{
> + return x == y;
> +}
> +
> +static inline bool ppc_inst_null(ppc_inst x)
> +{
> + return x == 0;
> +}
> +
> +static inline u32 ppc_inst_mask(ppc_inst x, u32 mask)
> +{
> + return ppc_inst_word(x) & mask;
> +}
> +
> +#endif /* _ASM_INST_H */
> diff --git a/arch/powerpc/include/asm/sstep.h
> b/arch/powerpc/include/asm/sstep.h
> index 769f055509c9..9353916fcba7 100644
> --- a/arch/powerpc/include/asm/sstep.h
> +++ b/arch/powerpc/include/asm/sstep.h
> @@ -2,6 +2,7 @@
> /*
> * Copyright (C) 2004 Paul Mackerras <paulus@au.ibm.com>, IBM
> */
> +#include <asm/inst.h>
>
> struct pt_regs;
>
> @@ -132,7 +133,7 @@ union vsx_reg {
> * otherwise.
> */
> extern int analyse_instr(struct instruction_op *op, const struct pt_regs
> *regs,
> - unsigned int instr);
> + ppc_inst instr);
>
> /*
> * Emulate an instruction that can be executed just by updating
> @@ -149,7 +150,7 @@ void emulate_update_regs(struct pt_regs *reg, struct
> instruction_op *op);
> * 0 if it could not be emulated, or -1 for an instruction that
> * should not be emulated (rfid, mtmsrd clearing MSR_RI, etc.).
> */
> -extern int emulate_step(struct pt_regs *regs, unsigned int instr);
> +extern int emulate_step(struct pt_regs *regs, ppc_inst instr);
>
> /*
> * Emulate a load or store instruction by reading/writing the
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 92045ed64976..34594aaa44de 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs, unsigned int
> reg,
>
> int fix_alignment(struct pt_regs *regs)
> {
> - unsigned int instr;
> + ppc_inst instr;
> struct instruction_op op;
> int r, type;
>
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c
> b/arch/powerpc/kernel/hw_breakpoint.c
> index 2462cd7c565c..06b97353d231 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -24,6 +24,7 @@
> #include <asm/debug.h>
> #include <asm/debugfs.h>
> #include <asm/hvcall.h>
> +#include <asm/inst.h>
> #include <linux/uaccess.h>
>
> /*
> @@ -243,7 +244,7 @@ dar_range_overlaps(unsigned long dar, int size, struct
> arch_hw_breakpoint *info)
> static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
> struct arch_hw_breakpoint *info)
> {
> - unsigned int instr = 0;
> + ppc_inst instr = 0;
> int ret, type, size;
> struct instruction_op op;
> unsigned long addr = info->address;
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 337516df17d4..e7205adc9820 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -225,7 +225,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
> static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
> {
> int ret;
> - unsigned int insn = *p->ainsn.insn;
> + ppc_inst insn = *p->ainsn.insn;
>
> /* regs->nip is also adjusted if emulate_step returns 1 */
> ret = emulate_step(regs, insn);
> diff --git a/arch/powerpc/kernel/mce_power.c
> b/arch/powerpc/kernel/mce_power.c
> index 1cbf7f1a4e3d..e65616bb3a3e 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -20,6 +20,7 @@
> #include <asm/sstep.h>
> #include <asm/exception-64s.h>
> #include <asm/extable.h>
> +#include <asm/inst.h>
>
> /*
> * Convert an address related to an mm to a PFN. NOTE: we are in real
> @@ -365,7 +366,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> *regs, uint64_t *addr,
> * in real-mode is tricky and can lead to recursive
> * faults
> */
> - int instr;
> + ppc_inst instr;
> unsigned long pfn, instr_addr;
> struct instruction_op op;
> struct pt_regs tmp = *regs;
> @@ -373,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> *regs, uint64_t *addr,
> pfn = addr_to_pfn(regs, regs->nip);
> if (pfn != ULONG_MAX) {
> instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
> - instr = *(unsigned int *)(instr_addr);
> + instr = *(ppc_inst *)(instr_addr);
> if (!analyse_instr(&op, &tmp, instr)) {
> pfn = addr_to_pfn(regs, op.ea);
> *addr = op.ea;
> diff --git a/arch/powerpc/kernel/optprobes.c
> b/arch/powerpc/kernel/optprobes.c
> index 024f7aad1952..f5e8cce438a3 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -189,8 +189,8 @@ void patch_imm64_load_insns(unsigned long val,
> kprobe_opcode_t *addr)
>
> int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe
> *p)
> {
> - kprobe_opcode_t *buff, branch_op_callback, branch_emulate_step;
> - kprobe_opcode_t *op_callback_addr, *emulate_step_addr;
> + ppc_inst branch_op_callback, branch_emulate_step;
> + kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> long b_offset;
> unsigned long nip, size;
> int rc, i;
> @@ -251,11 +251,11 @@ int arch_prepare_optimized_kprobe(struct
> optimized_kprobe *op, struct kprobe *p)
> goto error;
> }
>
> - branch_op_callback = create_branch((unsigned int *)buff +
> TMPL_CALL_HDLR_IDX,
> + branch_op_callback = create_branch((ppc_inst *)buff +
> TMPL_CALL_HDLR_IDX,
> (unsigned long)op_callback_addr,
> BRANCH_SET_LINK);
>
> - branch_emulate_step = create_branch((unsigned int *)buff +
> TMPL_EMULATE_IDX,
> + branch_emulate_step = create_branch((ppc_inst *)buff +
> TMPL_EMULATE_IDX,
> (unsigned long)emulate_step_addr,
> BRANCH_SET_LINK);
>
> @@ -316,7 +316,7 @@ void arch_optimize_kprobes(struct list_head *oplist)
> memcpy(op->optinsn.copied_insn, op->kp.addr,
> RELATIVEJUMP_SIZE);
> patch_instruction(op->kp.addr,
> - create_branch((unsigned int *)op->kp.addr,
> + create_branch((ppc_inst *)op->kp.addr,
> (unsigned long)op->optinsn.insn, 0));
> list_del_init(&op->list);
> }
> diff --git a/arch/powerpc/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c
> index 7ea0ca044b65..5787ccffb4df 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -27,6 +27,7 @@
> #include <asm/code-patching.h>
> #include <asm/ftrace.h>
> #include <asm/syscall.h>
> +#include <asm/inst.h>
>
>
> #ifdef CONFIG_DYNAMIC_FTRACE
> @@ -40,23 +41,23 @@
> #define NUM_FTRACE_TRAMPS 8
> static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
>
> -static unsigned int
> +static ppc_inst
> ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> {
> - unsigned int op;
> + ppc_inst op;
>
> addr = ppc_function_entry((void *)addr);
>
> /* if (link) set op to 'bl' else 'b' */
> - op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> + op = create_branch((ppc_inst *)ip, addr, link ? 1 : 0);
>
> return op;
> }
>
> static int
> -ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new)
> +ftrace_modify_code(unsigned long ip, ppc_inst old, ppc_inst new)
> {
> - unsigned int replaced;
> + ppc_inst replaced;
>
> /*
> * Note:
> @@ -78,7 +79,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> unsigned int new)
> }
>
> /* replace the text with the new text */
> - if (patch_instruction((unsigned int *)ip, new))
> + if (patch_instruction((ppc_inst *)ip, new))
> return -EPERM;
>
> return 0;
> @@ -87,25 +88,25 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> unsigned int new)
> /*
> * Helper functions that are the same for both PPC64 and PPC32.
> */
> -static int test_24bit_addr(unsigned long ip, unsigned long addr)
> +static ppc_inst test_24bit_addr(unsigned long ip, unsigned long addr)
> {
> addr = ppc_function_entry((void *)addr);
>
> /* use the create_branch to verify that this offset can be branched */
> - return create_branch((unsigned int *)ip, addr, 0);
> + return create_branch((ppc_inst *)ip, addr, 0);
> }
>
> -static int is_bl_op(unsigned int op)
> +static int is_bl_op(ppc_inst op)
> {
> return (op & 0xfc000003) == 0x48000001;
> }
>
> -static int is_b_op(unsigned int op)
> +static int is_b_op(ppc_inst op)
> {
> return (op & 0xfc000003) == 0x48000000;
> }
>
> -static unsigned long find_bl_target(unsigned long ip, unsigned int op)
> +static unsigned long find_bl_target(unsigned long ip, ppc_inst op)
> {
> int offset;
>
> @@ -125,7 +126,7 @@ __ftrace_make_nop(struct module *mod,
> {
> unsigned long entry, ptr, tramp;
> unsigned long ip = rec->ip;
> - unsigned int op, pop;
> + ppc_inst op, pop;
>
> /* read where this goes */
> if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> @@ -204,7 +205,7 @@ __ftrace_make_nop(struct module *mod,
> }
> #endif /* CONFIG_MPROFILE_KERNEL */
>
> - if (patch_instruction((unsigned int *)ip, pop)) {
> + if (patch_instruction((ppc_inst *)ip, pop)) {
> pr_err("Patching NOP failed.\n");
> return -EPERM;
> }
> @@ -217,7 +218,7 @@ static int
> __ftrace_make_nop(struct module *mod,
> struct dyn_ftrace *rec, unsigned long addr)
> {
> - unsigned int op;
> + ppc_inst op;
> unsigned int jmp[4];
> unsigned long ip = rec->ip;
> unsigned long tramp;
> @@ -276,7 +277,7 @@ __ftrace_make_nop(struct module *mod,
>
> op = PPC_INST_NOP;
>
> - if (patch_instruction((unsigned int *)ip, op))
> + if (patch_instruction((ppc_inst *)ip, op))
> return -EPERM;
>
> return 0;
> @@ -322,7 +323,8 @@ static int add_ftrace_tramp(unsigned long tramp)
> */
> static int setup_mcount_compiler_tramp(unsigned long tramp)
> {
> - int i, op;
> + int i;
> + ppc_inst op;
> unsigned long ptr;
> static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
>
> @@ -388,7 +390,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> tramp)
> static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long
> addr)
> {
> unsigned long tramp, ip = rec->ip;
> - unsigned int op;
> + ppc_inst op;
>
> /* Read where this goes */
> if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> @@ -416,7 +418,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace
> *rec, unsigned long addr)
> }
> }
>
> - if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> + if (patch_instruction((ppc_inst *)ip, PPC_INST_NOP)) {
> pr_err("Patching NOP failed.\n");
> return -EPERM;
> }
> @@ -428,7 +430,7 @@ int ftrace_make_nop(struct module *mod,
> struct dyn_ftrace *rec, unsigned long addr)
> {
> unsigned long ip = rec->ip;
> - unsigned int old, new;
> + ppc_inst old, new;
>
> /*
> * If the calling address is more that 24 bits away,
> @@ -481,7 +483,7 @@ int ftrace_make_nop(struct module *mod,
> */
> #ifndef CONFIG_MPROFILE_KERNEL
> static int
> -expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> +expected_nop_sequence(void *ip, ppc_inst op0, ppc_inst op1)
> {
> /*
> * We expect to see:
> @@ -510,7 +512,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> unsigned int op1)
> static int
> __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> {
> - unsigned int op[2];
> + ppc_inst op[2];
> void *ip = (void *)rec->ip;
> unsigned long entry, ptr, tramp;
> struct module *mod = rec->arch.mod;
> @@ -574,7 +576,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> static int
> __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> {
> - unsigned int op;
> + ppc_inst op;
> unsigned long ip = rec->ip;
>
> /* read where this goes */
> @@ -594,7 +596,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
> }
>
> /* create the branch to the trampoline */
> - op = create_branch((unsigned int *)ip,
> + op = create_branch((ppc_inst *)ip,
> rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> if (!op) {
> pr_err("REL24 out of range!\n");
> @@ -613,7 +615,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long
> addr)
>
> static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long
> addr)
> {
> - unsigned int op;
> + ppc_inst op;
> void *ip = (void *)rec->ip;
> unsigned long tramp, entry, ptr;
>
> @@ -661,7 +663,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace
> *rec, unsigned long addr)
> int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> {
> unsigned long ip = rec->ip;
> - unsigned int old, new;
> + ppc_inst old, new;
>
> /*
> * If the calling address is more that 24 bits away,
> @@ -700,7 +702,7 @@ static int
> __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
> unsigned long addr)
> {
> - unsigned int op;
> + ppc_inst op;
> unsigned long ip = rec->ip;
> unsigned long entry, ptr, tramp;
> struct module *mod = rec->arch.mod;
> @@ -748,7 +750,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> long old_addr,
> /* The new target may be within range */
> if (test_24bit_addr(ip, addr)) {
> /* within range */
> - if (patch_branch((unsigned int *)ip, addr, BRANCH_SET_LINK)) {
> + if (patch_branch((ppc_inst *)ip, addr, BRANCH_SET_LINK)) {
> pr_err("REL24 out of range!\n");
> return -EINVAL;
> }
> @@ -776,7 +778,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> long old_addr,
> }
>
> /* Ensure branch is within 24 bits */
> - if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> + if (!create_branch((ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
> pr_err("Branch out of range\n");
> return -EINVAL;
> }
> @@ -794,7 +796,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> long old_addr,
> unsigned long addr)
> {
> unsigned long ip = rec->ip;
> - unsigned int old, new;
> + ppc_inst old, new;
>
> /*
> * If the calling address is more that 24 bits away,
> @@ -834,7 +836,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned
> long old_addr,
> int ftrace_update_ftrace_func(ftrace_func_t func)
> {
> unsigned long ip = (unsigned long)(&ftrace_call);
> - unsigned int old, new;
> + ppc_inst old, new;
> int ret;
>
> old = *(unsigned int *)&ftrace_call;
> @@ -919,7 +921,7 @@ int ftrace_enable_ftrace_graph_caller(void)
> unsigned long ip = (unsigned long)(&ftrace_graph_call);
> unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> - unsigned int old, new;
> + ppc_inst old, new;
>
> old = ftrace_call_replace(ip, stub, 0);
> new = ftrace_call_replace(ip, addr, 0);
> @@ -932,7 +934,7 @@ int ftrace_disable_ftrace_graph_caller(void)
> unsigned long ip = (unsigned long)(&ftrace_graph_call);
> unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> - unsigned int old, new;
> + ppc_inst old, new;
>
> old = ftrace_call_replace(ip, addr, 0);
> new = ftrace_call_replace(ip, stub, 0);
> diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> b/arch/powerpc/kvm/emulate_loadstore.c
> index 1139bc56e004..1c9bcbfeb924 100644
> --- a/arch/powerpc/kvm/emulate_loadstore.c
> +++ b/arch/powerpc/kvm/emulate_loadstore.c
> @@ -21,6 +21,7 @@
> #include <asm/disassemble.h>
> #include <asm/ppc-opcode.h>
> #include <asm/sstep.h>
> +#include <asm/inst.h>
> #include "timing.h"
> #include "trace.h"
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index 3345f039a876..8492b9e2b8db 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -17,9 +17,10 @@
> #include <asm/page.h>
> #include <asm/code-patching.h>
> #include <asm/setup.h>
> +#include <asm/inst.h>
>
> -static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
> - unsigned int *patch_addr)
> +static int __patch_instruction(ppc_inst *exec_addr, ppc_inst instr,
> + ppc_inst *patch_addr)
> {
> int err = 0;
>
> @@ -33,7 +34,7 @@ static int __patch_instruction(unsigned int *exec_addr,
> unsigned int instr,
> return 0;
> }
>
> -int raw_patch_instruction(unsigned int *addr, unsigned int instr)
> +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> {
> return __patch_instruction(addr, instr, addr);
> }
> @@ -136,10 +137,10 @@ static inline int unmap_patch_area(unsigned long addr)
> return 0;
> }
>
> -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> {
> int err;
> - unsigned int *patch_addr = NULL;
> + ppc_inst *patch_addr = NULL;
> unsigned long flags;
> unsigned long text_poke_addr;
> unsigned long kaddr = (unsigned long)addr;
> @@ -176,7 +177,7 @@ static int do_patch_instruction(unsigned int *addr,
> unsigned int instr)
> }
> #else /* !CONFIG_STRICT_KERNEL_RWX */
>
> -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> {
> return raw_patch_instruction(addr, instr);
> }
> @@ -194,7 +195,7 @@ int patch_instruction(unsigned int *addr, unsigned int
> instr)
> }
> NOKPROBE_SYMBOL(patch_instruction);
>
> -int patch_branch(unsigned int *addr, unsigned long target, int flags)
> +int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> {
> return patch_instruction(addr, create_branch(addr, target, flags));
> }
> @@ -225,7 +226,7 @@ bool is_offset_in_branch_range(long offset)
> * Helper to check if a given instruction is a conditional branch
> * Derived from the conditional checks in analyse_instr()
> */
> -bool is_conditional_branch(unsigned int instr)
> +bool is_conditional_branch(ppc_inst instr)
> {
> unsigned int opcode = instr >> 26;
>
> @@ -243,10 +244,10 @@ bool is_conditional_branch(unsigned int instr)
> }
> NOKPROBE_SYMBOL(is_conditional_branch);
>
> -unsigned int create_branch(const unsigned int *addr,
> +ppc_inst create_branch(const ppc_inst *addr,
> unsigned long target, int flags)
> {
> - unsigned int instruction;
> + ppc_inst instruction;
> long offset;
>
> offset = target;
> @@ -266,7 +267,7 @@ unsigned int create_branch(const unsigned int *addr,
> unsigned int create_cond_branch(const unsigned int *addr,
> unsigned long target, int flags)
> {
> - unsigned int instruction;
> + ppc_inst instruction;
> long offset;
>
> offset = target;
> @@ -283,22 +284,22 @@ unsigned int create_cond_branch(const unsigned int
> *addr,
> return instruction;
> }
>
> -static unsigned int branch_opcode(unsigned int instr)
> +static unsigned int branch_opcode(ppc_inst instr)
> {
> return (instr >> 26) & 0x3F;
> }
>
> -static int instr_is_branch_iform(unsigned int instr)
> +static int instr_is_branch_iform(ppc_inst instr)
> {
> return branch_opcode(instr) == 18;
> }
>
> -static int instr_is_branch_bform(unsigned int instr)
> +static int instr_is_branch_bform(ppc_inst instr)
> {
> return branch_opcode(instr) == 16;
> }
>
> -int instr_is_relative_branch(unsigned int instr)
> +int instr_is_relative_branch(ppc_inst instr)
> {
> if (instr & BRANCH_ABSOLUTE)
> return 0;
> @@ -306,12 +307,12 @@ int instr_is_relative_branch(unsigned int instr)
> return instr_is_branch_iform(instr) || instr_is_branch_bform(instr);
> }
>
> -int instr_is_relative_link_branch(unsigned int instr)
> +int instr_is_relative_link_branch(ppc_inst instr)
> {
> return instr_is_relative_branch(instr) && (instr & BRANCH_SET_LINK);
> }
>
> -static unsigned long branch_iform_target(const unsigned int *instr)
> +static unsigned long branch_iform_target(const ppc_inst *instr)
> {
> signed long imm;
>
> @@ -327,7 +328,7 @@ static unsigned long branch_iform_target(const unsigned
> int *instr)
> return (unsigned long)imm;
> }
>
> -static unsigned long branch_bform_target(const unsigned int *instr)
> +static unsigned long branch_bform_target(const ppc_inst *instr)
> {
> signed long imm;
>
> @@ -343,7 +344,7 @@ static unsigned long branch_bform_target(const unsigned
> int *instr)
> return (unsigned long)imm;
> }
>
> -unsigned long branch_target(const unsigned int *instr)
> +unsigned long branch_target(const ppc_inst *instr)
> {
> if (instr_is_branch_iform(*instr))
> return branch_iform_target(instr);
> @@ -353,7 +354,7 @@ unsigned long branch_target(const unsigned int *instr)
> return 0;
> }
>
> -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr)
> +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr)
> {
> if (instr_is_branch_iform(*instr) || instr_is_branch_bform(*instr))
> return branch_target(instr) == addr;
> @@ -361,7 +362,7 @@ int instr_is_branch_to_addr(const unsigned int *instr,
> unsigned long addr)
> return 0;
> }
>
> -unsigned int translate_branch(const unsigned int *dest, const unsigned int
> *src)
> +ppc_inst translate_branch(const ppc_inst *dest, const ppc_inst *src)
> {
> unsigned long target;
>
> @@ -403,7 +404,7 @@ static void __init test_trampoline(void)
>
> static void __init test_branch_iform(void)
> {
> - unsigned int instr;
> + ppc_inst instr;
> unsigned long addr;
>
> addr = (unsigned long)&instr;
> @@ -478,11 +479,11 @@ static void __init test_branch_iform(void)
>
> static void __init test_create_function_call(void)
> {
> - unsigned int *iptr;
> + ppc_inst *iptr;
> unsigned long dest;
>
> /* Check we can create a function call */
> - iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> + iptr = (ppc_inst *)ppc_function_entry(test_trampoline);
> dest = ppc_function_entry(test_create_function_call);
> patch_instruction(iptr, create_branch(iptr, dest, BRANCH_SET_LINK));
> check(instr_is_branch_to_addr(iptr, dest));
> @@ -491,7 +492,8 @@ static void __init test_create_function_call(void)
> static void __init test_branch_bform(void)
> {
> unsigned long addr;
> - unsigned int *iptr, instr, flags;
> + ppc_inst *iptr, instr;
> + unsigned int flags;
>
> iptr = &instr;
> addr = (unsigned long)iptr;
> @@ -561,7 +563,7 @@ static void __init test_branch_bform(void)
> static void __init test_translate_branch(void)
> {
> unsigned long addr;
> - unsigned int *p, *q;
> + ppc_inst *p, *q;
> void *buf;
>
> buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index c077acb983a1..1d9c766a89fe 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1163,7 +1163,7 @@ static nokprobe_inline int trap_compare(long v1, long
> v2)
> * otherwise.
> */
> int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> - unsigned int instr)
> + ppc_inst instr)
> {
> unsigned int opcode, ra, rb, rc, rd, spr, u;
> unsigned long int imm;
> @@ -3101,7 +3101,7 @@ NOKPROBE_SYMBOL(emulate_loadstore);
> * or -1 if the instruction is one that should not be stepped,
> * such as an rfid, or a mtmsrd that would clear MSR_RI.
> */
> -int emulate_step(struct pt_regs *regs, unsigned int instr)
> +int emulate_step(struct pt_regs *regs, ppc_inst instr)
> {
> struct instruction_op op;
> int r, err, type;
> diff --git a/arch/powerpc/lib/test_emulate_step.c
> b/arch/powerpc/lib/test_emulate_step.c
> index 42347067739c..158efc8a0f53 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -460,7 +460,7 @@ struct compute_test {
> struct {
> char *descr;
> unsigned long flags;
> - unsigned int instr;
> + ppc_inst instr;
> struct pt_regs regs;
> } subtests[MAX_SUBTESTS + 1];
> };
> @@ -841,7 +841,7 @@ static struct compute_test compute_tests[] = {
> };
>
> static int __init emulate_compute_instr(struct pt_regs *regs,
> - unsigned int instr)
> + ppc_inst instr)
> {
> struct instruction_op op;
>
> @@ -859,7 +859,7 @@ static int __init emulate_compute_instr(struct pt_regs
> *regs,
> }
>
> static int __init execute_compute_instr(struct pt_regs *regs,
> - unsigned int instr)
> + ppc_inst instr)
> {
> extern int exec_instr(struct pt_regs *regs);
> extern s32 patch__exec_instr;
> @@ -890,7 +890,8 @@ static void __init run_tests_compute(void)
> unsigned long flags;
> struct compute_test *test;
> struct pt_regs *regs, exp, got;
> - unsigned int i, j, k, instr;
> + unsigned int i, j, k;
> + ppc_inst instr;
> bool ignore_gpr, ignore_xer, ignore_ccr, passed;
>
> for (i = 0; i < ARRAY_SIZE(compute_tests); i++) {
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 7875d1a37770..a0bc442f9557 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -892,7 +892,7 @@ static struct bpt *new_breakpoint(unsigned long a)
> static void insert_bpts(void)
> {
> int i;
> - unsigned int instr;
> + ppc_inst instr;
> struct bpt *bp;
>
> bp = bpts;
> @@ -914,7 +914,7 @@ static void insert_bpts(void)
> patch_instruction(bp->instr, instr);
> if (bp->enabled & BP_CIABR)
> continue;
> - if (patch_instruction((unsigned int *)bp->address,
> + if (patch_instruction((ppc_inst *)bp->address,
> bpinstr) != 0) {
> printf("Couldn't write instruction at %lx, "
> "disabling breakpoint there\n", bp->address);
> @@ -943,7 +943,7 @@ static void remove_bpts(void)
> {
> int i;
> struct bpt *bp;
> - unsigned instr;
> + ppc_inst instr;
>
> bp = bpts;
> for (i = 0; i < NBPTS; ++i, ++bp) {
> @@ -952,7 +952,7 @@ static void remove_bpts(void)
> if (mread(bp->address, &instr, 4) == 4
> && instr == bpinstr
> && patch_instruction(
> - (unsigned int *)bp->address, bp->instr[0]) != 0)
> + (ppc_inst *)bp->address, bp->instr[0]) != 0)
> printf("Couldn't remove breakpoint at %lx\n",
> bp->address);
> }
> @@ -1159,7 +1159,7 @@ static int do_step(struct pt_regs *regs)
> */
> static int do_step(struct pt_regs *regs)
> {
> - unsigned int instr;
> + ppc_inst instr;
> int stepped;
>
> force_enable_xmon();
> @@ -1325,7 +1325,7 @@ csum(void)
> */
> static long check_bp_loc(unsigned long addr)
> {
> - unsigned int instr;
> + ppc_inst instr;
>
> addr &= ~3;
> if (!is_kernel_addr(addr)) {
^ permalink raw reply
* Re: [PATCH v4 02/16] xmon: Move out-of-line instructions to text section
From: Balamuruhan S @ 2020-03-23 5:59 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, npiggin
In-Reply-To: <20200320051809.24332-3-jniethe5@gmail.com>
On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> To execute an instruction out of line after a breakpoint, the NIP is
> set
> to the address of struct bpt::instr. Here a copy of the instruction
> that
> was replaced with a breakpoint is kept, along with a trap so normal
> flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked
> as
> no execute.
>
> Instead of each struct bpt holding the instructions to be XOL'd, make
> a
> new array, bpt_table[], with enough space to hold instructions for
> the
> number of supported breakpoints. Place this array in the text
> section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple
> mapping:
> bpts[n] -> bpt_table[n * words per breakpoint].
Can it separate commits ?
* introduce the array bpt_table[] and make struct bpt::instr a
pointer to the instructions in bpt_table[].
* place the array in text section.
> Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/kernel/vmlinux.lds.S | 2 +-
> arch/powerpc/xmon/xmon.c | 22 +++++++++++++---------
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S
> b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..e90845b8c300 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -86,7 +86,7 @@ SECTIONS
> ALIGN_FUNCTION();
> #endif
> /* careful! __ftr_alt_* sections need to be close to
> .text */
> - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text);
> + *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text .text.xmon_bpts);
> #ifdef CONFIG_PPC64
> *(.tramp.ftrace.text);
> #endif
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..7875d1a37770 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> /* Breakpoint stuff */
> struct bpt {
> unsigned long address;
> - unsigned int instr[2];
> + unsigned int *instr;
> atomic_t ref_count;
> int enabled;
> unsigned long pad;
> @@ -109,6 +109,7 @@ struct bpt {
> #define BP_DABR 4
>
> #define NBPTS 256
> +#define BPT_WORDS 2
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008; /* trap
> */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> BPT_WORDS];
> +
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long
> pc)
> static struct bpt *in_breakpoint_table(unsigned long nip, unsigned
> long *offp)
> {
> unsigned long off;
> + unsigned long bp_off;
>
> - off = nip - (unsigned long) bpts;
> - if (off >= sizeof(bpts))
> + off = nip - (unsigned long) bpt_table;
> + if (off >= sizeof(bpt_table))
> return NULL;
> - off %= sizeof(struct bpt);
> - if (off != offsetof(struct bpt, instr[0])
> - && off != offsetof(struct bpt, instr[1]))
> + bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> + if (bp_off != 0 && bp_off != 4)
> return NULL;
> - *offp = off - offsetof(struct bpt, instr[0]);
> - return (struct bpt *) (nip - off);
> + *offp = bp_off;
> + return bpts + ((off - bp_off) / (sizeof(unsigned int) *
> BPT_WORDS));
`(off - bp_off) / (sizeof(unsigned int) * BPT_WORDS)` seems to be the
actual breakpoint offset. Can we have something like,
#define NBPTS 256
#define BPT_WORDS 2
#define BPT_WORDS_SIZE (sizeof(unsigned int) * BPT_WORDS)
#define BPT_OFFSET(off, bp_word_off) ((off - bp_word_off) / BPT_WOR
DS_SIZE);
:::
:::
:::
bp_word_off = off % BPT_WORDS_SIZE;
if (bp_word_off != 0 && bp_word_off != 4)
return NULL;
*offp = bp_word_off;
return bpts + BPT_OFFSET(off, bp_word_off);
-- Bala
> }
static struct bpt *new_breakpoint(unsigned long a)
@@ -876,7
> +879,8 @@ static struct bpt *new_breakpoint(unsigned long a)
for (bp
> = bpts; bp < &bpts[NBPTS]; ++bp) {
if (!bp->enabled &&
> atomic_read(&bp->ref_count) == 0) {
bp->address =
> a;
- patch_instruction(&bp->instr[1], bpinstr);
+
> bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
+
> patch_instruction(bp->instr + 1, bpinstr);
> return bp;
}
}
^ permalink raw reply
* Re: [PATCH v4 02/16] xmon: Move out-of-line instructions to text section
From: Balamuruhan S @ 2020-03-23 6:05 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, dja, npiggin
In-Reply-To: <20200320051809.24332-3-jniethe5@gmail.com>
On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> To execute an instruction out of line after a breakpoint, the NIP is
> set
> to the address of struct bpt::instr. Here a copy of the instruction
> that
> was replaced with a breakpoint is kept, along with a trap so normal
> flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked
> as
> no execute.
>
> Instead of each struct bpt holding the instructions to be XOL'd, make
> a
> new array, bpt_table[], with enough space to hold instructions for
> the
> number of supported breakpoints. Place this array in the text
> section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple
> mapping:
> bpts[n] -> bpt_table[n * words per breakpoint].
Can we have it in separate commits ?
* introduce the array bpt_table[] and make struct bpt::instr a
pointer to the instructions in bpt_table[].
* place the array in text section.
> Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
> arch/powerpc/kernel/vmlinux.lds.S | 2 +-
> arch/powerpc/xmon/xmon.c | 22 +++++++++++++---------
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S
> b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..e90845b8c300 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -86,7 +86,7 @@ SECTIONS
> ALIGN_FUNCTION();
> #endif
> /* careful! __ftr_alt_* sections need to be close to
> .text */
> - *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text);
> + *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text .text.xmon_bpts);
> #ifdef CONFIG_PPC64
> *(.tramp.ftrace.text);
> #endif
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..7875d1a37770 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> /* Breakpoint stuff */
> struct bpt {
> unsigned long address;
> - unsigned int instr[2];
> + unsigned int *instr;
> atomic_t ref_count;
> int enabled;
> unsigned long pad;
> @@ -109,6 +109,7 @@ struct bpt {
> #define BP_DABR 4
>
> #define NBPTS 256
> +#define BPT_WORDS 2
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008; /* trap
> */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> BPT_WORDS];
> +
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long
> pc)
> static struct bpt *in_breakpoint_table(unsigned long nip, unsigned
> long *offp)
> {
> unsigned long off;
> + unsigned long bp_off;
>
> - off = nip - (unsigned long) bpts;
> - if (off >= sizeof(bpts))
> + off = nip - (unsigned long) bpt_table;
> + if (off >= sizeof(bpt_table))
> return NULL;
> - off %= sizeof(struct bpt);
> - if (off != offsetof(struct bpt, instr[0])
> - && off != offsetof(struct bpt, instr[1]))
> + bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> + if (bp_off != 0 && bp_off != 4)
> return NULL;
> - *offp = off - offsetof(struct bpt, instr[0]);
> - return (struct bpt *) (nip - off);
> + *offp = bp_off;
> + return bpts + ((off - bp_off) / (sizeof(unsigned int) *
> BPT_WORDS));
`(off - bp_off) / (sizeof(unsigned int) * BPT_WORDS)` seems to be the
actual breakpoint offset. Can we have something like,
#define NBPTS 256
#define BPT_WORDS 2
#define BPT_WORDS_SIZE (sizeof(unsigned int) * BPT_WORDS)
#define BPT_OFFSET(off, bp_word_off) ((off - bp_word_off) / \ BPT_WORDS_SIZE)
;
:::
:::
:::
bp_word_off = off % BPT_WORDS_SIZE;
if (bp_word_off != 0 && bp_word_off != 4)
return NULL;
*offp = bp_word_off;
return bpts + BPT_OFFSET(off, bp_word_off);
-- Bala
^ 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;
as well as URLs for NNTP newsgroup(s).