* [PATCH kernel v2 0/7] powerpc/powenv/ioda: Allow 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
Here is an attempt to support bigger DMA space for devices
supporting DMA masks less than 59 bits (GPUs come into mind
first). POWER9 PHBs have an option to map 2 windows at 0
and select a windows based on DMA address being below or above
4GB.
This adds the "iommu=iommu_bypass" kernel parameter and
supports VFIO+pseries machine - current this requires telling
upstream+unmodified QEMU about this via
-global spapr-pci-host-bridge.dma64_win_addr=0x100000000
or per-phb property. 4/4 advertises the new option but
there is no automation around it in QEMU (should it be?).
For now it is either 1<<59 or 4GB mode; dynamic switching is
not supported (could be via sysfs).
This is a rebased version of
https://lore.kernel.org/kvm/20191202015953.127902-1-aik@ozlabs.ru/
The main change since v1 is that now it is 7 patches with
clearer separation of steps.
This is based on 6c90b86a745a "Merge tag 'mmc-v5.6-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc"
Please comment. Thanks.
Alexey Kardashevskiy (7):
powerpc/powernv/ioda: Move TCE bypass base to PE
powerpc/powernv/ioda: Rework for huge DMA window at 4GB
powerpc/powernv/ioda: Allow smaller TCE table levels
powerpc/powernv/phb4: Use IOMMU instead of bypassing
powerpc/iommu: Add a window number to
iommu_table_group_ops::get_table_size
powerpc/powernv/phb4: Add 4GB IOMMU bypass mode
vfio/spapr_tce: Advertise and allow a huge DMA windows at 4GB
arch/powerpc/include/asm/iommu.h | 3 +
arch/powerpc/include/asm/opal-api.h | 9 +-
arch/powerpc/include/asm/opal.h | 2 +
arch/powerpc/platforms/powernv/pci.h | 4 +-
include/uapi/linux/vfio.h | 2 +
arch/powerpc/platforms/powernv/npu-dma.c | 1 +
arch/powerpc/platforms/powernv/opal-call.c | 2 +
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 4 +-
arch/powerpc/platforms/powernv/pci-ioda.c | 234 ++++++++++++++----
drivers/vfio/vfio_iommu_spapr_tce.c | 17 +-
10 files changed, 213 insertions(+), 65 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH kernel v2 7/7] vfio/spapr_tce: Advertise and allow a huge DMA windows 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>
So far the only option for a big 64big DMA window was a window located
at 0x800.0000.0000.0000 (1<<59) which creates problems for devices
supporting smaller DMA masks.
This exploits a POWER9 PHB option to allow the second DMA window to map
at 0 and advertises it with a 4GB offset to avoid overlap with
the default 32bit window.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
include/uapi/linux/vfio.h | 2 ++
drivers/vfio/vfio_iommu_spapr_tce.c | 13 +++++++------
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 9e843a147ead..c7f89d47335a 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -831,9 +831,11 @@ struct vfio_iommu_spapr_tce_info {
__u32 argsz;
__u32 flags;
#define VFIO_IOMMU_SPAPR_INFO_DDW (1 << 0) /* DDW supported */
+#define VFIO_IOMMU_SPAPR_INFO_DDW_START (1 << 1) /* DDW offset */
__u32 dma32_window_start; /* 32 bit window start (bytes) */
__u32 dma32_window_size; /* 32 bit window size (bytes) */
struct vfio_iommu_spapr_tce_ddw_info ddw;
+ __u64 dma64_window_start;
};
#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 750a0676e9b7..315fd56e51a7 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -691,7 +691,7 @@ static long tce_iommu_create_window(struct tce_container *container,
container->tables[num] = tbl;
/* Return start address assigned by platform in create_table() */
- *start_addr = tbl->it_offset << tbl->it_page_shift;
+ *start_addr = (tbl->it_offset + tbl->it_tceoff) << tbl->it_page_shift;
return 0;
@@ -777,7 +777,7 @@ static long tce_iommu_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
{
struct tce_container *container = iommu_data;
- unsigned long minsz, ddwsz;
+ unsigned long minsz;
long ret;
switch (cmd) {
@@ -842,12 +842,13 @@ static long tce_iommu_ioctl(void *iommu_data,
info.ddw.levels = table_group->max_levels;
}
- ddwsz = offsetofend(struct vfio_iommu_spapr_tce_info, ddw);
+ info.flags |= VFIO_IOMMU_SPAPR_INFO_DDW_START;
+ info.dma64_window_start = table_group->tce64_start;
- if (info.argsz >= ddwsz)
- minsz = ddwsz;
+ if (info.argsz > sizeof(info))
+ info.argsz = sizeof(info);
- if (copy_to_user((void __user *)arg, &info, minsz))
+ if (copy_to_user((void __user *)arg, &info, info.argsz))
return -EFAULT;
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH kernel v2 6/7] powerpc/powernv/phb4: Add 4GB IOMMU bypass mode
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>
IODA2 systems (POWER8/9) allow DMA windows at 2 fixed locations - 0 and
0x800.0000.0000.0000==1<<59, stored in TVT as TVE0/1. PHB4 on POWER9 has
a "TVT Select 'GTE4GB' Option" which allows mapping both windows at 0 and
selecting one based on IOBA address - accesses below 4GB go via TVE0 and
above 4GB - via TVE1. Note that the TVE1's window still has to allocate
TCEs for below 4GB.
This changes iommu=iommy_bypass mode to move the second window at 4GB
if possible. When TVE1_4GB enabled, this creates a small (2GB typically)
32 bit window as there is no need to cover as much of lower DMA space -
the 4GB+ window does it better anyway.
As the physical TCE table from TVE1 maps PCI space from 0 and we want it
look like a 1:1 mapping with a fixed 4GB offset, this adds
a iommu_table::it_tceoff field which is a number of reserved TCEs covering
first 4GB of DMA space.
This keeps the existing behavior by default as the TVE1_4GB flag is set
per PHB by device assignment is done on PE basis and managing both modes
dynamically might get nasty.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/iommu.h | 1 +
arch/powerpc/include/asm/opal-api.h | 9 ++-
arch/powerpc/include/asm/opal.h | 2 +
arch/powerpc/platforms/powernv/opal-call.c | 2 +
arch/powerpc/platforms/powernv/pci-ioda.c | 66 +++++++++++++++++++---
5 files changed, 70 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index acf64a73ead1..b9c4af9f129c 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -97,6 +97,7 @@ struct iommu_table {
unsigned long it_level_size;
unsigned long it_allocated_size;
unsigned long it_offset; /* Offset into global table */
+ unsigned long it_tceoff;
unsigned long it_base; /* mapped address of tce table */
unsigned long it_index; /* which iommu table this is */
unsigned long it_type; /* type: PCI or Virtual Bus */
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index c1f25a760eb1..7873754f5ea6 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -214,7 +214,9 @@
#define OPAL_SECVAR_GET 176
#define OPAL_SECVAR_GET_NEXT 177
#define OPAL_SECVAR_ENQUEUE_UPDATE 178
-#define OPAL_LAST 178
+#define OPAL_PHB_SET_OPTION 179
+#define OPAL_PHB_GET_OPTION 180
+#define OPAL_LAST 180
#define QUIESCE_HOLD 1 /* Spin all calls at entry */
#define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
@@ -437,6 +439,11 @@ enum OpalSlotLedState {
OPAL_SLOT_LED_STATE_ON = 1 /* LED is ON */
};
+enum OpalPhbOption {
+ OPAL_PHB_OPTION_TVE1_4GB = 0x1,
+ OPAL_PHB_OPTION_MMIO_EEH_DISABLE = 0x2,
+};
+
/*
* Address cycle types for LPC accesses. These also correspond
* to the content of the first cell of the "reg" property for
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9986ac34b8e2..89b712288cdd 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -142,6 +142,8 @@ int64_t opal_pci_map_pe_dma_window(uint64_t phb_id, uint16_t pe_number, uint16_t
int64_t opal_pci_map_pe_dma_window_real(uint64_t phb_id, uint16_t pe_number,
uint16_t dma_window_number, uint64_t pci_start_addr,
uint64_t pci_mem_size);
+int64_t opal_phb_set_option(uint64_t phb_id, uint64_t opt, uint64_t setting);
+int64_t opal_phb_get_option(uint64_t phb_id, uint64_t opt, uint64_t *setting);
int64_t opal_pci_reset(uint64_t id, uint8_t reset_scope, uint8_t assert_state);
int64_t opal_pci_get_hub_diag_data(uint64_t hub_id, void *diag_buffer,
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 5cd0f52d258f..3130d5a41570 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -293,3 +293,5 @@ OPAL_CALL(opal_mpipl_query_tag, OPAL_MPIPL_QUERY_TAG);
OPAL_CALL(opal_secvar_get, OPAL_SECVAR_GET);
OPAL_CALL(opal_secvar_get_next, OPAL_SECVAR_GET_NEXT);
OPAL_CALL(opal_secvar_enqueue_update, OPAL_SECVAR_ENQUEUE_UPDATE);
+OPAL_CALL(opal_phb_set_option, OPAL_PHB_SET_OPTION);
+OPAL_CALL(opal_phb_get_option, OPAL_PHB_GET_OPTION);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 27a505a5edb4..cba2cb2e1119 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2367,7 +2367,7 @@ static long pnv_pci_ioda2_set_window(struct iommu_table_group *table_group,
}
static long pnv_pci_ioda2_set_bypass_iommu(struct pnv_ioda_pe *pe,
- unsigned long bus_offset)
+ unsigned long bus_offset, unsigned long tbl_offset)
{
struct pnv_phb *phb = pe->phb;
long rc;
@@ -2376,6 +2376,8 @@ static long pnv_pci_ioda2_set_bypass_iommu(struct pnv_ioda_pe *pe,
pgsizes = pnv_ioda_parse_tce_sizes(phb);
+ /* Filter sizes to have round number of TCEs to cover 0..tbl_offset */
+ pgsizes &= tbl_offset | (tbl_offset - 1);
if (!pgsizes)
return -1;
@@ -2386,17 +2388,19 @@ static long pnv_pci_ioda2_set_bypass_iommu(struct pnv_ioda_pe *pe,
1 /* window number */,
bus_offset,
__fls(pgsizes),
- roundup_pow_of_two(memory_hotplug_max()),
+ roundup_pow_of_two(memory_hotplug_max() +
+ tbl_offset),
2 /* levels */,
false /* userspace cache */,
&tbl);
if (rc)
return -1;
+ tbl->it_tceoff = tbl_offset >> tbl->it_page_shift;
for_each_memblock(memory, r)
pnv_ioda2_tce_build(tbl,
(r->base >> tbl->it_page_shift) +
- tbl->it_offset,
+ tbl->it_offset + tbl->it_tceoff,
r->size >> tbl->it_page_shift,
(unsigned long) __va(r->base),
DMA_BIDIRECTIONAL,
@@ -2438,8 +2442,22 @@ static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
}
if (pnv_iommu_bypass_mode == PNV_IOMMU_TCE_BYPASS) {
+ if (!opal_phb_set_option(phb->opal_id,
+ OPAL_PHB_OPTION_TVE1_4GB, 1)) {
+ pe->table_group.tce64_start = SZ_4G;
+ if (!pnv_pci_ioda2_set_bypass_iommu(pe,
+ pe->table_group.tce64_start, SZ_4G)) {
+ pe->tce_bypass_enabled = true;
+ pe_info(pe, "Enabled 64-bit IOMMU bypass at %llx\n",
+ pe->table_group.tce64_start);
+ return;
+ }
+ pe_err(pe, "Enabled TVE1_4GB but failed to configure TCE table");
+ opal_phb_set_option(phb->opal_id,
+ OPAL_PHB_OPTION_TVE1_4GB, 0);
+ }
if (!pnv_pci_ioda2_set_bypass_iommu(pe,
- pe->table_group.tce64_start)) {
+ pe->table_group.tce64_start, 0)) {
pe->tce_bypass_enabled = true;
pe_info(pe, "Enabled 64-bit IOMMU bypass at %llx\n",
pe->table_group.tce64_start);
@@ -2450,6 +2468,10 @@ static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
}
if (pnv_iommu_bypass_mode == PNV_IOMMU_NO_TRANSLATE) {
+ /*
+ * FIXME: if we enable dynamic switch, here we need to disable
+ * OPAL_PCI_PHB_FLAG_TVE1_4GB
+ */
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,
@@ -2521,6 +2543,15 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
*/
/* iommu_table::it_map uses 1 bit per IOMMU page, hence 8 */
window_size = min((maxblock * 8) << tceshift, max_memory);
+
+ /*
+ * If we get TVE#1_4GB on, there is no point in having a huge default
+ * DMA window.
+ */
+ if (pnv_iommu_bypass_mode == PNV_IOMMU_TCE_BYPASS)
+ window_size = min_t(u64, pe->table_group.tce32_size,
+ window_size);
+
/* Each TCE level cannot exceed maxblock so go multilevel if needed */
tces_order = ilog2(window_size >> tceshift);
tcelevel_order = ilog2(maxblock >> 3);
@@ -2611,6 +2642,9 @@ unsigned long pnv_pci_ioda2_get_table_size(int num, __u32 page_shift,
!is_power_of_2(window_size))
return 0;
+ if (pnv_iommu_bypass_mode == PNV_IOMMU_TCE_BYPASS && num == 1)
+ window_size = roundup_pow_of_two(window_size + SZ_4G);
+
/* Calculate a direct table size from window_size and levels */
entries_shift = (entries_shift + levels - 1) / levels;
table_shift = entries_shift + 3;
@@ -2636,15 +2670,29 @@ static long pnv_pci_ioda2_create_table_userspace(
{
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,
+ __u64 bus_offset, tce_offset = 0, win_size = window_size;
+ long ret;
+
+ if (num == 0) {
+ bus_offset = table_group->tce32_start;
+ } else if (table_group->tce64_start == SZ_4G) {
+ bus_offset = table_group->tce32_start;
+ tce_offset = SZ_4G;
+ win_size = roundup_pow_of_two(window_size + tce_offset);
+ } else {
+ bus_offset = table_group->tce64_start;
+ }
+
+ ret = pnv_pci_ioda2_create_table(pe->phb->hose->node,
+ num, bus_offset, page_shift, win_size, levels, true,
ptbl);
- if (!ret)
+ if (!ret) {
(*ptbl)->it_allocated_size = pnv_pci_ioda2_get_table_size(
num, page_shift, window_size, levels);
+ (*ptbl)->it_tceoff = tce_offset >> page_shift;
+ }
+
return ret;
}
--
2.17.1
^ permalink raw reply related
* [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
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