[PATCH v3 2/2] powerpc/powernv: Re-enable imc trace-mode in kernel
commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
mode switching between accumulation mode and trace mode via the spr LDBAR
in the hardware can trigger a checkstop(system crash).
Patch to re-enable imc-trace mode in kernel.
The previous patch(1/2) in this series will address the mode switching issue
by implementing a global lock, and will restrict the usage of
accumulation and trace-mode at a time.
Signed-off-by: Anju T Sudhakar
---
arch/powerpc/platforms/powernv/opal-imc.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c
b/arch/powerpc/platforms/powernv/opal-imc.c
index 000b350d4060..3b4518f4b643 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -278,14 +278,7 @@ static int opal_imc_counters_probe(struct platform_device
*pdev)
domain = IMC_DOMAIN_THREAD;
break;
case IMC_TYPE_TRACE:
- /*
-* FIXME. Using trace_imc events to monitor application
-* or KVM thread performance can cause a checkstop
-* (system crash).
-* Disable it for now.
-*/
- pr_info_once("IMC: disabling trace_imc PMU\n");
- domain = -1;
+ domain = IMC_DOMAIN_TRACE;
break;
default:
pr_warn("IMC Unknown Device type \n");
--
2.20.1
[PATCH v3 1/2] powerpc/perf: Implement a global lock to avoid races between trace, core and thread imc events.
IMC(In-memory Collection Counters) does performance monitoring in
two different modes, i.e accumulation mode(core-imc and thread-imc events),
and trace mode(trace-imc events). A cpu thread can either be in
accumulation-mode or trace-mode at a time and this is done via the LDBAR
register in POWER architecture. The current design does not address the
races between thread-imc and trace-imc events.
Patch implements a global id and lock to avoid the races between
core, trace and thread imc events. With this global id-lock
implementation, the system can either run core, thread or trace imc
events at a time. i.e. to run any core-imc events, thread/trace imc events
should not be enabled/monitored.
Signed-off-by: Anju T Sudhakar
---
Changes from v2->v3:
- Addressed the off-line comments from Michael Ellerman
- Optimized the *_event_init code path for trace, core and thread imc
- Handled the global refc in cpuhotplug scenario
- Re-order the patch series
- Removed the selftest patches and will send as a follow up patch
Changes from v1 -> v2:
- Added self test patches to the series.
---
arch/powerpc/perf/imc-pmu.c | 165 ++--
1 file changed, 141 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..a366e2ec0351 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -44,6 +44,16 @@ static DEFINE_PER_CPU(u64 *, trace_imc_mem);
static struct imc_pmu_ref *trace_imc_refc;
static int trace_imc_mem_size;
+/*
+ * Global data structure used to avoid races between thread,
+ * core and trace-imc
+ */
+static struct imc_pmu_ref imc_global_refc = {
+ .lock = __MUTEX_INITIALIZER(imc_global_refc.lock),
+ .id = 0,
+ .refc = 0,
+};
+
static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
{
return container_of(event->pmu, struct imc_pmu, pmu);
@@ -698,6 +708,13 @@ static int ppc_core_imc_cpu_offline(unsigned int cpu)
return -EINVAL;
ref->refc = 0;
+ /*
+* Reduce the global reference count, if this is the
+* last cpu in this core and core-imc event running
+* in this cpu.
+*/
+ if (imc_global_refc.id == IMC_DOMAIN_CORE)
+ imc_global_refc.refc--;
}
return 0;
}
@@ -710,6 +727,23 @@ static int core_imc_pmu_cpumask_init(void)
ppc_core_imc_cpu_offline);
}
+static void reset_global_refc(struct perf_event *event)
+{
+ mutex_lock(_global_refc.lock);
+ imc_global_refc.refc--;
+
+ /*
+* If no other thread is running any
+* event for this domain(thread/core/trace),
+* set the global id to zero.
+*/
+ if (imc_global_refc.refc <= 0) {
+ imc_global_refc.refc = 0;
+ imc_global_refc.id = 0;
+ }
+ mutex_unlock(_global_refc.lock);
+}
+
static void core_imc_counters_release(struct perf_event *event)
{
int rc, core_id;
@@ -759,6 +793,8 @@ static void core_imc_counters_release(struct perf_event
*event)
ref->refc = 0;
}
mutex_unlock(>lock);
+
+ reset_global_refc(event);
}
static int core_imc_event_init(struct perf_event *event)
@@ -819,6 +855,29 @@ static int core_imc_event_init(struct perf_event *event)
++ref->refc;
mutex_unlock(>lock);
+ /*
+* Since the system can run either in accumulation or trace-mode
+* of IMC at a time, core-imc events are allowed only if no other
+* trace/thread imc events are enabled/monitored.
+*
+* Take the global lock, and check the refc.id
+* to know whether any other trace/thread imc
+* events are running.
+*/
+ mutex_lock(_global_refc.lock);
+ if (imc_global_refc.id == 0 || imc_global_refc.id == IMC_DOMAIN_CORE) {
+ /*
+* No other trace/thread imc events are running in
+* the system, so set the refc.id to core-imc.
+*/
+ imc_global_refc.id = IMC_DOMAIN_CORE;
+ imc_global_refc.refc++;
+ } else {
+ mutex_unlock(_global_refc.lock);
+ return -EBUSY;
+ }
+ mutex_unlock(_global_refc.lock);
+
event->hw.event_base = (u64)pcmi->vbase + (config &
IMC_EVENT_OFFSET_MASK);
event->destroy = core_imc_counters_release;
return 0;
@@ -877,7 +936,20 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
static int ppc_thread_imc_cpu_offline(unsigned int cpu)
{
- mtspr(SPRN_LDBAR, 0);
+ /*
+* Set the bit 0 of LDBAR to zero.
+*
+* If bit 0 of LDBAR is unset, it will stop posting
+* the counetr data to memory.
+* For
Re: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > +{
> > + int i, rc;
> > +
> > + for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > + rc = device_create_file(>dev, [i]);
> > + if (rc) {
> > + for (; --i >= 0;)
> > + device_remove_file(>dev, [i]);
>
> I'd rather avoid weird for loop constructs if possible.
>
> Is it actually dangerous to call device_remove_file() on an attr that hasn't
> been added? If not then I'd rather define an err: label and loop over the
> whole array there.
None of this should be used at all, just use attribute groups properly
and the driver core will handle this all for you.
device_create/remove_file should never be called by anyone anymore if at all
possible.
thanks,
greg k-h
Re: [PATCH v3 2/4] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
On Fri, Feb 28, 2020 at 2:41 AM Nicolin Chen wrote: > > On Thu, Feb 27, 2020 at 10:41:56AM +0800, Shengjiu Wang wrote: > > There is a new ASRC included in i.MX serial platform, there > > are some common definition can be shared with each other. > > So move the common definition to a separate header file. > > > > And add fsl_asrc_pair_internal and fsl_asrc_internal for > > the variable specific for the module, which can be used > > internally. > > I think we can just call it "priv", instead of "internal", since > it's passed by the "void *private" pointer. > > And it'd be nicer to have an extra preparational patch to rename > existing "struct fsl_asrc *asrc_priv" to "struct fsl_asrc *asrc". > > Something like: > struct fsl_asrc *asrc = ; > struct fsl_asrc_pair *pair = ; > struct fsl_asrc_priv *asrc_priv = asrc->private; > struct fsl_asrc_pair_priv *pair_priv = pair->private; > > Thanks > -- > ok, will change it. best regards wang shengjiu
Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
On Fri, Feb 28, 2020 at 2:40 PM Nicolin Chen wrote: > > On Fri, Feb 28, 2020 at 10:54:02AM +0800, Shengjiu Wang wrote: > > Hi > > > > On Fri, Feb 28, 2020 at 1:45 AM Nicolin Chen wrote: > > > > > > On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote: > > > > On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen > > > > wrote: > > > > > > > > > > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote: > > > > > > asrc_format is more inteligent variable, which is align > > > > > > with the alsa definition snd_pcm_format_t. > > > > > > > > > > > > Signed-off-by: Shengjiu Wang > > > > > > --- > > > > > > sound/soc/fsl/fsl_asrc.c | 23 +++ > > > > > > sound/soc/fsl/fsl_asrc.h | 4 ++-- > > > > > > sound/soc/fsl/fsl_asrc_dma.c | 2 +- > > > > > > 3 files changed, 14 insertions(+), 15 deletions(-) > > > > > > > > > > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c > > > > > > index 0dcebc24c312..2b6a1643573c 100644 > > > > > > --- a/sound/soc/fsl/fsl_asrc.c > > > > > > +++ b/sound/soc/fsl/fsl_asrc.c > > > > > > > > > > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct > > > > > > snd_pcm_substream *substream, > > > > > > > > > > > > pair->config = > > > > > > > > > > > > - if (asrc_priv->asrc_width == 16) > > > > > > - format = SNDRV_PCM_FORMAT_S16_LE; > > > > > > - else > > > > > > - format = SNDRV_PCM_FORMAT_S24_LE; > > > > > > > > > > It feels better to me that we have format settings in hw_params(). > > > > > > > > > > Why not let fsl_easrc align with this? Any reason that I'm missing? > > > > > > > > because the asrc_width is not formal, in the future we can direct > > > > > > Hmm..that's our DT binding. And I don't feel it is a problem > > > to be ASoC irrelative. > > > > > > > input the format from the dts. format involve the info about width. > > > > > > Is there such any formal ASoC binding? I don't see those PCM > > > formats under include/dt-bindings/ folder. How are we going > > > to involve those formats in DT? > > > > There is no formal binding of this case. > > > > I think it is not good to convert width to format, because, for example > > The thing is that fsl_easrc does the conversion too... It just > does in the probe instead of hw_params(), and then copies them > in the hw_params(). So I don't see obvious benefit by doing so. > > > width = 24, there is two option, we can select format S24_LE, or > > format S24_3LE, width is ambiguous for selecting. > > > > In EASRC, it support other two 24bit format U24_LE, U24_3LE . > > I understood the reason here, but am not seeing the necessity, > at this point. > > > if we use the format in DT, then it is clear for usage in driver. > > I think this is the thing that we should address first. If we > have such a binding being added with the new fsl_easrc driver, > I'd love to see the old driver aligning with the new one. > > Otherwise, we can keep the old way, and change it when the new > binding is ready. ok, I will change the binding this time in next version. best regards wang shengjiu
Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
if dumping to fsdax, it will take a very long time.
Take a closer look, during the papr_scm initialization, the only
configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
...), which helps to set up the bound address.
On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
step can be stepped around to save times. So the pmem bound address can be
passed to the 2nd kernel through a dynamic added property "bound-addr" in
dt node 'ibm,pmemory'.
Signed-off-by: Pingfan Liu
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Hari Bathini
Cc: Aneesh Kumar K.V
Cc: Oliver O'Halloran
Cc: Dan Williams
Cc: ke...@lists.infradead.org
---
note: I can not find such a pseries machine, and not finish it yet.
---
arch/powerpc/platforms/pseries/papr_scm.c | 32 +--
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index c2ef320..555e746 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
u32 drc_index, metadata_size;
- u64 blocks, block_size;
+ u64 blocks, block_size, bound_addr = 0;
struct papr_scm_priv *p;
const char *uuid_str;
u64 uuid[2];
@@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
p->metadata_size = metadata_size;
p->pdev = pdev;
- /* request the hypervisor to bind this region to somewhere in memory */
- rc = drc_pmem_bind(p);
+ of_property_read_u64(dn, "bound-addr", _addr);
+ if (bound_addr)
+ p->bound_addr = bound_addr;
+ else {
All legs of an if/else must have { } when one leg need them, see codying
style.
+ struct property *property;
+ u64 big;
- /* If phyp says drc memory still bound then force unbound and retry */
- if (rc == H_OVERLAP)
- rc = drc_pmem_query_n_bind(p);
+ /* request the hypervisor to bind this region to somewhere in
memory */
+ rc = drc_pmem_bind(p);
- if (rc != H_SUCCESS) {
- dev_err(>pdev->dev, "bind err: %d\n", rc);
- rc = -ENXIO;
- goto err;
+ /* If phyp says drc memory still bound then force unbound and
retry */
+ if (rc == H_OVERLAP)
+ rc = drc_pmem_query_n_bind(p);
+
+ if (rc != H_SUCCESS) {
+ dev_err(>pdev->dev, "bind err: %d\n", rc);
+ rc = -ENXIO;
+ goto err;
+ }
+ big = cpu_to_be64(p->bound_addr);
+ property = new_property("bound-addr", sizeof(u64), ,
+ NULL);
Why plitting this line in two parts ? You have lines far longer above.
In powerpc we allow lines 90 chars long.
+ of_add_property(dn, property);
}
/* setup the resource for the newly bound range */
Christophe
[PATCH] powerpc/perf: Use SIER_USER_MASK while updating SPRN_SIER for EBB events
commit 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit book3s")
defines user mask for SIER. But this is not used in ebb_switch_out while
saving SPR's. Patch fixes this by updating SPRN_SIER with the user mask.
Fixes: 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit book3s")
Signed-off-by: Athira Rajeev
---
arch/powerpc/perf/core-book3s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3086055..48b61cc 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -579,7 +579,7 @@ static void ebb_switch_out(unsigned long mmcr0)
return;
current->thread.siar = mfspr(SPRN_SIAR);
- current->thread.sier = mfspr(SPRN_SIER);
+ current->thread.sier = mfspr(SPRN_SIER) & SIER_USER_MASK;
current->thread.sdar = mfspr(SPRN_SDAR);
current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
--
1.8.3.1
Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
Since new_property() is used in several calling sites, splitting it out for
reusing.
To ease the review, although the split out part has coding style issue,
keeping it untouched and fixed in next patch.
The moved function fits in one screen. I don't think it is worth
splitting out for coding style that applies on the new lines only. You
can squash the two patches, it will still be easy to review.
Signed-off-by: Pingfan Liu
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Hari Bathini
Cc: Aneesh Kumar K.V
Cc: Oliver O'Halloran
Cc: Dan Williams
Cc: ke...@lists.infradead.org
---
arch/powerpc/include/asm/prom.h | 2 ++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/of_property.c | 32 +++
arch/powerpc/mm/drmem.c | 26 -
arch/powerpc/platforms/pseries/reconfig.c | 26 -
5 files changed, 35 insertions(+), 53 deletions(-)
create mode 100644 arch/powerpc/kernel/of_property.c
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 94e3fd5..02f7b1b 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -90,6 +90,8 @@ struct of_drc_info {
extern int of_read_drc_info_cell(struct property **prop,
const __be32 **curval, struct of_drc_info *data);
+extern struct property *new_property(const char *name, const int length,
+ const unsigned char *value, struct property *last);
Don't add useless 'extern' keyword.
/*
* There are two methods for telling firmware what our capabilities are.
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 157b014..23375fd 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -47,7 +47,7 @@ obj-y := cputable.o ptrace.o
syscalls.o \
signal.o sysfs.o cacheinfo.o time.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
- of_platform.o prom_parse.o
+ of_platform.o prom_parse.o of_property.o
obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \
signal_64.o ptrace32.o \
paca.o nvram_64.o firmware.o note.o
diff --git a/arch/powerpc/kernel/of_property.c
b/arch/powerpc/kernel/of_property.c
new file mode 100644
index 000..e56c832
--- /dev/null
+++ b/arch/powerpc/kernel/of_property.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include
+#include
+#include
+#include
+
+struct property *new_property(const char *name, const int length,
+const unsigned char *value, struct
property *last)
+{
+ struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
+
+ if (!new)
+ return NULL;
+
+ if (!(new->name = kstrdup(name, GFP_KERNEL)))
+ goto cleanup;
+ if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
+ goto cleanup;
+
+ memcpy(new->value, value, length);
+ *(((char *)new->value) + length) = 0;
+ new->length = length;
+ new->next = last;
+ return new;
+
+cleanup:
+ kfree(new->name);
+ kfree(new->value);
+ kfree(new);
+ return NULL;
+}
+
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 85b088a..888227e 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2
*dr_cell,
extern int test_hotplug;
-static struct property *new_property(const char *name, const int length,
-const unsigned char *value, struct
property *last)
-{
- struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
- if (!new)
- return NULL;
-
- if (!(new->name = kstrdup(name, GFP_KERNEL)))
- goto cleanup;
- if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
- goto cleanup;
-
- memcpy(new->value, value, length);
- *(((char *)new->value) + length) = 0;
- new->length = length;
- new->next = last;
- return new;
-
-cleanup:
- kfree(new->name);
- kfree(new->value);
- kfree(new);
- return NULL;
-}
-
static int drmem_update_dt_v2(struct device_node *memory,
struct property *prop)
{
diff --git a/arch/powerpc/platforms/pseries/reconfig.c
b/arch/powerpc/platforms/pseries/reconfig.c
index 7f7369f..8e5a2ba 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -165,32 +165,6 @@ static char *
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
在 2020/2/28 13:53, Scott Wood 写道: On Wed, 2020-02-26 at 16:18 +0800, Jason Yan wrote: Hi Daniel, 在 2020/2/26 15:16, Daniel Axtens 写道: Hi Jason, This is a try to implement KASLR for Freescale BookE64 which is based on my earlier implementation for Freescale BookE32: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718 The implementation for Freescale BookE64 is similar as BookE32. One difference is that Freescale BookE64 set up a TLB mapping of 1G during booting. Another difference is that ppc64 needs the kernel to be 64K-aligned. So we can randomize the kernel in this 1G mapping and make it 64K-aligned. This can save some code to creat another TLB map at early boot. The disadvantage is that we only have about 1G/64K = 16384 slots to put the kernel in. KERNELBASE 64K |--> kernel <--| | | | +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ | | | || | | | | | | | | || | | +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ | |1G |-> offset<-| kernstart_virt_addr I'm not sure if the slot numbers is enough or the design has any defects. If you have some better ideas, I would be happy to hear that. Thank you all. Are you making any attempt to hide kernel address leaks in this series? Yes. I've just been looking at the stackdump code just now, and it directly prints link registers and stack pointers, which is probably enough to determine the kernel base address: SPs: LRs: %pS pointer [0.424506] [c000de403970] [c1fc0458] dump_stack+0xfc/0x154 (unreliable) [0.424593] [c000de4039c0] [c0267eec] panic+0x258/0x5ac [0.424659] [c000de403a60] [c24d7a00] mount_block_root+0x634/0x7c0 [0.424734] [c000de403be0] [c24d8100] prepare_namespace+0x1ec/0x23c [0.424811] [c000de403c60] [c24d7010] kernel_init_freeable+0x804/0x880 git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all in process.c or in xmon. Thanks for reminding this. Maybe replacing the REG format string in KASLR mode would be sufficient? Most archs have removed the address printing when dumping stack. Do we really have to print this? If we have to do this, maybe we can use "%pK" so that they will be hidden from unprivileged users. I've found the addresses to be useful, especially if I had a way to dump the stack data itself. Wouldn't the register dump also be likely to give away the addresses? If we have to print the address, then kptr_restrict and dmesg_restrict must be set properly so that unprivileged users cannot see them. I don't see any debug setting for %pK (or %p) to always print the actual address (closest is kptr_restrict=1 but that only works in certain contexts)... from looking at the code it seems it hashes even if kaslr is entirely disabled? Or am I missing something? Yes, %pK (or %p) always hashes whether kaslr is disabled or not. So if we want the real value of the address, we cannot use it. But if you only want to distinguish if two pointers are the same, it's ok. -Scott .
Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
On Fri, Feb 28, 2020 at 10:54:02AM +0800, Shengjiu Wang wrote: > Hi > > On Fri, Feb 28, 2020 at 1:45 AM Nicolin Chen wrote: > > > > On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote: > > > On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen > > > wrote: > > > > > > > > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote: > > > > > asrc_format is more inteligent variable, which is align > > > > > with the alsa definition snd_pcm_format_t. > > > > > > > > > > Signed-off-by: Shengjiu Wang > > > > > --- > > > > > sound/soc/fsl/fsl_asrc.c | 23 +++ > > > > > sound/soc/fsl/fsl_asrc.h | 4 ++-- > > > > > sound/soc/fsl/fsl_asrc_dma.c | 2 +- > > > > > 3 files changed, 14 insertions(+), 15 deletions(-) > > > > > > > > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c > > > > > index 0dcebc24c312..2b6a1643573c 100644 > > > > > --- a/sound/soc/fsl/fsl_asrc.c > > > > > +++ b/sound/soc/fsl/fsl_asrc.c > > > > > > > > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct > > > > > snd_pcm_substream *substream, > > > > > > > > > > pair->config = > > > > > > > > > > - if (asrc_priv->asrc_width == 16) > > > > > - format = SNDRV_PCM_FORMAT_S16_LE; > > > > > - else > > > > > - format = SNDRV_PCM_FORMAT_S24_LE; > > > > > > > > It feels better to me that we have format settings in hw_params(). > > > > > > > > Why not let fsl_easrc align with this? Any reason that I'm missing? > > > > > > because the asrc_width is not formal, in the future we can direct > > > > Hmm..that's our DT binding. And I don't feel it is a problem > > to be ASoC irrelative. > > > > > input the format from the dts. format involve the info about width. > > > > Is there such any formal ASoC binding? I don't see those PCM > > formats under include/dt-bindings/ folder. How are we going > > to involve those formats in DT? > > There is no formal binding of this case. > > I think it is not good to convert width to format, because, for example The thing is that fsl_easrc does the conversion too... It just does in the probe instead of hw_params(), and then copies them in the hw_params(). So I don't see obvious benefit by doing so. > width = 24, there is two option, we can select format S24_LE, or > format S24_3LE, width is ambiguous for selecting. > > In EASRC, it support other two 24bit format U24_LE, U24_3LE . I understood the reason here, but am not seeing the necessity, at this point. > if we use the format in DT, then it is clear for usage in driver. I think this is the thing that we should address first. If we have such a binding being added with the new fsl_easrc driver, I'd love to see the old driver aligning with the new one. Otherwise, we can keep the old way, and change it when the new binding is ready.
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Wed, 2020-02-26 at 16:18 +0800, Jason Yan wrote: > Hi Daniel, > > 在 2020/2/26 15:16, Daniel Axtens 写道: > > Hi Jason, > > > > > This is a try to implement KASLR for Freescale BookE64 which is based on > > > my earlier implementation for Freescale BookE32: > > > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718 > > > > > > The implementation for Freescale BookE64 is similar as BookE32. One > > > difference is that Freescale BookE64 set up a TLB mapping of 1G during > > > booting. Another difference is that ppc64 needs the kernel to be > > > 64K-aligned. So we can randomize the kernel in this 1G mapping and make > > > it 64K-aligned. This can save some code to creat another TLB map at > > > early boot. The disadvantage is that we only have about 1G/64K = 16384 > > > slots to put the kernel in. > > > > > > KERNELBASE > > > > > >64K |--> kernel <--| > > > | | | > > > +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ > > > | | | || | | | | | | | | || | | > > > +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ > > > | |1G > > > |-> offset<-| > > > > > >kernstart_virt_addr > > > > > > I'm not sure if the slot numbers is enough or the design has any > > > defects. If you have some better ideas, I would be happy to hear that. > > > > > > Thank you all. > > > > > > > Are you making any attempt to hide kernel address leaks in this series? > > Yes. > > > I've just been looking at the stackdump code just now, and it directly > > prints link registers and stack pointers, which is probably enough to > > determine the kernel base address: > > > >SPs: LRs: %pS pointer > > [0.424506] [c000de403970] [c1fc0458] dump_stack+0xfc/0x154 > > (unreliable) > > [0.424593] [c000de4039c0] [c0267eec] panic+0x258/0x5ac > > [0.424659] [c000de403a60] [c24d7a00] > > mount_block_root+0x634/0x7c0 > > [0.424734] [c000de403be0] [c24d8100] > > prepare_namespace+0x1ec/0x23c > > [0.424811] [c000de403c60] [c24d7010] > > kernel_init_freeable+0x804/0x880 > > > > git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all > > in process.c or in xmon. > > > > Thanks for reminding this. > > > Maybe replacing the REG format string in KASLR mode would be sufficient? > > > > Most archs have removed the address printing when dumping stack. Do we > really have to print this? > > If we have to do this, maybe we can use "%pK" so that they will be > hidden from unprivileged users. I've found the addresses to be useful, especially if I had a way to dump the stack data itself. Wouldn't the register dump also be likely to give away the addresses? I don't see any debug setting for %pK (or %p) to always print the actual address (closest is kptr_restrict=1 but that only works in certain contexts)... from looking at the code it seems it hashes even if kaslr is entirely disabled? Or am I missing something? -Scott
Re: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
+int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
+{
+ int i, rc;
+
+ for (i = 0; i < ARRAY_SIZE(attrs); i++) {
+ rc = device_create_file(>dev, [i]);
+ if (rc) {
+ for (; --i >= 0;)
+ device_remove_file(>dev, [i]);
I'd rather avoid weird for loop constructs if possible.
Is it actually dangerous to call device_remove_file() on an attr that
hasn't been added? If not then I'd rather define an err: label and loop
over the whole array there.
--
Andrew Donnellan OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
On Fri, Feb 28, 2020 at 2:03 PM Andrew Donnellan wrote: > > On 28/2/20 4:53 pm, Pingfan Liu wrote: > > Since new_property() is used in several calling sites, splitting it out for > > reusing. > > > > To ease the review, although the split out part has coding style issue, > > keeping it untouched and fixed in next patch. > > > > Signed-off-by: Pingfan Liu > > To: linuxppc-dev@lists.ozlabs.org > > Cc: Benjamin Herrenschmidt > > Cc: Paul Mackerras > > Cc: Michael Ellerman > > Cc: Hari Bathini > > Cc: Aneesh Kumar K.V > > Cc: Oliver O'Halloran > > Cc: Dan Williams > > Cc: ke...@lists.infradead.org > > Which tree does this apply to? I don't see a new_property() in mm/drmem.c... Sorry, there is mud in my git tree, I check, either linux git or powerpc git tree does not have this function. Nack this series, and I will send out V2 for patch 3/3. Thanks, Pingfan > > -- > Andrew Donnellan OzLabs, ADL Canberra > a...@linux.ibm.com IBM Australia Limited >
Re: [PATCH v3 22/27] powerpc/powernv/pmem: Implement the heartbeat command
On 21/2/20 2:27 pm, Alastair D'Silva wrote: From: Alastair D'Silva The heartbeat admin command is a simple admin command that exercises the communication mechanisms within the controller. This patch issues a heartbeat command to the card during init to ensure we can communicate with the card's controller. > Signed-off-by: Alastair D'Silva Looks okay. Reviewed-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v3 21/27] powerpc/powernv/pmem: Add an IOCTL to request controller health & perf data
On 21/2/20 2:27 pm, Alastair D'Silva wrote: From: Alastair D'Silva When health & performance data is requested from the controller, it responds with an error log containing the requested information. This patch allows the request to me issued via an IOCTL. A better explanation would be good - this IOCTL triggers a request to the controller to collect controller health/perf data, and the controller will later respond with an error log that can be picked up via the error log IOCTL that you've defined earlier. -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
While providing guests, it's desirable to resize it's memory on demand.
By now, it's possible to do so by creating a guest with a small base
memory, hot-plugging all the rest, and using 'movable_node' kernel
command-line parameter, which puts all hot-plugged memory in
ZONE_MOVABLE, allowing it to be removed whenever needed.
But there is an issue regarding guest reboot:
If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
It usually prevents this memory to be hot-removed from the guest.
It's possible to use device-tree information to fix that behavior, as
it stores flags for LMB ranges on ibm,dynamic-memory-vN.
It involves marking each memblock with the correct flags as hotpluggable
memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
'movable_node' is passed.
For base memory, qemu assigns these flags for it's LMBs:
(DRCONF_MEM_AI_INVALID | DRCONF_MEM_RESERVED)
For hot-plugged memory, it assigns (DRCONF_MEM_ASSIGNED).
While guest kernel reads the device-tree, early_init_drmem_lmb() is
called for every added LMBs, doing nothing for base memory, and adding
memblocks for hot-plugged memory. Skipping base memory happens here:
if ((lmb->flags & DRCONF_MEM_RESERVED) ||
!(lmb->flags & DRCONF_MEM_ASSIGNED))
return;
Marking memblocks added by this function as hotplugable memory
is enough to get the desirable behavior, and should cause no change
if 'movable_node' parameter is not passed to kernel.
Signed-off-by: Leonardo Bras
---
arch/powerpc/kernel/prom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6620f37abe73..f4d14c67bf53 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -518,6 +518,8 @@ static void __init early_init_drmem_lmb(struct drmem_lmb
*lmb,
DBG("Adding: %llx -> %llx\n", base, size);
if (validate_mem_limit(base, ))
memblock_add(base, size);
+
+ early_init_dt_mark_hotplug_memory_arch(base, size);
} while (--rngs);
}
#endif /* CONFIG_PPC_PSERIES */
--
2.24.1
Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
On 28/2/20 4:53 pm, Pingfan Liu wrote: Since new_property() is used in several calling sites, splitting it out for reusing. To ease the review, although the split out part has coding style issue, keeping it untouched and fixed in next patch. Signed-off-by: Pingfan Liu To: linuxppc-dev@lists.ozlabs.org Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Hari Bathini Cc: Aneesh Kumar K.V Cc: Oliver O'Halloran Cc: Dan Williams Cc: ke...@lists.infradead.org Which tree does this apply to? I don't see a new_property() in mm/drmem.c... -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
if dumping to fsdax, it will take a very long time.
Take a closer look, during the papr_scm initialization, the only
configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
...), which helps to set up the bound address.
On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
step can be stepped around to save times. So the pmem bound address can be
passed to the 2nd kernel through a dynamic added property "bound-addr" in
dt node 'ibm,pmemory'.
Signed-off-by: Pingfan Liu
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Hari Bathini
Cc: Aneesh Kumar K.V
Cc: Oliver O'Halloran
Cc: Dan Williams
Cc: ke...@lists.infradead.org
---
note: I can not find such a pseries machine, and not finish it yet.
---
arch/powerpc/platforms/pseries/papr_scm.c | 32 +--
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index c2ef320..555e746 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
u32 drc_index, metadata_size;
- u64 blocks, block_size;
+ u64 blocks, block_size, bound_addr = 0;
struct papr_scm_priv *p;
const char *uuid_str;
u64 uuid[2];
@@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
p->metadata_size = metadata_size;
p->pdev = pdev;
- /* request the hypervisor to bind this region to somewhere in memory */
- rc = drc_pmem_bind(p);
+ of_property_read_u64(dn, "bound-addr", _addr);
+ if (bound_addr)
+ p->bound_addr = bound_addr;
+ else {
+ struct property *property;
+ u64 big;
- /* If phyp says drc memory still bound then force unbound and retry */
- if (rc == H_OVERLAP)
- rc = drc_pmem_query_n_bind(p);
+ /* request the hypervisor to bind this region to somewhere in
memory */
+ rc = drc_pmem_bind(p);
- if (rc != H_SUCCESS) {
- dev_err(>pdev->dev, "bind err: %d\n", rc);
- rc = -ENXIO;
- goto err;
+ /* If phyp says drc memory still bound then force unbound and
retry */
+ if (rc == H_OVERLAP)
+ rc = drc_pmem_query_n_bind(p);
+
+ if (rc != H_SUCCESS) {
+ dev_err(>pdev->dev, "bind err: %d\n", rc);
+ rc = -ENXIO;
+ goto err;
+ }
+ big = cpu_to_be64(p->bound_addr);
+ property = new_property("bound-addr", sizeof(u64), ,
+ NULL);
+ of_add_property(dn, property);
}
/* setup the resource for the newly bound range */
--
2.7.5
[PATCH 2/3] powerpc/of: coding style cleanup
Signed-off-by: Pingfan Liu
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Hari Bathini
Cc: Aneesh Kumar K.V
Cc: Oliver O'Halloran
Cc: Dan Williams
Cc: ke...@lists.infradead.org
---
arch/powerpc/kernel/of_property.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/of_property.c
b/arch/powerpc/kernel/of_property.c
index e56c832..c6abf7e 100644
--- a/arch/powerpc/kernel/of_property.c
+++ b/arch/powerpc/kernel/of_property.c
@@ -5,16 +5,18 @@
#include
struct property *new_property(const char *name, const int length,
-const unsigned char *value, struct
property *last)
+ const unsigned char *value, struct property *last)
{
struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
if (!new)
return NULL;
- if (!(new->name = kstrdup(name, GFP_KERNEL)))
+ new->name = kstrdup(name, GFP_KERNEL);
+ if (!new->name)
goto cleanup;
- if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
+ new->value = kmalloc(length + 1, GFP_KERNEL);
+ if (!new->value)
goto cleanup;
memcpy(new->value, value, length);
--
2.7.5
[PATCH 1/3] powerpc/of: split out new_property() for reusing
Since new_property() is used in several calling sites, splitting it out for
reusing.
To ease the review, although the split out part has coding style issue,
keeping it untouched and fixed in next patch.
Signed-off-by: Pingfan Liu
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: Hari Bathini
Cc: Aneesh Kumar K.V
Cc: Oliver O'Halloran
Cc: Dan Williams
Cc: ke...@lists.infradead.org
---
arch/powerpc/include/asm/prom.h | 2 ++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/of_property.c | 32 +++
arch/powerpc/mm/drmem.c | 26 -
arch/powerpc/platforms/pseries/reconfig.c | 26 -
5 files changed, 35 insertions(+), 53 deletions(-)
create mode 100644 arch/powerpc/kernel/of_property.c
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 94e3fd5..02f7b1b 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -90,6 +90,8 @@ struct of_drc_info {
extern int of_read_drc_info_cell(struct property **prop,
const __be32 **curval, struct of_drc_info *data);
+extern struct property *new_property(const char *name, const int length,
+ const unsigned char *value, struct property *last);
/*
* There are two methods for telling firmware what our capabilities are.
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 157b014..23375fd 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -47,7 +47,7 @@ obj-y := cputable.o ptrace.o
syscalls.o \
signal.o sysfs.o cacheinfo.o time.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
- of_platform.o prom_parse.o
+ of_platform.o prom_parse.o of_property.o
obj-$(CONFIG_PPC64)+= setup_64.o sys_ppc32.o \
signal_64.o ptrace32.o \
paca.o nvram_64.o firmware.o note.o
diff --git a/arch/powerpc/kernel/of_property.c
b/arch/powerpc/kernel/of_property.c
new file mode 100644
index 000..e56c832
--- /dev/null
+++ b/arch/powerpc/kernel/of_property.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include
+#include
+#include
+#include
+
+struct property *new_property(const char *name, const int length,
+const unsigned char *value, struct
property *last)
+{
+ struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
+
+ if (!new)
+ return NULL;
+
+ if (!(new->name = kstrdup(name, GFP_KERNEL)))
+ goto cleanup;
+ if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
+ goto cleanup;
+
+ memcpy(new->value, value, length);
+ *(((char *)new->value) + length) = 0;
+ new->length = length;
+ new->next = last;
+ return new;
+
+cleanup:
+ kfree(new->name);
+ kfree(new->value);
+ kfree(new);
+ return NULL;
+}
+
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 85b088a..888227e 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2
*dr_cell,
extern int test_hotplug;
-static struct property *new_property(const char *name, const int length,
-const unsigned char *value, struct
property *last)
-{
- struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
- if (!new)
- return NULL;
-
- if (!(new->name = kstrdup(name, GFP_KERNEL)))
- goto cleanup;
- if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
- goto cleanup;
-
- memcpy(new->value, value, length);
- *(((char *)new->value) + length) = 0;
- new->length = length;
- new->next = last;
- return new;
-
-cleanup:
- kfree(new->name);
- kfree(new->value);
- kfree(new);
- return NULL;
-}
-
static int drmem_update_dt_v2(struct device_node *memory,
struct property *prop)
{
diff --git a/arch/powerpc/platforms/pseries/reconfig.c
b/arch/powerpc/platforms/pseries/reconfig.c
index 7f7369f..8e5a2ba 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end,
char **name, int *length
return tmp;
}
-static struct property *new_property(const char *name, const int length,
-const unsigned char *value, struct
property *last)
-{
- struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
-
Re: [PATCH v3 11/14] powerpc/kprobes: Support kprobes on prefixed instructions
Jordan Niethe's on February 28, 2020 1:23 pm:
> On Fri, Feb 28, 2020 at 12:48 PM Nicholas Piggin wrote:
>>
>> Jordan Niethe's on February 27, 2020 10:58 am:
>> > On Wed, Feb 26, 2020 at 6:18 PM Nicholas Piggin wrote:
>> >>
>> >> Jordan Niethe's on February 26, 2020 2:07 pm:
>> >> > @@ -136,11 +148,14 @@ int arch_prepare_kprobe(struct kprobe *p)
>> >> > }
>> >> >
>> >> > if (!ret) {
>> >> > - patch_instruction(p->ainsn.insn, *p->addr);
>> >> > + patch_instruction(>ainsn.insn[0], p->addr[0]);
>> >> > + if (IS_PREFIX(insn))
>> >> > + patch_instruction(>ainsn.insn[1], p->addr[1]);
>> >> > p->opcode = *p->addr;
>> >>
>> >> Not to single out this hunk or this patch even, but what do you reckon
>> >> about adding an instruction data type, and then use that in all these
>> >> call sites rather than adding the extra arg or doing the extra copy
>> >> manually in each place depending on prefix?
>> >>
>> >> instrs_are_equal, get_user_instr, analyse_instr, patch_instruction,
>> >> etc., would all take this new instr. Places that open code a memory
>> >> access like your MCE change need some accessor
>> >>
>> >>instr = *(unsigned int *)(instr_addr);
>> >> - if (!analyse_instr(, , instr, PPC_NO_SUFFIX)) {
>> >> + if (IS_PREFIX(instr))
>> >> + suffix = *(unsigned int *)(instr_addr + 4);
>> >>
>> >> Becomes
>> >>read_instr(instr_addr, );
>> >>if (!analyse_instr(, , instr)) ...
>> >>
>> >> etc.
>> > Daniel Axtens also talked about this and my reasons not to do so were
>> > pretty unconvincing, so I started trying something like this. One
>>
>> Okay.
>>
>> > thing I have been wondering is how pervasive should the new type be.
>>
>> I wouldn't mind it being quite pervasive. We have to be careful not
>> to copy it directly to/from memory now, but if we have accessors to
>> do all that with, I think it should be okay.
>>
>> > Below is data type I have started using, which I think works
>> > reasonably for replacing unsigned ints everywhere (like within
>> > code-patching.c). In a few architecture independent places such as
>> > uprobes which want to do ==, etc the union type does not work so well.
>>
>> There will be some places you have to make the boundary. I would start
>> by just making it a powerpc thing, but possibly there is or could be
>> some generic helpers. How does something like x86 cope with this?
>
> One of the places I was thinking of was is_swbp_insn() in
> kernel/events/uprobes.c. The problem was I wanted to typedef
> uprobe_opcode_t as ppc_insn type which was probably the wrong thing to
> do. x86 typedef's it as u8 (size of the trap they use). So we probably
> can do the same thing and just keep it as a u32.
Sounds good.
>> > I will have the next revision of the series start using a type.
>>
>> Thanks for doing that.
>>
>> >
>> > diff --git a/arch/powerpc/include/asm/inst.h
>> > b/arch/powerpc/include/asm/inst.h
>> > new file mode 100644
>> > index ..50adb3dbdeb4
>> > --- /dev/null
>> > +++ b/arch/powerpc/include/asm/inst.h
>> > @@ -0,0 +1,87 @@
>> > +
>> > +#ifndef _ASM_INST_H
>> > +#define _ASM_INST_H
>> > +
>> > +#ifdef __powerpc64__
>> > +
>> > +/* 64 bit Instruction */
>> > +
>> > +typedef struct {
>> > +unsigned int prefix;
>> > +unsigned int suffix;
>>
>> u32?
> Sure.
>>
>> > +} __packed ppc_prefixed_inst;
>> > +
>> > +typedef union ppc_inst {
>> > +unsigned int w;
>> > +ppc_prefixed_inst p;
>> > +} ppc_inst;
>>
>> I'd make it a struct and use nameless structs/unions inside it (with
>> appropriate packed annotation):
> Yeah that will be nicer.
>>
>> struct ppc_inst {
>> union {
>> struct {
>> u32 word;
>> u32 pad;
>> };
>> struct {
>> u32 prefix;
>> u32 suffix;
>> };
>> };
>> };
>>
>> > +
>> > +#define PPC_INST_IS_PREFIXED(inst) (((inst).w >> 26) == 1)
>> > +#define PPC_INST_LEN(inst) (PPC_INST_IS_PREFIXED((inst)) ?
>> > sizeof((inst).p) : sizeof((inst).w))
>>
>> Good accessors, I'd make them all C inline functions and lower case.
> Will change.
>>
>> > +
>> > +#define PPC_INST_NEW_WORD(x) ((ppc_inst) { .w = (x) })
>> > +#define PPC_INST_NEW_WORD_PAD(x) ((ppc_inst) { .p.prefix = (x),
>> > .p.suffix = (0x6000) })
>> > +#define PPC_INST_NEW_PREFIXED(x, y) ((ppc_inst) { .p.prefix = (x),
>> > .p.suffix = (y) })
>>
>> If these are widely used, I'd make them a bit shorter
>>
>> #define PPC_INST(x)
>> #define PPC_INST_PREFIXED(x)
> Good idea, they do end up used quite widely.
>>
>> I'd also set padding to something invalid 0 or 0x for the first
>> case, and have your accessors check that rather than carrying around
>> another type of ppc_inst (prefixed, padded, non-padded).
>>
>> > +
>> > +#define PPC_INST_WORD(x) ((x).w)
>> > +#define PPC_INST_PREFIX(x) (x.p.prefix)
>> > +#define PPC_INST_SUFFIX(x)
Re: [PATCH v3 01/14] powerpc: Enable Prefixed Instructions
Jordan Niethe's on February 28, 2020 12:52 pm:
> On Wed, Feb 26, 2020 at 5:50 PM Nicholas Piggin wrote:
>>
>> Jordan Niethe's on February 26, 2020 2:07 pm:
>> > From: Alistair Popple
>> >
>> > 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.
>> >
>> > Signed-off-by: Alistair Popple
>> > ---
>> > arch/powerpc/include/asm/reg.h| 3 +++
>> > arch/powerpc/kernel/dt_cpu_ftrs.c | 23 +++
>> > 2 files changed, 26 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_RWMR0x375 /* 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_FSCR0x099 /* Facility Status & Control Register */
>> > +#define FSCR_PREFIX__MASK(FSCR_PREFIX_LG)
>>
>> When you add a new FSCR, there's a couple more things to do, check
>> out traps.c.
>>
>> > #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/dt_cpu_ftrs.c
>> > b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> > index 182b4047c1ef..396f2c6c588e 100644
>> > --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> > +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> > @@ -553,6 +553,28 @@ static int __init feat_enable_large_ci(struct
>> > dt_cpu_feature *f)
>> > return 1;
>> > }
>> >
>> > +static int __init feat_enable_prefix(struct dt_cpu_feature *f)
>> > +{
>> > + u64 fscr, hfscr;
>> > +
>> > + if (f->usable_privilege & USABLE_HV) {
>> > + hfscr = mfspr(SPRN_HFSCR);
>> > + hfscr |= HFSCR_PREFIX;
>> > + mtspr(SPRN_HFSCR, hfscr);
>> > + }
>> > +
>> > + if (f->usable_privilege & USABLE_OS) {
>> > + fscr = mfspr(SPRN_FSCR);
>> > + fscr |= FSCR_PREFIX;
>> > + mtspr(SPRN_FSCR, fscr);
>> > +
>> > + if (f->usable_privilege & USABLE_PR)
>> > + current->thread.fscr |= FSCR_PREFIX;
>> > + }
>> > +
>> > + return 1;
>> > +}
>>
>> It would be good to be able to just use the default feature matching
>> for this, if possible? Do we not do the right thing with
>> init_thread.fscr?
> The default feature matching do you mean feat_enable()?
> I just tested using that again, within feat_enable() I can print and
> see that the [h]fscr gets the bits set for enabling prefixed
> instructions. However once I get to the shell and start xmon, the fscr
> bit for prefixed instructions can be seen to be unset. What we are
> doing in feat_enable_prefix() avoids that problem. So it seems maybe
> something is not quite right with the init_thread.fscr. I will look
> into further.
Okay it probably gets overwritten somewhere.
But hmm, the dt_cpu_ftrs parsing should be picking those up and setting
them by default I would think (or maybe not because you don't expect
FSCR bit if OS support is not required).
All the other FSCR bits are done similarly to this AFAIKS:
https://patchwork.ozlabs.org/patch/1244476/
I would do that for now rather than digging into it too far, but we
should look at cleaning that up and doing something more like what
you have here.
>> > struct dt_cpu_feature_match {
>> > const char *name;
>> > int (*enable)(struct dt_cpu_feature *f);
>> > @@ -626,6 +648,7 @@ static struct dt_cpu_feature_match __initdata
>> > {"vector-binary128", feat_enable, 0},
>> > {"vector-binary16", feat_enable, 0},
>> > {"wait-v3", feat_enable, 0},
>> > + {"prefix-instructions", feat_enable_prefix, 0},
>>
>> That's reasonable to make that a feature, will it specify a minimum
>> base set of prefix instructions or just that prefix instructions
>> with the prefix/suffix arrangement exist?
> This was just going to be that they exist.
>>
>> You may not need "-instructions" on the end, none of the other
>> instructions do.
> Good point.
>>
>> I would maybe just hold off
Re: [PATCH v4 06/13] powerpc/ptrace: split out ALTIVEC related functions.
Hi Christophe, Thank you for the patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on v5.6-rc3 next-20200227] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Reduce-ifdef-mess-in-ptrace/20200228-094814 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-defconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): >> arch/powerpc/kernel/ptrace/ptrace-altivec.c:8:10: fatal error: >> ptrace_decl.h: No such file or directory #include "ptrace_decl.h" ^~~ compilation terminated. vim +8 arch/powerpc/kernel/ptrace/ptrace-altivec.c 7 > 8 #include "ptrace_decl.h" 9 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
This was my fault, I should really test changes like these before they go live. Apologies for the confusion caused -- Russell Currey rus...@russell.cc On Fri, Feb 28, 2020, at 2:59 PM, Andrew Donnellan wrote: > On 28/2/20 9:16 am, Michael Ellerman wrote: > > Christophe Leroy writes: > >> Russel, > >> > >> Le 27/02/2020 à 12:49, Christophe Leroy a écrit : > >>> ptrace_triggered() is declared in asm/hw_breakpoint.h and > >>> only needed when CONFIG_HW_BREAKPOINT is set, so move it > >>> into hw_breakpoint.c > >> > >> My series v4 is definitely buggy (I included ptrace_decl.h instead > >> instead of ptrace-decl.h), how can Snowpatch say build succeeded > >> (https://patchwork.ozlabs.org/patch/1245807/) ? > > > > Which links to: > > > > https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895//artifact/linux/report.txt > > > > The actual build log of which is: > > > > https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895/artifact/linux/build_new.log > > > > Which contains: > >scripts/Makefile.build:267: recipe for target > > 'arch/powerpc/kernel/ptrace/ptrace-altivec.o' failed > >make[3]: *** [arch/powerpc/kernel/ptrace/ptrace-altivec.o] Error 1 > >make[3]: *** Waiting for unfinished jobs > >scripts/Makefile.build:505: recipe for target > > 'arch/powerpc/kernel/ptrace' failed > >make[2]: *** [arch/powerpc/kernel/ptrace] Error 2 > >make[2]: *** Waiting for unfinished jobs > >scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel' > > failed > >make[1]: *** [arch/powerpc/kernel] Error 2 > >make[1]: *** Waiting for unfinished jobs > >Makefile:1681: recipe for target 'arch/powerpc' failed > >make: *** [arch/powerpc] Error 2 > >make: *** Waiting for unfinished jobs > > > > Same for ppc64le: > > > > https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15896/artifact/linux/build_new.log > > > > > > So it seems like snowpatch always reports the build as succeeded even > > when it fails. > > Turns out there was an issue in a recent change in our build script > which caused build failures to return the wrong exit code and put the > wrong text in the reports, because of some confusion with bash > subshells. I've fixed it (I think). > > -- > Andrew Donnellan OzLabs, ADL Canberra > a...@linux.ibm.com IBM Australia Limited > >
Re: [PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
On 28/2/20 9:16 am, Michael Ellerman wrote: Christophe Leroy writes: Russel, Le 27/02/2020 à 12:49, Christophe Leroy a écrit : ptrace_triggered() is declared in asm/hw_breakpoint.h and only needed when CONFIG_HW_BREAKPOINT is set, so move it into hw_breakpoint.c My series v4 is definitely buggy (I included ptrace_decl.h instead instead of ptrace-decl.h), how can Snowpatch say build succeeded (https://patchwork.ozlabs.org/patch/1245807/) ? Which links to: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895//artifact/linux/report.txt The actual build log of which is: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895/artifact/linux/build_new.log Which contains: scripts/Makefile.build:267: recipe for target 'arch/powerpc/kernel/ptrace/ptrace-altivec.o' failed make[3]: *** [arch/powerpc/kernel/ptrace/ptrace-altivec.o] Error 1 make[3]: *** Waiting for unfinished jobs scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel/ptrace' failed make[2]: *** [arch/powerpc/kernel/ptrace] Error 2 make[2]: *** Waiting for unfinished jobs scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel' failed make[1]: *** [arch/powerpc/kernel] Error 2 make[1]: *** Waiting for unfinished jobs Makefile:1681: recipe for target 'arch/powerpc' failed make: *** [arch/powerpc] Error 2 make: *** Waiting for unfinished jobs Same for ppc64le: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15896/artifact/linux/build_new.log So it seems like snowpatch always reports the build as succeeded even when it fails. Turns out there was an issue in a recent change in our build script which caused build failures to return the wrong exit code and put the wrong text in the reports, because of some confusion with bash subshells. I've fixed it (I think). -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v3 11/14] powerpc/kprobes: Support kprobes on prefixed instructions
On Fri, Feb 28, 2020 at 12:48 PM Nicholas Piggin wrote:
>
> Jordan Niethe's on February 27, 2020 10:58 am:
> > On Wed, Feb 26, 2020 at 6:18 PM Nicholas Piggin wrote:
> >>
> >> Jordan Niethe's on February 26, 2020 2:07 pm:
> >> > @@ -136,11 +148,14 @@ int arch_prepare_kprobe(struct kprobe *p)
> >> > }
> >> >
> >> > if (!ret) {
> >> > - patch_instruction(p->ainsn.insn, *p->addr);
> >> > + patch_instruction(>ainsn.insn[0], p->addr[0]);
> >> > + if (IS_PREFIX(insn))
> >> > + patch_instruction(>ainsn.insn[1], p->addr[1]);
> >> > p->opcode = *p->addr;
> >>
> >> Not to single out this hunk or this patch even, but what do you reckon
> >> about adding an instruction data type, and then use that in all these
> >> call sites rather than adding the extra arg or doing the extra copy
> >> manually in each place depending on prefix?
> >>
> >> instrs_are_equal, get_user_instr, analyse_instr, patch_instruction,
> >> etc., would all take this new instr. Places that open code a memory
> >> access like your MCE change need some accessor
> >>
> >>instr = *(unsigned int *)(instr_addr);
> >> - if (!analyse_instr(, , instr, PPC_NO_SUFFIX)) {
> >> + if (IS_PREFIX(instr))
> >> + suffix = *(unsigned int *)(instr_addr + 4);
> >>
> >> Becomes
> >>read_instr(instr_addr, );
> >>if (!analyse_instr(, , instr)) ...
> >>
> >> etc.
> > Daniel Axtens also talked about this and my reasons not to do so were
> > pretty unconvincing, so I started trying something like this. One
>
> Okay.
>
> > thing I have been wondering is how pervasive should the new type be.
>
> I wouldn't mind it being quite pervasive. We have to be careful not
> to copy it directly to/from memory now, but if we have accessors to
> do all that with, I think it should be okay.
>
> > Below is data type I have started using, which I think works
> > reasonably for replacing unsigned ints everywhere (like within
> > code-patching.c). In a few architecture independent places such as
> > uprobes which want to do ==, etc the union type does not work so well.
>
> There will be some places you have to make the boundary. I would start
> by just making it a powerpc thing, but possibly there is or could be
> some generic helpers. How does something like x86 cope with this?
One of the places I was thinking of was is_swbp_insn() in
kernel/events/uprobes.c. The problem was I wanted to typedef
uprobe_opcode_t as ppc_insn type which was probably the wrong thing to
do. x86 typedef's it as u8 (size of the trap they use). So we probably
can do the same thing and just keep it as a u32.
>
> > I will have the next revision of the series start using a type.
>
> Thanks for doing that.
>
> >
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h
> > new file mode 100644
> > index ..50adb3dbdeb4
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -0,0 +1,87 @@
> > +
> > +#ifndef _ASM_INST_H
> > +#define _ASM_INST_H
> > +
> > +#ifdef __powerpc64__
> > +
> > +/* 64 bit Instruction */
> > +
> > +typedef struct {
> > +unsigned int prefix;
> > +unsigned int suffix;
>
> u32?
Sure.
>
> > +} __packed ppc_prefixed_inst;
> > +
> > +typedef union ppc_inst {
> > +unsigned int w;
> > +ppc_prefixed_inst p;
> > +} ppc_inst;
>
> I'd make it a struct and use nameless structs/unions inside it (with
> appropriate packed annotation):
Yeah that will be nicer.
>
> struct ppc_inst {
> union {
> struct {
> u32 word;
> u32 pad;
> };
> struct {
> u32 prefix;
> u32 suffix;
> };
> };
> };
>
> > +
> > +#define PPC_INST_IS_PREFIXED(inst) (((inst).w >> 26) == 1)
> > +#define PPC_INST_LEN(inst) (PPC_INST_IS_PREFIXED((inst)) ?
> > sizeof((inst).p) : sizeof((inst).w))
>
> Good accessors, I'd make them all C inline functions and lower case.
Will change.
>
> > +
> > +#define PPC_INST_NEW_WORD(x) ((ppc_inst) { .w = (x) })
> > +#define PPC_INST_NEW_WORD_PAD(x) ((ppc_inst) { .p.prefix = (x),
> > .p.suffix = (0x6000) })
> > +#define PPC_INST_NEW_PREFIXED(x, y) ((ppc_inst) { .p.prefix = (x),
> > .p.suffix = (y) })
>
> If these are widely used, I'd make them a bit shorter
>
> #define PPC_INST(x)
> #define PPC_INST_PREFIXED(x)
Good idea, they do end up used quite widely.
>
> I'd also set padding to something invalid 0 or 0x for the first
> case, and have your accessors check that rather than carrying around
> another type of ppc_inst (prefixed, padded, non-padded).
>
> > +
> > +#define PPC_INST_WORD(x) ((x).w)
> > +#define PPC_INST_PREFIX(x) (x.p.prefix)
> > +#define PPC_INST_SUFFIX(x) (x.p.suffix)
> > +#define PPC_INST_EMPTY(x) (PPC_INST_WORD(x) == 0)
>
> I'd avoid simple accessors like this completely. If they have any use
> it would be to ensure you don't try to use prefix/suffix on a
>
[PATCH 2/2] powerpc/powernv: Wire up OPAL address lookups
Use ARCH_HAS_ADDRESS_LOOKUP to look up the opal symbol table. This allows crashes and xmon debugging to print firmware symbols. Oops: System Reset, sig: 6 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc2-dirty #903 NIP: 30020434 LR: 3000378c CTR: 30020414 REGS: c000fffc3d70 TRAP: 0100 Not tainted (5.6.0-rc2-dirty) MSR: 92101002 CR: 28022284 XER: 2004 CFAR: 30003788 IRQMASK: 3 GPR00: 3000378c 31c13c90 30136200 c12cfa10 GPR04: c12cfa10 0010 31c10060 GPR08: c12cfaaf 30003640 0001 GPR12: 300e c149 c139c588 GPR16: 31c1 c125a900 c12076a8 GPR20: c12a3950 0001 31c10060 c12cfaaf GPR24: 0019 30003640 GPR28: 0010 c12cfa10 NIP [30020434] .dummy_console_write_buffer_space+0x20/0x64 [OPAL] LR [3000378c] opal_entry+0x14c/0x17c [OPAL] This won't unwind the firmware stack (or its Linux caller) properly if firmware and kernel endians don't match, but that problem could be solved in powerpc's unwinder. Signed-off-by: Nicholas Piggin --- arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/opal-api.h| 6 +++- arch/powerpc/include/asm/opal.h| 3 ++ arch/powerpc/platforms/powernv/opal-call.c | 2 ++ arch/powerpc/platforms/powernv/opal.c | 40 ++ 5 files changed, 51 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 497b7d0b2d7e..4d32b02d35e8 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -115,6 +115,7 @@ config PPC # Please keep this list sorted alphabetically. # select ARCH_32BIT_OFF_T if PPC32 + select ARCH_HAS_ADDRESS_LOOKUP if PPC_POWERNV select ARCH_HAS_DEBUG_VIRTUAL select ARCH_HAS_DEVMEM_IS_ALLOWED select ARCH_HAS_ELF_RANDOMIZE diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h index c1f25a760eb1..c3a2a797177a 100644 --- a/arch/powerpc/include/asm/opal-api.h +++ b/arch/powerpc/include/asm/opal-api.h @@ -214,7 +214,11 @@ #define OPAL_SECVAR_GET176 #define OPAL_SECVAR_GET_NEXT 177 #define OPAL_SECVAR_ENQUEUE_UPDATE 178 -#define OPAL_LAST 178 +#define OPAL_PHB_SET_OPTION179 +#define OPAL_PHB_GET_OPTION180 +#define OPAL_GET_SYMBOL181 +#define OPAL_LOOKUP_SYMBOL 182 +#define OPAL_LAST 182 #define QUIESCE_HOLD 1 /* Spin all calls at entry */ #define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */ diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h index 9986ac34b8e2..ef2d9273f06f 100644 --- a/arch/powerpc/include/asm/opal.h +++ b/arch/powerpc/include/asm/opal.h @@ -312,6 +312,9 @@ s64 opal_mpipl_query_tag(enum opal_mpipl_tags tag, u64 *addr); s64 opal_signal_system_reset(s32 cpu); s64 opal_quiesce(u64 shutdown_type, s32 cpu); +int64_t opal_get_symbol(uint64_t addr, __be64 *symaddr, __be64 *symsize, char *namebuf, uint64_t buflen); +int64_t opal_lookup_symbol(const char *name, __be64 *symaddr, __be64 *symsize); + /* Internal functions */ extern int early_init_dt_scan_opal(unsigned long node, const char *uname, int depth, void *data); diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c index 5cd0f52d258f..ba2d94df 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_get_symbol, OPAL_GET_SYMBOL); +OPAL_CALL(opal_lookup_symbol, OPAL_LOOKUP_SYMBOL); diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 2b3dfd0b6cdd..fdf6c4e6f7f9 100644 --- a/arch/powerpc/platforms/powernv/opal.c +++ b/arch/powerpc/platforms/powernv/opal.c @@ -107,6 +107,46 @@ void opal_configure_cores(void) cur_cpu_spec->cpu_restore(); } +const char *arch_address_lookup(unsigned long addr, + unsigned long
[PATCH 1/2] kallsyms: architecture specific symbol lookups
Provide CONFIG_ARCH_HAS_ADDRESS_LOOKUP which allows architectures to
do their own symbol/address lookup if kernel and module lookups miss.
powerpc will use this to deal with firmware symbols.
Signed-off-by: Nicholas Piggin
---
include/linux/kallsyms.h | 20
kernel/kallsyms.c| 13 -
lib/Kconfig | 3 +++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 657a83b943f0..8fdd44873373 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -83,6 +83,26 @@ extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
unsigned long *offset);
+#ifdef CONFIG_ARCH_HAS_ADDRESS_LOOKUP
+const char *arch_address_lookup(unsigned long addr,
+ unsigned long *symbolsize,
+ unsigned long *offset,
+ char **modname, char *namebuf);
+unsigned long arch_address_lookup_name(const char *name);
+#else
+static inline const char *arch_address_lookup(unsigned long addr,
+ unsigned long *symbolsize,
+ unsigned long *offset,
+ char **modname, char *namebuf)
+{
+ return NULL;
+}
+static inline unsigned long arch_address_lookup_name(const char *name)
+{
+ return 0;
+}
+#endif
+
/* Lookup an address. modname is set to NULL if it's in the kernel. */
const char *kallsyms_lookup(unsigned long addr,
unsigned long *symbolsize,
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index a9b3f660dee7..580c762fadd8 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -164,6 +164,7 @@ static unsigned long kallsyms_sym_address(int idx)
unsigned long kallsyms_lookup_name(const char *name)
{
char namebuf[KSYM_NAME_LEN];
+ unsigned long ret;
unsigned long i;
unsigned int off;
@@ -173,7 +174,12 @@ unsigned long kallsyms_lookup_name(const char *name)
if (strcmp(namebuf, name) == 0)
return kallsyms_sym_address(i);
}
- return module_kallsyms_lookup_name(name);
+
+ ret = module_kallsyms_lookup_name(name);
+ if (ret)
+ return ret;
+
+ return arch_address_lookup_name(name);
}
EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
@@ -311,6 +317,11 @@ const char *kallsyms_lookup(unsigned long addr,
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
offset, modname, namebuf);
+
+ if (!ret)
+ ret = arch_address_lookup(addr, symbolsize,
+ offset, modname, namebuf);
+
return ret;
}
diff --git a/lib/Kconfig b/lib/Kconfig
index bc7e56370129..16d3b8dbcadf 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -80,6 +80,9 @@ config ARCH_USE_CMPXCHG_LOCKREF
config ARCH_HAS_FAST_MULTIPLIER
bool
+config ARCH_HAS_ADDRESS_LOOKUP
+ bool
+
config INDIRECT_PIO
bool "Access I/O in non-MMIO mode"
depends on ARM64
--
2.23.0
[PATCH] Add OPAL_GET_SYMBOL / OPAL_LOOKUP_SYMBOL
These calls can be used by Linux to annotate BUG addresses with symbols,
look up symbol addresses in xmon, etc.
This is preferable over having Linux parse the OPAL symbol map itself,
because OPAL's parsing code already exists for its own symbol printing,
and it can support other code regions than the skiboot symbols, e.g.,
the wake-up code in the HOMER (where CPUs have been seen to get stuck).
Signed-off-by: Nicholas Piggin
---
core/opal.c | 2 +
core/utils.c| 92 +++--
doc/opal-api/opal-get-symbol-181.rst| 42 +++
doc/opal-api/opal-lookup-symbol-182.rst | 35 ++
include/opal-api.h | 4 +-
5 files changed, 168 insertions(+), 7 deletions(-)
create mode 100644 doc/opal-api/opal-get-symbol-181.rst
create mode 100644 doc/opal-api/opal-lookup-symbol-182.rst
diff --git a/core/opal.c b/core/opal.c
index d6ff6027b..d9fc4fe05 100644
--- a/core/opal.c
+++ b/core/opal.c
@@ -142,6 +142,8 @@ int64_t opal_entry_check(struct stack_frame *eframe)
case OPAL_CEC_REBOOT:
case OPAL_CEC_REBOOT2:
case OPAL_SIGNAL_SYSTEM_RESET:
+ case OPAL_GET_SYMBOL:
+ case OPAL_LOOKUP_SYMBOL:
break;
default:
printf("CPU ATTEMPT TO RE-ENTER FIRMWARE! PIR=%04lx cpu
@%p -> pir=%04x token=%llu\n",
diff --git a/core/utils.c b/core/utils.c
index 8fd63fcb7..5f0d5130b 100644
--- a/core/utils.c
+++ b/core/utils.c
@@ -48,40 +48,120 @@ char __attrconst tohex(uint8_t nibble)
return __tohex[nibble];
}
-static unsigned long get_symbol(unsigned long addr, char **sym, char **sym_end)
+static unsigned long get_symbol(unsigned long addr, char **sym, char
**sym_end, unsigned long *size)
{
unsigned long prev = 0, next;
char *psym = NULL, *p = __sym_map_start;
*sym = *sym_end = NULL;
- while(p < __sym_map_end) {
+ while (p < __sym_map_end) {
next = strtoul(p, , 16) | SKIBOOT_BASE;
if (next > addr && prev <= addr) {
- p = psym + 3;;
+ p = psym + 3;
if (p >= __sym_map_end)
return 0;
*sym = p;
- while(p < __sym_map_end && *p != 10)
+ while (p < __sym_map_end && *p != '\n')
p++;
*sym_end = p;
+ *size = next - prev;
return prev;
}
prev = next;
psym = p;
- while(p < __sym_map_end && *p != 10)
+ while (p < __sym_map_end && *p != '\n')
p++;
p++;
}
return 0;
}
+static unsigned long lookup_symbol(const char *name, unsigned long *size)
+{
+ size_t len = strlen(name);
+ unsigned long addr = 0;
+ char *sym;
+ char *p = __sym_map_start;
+
+ while (p < __sym_map_end) {
+ addr = strtoul(p, , 16) | SKIBOOT_BASE;
+ p += 3;
+ if (p >= __sym_map_end)
+ return 0;
+
+ if (*(p + len) == '\n' && !strncmp(name, p, len)) {
+ char *sym_end;
+
+ if (get_symbol(addr, , _end, size) == 0) {
+ assert(!strcmp(name, "_end"));
+ *size = 0;
+ }
+
+ /*
+* May be more than one symbol at this address but
+* symbol length calculation should still work in
+* that case.
+*/
+
+ return addr;
+ }
+
+ while(p < __sym_map_end && *p != '\n')
+ p++;
+ p++;
+ }
+ return 0;
+}
+
+static int64_t opal_get_symbol(uint64_t addr, __be64 *symaddr, __be64
*symsize, char *namebuf, uint64_t buflen)
+{
+ unsigned long saddr;
+ unsigned long ssize;
+ char *sym, *sym_end;
+ size_t l;
+
+ saddr = get_symbol(addr, , _end, );
+ if (!saddr)
+ return OPAL_RESOURCE;
+
+ if (buflen > sym_end - sym)
+ l = sym_end - sym;
+ else
+ l = buflen - 1;
+ memcpy(namebuf, sym, l);
+ namebuf[l] = '\0';
+
+ *symaddr = cpu_to_be64(saddr);
+ *symsize = cpu_to_be64(ssize);
+
+ return OPAL_SUCCESS;
+}
+opal_call(OPAL_GET_SYMBOL, opal_get_symbol, 5);
+
+static int64_t opal_lookup_symbol(const char *name, __be64 *symaddr, __be64
*symsize)
+{
+ unsigned long saddr;
+ unsigned long ssize;
+
+ saddr = lookup_symbol(name, );
+ if (!saddr)
+ return OPAL_RESOURCE;
+
+ *symaddr = cpu_to_be64(saddr);
+
Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
Hi On Fri, Feb 28, 2020 at 1:45 AM Nicolin Chen wrote: > > On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote: > > On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen > > wrote: > > > > > > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote: > > > > asrc_format is more inteligent variable, which is align > > > > with the alsa definition snd_pcm_format_t. > > > > > > > > Signed-off-by: Shengjiu Wang > > > > --- > > > > sound/soc/fsl/fsl_asrc.c | 23 +++ > > > > sound/soc/fsl/fsl_asrc.h | 4 ++-- > > > > sound/soc/fsl/fsl_asrc_dma.c | 2 +- > > > > 3 files changed, 14 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c > > > > index 0dcebc24c312..2b6a1643573c 100644 > > > > --- a/sound/soc/fsl/fsl_asrc.c > > > > +++ b/sound/soc/fsl/fsl_asrc.c > > > > > > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct > > > > snd_pcm_substream *substream, > > > > > > > > pair->config = > > > > > > > > - if (asrc_priv->asrc_width == 16) > > > > - format = SNDRV_PCM_FORMAT_S16_LE; > > > > - else > > > > - format = SNDRV_PCM_FORMAT_S24_LE; > > > > > > It feels better to me that we have format settings in hw_params(). > > > > > > Why not let fsl_easrc align with this? Any reason that I'm missing? > > > > because the asrc_width is not formal, in the future we can direct > > Hmm..that's our DT binding. And I don't feel it is a problem > to be ASoC irrelative. > > > input the format from the dts. format involve the info about width. > > Is there such any formal ASoC binding? I don't see those PCM > formats under include/dt-bindings/ folder. How are we going > to involve those formats in DT? There is no formal binding of this case. I think it is not good to convert width to format, because, for example width = 24, there is two option, we can select format S24_LE, or format S24_3LE, width is ambiguous for selecting. In EASRC, it support other two 24bit format U24_LE, U24_3LE . if we use the format in DT, then it is clear for usage in driver. best regards wang shengjiu
Re: [PATCH v3 01/14] powerpc: Enable Prefixed Instructions
On Wed, Feb 26, 2020 at 5:50 PM Nicholas Piggin wrote:
>
> Jordan Niethe's on February 26, 2020 2:07 pm:
> > From: Alistair Popple
> >
> > 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.
> >
> > Signed-off-by: Alistair Popple
> > ---
> > arch/powerpc/include/asm/reg.h| 3 +++
> > arch/powerpc/kernel/dt_cpu_ftrs.c | 23 +++
> > 2 files changed, 26 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_RWMR0x375 /* 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_FSCR0x099 /* Facility Status & Control Register */
> > +#define FSCR_PREFIX__MASK(FSCR_PREFIX_LG)
>
> When you add a new FSCR, there's a couple more things to do, check
> out traps.c.
>
> > #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/dt_cpu_ftrs.c
> > b/arch/powerpc/kernel/dt_cpu_ftrs.c
> > index 182b4047c1ef..396f2c6c588e 100644
> > --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> > +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> > @@ -553,6 +553,28 @@ static int __init feat_enable_large_ci(struct
> > dt_cpu_feature *f)
> > return 1;
> > }
> >
> > +static int __init feat_enable_prefix(struct dt_cpu_feature *f)
> > +{
> > + u64 fscr, hfscr;
> > +
> > + if (f->usable_privilege & USABLE_HV) {
> > + hfscr = mfspr(SPRN_HFSCR);
> > + hfscr |= HFSCR_PREFIX;
> > + mtspr(SPRN_HFSCR, hfscr);
> > + }
> > +
> > + if (f->usable_privilege & USABLE_OS) {
> > + fscr = mfspr(SPRN_FSCR);
> > + fscr |= FSCR_PREFIX;
> > + mtspr(SPRN_FSCR, fscr);
> > +
> > + if (f->usable_privilege & USABLE_PR)
> > + current->thread.fscr |= FSCR_PREFIX;
> > + }
> > +
> > + return 1;
> > +}
>
> It would be good to be able to just use the default feature matching
> for this, if possible? Do we not do the right thing with
> init_thread.fscr?
The default feature matching do you mean feat_enable()?
I just tested using that again, within feat_enable() I can print and
see that the [h]fscr gets the bits set for enabling prefixed
instructions. However once I get to the shell and start xmon, the fscr
bit for prefixed instructions can be seen to be unset. What we are
doing in feat_enable_prefix() avoids that problem. So it seems maybe
something is not quite right with the init_thread.fscr. I will look
into further.
>
>
> > +
> > struct dt_cpu_feature_match {
> > const char *name;
> > int (*enable)(struct dt_cpu_feature *f);
> > @@ -626,6 +648,7 @@ static struct dt_cpu_feature_match __initdata
> > {"vector-binary128", feat_enable, 0},
> > {"vector-binary16", feat_enable, 0},
> > {"wait-v3", feat_enable, 0},
> > + {"prefix-instructions", feat_enable_prefix, 0},
>
> That's reasonable to make that a feature, will it specify a minimum
> base set of prefix instructions or just that prefix instructions
> with the prefix/suffix arrangement exist?
This was just going to be that they exist.
>
> You may not need "-instructions" on the end, none of the other
> instructions do.
Good point.
>
> I would maybe just hold off upstreaming the dt_cpu_ftrs changes for
> a bit. We have to do a pass over new CPU feature device tree, and
> some compatibility questions have come up recently.
>
> If you wouldn't mind just adding the new [H]FSCR bits and faults
> upstream for now, that would be good.
No problem.
>
> Thanks,
> Nick
Re: [PATCH v3 07/14] powerpc/traps: Check for prefixed instructions in facility_unavailable_exception()
Jordan Niethe's on February 27, 2020 9:52 am: > On Wed, Feb 26, 2020 at 5:53 PM Nicholas Piggin wrote: >> >> Jordan Niethe's on February 26, 2020 2:07 pm: >> > 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. >> > >> > Signed-off-by: Jordan Niethe >> >> Oh you've got it here, I would just squash this together with the first >> patch. > Sure, I'll put them together. When you mentioned a couple more things > to do in traps.c, was it just this? Or is there still more to be done > adding an FSCR? Hmm... maybe it's just that one. Thanks, Nick
Re: [PATCH v3 11/14] powerpc/kprobes: Support kprobes on prefixed instructions
Nicholas Piggin's on February 28, 2020 11:47 am:
> Jordan Niethe's on February 27, 2020 10:58 am:
>> On Wed, Feb 26, 2020 at 6:18 PM Nicholas Piggin wrote:
>> +
>> +#define DEREF_PPC_INST_PTR(ptr)\
>> +({\
>> +ppc_inst __inst;\
>> +__inst.w = *(unsigned int *)(ptr);\
>> +if (PPC_INST_IS_PREFIXED(__inst))\
>> +__inst.p = *(ppc_prefixed_inst *)(ptr);\
>> +__inst;\
>> +})
>
> I'd go an inline with shorter lowercase names. ppc_inst_read();
Probably inst = ppc_inst_read(mem), rather.
Thanks,
Nick
Re: [PATCH v3 11/14] powerpc/kprobes: Support kprobes on prefixed instructions
Jordan Niethe's on February 27, 2020 10:58 am:
> On Wed, Feb 26, 2020 at 6:18 PM Nicholas Piggin wrote:
>>
>> Jordan Niethe's on February 26, 2020 2:07 pm:
>> > @@ -136,11 +148,14 @@ int arch_prepare_kprobe(struct kprobe *p)
>> > }
>> >
>> > if (!ret) {
>> > - patch_instruction(p->ainsn.insn, *p->addr);
>> > + patch_instruction(>ainsn.insn[0], p->addr[0]);
>> > + if (IS_PREFIX(insn))
>> > + patch_instruction(>ainsn.insn[1], p->addr[1]);
>> > p->opcode = *p->addr;
>>
>> Not to single out this hunk or this patch even, but what do you reckon
>> about adding an instruction data type, and then use that in all these
>> call sites rather than adding the extra arg or doing the extra copy
>> manually in each place depending on prefix?
>>
>> instrs_are_equal, get_user_instr, analyse_instr, patch_instruction,
>> etc., would all take this new instr. Places that open code a memory
>> access like your MCE change need some accessor
>>
>>instr = *(unsigned int *)(instr_addr);
>> - if (!analyse_instr(, , instr, PPC_NO_SUFFIX)) {
>> + if (IS_PREFIX(instr))
>> + suffix = *(unsigned int *)(instr_addr + 4);
>>
>> Becomes
>>read_instr(instr_addr, );
>>if (!analyse_instr(, , instr)) ...
>>
>> etc.
> Daniel Axtens also talked about this and my reasons not to do so were
> pretty unconvincing, so I started trying something like this. One
Okay.
> thing I have been wondering is how pervasive should the new type be.
I wouldn't mind it being quite pervasive. We have to be careful not
to copy it directly to/from memory now, but if we have accessors to
do all that with, I think it should be okay.
> Below is data type I have started using, which I think works
> reasonably for replacing unsigned ints everywhere (like within
> code-patching.c). In a few architecture independent places such as
> uprobes which want to do ==, etc the union type does not work so well.
There will be some places you have to make the boundary. I would start
by just making it a powerpc thing, but possibly there is or could be
some generic helpers. How does something like x86 cope with this?
> I will have the next revision of the series start using a type.
Thanks for doing that.
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> new file mode 100644
> index ..50adb3dbdeb4
> --- /dev/null
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -0,0 +1,87 @@
> +
> +#ifndef _ASM_INST_H
> +#define _ASM_INST_H
> +
> +#ifdef __powerpc64__
> +
> +/* 64 bit Instruction */
> +
> +typedef struct {
> +unsigned int prefix;
> +unsigned int suffix;
u32?
> +} __packed ppc_prefixed_inst;
> +
> +typedef union ppc_inst {
> +unsigned int w;
> +ppc_prefixed_inst p;
> +} ppc_inst;
I'd make it a struct and use nameless structs/unions inside it (with
appropriate packed annotation):
struct ppc_inst {
union {
struct {
u32 word;
u32 pad;
};
struct {
u32 prefix;
u32 suffix;
};
};
};
> +
> +#define PPC_INST_IS_PREFIXED(inst) (((inst).w >> 26) == 1)
> +#define PPC_INST_LEN(inst) (PPC_INST_IS_PREFIXED((inst)) ?
> sizeof((inst).p) : sizeof((inst).w))
Good accessors, I'd make them all C inline functions and lower case.
> +
> +#define PPC_INST_NEW_WORD(x) ((ppc_inst) { .w = (x) })
> +#define PPC_INST_NEW_WORD_PAD(x) ((ppc_inst) { .p.prefix = (x),
> .p.suffix = (0x6000) })
> +#define PPC_INST_NEW_PREFIXED(x, y) ((ppc_inst) { .p.prefix = (x),
> .p.suffix = (y) })
If these are widely used, I'd make them a bit shorter
#define PPC_INST(x)
#define PPC_INST_PREFIXED(x)
I'd also set padding to something invalid 0 or 0x for the first
case, and have your accessors check that rather than carrying around
another type of ppc_inst (prefixed, padded, non-padded).
> +
> +#define PPC_INST_WORD(x) ((x).w)
> +#define PPC_INST_PREFIX(x) (x.p.prefix)
> +#define PPC_INST_SUFFIX(x) (x.p.suffix)
> +#define PPC_INST_EMPTY(x) (PPC_INST_WORD(x) == 0)
I'd avoid simple accessors like this completely. If they have any use
it would be to ensure you don't try to use prefix/suffix on a
non-prefixed instruction for example. If you want to do that I'd use
inline functions for it.
> +
> +#define DEREF_PPC_INST_PTR(ptr)\
> +({\
> +ppc_inst __inst;\
> +__inst.w = *(unsigned int *)(ptr);\
> +if (PPC_INST_IS_PREFIXED(__inst))\
> +__inst.p = *(ppc_prefixed_inst *)(ptr);\
> +__inst;\
> +})
I'd go an inline with shorter lowercase names. ppc_inst_read();
> +#define PPC_INST_NEXT(ptr) ((ptr) += PPC_INST_LEN(DEREF_PPC_INST_PTR((ptr
> +#define PPC_INST_PREV(ptr) ((ptr) -= PPC_INST_LEN(DEREF_PPC_INST_PTR((ptr
Wouldn't bother with this accessor, caller could
Re: [PATCH v3 09/14] powerpc/xmon: Add initial support for prefixed instructions
Jordan Niethe's on February 28, 2020 10:37 am:
> On Thu, Feb 27, 2020 at 6:14 PM Christophe Leroy
> wrote:
>>
>>
>>
>> Le 27/02/2020 à 01:11, Jordan Niethe a écrit :
>> > On Wed, Feb 26, 2020 at 6:10 PM Nicholas Piggin wrote:
>> >>
>> >> Jordan Niethe's on February 26, 2020 2:07 pm:
>> >>> A prefixed instruction is composed of a word prefix and a word suffix.
>> >>> It does not make sense to be able to have a breakpoint on the suffix of
>> >>> a prefixed instruction, so make this impossible.
>> >>>
>> >>> When leaving xmon_core() we check to see if we are currently at a
>> >>> breakpoint. If this is the case, the breakpoint needs to be proceeded
>> >>> from. Initially emulate_step() is tried, but if this fails then we need
>> >>> to execute the saved instruction out of line. The NIP is set to the
>> >>> address of bpt::instr[] for the current breakpoint. bpt::instr[]
>> >>> contains the instruction replaced by the breakpoint, followed by a trap
>> >>> instruction. After bpt::instr[0] is executed and we hit the trap we
>> >>> enter back into xmon_bpt(). We know that if we got here and the offset
>> >>> indicates we are at bpt::instr[1] then we have just executed out of line
>> >>> so we can put the NIP back to the instruction after the breakpoint
>> >>> location and continue on.
>> >>>
>> >>> Adding prefixed instructions complicates this as the bpt::instr[1] needs
>> >>> to be used to hold the suffix. To deal with this make bpt::instr[] big
>> >>> enough for three word instructions. bpt::instr[2] contains the trap,
>> >>> and in the case of word instructions pad bpt::instr[1] with a noop.
>> >>>
>> >>> No support for disassembling prefixed instructions.
>> >>>
>> >>> Signed-off-by: Jordan Niethe
>> >>> ---
>> >>> v2: Rename sufx to suffix
>> >>> v3: - Just directly use PPC_INST_NOP
>> >>> - Typo: plac -> place
>> >>> - Rename read_inst() to mread_inst(). Do not have it call mread().
>> >>> ---
>> >>> arch/powerpc/xmon/xmon.c | 90 ++--
>> >>> 1 file changed, 78 insertions(+), 12 deletions(-)
>> >>>
>> >>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> >>> index a673cf55641c..a73a35aa4a75 100644
>> >>> --- a/arch/powerpc/xmon/xmon.c
>> >>> +++ b/arch/powerpc/xmon/xmon.c
>> >>> @@ -97,7 +97,8 @@ static long *xmon_fault_jmp[NR_CPUS];
>> >>> /* Breakpoint stuff */
>> >>> struct bpt {
>> >>>unsigned long address;
>> >>> - unsigned intinstr[2];
>> >>> + /* Prefixed instructions can not cross 64-byte boundaries */
>> >>> + unsigned intinstr[3] __aligned(64);
>> >>
>> >> This is pretty wild, I didn't realize xmon executes breakpoints out
>> >> of line like this.
>>
>> Neither did I. That's problematic. Kernel data is mapped NX on some
>> platforms.
>>
>> >>
>> >> IMO the break point entries here should correspond with a range of
>> >> reserved bytes in .text so we patch instructions into normal executable
>> >> pages rather than .data.
>> > Would it make sense to use vmalloc_exec() and use that like we are
>> > going to do in kprobes()?
>>
>> As we are (already) doing in kprobes() you mean ?
> Sorry for the confusion, I was mainly thinking of the patch that you
> pointed out before:
> https://patchwork.ozlabs.org/patch/1232619/
>>
>> In fact kprobes uses module_alloc(), and it works because kprobe depends
>> on module. On some platforms (i.e. book3s/32) vmalloc space is marked NX
>> in segment registers when CONFIG_MODULES is not set, see
>> mmu_mark_initmem_nx(). On other ones the Instruction TLB miss exception
>> does not manage misses at kernel addresses when CONFIG_MODULES is not
>> selected.
>>
>> So if we want XMON to work at all time, we need to use some (linear)
>> text address and use patch_instruction() to change it.
> Thank you for the detailed clarification, I will do it like that.
Yeah I would just make a little array in .text.xmon_bpts or something,
which should do the trick.
That wouldn't depend on this series, but if you want to do it as a fix
up patch before it, that would be good.
Thanks,
Nick
Re: [PATCH v3 10/27] powerpc: Add driver for OpenCAPI Persistent Memory
On Thu, 2020-02-27 at 21:44 +0100, Frederic Barrat wrote:
>
> Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > From: Alastair D'Silva
> >
> > This driver exposes LPC memory on OpenCAPI pmem cards
> > as an NVDIMM, allowing the existing nvram infrastructure
> > to be used.
> >
> > Namespace metadata is stored on the media itself, so
> > scm_reserve_metadata() maps 1 section's worth of PMEM storage
> > at the start to hold this. The rest of the PMEM range is registered
> > with libnvdimm as an nvdimm. scm_ndctl_config_read/write/size()
> > provide
> > callbacks to libnvdimm to access the metadata.
> >
> > Signed-off-by: Alastair D'Silva
> > ---
> > arch/powerpc/platforms/powernv/Kconfig| 3 +
> > arch/powerpc/platforms/powernv/Makefile | 1 +
> > arch/powerpc/platforms/powernv/pmem/Kconfig | 15 +
> > arch/powerpc/platforms/powernv/pmem/Makefile | 7 +
> > arch/powerpc/platforms/powernv/pmem/ocxl.c| 473
> > ++
> > .../platforms/powernv/pmem/ocxl_internal.h| 28 ++
> > 6 files changed, 527 insertions(+)
> > create mode 100644 arch/powerpc/platforms/powernv/pmem/Kconfig
> > create mode 100644 arch/powerpc/platforms/powernv/pmem/Makefile
> > create mode 100644 arch/powerpc/platforms/powernv/pmem/ocxl.c
> > create mode 100644
> > arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> >
> > diff --git a/arch/powerpc/platforms/powernv/Kconfig
> > b/arch/powerpc/platforms/powernv/Kconfig
> > index 938803eab0ad..fc8976af0e52 100644
> > --- a/arch/powerpc/platforms/powernv/Kconfig
> > +++ b/arch/powerpc/platforms/powernv/Kconfig
> > @@ -50,3 +50,6 @@ config PPC_VAS
> > config SCOM_DEBUGFS
> > bool "Expose SCOM controllers via debugfs"
> > depends on DEBUG_FS
> > +
> > +source "arch/powerpc/platforms/powernv/pmem/Kconfig"
> > +
> > diff --git a/arch/powerpc/platforms/powernv/Makefile
> > b/arch/powerpc/platforms/powernv/Makefile
> > index c0f8120045c3..0bbd72988b6f 100644
> > --- a/arch/powerpc/platforms/powernv/Makefile
> > +++ b/arch/powerpc/platforms/powernv/Makefile
> > @@ -21,3 +21,4 @@ obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o
> > vas-debug.o
> > obj-$(CONFIG_OCXL_BASE) += ocxl.o
> > obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
> > obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
> > +obj-$(CONFIG_LIBNVDIMM) += pmem/
> > diff --git a/arch/powerpc/platforms/powernv/pmem/Kconfig
> > b/arch/powerpc/platforms/powernv/pmem/Kconfig
> > new file mode 100644
> > index ..c5d927520920
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/powernv/pmem/Kconfig
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +if LIBNVDIMM
> > +
> > +config OCXL_PMEM
> > + tristate "OpenCAPI Persistent Memory"
> > + depends on LIBNVDIMM && PPC_POWERNV && PCI && EEH &&
> > ZONE_DEVICE && OCXL
> > + help
> > + Exposes devices that implement the OpenCAPI Storage Class
> > Memory
> > + specification as persistent memory regions. You may also want
> > + DEV_DAX, DEV_DAX_PMEM & FS_DAX if you plan on using DAX
> > devices
> > + stacked on top of this driver.
> > +
> > + Select N if unsure.
> > +
> > +endif
> > diff --git a/arch/powerpc/platforms/powernv/pmem/Makefile
> > b/arch/powerpc/platforms/powernv/pmem/Makefile
> > new file mode 100644
> > index ..1c55c4193175
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/powernv/pmem/Makefile
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +ccflags-$(CONFIG_PPC_WERROR) += -Werror
> > +
> > +obj-$(CONFIG_OCXL_PMEM) += ocxlpmem.o
> > +
> > +ocxlpmem-y := ocxl.o
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > new file mode 100644
> > index ..3c4eeb5dcc0f
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -0,0 +1,473 @@
> > +// SPDX-License-Id
> > +// Copyright 2019 IBM Corp.
> > +
> > +/*
> > + * A driver for OpenCAPI devices that implement the Storage Class
> > + * Memory specification.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "ocxl_internal.h"
> > +
> > +
> > +static const struct pci_device_id ocxlpmem_pci_tbl[] = {
> > + { PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0625), },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(pci, ocxlpmem_pci_tbl);
> > +
> > +#define NUM_MINORS 256 // Total to reserve
> > +
> > +static dev_t ocxlpmem_dev;
> > +static struct class *ocxlpmem_class;
> > +static struct mutex minors_idr_lock;
> > +static struct idr minors_idr;
> > +
> > +/**
> > + * ndctl_config_write() - Handle a ND_CMD_SET_CONFIG_DATA command
> > from ndctl
> > + * @ocxlpmem: the device metadata
> > + * @command: the incoming data to write
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int ndctl_config_write(struct ocxlpmem *ocxlpmem,
> > + struct nd_cmd_set_config_hdr *command)
> > +{
> > + if
Re: [PATCH v3 09/14] powerpc/xmon: Add initial support for prefixed instructions
On Thu, Feb 27, 2020 at 6:14 PM Christophe Leroy
wrote:
>
>
>
> Le 27/02/2020 à 01:11, Jordan Niethe a écrit :
> > On Wed, Feb 26, 2020 at 6:10 PM Nicholas Piggin wrote:
> >>
> >> Jordan Niethe's on February 26, 2020 2:07 pm:
> >>> A prefixed instruction is composed of a word prefix and a word suffix.
> >>> It does not make sense to be able to have a breakpoint on the suffix of
> >>> a prefixed instruction, so make this impossible.
> >>>
> >>> When leaving xmon_core() we check to see if we are currently at a
> >>> breakpoint. If this is the case, the breakpoint needs to be proceeded
> >>> from. Initially emulate_step() is tried, but if this fails then we need
> >>> to execute the saved instruction out of line. The NIP is set to the
> >>> address of bpt::instr[] for the current breakpoint. bpt::instr[]
> >>> contains the instruction replaced by the breakpoint, followed by a trap
> >>> instruction. After bpt::instr[0] is executed and we hit the trap we
> >>> enter back into xmon_bpt(). We know that if we got here and the offset
> >>> indicates we are at bpt::instr[1] then we have just executed out of line
> >>> so we can put the NIP back to the instruction after the breakpoint
> >>> location and continue on.
> >>>
> >>> Adding prefixed instructions complicates this as the bpt::instr[1] needs
> >>> to be used to hold the suffix. To deal with this make bpt::instr[] big
> >>> enough for three word instructions. bpt::instr[2] contains the trap,
> >>> and in the case of word instructions pad bpt::instr[1] with a noop.
> >>>
> >>> No support for disassembling prefixed instructions.
> >>>
> >>> Signed-off-by: Jordan Niethe
> >>> ---
> >>> v2: Rename sufx to suffix
> >>> v3: - Just directly use PPC_INST_NOP
> >>> - Typo: plac -> place
> >>> - Rename read_inst() to mread_inst(). Do not have it call mread().
> >>> ---
> >>> arch/powerpc/xmon/xmon.c | 90 ++--
> >>> 1 file changed, 78 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> >>> index a673cf55641c..a73a35aa4a75 100644
> >>> --- a/arch/powerpc/xmon/xmon.c
> >>> +++ b/arch/powerpc/xmon/xmon.c
> >>> @@ -97,7 +97,8 @@ static long *xmon_fault_jmp[NR_CPUS];
> >>> /* Breakpoint stuff */
> >>> struct bpt {
> >>>unsigned long address;
> >>> - unsigned intinstr[2];
> >>> + /* Prefixed instructions can not cross 64-byte boundaries */
> >>> + unsigned intinstr[3] __aligned(64);
> >>
> >> This is pretty wild, I didn't realize xmon executes breakpoints out
> >> of line like this.
>
> Neither did I. That's problematic. Kernel data is mapped NX on some
> platforms.
>
> >>
> >> IMO the break point entries here should correspond with a range of
> >> reserved bytes in .text so we patch instructions into normal executable
> >> pages rather than .data.
> > Would it make sense to use vmalloc_exec() and use that like we are
> > going to do in kprobes()?
>
> As we are (already) doing in kprobes() you mean ?
Sorry for the confusion, I was mainly thinking of the patch that you
pointed out before:
https://patchwork.ozlabs.org/patch/1232619/
>
> In fact kprobes uses module_alloc(), and it works because kprobe depends
> on module. On some platforms (i.e. book3s/32) vmalloc space is marked NX
> in segment registers when CONFIG_MODULES is not set, see
> mmu_mark_initmem_nx(). On other ones the Instruction TLB miss exception
> does not manage misses at kernel addresses when CONFIG_MODULES is not
> selected.
>
> So if we want XMON to work at all time, we need to use some (linear)
> text address and use patch_instruction() to change it.
Thank you for the detailed clarification, I will do it like that.
>
> Christophe
>
> >>
> >> Anyway that's for patch.
> >>
> >> Thanks,
> >> Nick
[PATCH v5 11/13] powerpc/ptrace: create ptrace_get_debugreg()
Create ptrace_get_debugreg() to handle PTRACE_GET_DEBUGREG and
reduce ifdef mess
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/ptrace/ptrace-adv.c | 9 +
arch/powerpc/kernel/ptrace/ptrace-decl.h | 2 ++
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 13 +
arch/powerpc/kernel/ptrace/ptrace.c | 18 ++
4 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c
b/arch/powerpc/kernel/ptrace/ptrace-adv.c
index eebcd41edc3d..c71bedd54a8b 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
@@ -56,6 +56,15 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
+ unsigned long __user *datalp)
+{
+ /* We only support one DABR and no IABRS at the moment */
+ if (addr > 0)
+ return -EINVAL;
+ return put_user(child->thread.debug.dac1, datalp);
+}
+
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned
long data)
{
#ifdef CONFIG_HAVE_HW_BREAKPOINT
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index bdba09a87aea..4b4b6a1d508a 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -176,6 +176,8 @@ int tm_cgpr32_set(struct task_struct *target, const struct
user_regset *regset,
extern const struct user_regset_view user_ppc_native_view;
/* ptrace-(no)adv */
+int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
+ unsigned long __user *datalp);
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned
long data);
long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint
*bp_info);
long ppc_del_hwdebug(struct task_struct *child, long data);
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index cf05fadba0d5..9c1ef9c2ffd4 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -39,6 +39,19 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
+ unsigned long __user *datalp)
+{
+ unsigned long dabr_fake;
+
+ /* We only support one DABR and no IABRS at the moment */
+ if (addr > 0)
+ return -EINVAL;
+ dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
+(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
+ return put_user(dabr_fake, datalp);
+}
+
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned
long data)
{
#ifdef CONFIG_HAVE_HW_BREAKPOINT
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index 48e095e88a2f..d6e1a301d20e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -191,23 +191,9 @@ long arch_ptrace(struct task_struct *child, long request,
break;
}
- case PTRACE_GET_DEBUGREG: {
-#ifndef CONFIG_PPC_ADV_DEBUG_REGS
- unsigned long dabr_fake;
-#endif
- ret = -EINVAL;
- /* We only support one DABR and no IABRS at the moment */
- if (addr > 0)
- break;
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- ret = put_user(child->thread.debug.dac1, datalp);
-#else
- dabr_fake = ((child->thread.hw_brk.address &
(~HW_BRK_TYPE_DABR)) |
-(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
- ret = put_user(dabr_fake, datalp);
-#endif
+ case PTRACE_GET_DEBUGREG:
+ ret = ptrace_get_debugreg(child, addr, datalp);
break;
- }
case PTRACE_SET_DEBUGREG:
ret = ptrace_set_debugreg(child, addr, data);
--
2.25.0
[PATCH v5 12/13] powerpc/ptrace: create ppc_gethwdinfo()
Create ippc_gethwdinfo() to handle PPC_PTRACE_GETHWDBGINFO and
reduce ifdef mess
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/ptrace/ptrace-adv.c | 15 +++
arch/powerpc/kernel/ptrace/ptrace-decl.h | 1 +
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 20 ++
arch/powerpc/kernel/ptrace/ptrace.c | 32 +--
4 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c
b/arch/powerpc/kernel/ptrace/ptrace-adv.c
index c71bedd54a8b..3990c01ef8cf 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
@@ -56,6 +56,21 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
+{
+ dbginfo->version = 1;
+ dbginfo->num_instruction_bps = CONFIG_PPC_ADV_DEBUG_IACS;
+ dbginfo->num_data_bps = CONFIG_PPC_ADV_DEBUG_DACS;
+ dbginfo->num_condition_regs = CONFIG_PPC_ADV_DEBUG_DVCS;
+ dbginfo->data_bp_alignment = 4;
+ dbginfo->sizeof_condition = 4;
+ dbginfo->features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
+ PPC_DEBUG_FEATURE_INSN_BP_MASK;
+ if (IS_ENABLED(CONFIG_PPC_ADV_DEBUG_DAC_RANGE))
+ dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_RANGE |
+PPC_DEBUG_FEATURE_DATA_BP_MASK;
+}
+
int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
unsigned long __user *datalp)
{
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index 4b4b6a1d508a..3c8a81999292 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -176,6 +176,7 @@ int tm_cgpr32_set(struct task_struct *target, const struct
user_regset *regset,
extern const struct user_regset_view user_ppc_native_view;
/* ptrace-(no)adv */
+void ppc_gethwdinfo(struct ppc_debug_info *dbginfo);
int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
unsigned long __user *datalp);
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned
long data);
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 9c1ef9c2ffd4..8a616d87fffb 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -39,6 +39,26 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
+{
+ dbginfo->version = 1;
+ dbginfo->num_instruction_bps = 0;
+ if (ppc_breakpoint_available())
+ dbginfo->num_data_bps = 1;
+ else
+ dbginfo->num_data_bps = 0;
+ dbginfo->num_condition_regs = 0;
+ dbginfo->data_bp_alignment = sizeof(long);
+ dbginfo->sizeof_condition = 0;
+ if (IS_ENABLED(CONFIG_HAVE_HW_BREAKPOINT)) {
+ dbginfo->features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
+ if (dawr_enabled())
+ dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_DAWR;
+ } else {
+ dbginfo->features = 0;
+ }
+}
+
int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
unsigned long __user *datalp)
{
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index d6e1a301d20e..a44f6e5e05ff 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -139,37 +139,7 @@ long arch_ptrace(struct task_struct *child, long request,
case PPC_PTRACE_GETHWDBGINFO: {
struct ppc_debug_info dbginfo;
- dbginfo.version = 1;
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- dbginfo.num_instruction_bps = CONFIG_PPC_ADV_DEBUG_IACS;
- dbginfo.num_data_bps = CONFIG_PPC_ADV_DEBUG_DACS;
- dbginfo.num_condition_regs = CONFIG_PPC_ADV_DEBUG_DVCS;
- dbginfo.data_bp_alignment = 4;
- dbginfo.sizeof_condition = 4;
- dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
- PPC_DEBUG_FEATURE_INSN_BP_MASK;
-#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
- dbginfo.features |=
- PPC_DEBUG_FEATURE_DATA_BP_RANGE |
- PPC_DEBUG_FEATURE_DATA_BP_MASK;
-#endif
-#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
- dbginfo.num_instruction_bps = 0;
- if (ppc_breakpoint_available())
- dbginfo.num_data_bps = 1;
- else
- dbginfo.num_data_bps = 0;
- dbginfo.num_condition_regs = 0;
- dbginfo.data_bp_alignment = sizeof(long);
- dbginfo.sizeof_condition = 0;
Re: [PATCH] selftests: pidfd: Add pidfd_fdinfo_test in .gitignore
On February 28, 2020 1:00:08 AM GMT+01:00, Christophe Leroy
wrote:
>The commit identified below added pidfd_fdinfo_test
>but failed to add it to .gitignore
>
>Fixes: 2def297ec7fb ("pidfd: add tests for NSpid info in fdinfo")
>Cc: sta...@vger.kernel.org
>Signed-off-by: Christophe Leroy
>---
> tools/testing/selftests/pidfd/.gitignore | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/tools/testing/selftests/pidfd/.gitignore
>b/tools/testing/selftests/pidfd/.gitignore
>index 3a779c084d96..39559d723c41 100644
>--- a/tools/testing/selftests/pidfd/.gitignore
>+++ b/tools/testing/selftests/pidfd/.gitignore
>@@ -2,4 +2,5 @@ pidfd_open_test
> pidfd_poll_test
> pidfd_test
> pidfd_wait
>+pidfd_fdinfo_test
> pidfd_getfd_test
Thanks for spotting this.
I'll pick this up along with other fixes I have waiting.
Acked-by: Christian Brauner
[PATCH v5 09/13] powerpc/ptrace: move register viewing functions out of ptrace.c
Create a dedicated ptrace-view.c file.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/ptrace/Makefile | 4 +-
arch/powerpc/kernel/ptrace/ptrace-decl.h | 43 +
arch/powerpc/kernel/ptrace/ptrace-view.c | 904 +
arch/powerpc/kernel/ptrace/ptrace.c | 966 ---
4 files changed, 949 insertions(+), 968 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-view.c
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
index 2d7f5f301536..7addc5994bb9 100644
--- a/arch/powerpc/kernel/ptrace/Makefile
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -3,9 +3,9 @@
# Makefile for the linux kernel.
#
-CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
+CFLAGS_ptrace-view.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
-obj-y += ptrace.o
+obj-y += ptrace.o ptrace-view.o
obj-$(CONFIG_PPC64)+= ptrace32.o
obj-$(CONFIG_VSX) += ptrace-vsx.o
ifneq ($(CONFIG_VSX),y)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index 8d076818f1de..e12f6615fc1d 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -22,6 +22,45 @@
#define TFSO(f)(offsetof(struct thread_fp_state, f))
#define TSO(f) (offsetof(struct thread_struct, f))
+/*
+ * These are our native regset flavors.
+ */
+enum powerpc_regset {
+ REGSET_GPR,
+ REGSET_FPR,
+#ifdef CONFIG_ALTIVEC
+ REGSET_VMX,
+#endif
+#ifdef CONFIG_VSX
+ REGSET_VSX,
+#endif
+#ifdef CONFIG_SPE
+ REGSET_SPE,
+#endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ REGSET_TM_CGPR, /* TM checkpointed GPR registers */
+ REGSET_TM_CFPR, /* TM checkpointed FPR registers */
+ REGSET_TM_CVMX, /* TM checkpointed VMX registers */
+ REGSET_TM_CVSX, /* TM checkpointed VSX registers */
+ REGSET_TM_SPR, /* TM specific SPR registers */
+ REGSET_TM_CTAR, /* TM checkpointed TAR register */
+ REGSET_TM_CPPR, /* TM checkpointed PPR register */
+ REGSET_TM_CDSCR,/* TM checkpointed DSCR register */
+#endif
+#ifdef CONFIG_PPC64
+ REGSET_PPR, /* PPR register */
+ REGSET_DSCR,/* DSCR register */
+#endif
+#ifdef CONFIG_PPC_BOOK3S_64
+ REGSET_TAR, /* TAR register */
+ REGSET_EBB, /* EBB registers */
+ REGSET_PMR, /* Performance Monitor Registers */
+#endif
+#ifdef CONFIG_PPC_MEM_KEYS
+ REGSET_PKEY,/* AMR register */
+#endif
+};
+
/* ptrace-(no)vsx */
int fpr_get(struct task_struct *target, const struct user_regset *regset,
@@ -131,3 +170,7 @@ int tm_cgpr32_get(struct task_struct *target, const struct
user_regset *regset,
int tm_cgpr32_set(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf);
+
+/* ptrace-view */
+
+extern const struct user_regset_view user_ppc_native_view;
diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c
b/arch/powerpc/kernel/ptrace/ptrace-view.c
new file mode 100644
index ..15e3b79b6395
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@ -0,0 +1,904 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+#include
+#include
+#include
+
+#include "ptrace-decl.h"
+
+struct pt_regs_offset {
+ const char *name;
+ int offset;
+};
+
+#define STR(s) #s /* convert to string */
+#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs, r)}
+#define GPR_OFFSET_NAME(num) \
+ {.name = STR(r##num), .offset = offsetof(struct pt_regs, gpr[num])}, \
+ {.name = STR(gpr##num), .offset = offsetof(struct pt_regs, gpr[num])}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+
+static const struct pt_regs_offset regoffset_table[] = {
+ GPR_OFFSET_NAME(0),
+ GPR_OFFSET_NAME(1),
+ GPR_OFFSET_NAME(2),
+ GPR_OFFSET_NAME(3),
+ GPR_OFFSET_NAME(4),
+ GPR_OFFSET_NAME(5),
+ GPR_OFFSET_NAME(6),
+ GPR_OFFSET_NAME(7),
+ GPR_OFFSET_NAME(8),
+ GPR_OFFSET_NAME(9),
+ GPR_OFFSET_NAME(10),
+ GPR_OFFSET_NAME(11),
+ GPR_OFFSET_NAME(12),
+ GPR_OFFSET_NAME(13),
+ GPR_OFFSET_NAME(14),
+ GPR_OFFSET_NAME(15),
+ GPR_OFFSET_NAME(16),
+ GPR_OFFSET_NAME(17),
+ GPR_OFFSET_NAME(18),
+ GPR_OFFSET_NAME(19),
+ GPR_OFFSET_NAME(20),
+ GPR_OFFSET_NAME(21),
+ GPR_OFFSET_NAME(22),
+ GPR_OFFSET_NAME(23),
+ GPR_OFFSET_NAME(24),
+ GPR_OFFSET_NAME(25),
+ GPR_OFFSET_NAME(26),
+ GPR_OFFSET_NAME(27),
+ GPR_OFFSET_NAME(28),
+ GPR_OFFSET_NAME(29),
+ GPR_OFFSET_NAME(30),
+ GPR_OFFSET_NAME(31),
+
[PATCH v5 06/13] powerpc/ptrace: split out ALTIVEC related functions.
Move CONFIG_ALTIVEC functions out of ptrace.c, into
ptrace-altivec.c
Signed-off-by: Christophe Leroy
---
v4: add missing ptrace_decl.h
v5: that's ptrace-decl.h in fact
---
arch/powerpc/kernel/ptrace/Makefile | 1 +
arch/powerpc/kernel/ptrace/ptrace-altivec.c | 128
arch/powerpc/kernel/ptrace/ptrace-decl.h| 9 ++
arch/powerpc/kernel/ptrace/ptrace.c | 124 ---
4 files changed, 138 insertions(+), 124 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-altivec.c
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
index 238c27189078..522e6fd0b5b8 100644
--- a/arch/powerpc/kernel/ptrace/Makefile
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_VSX) += ptrace-vsx.o
ifneq ($(CONFIG_VSX),y)
obj-y += ptrace-novsx.o
endif
+obj-$(CONFIG_ALTIVEC) += ptrace-altivec.o
diff --git a/arch/powerpc/kernel/ptrace/ptrace-altivec.c
b/arch/powerpc/kernel/ptrace/ptrace-altivec.c
new file mode 100644
index ..dd8b75dfbd06
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-altivec.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+#include
+
+#include
+
+#include "ptrace-decl.h"
+
+/*
+ * Get/set all the altivec registers vr0..vr31, vscr, vrsave, in one go.
+ * The transfer totals 34 quadword. Quadwords 0-31 contain the
+ * corresponding vector registers. Quadword 32 contains the vscr as the
+ * last word (offset 12) within that quadword. Quadword 33 contains the
+ * vrsave as the first word (offset 0) within the quadword.
+ *
+ * This definition of the VMX state is compatible with the current PPC32
+ * ptrace interface. This allows signal handling and ptrace to use the
+ * same structures. This also simplifies the implementation of a bi-arch
+ * (combined (32- and 64-bit) gdb.
+ */
+
+int vr_active(struct task_struct *target, const struct user_regset *regset)
+{
+ flush_altivec_to_thread(target);
+ return target->thread.used_vr ? regset->n : 0;
+}
+
+/*
+ * Regardless of transactions, 'vr_state' holds the current running
+ * value of all the VMX registers and 'ckvr_state' holds the last
+ * checkpointed value of all the VMX registers for the current
+ * transaction to fall back on in case it aborts.
+ *
+ * Userspace interface buffer layout:
+ *
+ * struct data {
+ * vector128 vr[32];
+ * vector128 vscr;
+ * vector128 vrsave;
+ * };
+ */
+int vr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf)
+{
+ int ret;
+
+ flush_altivec_to_thread(target);
+
+ BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
+offsetof(struct thread_vr_state, vr[32]));
+
+ ret = user_regset_copyout(, , , ,
+ >thread.vr_state, 0,
+ 33 * sizeof(vector128));
+ if (!ret) {
+ /*
+* Copy out only the low-order word of vrsave.
+*/
+ int start, end;
+ union {
+ elf_vrreg_t reg;
+ u32 word;
+ } vrsave;
+ memset(, 0, sizeof(vrsave));
+
+ vrsave.word = target->thread.vrsave;
+
+ start = 33 * sizeof(vector128);
+ end = start + sizeof(vrsave);
+ ret = user_regset_copyout(, , , , ,
+ start, end);
+ }
+
+ return ret;
+}
+
+/*
+ * Regardless of transactions, 'vr_state' holds the current running
+ * value of all the VMX registers and 'ckvr_state' holds the last
+ * checkpointed value of all the VMX registers for the current
+ * transaction to fall back on in case it aborts.
+ *
+ * Userspace interface buffer layout:
+ *
+ * struct data {
+ * vector128 vr[32];
+ * vector128 vscr;
+ * vector128 vrsave;
+ * };
+ */
+int vr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ int ret;
+
+ flush_altivec_to_thread(target);
+
+ BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
+offsetof(struct thread_vr_state, vr[32]));
+
+ ret = user_regset_copyin(, , , ,
+>thread.vr_state, 0,
+33 * sizeof(vector128));
+ if (!ret && count > 0) {
+ /*
+* We use only the first word of vrsave.
+*/
+ int start, end;
+ union {
+ elf_vrreg_t reg;
+ u32 word;
+ } vrsave;
+ memset(, 0, sizeof(vrsave));
+
+ vrsave.word = target->thread.vrsave;
+
+
[PATCH v5 10/13] powerpc/ptrace: split out ADV_DEBUG_REGS related functions.
Move ADV_DEBUG_REGS functions out of ptrace.c, into
ptrace-adv.c and ptrace-noadv.c
Signed-off-by: Christophe Leroy
---
v4: Leave hw_breakpoint.h for ptrace.c
---
arch/powerpc/kernel/ptrace/Makefile | 4 +
arch/powerpc/kernel/ptrace/ptrace-adv.c | 468
arch/powerpc/kernel/ptrace/ptrace-decl.h | 5 +
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 236
arch/powerpc/kernel/ptrace/ptrace.c | 650 --
5 files changed, 713 insertions(+), 650 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-adv.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-noadv.c
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
index 7addc5994bb9..e9d97c2d063e 100644
--- a/arch/powerpc/kernel/ptrace/Makefile
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -14,3 +14,7 @@ endif
obj-$(CONFIG_ALTIVEC) += ptrace-altivec.o
obj-$(CONFIG_SPE) += ptrace-spe.o
obj-$(CONFIG_PPC_TRANSACTIONAL_MEM)+= ptrace-tm.o
+obj-$(CONFIG_PPC_ADV_DEBUG_REGS) += ptrace-adv.o
+ifneq ($(CONFIG_PPC_ADV_DEBUG_REGS),y)
+obj-y += ptrace-noadv.o
+endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c
b/arch/powerpc/kernel/ptrace/ptrace-adv.c
new file mode 100644
index ..eebcd41edc3d
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
@@ -0,0 +1,468 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+#include
+
+#include "ptrace-decl.h"
+
+void user_enable_single_step(struct task_struct *task)
+{
+ struct pt_regs *regs = task->thread.regs;
+
+ if (regs != NULL) {
+ task->thread.debug.dbcr0 &= ~DBCR0_BT;
+ task->thread.debug.dbcr0 |= DBCR0_IDM | DBCR0_IC;
+ regs->msr |= MSR_DE;
+ }
+ set_tsk_thread_flag(task, TIF_SINGLESTEP);
+}
+
+void user_enable_block_step(struct task_struct *task)
+{
+ struct pt_regs *regs = task->thread.regs;
+
+ if (regs != NULL) {
+ task->thread.debug.dbcr0 &= ~DBCR0_IC;
+ task->thread.debug.dbcr0 = DBCR0_IDM | DBCR0_BT;
+ regs->msr |= MSR_DE;
+ }
+ set_tsk_thread_flag(task, TIF_SINGLESTEP);
+}
+
+void user_disable_single_step(struct task_struct *task)
+{
+ struct pt_regs *regs = task->thread.regs;
+
+ if (regs != NULL) {
+ /*
+* The logic to disable single stepping should be as
+* simple as turning off the Instruction Complete flag.
+* And, after doing so, if all debug flags are off, turn
+* off DBCR0(IDM) and MSR(DE) Torez
+*/
+ task->thread.debug.dbcr0 &= ~(DBCR0_IC | DBCR0_BT);
+ /*
+* Test to see if any of the DBCR_ACTIVE_EVENTS bits are set.
+*/
+ if (!DBCR_ACTIVE_EVENTS(task->thread.debug.dbcr0,
+ task->thread.debug.dbcr1)) {
+ /*
+* All debug events were off.
+*/
+ task->thread.debug.dbcr0 &= ~DBCR0_IDM;
+ regs->msr &= ~MSR_DE;
+ }
+ }
+ clear_tsk_thread_flag(task, TIF_SINGLESTEP);
+}
+
+int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned
long data)
+{
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ int ret;
+ struct thread_struct *thread = >thread;
+ struct perf_event *bp;
+ struct perf_event_attr attr;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+
+ /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
+* For embedded processors we support one DAC and no IAC's at the
+* moment.
+*/
+ if (addr > 0)
+ return -EINVAL;
+
+ /* The bottom 3 bits in dabr are flags */
+ if ((data & ~0x7UL) >= TASK_SIZE)
+ return -EIO;
+
+ /* As described above, it was assumed 3 bits were passed with the data
+* address, but we will assume only the mode bits will be passed
+* as to not cause alignment restrictions for DAC-based processors.
+*/
+
+ /* DAC's hold the whole address without any mode flags */
+ task->thread.debug.dac1 = data & ~0x3UL;
+
+ if (task->thread.debug.dac1 == 0) {
+ dbcr_dac(task) &= ~(DBCR_DAC1R | DBCR_DAC1W);
+ if (!DBCR_ACTIVE_EVENTS(task->thread.debug.dbcr0,
+ task->thread.debug.dbcr1)) {
+ task->thread.regs->msr &= ~MSR_DE;
+ task->thread.debug.dbcr0 &= ~DBCR0_IDM;
+ }
+ return 0;
+ }
+
+ /* Read or Write bits must be set */
+
+ if (!(data & 0x3UL))
+ return -EINVAL;
+
+ /* Set the Internal Debugging flag (IDM bit 1) for the DBCR0 register */
+ task->thread.debug.dbcr0 |=
[PATCH v5 07/13] powerpc/ptrace: split out SPE related functions.
Move CONFIG_SPE functions out of ptrace.c, into
ptrace-spe.c
Signed-off-by: Christophe Leroy
---
v5: Added ptrace-decl.h
---
arch/powerpc/kernel/ptrace/Makefile | 1 +
arch/powerpc/kernel/ptrace/ptrace-decl.h | 9
arch/powerpc/kernel/ptrace/ptrace-spe.c | 68
arch/powerpc/kernel/ptrace/ptrace.c | 66 ---
4 files changed, 78 insertions(+), 66 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-spe.c
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
index 522e6fd0b5b8..f87eadf6e072 100644
--- a/arch/powerpc/kernel/ptrace/Makefile
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -12,3 +12,4 @@ ifneq ($(CONFIG_VSX),y)
obj-y += ptrace-novsx.o
endif
obj-$(CONFIG_ALTIVEC) += ptrace-altivec.o
+obj-$(CONFIG_SPE) += ptrace-spe.o
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index 0f9282cb52fc..8a362f97f1d6 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -26,6 +26,15 @@ int vr_set(struct task_struct *target, const struct
user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf);
+/* ptrace-spe */
+
+int evr_active(struct task_struct *target, const struct user_regset *regset);
+int evr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int evr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+
/* ptrace */
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
diff --git a/arch/powerpc/kernel/ptrace/ptrace-spe.c
b/arch/powerpc/kernel/ptrace/ptrace-spe.c
new file mode 100644
index ..68b86b4a4be4
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-spe.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+
+#include
+
+#include "ptrace-decl.h"
+
+/*
+ * For get_evrregs/set_evrregs functions 'data' has the following layout:
+ *
+ * struct {
+ * u32 evr[32];
+ * u64 acc;
+ * u32 spefscr;
+ * }
+ */
+
+int evr_active(struct task_struct *target, const struct user_regset *regset)
+{
+ flush_spe_to_thread(target);
+ return target->thread.used_spe ? regset->n : 0;
+}
+
+int evr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf)
+{
+ int ret;
+
+ flush_spe_to_thread(target);
+
+ ret = user_regset_copyout(, , , ,
+ >thread.evr,
+ 0, sizeof(target->thread.evr));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, acc) + sizeof(u64) !=
+offsetof(struct thread_struct, spefscr));
+
+ if (!ret)
+ ret = user_regset_copyout(, , , ,
+ >thread.acc,
+ sizeof(target->thread.evr), -1);
+
+ return ret;
+}
+
+int evr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ int ret;
+
+ flush_spe_to_thread(target);
+
+ ret = user_regset_copyin(, , , ,
+>thread.evr,
+0, sizeof(target->thread.evr));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, acc) + sizeof(u64) !=
+offsetof(struct thread_struct, spefscr));
+
+ if (!ret)
+ ret = user_regset_copyin(, , , ,
+>thread.acc,
+sizeof(target->thread.evr), -1);
+
+ return ret;
+}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index c383325db4a6..ca2b4d804992 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -403,72 +403,6 @@ static int gpr_set(struct task_struct *target, const
struct user_regset *regset,
return ret;
}
-#ifdef CONFIG_SPE
-
-/*
- * For get_evrregs/set_evrregs functions 'data' has the following layout:
- *
- * struct {
- * u32 evr[32];
- * u64 acc;
- * u32 spefscr;
- * }
- */
-
-static int evr_active(struct task_struct *target,
- const struct user_regset *regset)
-{
- flush_spe_to_thread(target);
- return target->thread.used_spe ? regset->n : 0;
-}
-
-static int evr_get(struct task_struct *target, const struct user_regset
*regset,
- unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf)
-{
- int ret;
-
- flush_spe_to_thread(target);
-
- ret = user_regset_copyout(,
[PATCH v5 08/13] powerpc/ptrace: split out TRANSACTIONAL_MEM related functions.
Move TRANSACTIONAL_MEM functions out of ptrace.c, into
ptrace-tm.c
Signed-off-by: Christophe Leroy
---
v4: leave asm-prototypes.h
---
arch/powerpc/kernel/ptrace/Makefile | 1 +
arch/powerpc/kernel/ptrace/ptrace-decl.h | 89 +++
arch/powerpc/kernel/ptrace/ptrace-tm.c | 851 +
arch/powerpc/kernel/ptrace/ptrace.c | 916 +--
4 files changed, 943 insertions(+), 914 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-tm.c
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
index f87eadf6e072..2d7f5f301536 100644
--- a/arch/powerpc/kernel/ptrace/Makefile
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -13,3 +13,4 @@ obj-y += ptrace-novsx.o
endif
obj-$(CONFIG_ALTIVEC) += ptrace-altivec.o
obj-$(CONFIG_SPE) += ptrace-spe.o
+obj-$(CONFIG_PPC_TRANSACTIONAL_MEM)+= ptrace-tm.o
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index 8a362f97f1d6..8d076818f1de 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -1,5 +1,27 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Set of msr bits that gdb can change on behalf of a process.
+ */
+#ifdef CONFIG_PPC_ADV_DEBUG_REGS
+#define MSR_DEBUGCHANGE0
+#else
+#define MSR_DEBUGCHANGE(MSR_SE | MSR_BE)
+#endif
+
+/*
+ * Max register writeable via put_reg
+ */
+#ifdef CONFIG_PPC32
+#define PT_MAX_PUT_REG PT_MQ
+#else
+#define PT_MAX_PUT_REG PT_CCR
+#endif
+
+#define TVSO(f)(offsetof(struct thread_vr_state, f))
+#define TFSO(f)(offsetof(struct thread_fp_state, f))
+#define TSO(f) (offsetof(struct thread_struct, f))
+
/* ptrace-(no)vsx */
int fpr_get(struct task_struct *target, const struct user_regset *regset,
@@ -37,8 +59,75 @@ int evr_set(struct task_struct *target, const struct
user_regset *regset,
/* ptrace */
+int gpr32_get_common(struct task_struct *target,
+const struct user_regset *regset,
+unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf,
+ unsigned long *regs);
+int gpr32_set_common(struct task_struct *target,
+const struct user_regset *regset,
+unsigned int pos, unsigned int count,
+const void *kbuf, const void __user *ubuf,
+unsigned long *regs);
+
+/* ptrace-tm */
+
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
void flush_tmregs_to_thread(struct task_struct *tsk);
#else
static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }
#endif
+
+int tm_cgpr_active(struct task_struct *target, const struct user_regset
*regset);
+int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int tm_cgpr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+int tm_cfpr_active(struct task_struct *target, const struct user_regset
*regset);
+int tm_cfpr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int tm_cfpr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+int tm_cvmx_active(struct task_struct *target, const struct user_regset
*regset);
+int tm_cvmx_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int tm_cvmx_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+int tm_cvsx_active(struct task_struct *target, const struct user_regset
*regset);
+int tm_cvsx_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int tm_cvsx_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+int tm_spr_active(struct task_struct *target, const struct user_regset
*regset);
+int tm_spr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int tm_spr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+int tm_tar_active(struct task_struct *target, const struct
[PATCH v5 03/13] powerpc/ptrace: drop unnecessary #ifdefs CONFIG_PPC64
Drop a bunch of #ifdefs CONFIG_PPC64 that are not vital.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/ptrace.h | 2 ++
arch/powerpc/kernel/ptrace/ptrace.c | 18 +++---
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h
b/arch/powerpc/include/asm/ptrace.h
index ee3ada66deb5..8e1953d99353 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -276,6 +276,8 @@ static inline unsigned long
regs_get_kernel_stack_nth(struct pt_regs *regs,
#endif /* __ASSEMBLY__ */
#ifndef __powerpc64__
+/* We need PT_SOFTE defined at all time to avoid #ifdefs */
+#define PT_SOFTE PT_MQ
#else /* __powerpc64__ */
#define PT_FPSCR32 (PT_FPR0 + 2*32 + 1)/* each FP reg occupies 2
32-bit userspace slots */
#define PT_VR0_32 164 /* each Vector reg occupies 4 slots in 32-bit */
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index 7ed54dbb2d7e..3dd94c296ac7 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -274,17 +274,15 @@ int ptrace_get_reg(struct task_struct *task, int regno,
unsigned long *data)
if (regno == PT_DSCR)
return get_user_dscr(task, data);
-#ifdef CONFIG_PPC64
/*
* softe copies paca->irq_soft_mask variable state. Since irq_soft_mask
is
* no more used as a flag, lets force usr to alway see the softe value
as 1
* which means interrupts are not soft disabled.
*/
- if (regno == PT_SOFTE) {
+ if (IS_ENABLED(CONFIG_PPC64) && regno == PT_SOFTE) {
*data = 1;
return 0;
}
-#endif
regs_max = sizeof(struct user_pt_regs) / sizeof(unsigned long);
if (regno < regs_max) {
@@ -1998,7 +1996,6 @@ static const struct user_regset_view user_ppc_native_view
= {
.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
};
-#ifdef CONFIG_PPC64
#include
static int gpr32_get_common(struct task_struct *target,
@@ -2272,14 +2269,11 @@ static const struct user_regset_view
user_ppc_compat_view = {
.name = "ppc", .e_machine = EM_PPC, .ei_osabi = ELF_OSABI,
.regsets = compat_regsets, .n = ARRAY_SIZE(compat_regsets)
};
-#endif /* CONFIG_PPC64 */
const struct user_regset_view *task_user_regset_view(struct task_struct *task)
{
-#ifdef CONFIG_PPC64
- if (test_tsk_thread_flag(task, TIF_32BIT))
+ if (IS_ENABLED(CONFIG_PPC64) && test_tsk_thread_flag(task, TIF_32BIT))
return _ppc_compat_view;
-#endif
return _ppc_native_view;
}
@@ -3063,11 +3057,7 @@ long arch_ptrace(struct task_struct *child, long request,
else
dbginfo.num_data_bps = 0;
dbginfo.num_condition_regs = 0;
-#ifdef CONFIG_PPC64
- dbginfo.data_bp_alignment = 8;
-#else
- dbginfo.data_bp_alignment = 4;
-#endif
+ dbginfo.data_bp_alignment = sizeof(long);
dbginfo.sizeof_condition = 0;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
dbginfo.features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
@@ -3304,12 +3294,10 @@ long do_syscall_trace_enter(struct pt_regs *regs)
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->gpr[0]);
-#ifdef CONFIG_PPC64
if (!is_32bit_task())
audit_syscall_entry(regs->gpr[0], regs->gpr[3], regs->gpr[4],
regs->gpr[5], regs->gpr[6]);
else
-#endif
audit_syscall_entry(regs->gpr[0],
regs->gpr[3] & 0x,
regs->gpr[4] & 0x,
--
2.25.0
[PATCH v5 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
ptrace_triggered() is declared in asm/hw_breakpoint.h and
only needed when CONFIG_HW_BREAKPOINT is set, so move it
into hw_breakpoint.c
Signed-off-by: Christophe Leroy
---
v4: removing inclusing of hw_breakpoint.h now. Previously it was done too early.
---
arch/powerpc/kernel/hw_breakpoint.c | 16
arch/powerpc/kernel/ptrace/ptrace.c | 19 ---
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 2462cd7c565c..2c0be9d941cf 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -427,3 +427,19 @@ void hw_breakpoint_pmu_read(struct perf_event *bp)
{
/* TODO */
}
+
+void ptrace_triggered(struct perf_event *bp,
+ struct perf_sample_data *data, struct pt_regs *regs)
+{
+ struct perf_event_attr attr;
+
+ /*
+* Disable the breakpoint request here since ptrace has defined a
+* one-shot behaviour for breakpoint exceptions in PPC64.
+* The SIGTRAP signal is generated automatically for us in do_dabr().
+* We don't have to do anything about that here
+*/
+ attr = bp->attr;
+ attr.disabled = true;
+ modify_user_hw_breakpoint(bp, );
+}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index a44f6e5e05ff..f6e51be47c6e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include
@@ -31,24 +30,6 @@
#include "ptrace-decl.h"
-#ifdef CONFIG_HAVE_HW_BREAKPOINT
-void ptrace_triggered(struct perf_event *bp,
- struct perf_sample_data *data, struct pt_regs *regs)
-{
- struct perf_event_attr attr;
-
- /*
-* Disable the breakpoint request here since ptrace has defined a
-* one-shot behaviour for breakpoint exceptions in PPC64.
-* The SIGTRAP signal is generated automatically for us in do_dabr().
-* We don't have to do anything about that here
-*/
- attr = bp->attr;
- attr.disabled = true;
- modify_user_hw_breakpoint(bp, );
-}
-#endif /* CONFIG_HAVE_HW_BREAKPOINT */
-
/*
* Called by kernel/ptrace.c when detaching..
*
--
2.25.0
[PATCH v5 05/13] powerpc/ptrace: split out VSX related functions.
Move CONFIG_VSX functions out of ptrace.c, into
ptrace-vsx.c and ptrace-novsx.c
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/ptrace/Makefile | 4 +
arch/powerpc/kernel/ptrace/ptrace-decl.h | 26
arch/powerpc/kernel/ptrace/ptrace-novsx.c | 57 +++
arch/powerpc/kernel/ptrace/ptrace-vsx.c | 151 +++
arch/powerpc/kernel/ptrace/ptrace.c | 175 +-
5 files changed, 241 insertions(+), 172 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-decl.h
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-novsx.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-vsx.c
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
index 02fb28eb3b55..238c27189078 100644
--- a/arch/powerpc/kernel/ptrace/Makefile
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -7,3 +7,7 @@ CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
obj-y += ptrace.o
obj-$(CONFIG_PPC64)+= ptrace32.o
+obj-$(CONFIG_VSX) += ptrace-vsx.o
+ifneq ($(CONFIG_VSX),y)
+obj-y += ptrace-novsx.o
+endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
new file mode 100644
index ..764df4ee9362
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/* ptrace-(no)vsx */
+
+int fpr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int fpr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+
+/* ptrace-vsx */
+
+int vsr_active(struct task_struct *target, const struct user_regset *regset);
+int vsr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user
*ubuf);
+int vsr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf);
+
+/* ptrace */
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+void flush_tmregs_to_thread(struct task_struct *tsk);
+#else
+static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }
+#endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace-novsx.c
b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
new file mode 100644
index ..b2dc4e92d11a
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-novsx.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+
+#include
+
+#include "ptrace-decl.h"
+
+/*
+ * Regardless of transactions, 'fp_state' holds the current running
+ * value of all FPR registers and 'ckfp_state' holds the last checkpointed
+ * value of all FPR registers for the current transaction.
+ *
+ * Userspace interface buffer layout:
+ *
+ * struct data {
+ * u64 fpr[32];
+ * u64 fpscr;
+ * };
+ */
+int fpr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf)
+{
+ BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+offsetof(struct thread_fp_state, fpr[32]));
+
+ flush_fp_to_thread(target);
+
+ return user_regset_copyout(, , , ,
+ >thread.fp_state, 0, -1);
+}
+
+/*
+ * Regardless of transactions, 'fp_state' holds the current running
+ * value of all FPR registers and 'ckfp_state' holds the last checkpointed
+ * value of all FPR registers for the current transaction.
+ *
+ * Userspace interface buffer layout:
+ *
+ * struct data {
+ * u64 fpr[32];
+ * u64 fpscr;
+ * };
+ *
+ */
+int fpr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+offsetof(struct thread_fp_state, fpr[32]));
+
+ flush_fp_to_thread(target);
+
+ return user_regset_copyin(, , , ,
+ >thread.fp_state, 0, -1);
+}
diff --git a/arch/powerpc/kernel/ptrace/ptrace-vsx.c
b/arch/powerpc/kernel/ptrace/ptrace-vsx.c
new file mode 100644
index ..d53466d49cc0
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/ptrace-vsx.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include
+
+#include
+
+#include "ptrace-decl.h"
+
+/*
+ * Regardless of transactions, 'fp_state' holds the current running
+ * value of all FPR registers and 'ckfp_state' holds the last checkpointed
+ * value of all FPR registers for the current transaction.
+ *
+ * Userspace interface buffer layout:
+ *
+ * struct data {
+ * u64 fpr[32];
+ * u64
[PATCH v5 02/13] powerpc/ptrace: remove unused header includes
Remove unused header includes in ptrace.c and ptrace32.c Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/ptrace/ptrace.c | 19 ++- arch/powerpc/kernel/ptrace/ptrace32.c | 11 --- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c index 25c0424e8868..7ed54dbb2d7e 100644 --- a/arch/powerpc/kernel/ptrace/ptrace.c +++ b/arch/powerpc/kernel/ptrace/ptrace.c @@ -15,35 +15,20 @@ * this archive for more details. */ -#include -#include -#include -#include -#include -#include #include #include #include -#include -#include -#include -#include #include -#include #include -#include #include #include - -#include +#include #include -#include -#include + #include #include #include #include -#include #define CREATE_TRACE_POINTS #include diff --git a/arch/powerpc/kernel/ptrace/ptrace32.c b/arch/powerpc/kernel/ptrace/ptrace32.c index f37eb53de1a1..7976ddf29c0e 100644 --- a/arch/powerpc/kernel/ptrace/ptrace32.c +++ b/arch/powerpc/kernel/ptrace/ptrace32.c @@ -17,21 +17,10 @@ * this archive for more details. */ -#include -#include -#include -#include -#include #include #include -#include -#include -#include #include -#include -#include -#include #include /* -- 2.25.0
[PATCH v5 00/13] Reduce ifdef mess in ptrace
The purpose of this series is to reduce the amount of #ifdefs
in ptrace.c
Link: https://github.com/linuxppc/issues/issues/128
v5:
- Big mistake fixed in ptrace-altivec.c (included ptrace-decl.h as
ptrace_decl.h does not exist)
- Added ptrace-decl.h in ptrace-spe.c
- Kisskb is happy at the moment at
http://kisskb.ellerman.id.au/kisskb/branch/chleroy/head/8402c516023da1371953a65af7df2008758ea0c4/
v4:
- Fixed a few header files inclusion, see details in relevant patchs (no
marking on unchanged patches).
v3:
- Droped part of #ifdef removals iaw mpe's comments
- Removed unneccesary includes
v2:
- Fixed several build failures. Now builts cleanly on kisskb, see
http://kisskb.ellerman.id.au/kisskb/head/840e53cf913d6096dd60181a085f102c85d6e526/
- Droped last patch which is not related to ptrace and can be applies
independently.
Christophe Leroy (13):
powerpc: move ptrace into a subdirectory.
powerpc/ptrace: remove unused header includes
powerpc/ptrace: drop unnecessary #ifdefs CONFIG_PPC64
powerpc/ptrace: drop PARAMETER_SAVE_AREA_OFFSET
powerpc/ptrace: split out VSX related functions.
powerpc/ptrace: split out ALTIVEC related functions.
powerpc/ptrace: split out SPE related functions.
powerpc/ptrace: split out TRANSACTIONAL_MEM related functions.
powerpc/ptrace: move register viewing functions out of ptrace.c
powerpc/ptrace: split out ADV_DEBUG_REGS related functions.
powerpc/ptrace: create ptrace_get_debugreg()
powerpc/ptrace: create ppc_gethwdinfo()
powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
arch/powerpc/include/asm/ptrace.h |2 +
arch/powerpc/kernel/Makefile|7 +-
arch/powerpc/kernel/hw_breakpoint.c | 16 +
arch/powerpc/kernel/ptrace.c| 3468 ---
arch/powerpc/kernel/ptrace/Makefile | 20 +
arch/powerpc/kernel/ptrace/ptrace-adv.c | 492 +++
arch/powerpc/kernel/ptrace/ptrace-altivec.c | 128 +
arch/powerpc/kernel/ptrace/ptrace-decl.h| 184 +
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 269 ++
arch/powerpc/kernel/ptrace/ptrace-novsx.c | 57 +
arch/powerpc/kernel/ptrace/ptrace-spe.c | 68 +
arch/powerpc/kernel/ptrace/ptrace-tm.c | 851 +
arch/powerpc/kernel/ptrace/ptrace-view.c| 904 +
arch/powerpc/kernel/ptrace/ptrace-vsx.c | 151 +
arch/powerpc/kernel/ptrace/ptrace.c | 481 +++
arch/powerpc/kernel/{ => ptrace}/ptrace32.c | 11 -
16 files changed, 3626 insertions(+), 3483 deletions(-)
delete mode 100644 arch/powerpc/kernel/ptrace.c
create mode 100644 arch/powerpc/kernel/ptrace/Makefile
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-adv.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-altivec.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-decl.h
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-noadv.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-novsx.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-spe.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-tm.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-view.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace-vsx.c
create mode 100644 arch/powerpc/kernel/ptrace/ptrace.c
rename arch/powerpc/kernel/{ => ptrace}/ptrace32.c (96%)
--
2.25.0
[PATCH v5 04/13] powerpc/ptrace: drop PARAMETER_SAVE_AREA_OFFSET
PARAMETER_SAVE_AREA_OFFSET is not used, drop it.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/ptrace/ptrace.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index 3dd94c296ac7..22826c942eae 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -33,16 +33,6 @@
#define CREATE_TRACE_POINTS
#include
-/*
- * The parameter save area on the stack is used to store arguments being passed
- * to callee function and is located at fixed offset from stack pointer.
- */
-#ifdef CONFIG_PPC32
-#define PARAMETER_SAVE_AREA_OFFSET 24 /* bytes */
-#else /* CONFIG_PPC32 */
-#define PARAMETER_SAVE_AREA_OFFSET 48 /* bytes */
-#endif
-
struct pt_regs_offset {
const char *name;
int offset;
--
2.25.0
[PATCH v5 01/13] powerpc: move ptrace into a subdirectory.
In order to allow splitting of ptrace depending on the
different CONFIG_ options, create a subdirectory dedicated to
ptrace and move ptrace.c and ptrace32.c into it.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/Makefile| 7 +++
arch/powerpc/kernel/ptrace/Makefile | 9 +
arch/powerpc/kernel/{ => ptrace}/ptrace.c | 0
arch/powerpc/kernel/{ => ptrace}/ptrace32.c | 0
4 files changed, 12 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/kernel/ptrace/Makefile
rename arch/powerpc/kernel/{ => ptrace}/ptrace.c (100%)
rename arch/powerpc/kernel/{ => ptrace}/ptrace32.c (100%)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 78a1b22d4fd8..d812d7760e50 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -3,8 +3,6 @@
# Makefile for the linux kernel.
#
-CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
-
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
endif
@@ -41,15 +39,16 @@ CFLAGS_cputable.o += -DDISABLE_BRANCH_PROFILING
CFLAGS_btext.o += -DDISABLE_BRANCH_PROFILING
endif
-obj-y := cputable.o ptrace.o syscalls.o \
+obj-y := cputable.o syscalls.o \
irq.o align.o signal_32.o pmc.o vdso.o \
process.o systbl.o idle.o \
signal.o sysfs.o cacheinfo.o time.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
of_platform.o prom_parse.o
+obj-y += ptrace/
obj-$(CONFIG_PPC64)+= setup_64.o sys_ppc32.o \
- signal_64.o ptrace32.o \
+ signal_64.o \
paca.o nvram_64.o firmware.o note.o
obj-$(CONFIG_VDSO32) += vdso32/
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
diff --git a/arch/powerpc/kernel/ptrace/Makefile
b/arch/powerpc/kernel/ptrace/Makefile
new file mode 100644
index ..02fb28eb3b55
--- /dev/null
+++ b/arch/powerpc/kernel/ptrace/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the linux kernel.
+#
+
+CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
+
+obj-y += ptrace.o
+obj-$(CONFIG_PPC64)+= ptrace32.o
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
similarity index 100%
rename from arch/powerpc/kernel/ptrace.c
rename to arch/powerpc/kernel/ptrace/ptrace.c
diff --git a/arch/powerpc/kernel/ptrace32.c
b/arch/powerpc/kernel/ptrace/ptrace32.c
similarity index 100%
rename from arch/powerpc/kernel/ptrace32.c
rename to arch/powerpc/kernel/ptrace/ptrace32.c
--
2.25.0
[PATCH] selftests: powerpc: Add tlbie_test in .gitignore
The commit identified below added tlbie_test but
forgot to add it in .gitignore.
Fixes: 93cad5f78995 ("selftests/powerpc: Add test case for tlbie vs mtpidr
ordering issue")
Cc: sta...@vger.kernel.org
Signed-off-by: Christophe Leroy
---
tools/testing/selftests/powerpc/mm/.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/powerpc/mm/.gitignore
b/tools/testing/selftests/powerpc/mm/.gitignore
index 0ebeaea22641..97f7922c52c5 100644
--- a/tools/testing/selftests/powerpc/mm/.gitignore
+++ b/tools/testing/selftests/powerpc/mm/.gitignore
@@ -6,3 +6,4 @@ segv_errors
wild_bctr
large_vm_fork_separation
bad_accesses
+tlbie_test
--
2.25.0
[PATCH] selftests: pidfd: Add pidfd_fdinfo_test in .gitignore
The commit identified below added pidfd_fdinfo_test
but failed to add it to .gitignore
Fixes: 2def297ec7fb ("pidfd: add tests for NSpid info in fdinfo")
Cc: sta...@vger.kernel.org
Signed-off-by: Christophe Leroy
---
tools/testing/selftests/pidfd/.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/pidfd/.gitignore
b/tools/testing/selftests/pidfd/.gitignore
index 3a779c084d96..39559d723c41 100644
--- a/tools/testing/selftests/pidfd/.gitignore
+++ b/tools/testing/selftests/pidfd/.gitignore
@@ -2,4 +2,5 @@ pidfd_open_test
pidfd_poll_test
pidfd_test
pidfd_wait
+pidfd_fdinfo_test
pidfd_getfd_test
--
2.25.0
RE: [PATCH v3 14/27] powerpc/powernv/pmem: Add support for Admin commands
On Thu, 2020-02-27 at 09:01 -0800, Dan Williams wrote: > On Thu, Feb 20, 2020 at 7:28 PM Alastair D'Silva < > alast...@au1.ibm.com> wrote: > > From: Alastair D'Silva > > > > This patch requests the metadata required to issue admin commands, > > as well > > as some helper functions to construct and check the completion of > > the > > commands. > > What are the admin commands? Any pointer to a spec? Why does Linux > need to support these commands? I'll flesh these out for the next spin, thanks. -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819
Re: [PATCH v3 15/27] powerpc/powernv/pmem: Add support for near storage commands
On Thu, 2020-02-27 at 19:30 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:> +int
> ns_response_handled(const struct ocxlpmem *ocxlpmem)
> > +{
> > + return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_CHIC,
> > + OCXL_LITTLE_ENDIAN,
> > GLOBAL_MMIO_CHI_NSCRA);
> > +}
>
> Same comment as on the last patch - I think we're meant to be
> clearing
> this bit, not setting it to 1,
>
Same reply :) Writing to the CHIC register clears the bit in CHI.
>
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
Re: [PATCH v3 14/27] powerpc/powernv/pmem: Add support for Admin commands
On Thu, 2020-02-27 at 19:27 +1100, Andrew Donnellan wrote:
> On 27/2/20 7:22 pm, Andrew Donnellan wrote:
> > > +int admin_command_request(struct ocxlpmem *ocxlpmem, u8 op_code)
> > > +{
> > > +u64 val;
> > > +int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > > GLOBAL_MMIO_CHI,
> > > + OCXL_LITTLE_ENDIAN, );
> > > +if (rc)
> > > +return rc;
> >
> > Ignoring the value here expected, you're just trying to verify that
> > you
> > don't see an error on the read?
>
> I see that in the next patch, in ns_command_request() you check that
> NSCRA is 1 - did you mean to check that ACRA = 1 here?
>
>
I was in one version, but that was causing problems in startup since
there was successful prior command to assert ACRA.
I should remove the NSCRA check too.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
Re: [PATCH v3 14/27] powerpc/powernv/pmem: Add support for Admin commands
On Thu, 2020-02-27 at 19:22 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva
> >
> > This patch requests the metadata required to issue admin commands,
> > as well
> > as some helper functions to construct and check the completion of
> > the
> > commands.
> >
> > Signed-off-by: Alastair D'Silva
> > ---
> > arch/powerpc/platforms/powernv/pmem/ocxl.c| 65
> > .../platforms/powernv/pmem/ocxl_internal.c| 153
> > ++
> > .../platforms/powernv/pmem/ocxl_internal.h| 61 +++
> > 3 files changed, 279 insertions(+)
> >
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > index 431212c9f0cc..4e782d22605b 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -216,6 +216,58 @@ static int register_lpc_mem(struct ocxlpmem
> > *ocxlpmem)
> > return 0;
> > }
> >
> > +/**
> > + * extract_command_metadata() - Extract command data from MMIO &
> > save it for further use
> > + * @ocxlpmem: the device metadata
> > + * @offset: The base address of the command data structures
> > (address of CREQO)
> > + * @command_metadata: A pointer to the command metadata to
> > populate
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int extract_command_metadata(struct ocxlpmem *ocxlpmem, u32
> > offset,
> > + struct command_metadata
> > *command_metadata)
> > +{
> > + int rc;
> > + u64 tmp;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset,
> > OCXL_LITTLE_ENDIAN,
> > +);
> > + if (rc)
> > + return rc;
> > +
> > + command_metadata->request_offset = tmp >> 32;
> > + command_metadata->response_offset = tmp & 0x;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, offset + 8,
> > OCXL_LITTLE_ENDIAN,
> > +);
> > + if (rc)
> > + return rc;
> > +
> > + command_metadata->data_offset = tmp >> 32;
> > + command_metadata->data_size = tmp & 0x;
> > +
> > + command_metadata->id = 0;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * setup_command_metadata() - Set up the command metadata
> > + * @ocxlpmem: the device metadata
> > + */
> > +static int setup_command_metadata(struct ocxlpmem *ocxlpmem)
> > +{
> > + int rc;
> > +
> > + mutex_init(>admin_command.lock);
> > +
> > + rc = extract_command_metadata(ocxlpmem, GLOBAL_MMIO_ACMA_CREQO,
> > + >admin_command);
> > + if (rc)
> > + return rc;
> > +
> > + return 0;
> > +}
> > +
> > /**
> >* is_usable() - Is a controller usable?
> >* @ocxlpmem: the device metadata
> > @@ -456,6 +508,14 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > }
> > ocxlpmem->pdev = pdev;
> >
> > + ocxlpmem->timeouts[ADMIN_COMMAND_ERRLOG] = 2000; // ms
> > + ocxlpmem->timeouts[ADMIN_COMMAND_HEARTBEAT] = 100; // ms
> > + ocxlpmem->timeouts[ADMIN_COMMAND_SMART] = 100; // ms
> > + ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_DUMP] = 1000; // ms
> > + ocxlpmem->timeouts[ADMIN_COMMAND_CONTROLLER_STATS] = 100; // ms
> > + ocxlpmem->timeouts[ADMIN_COMMAND_SHUTDOWN] = 1000; // ms
> > + ocxlpmem->timeouts[ADMIN_COMMAND_FW_UPDATE] = 16000; // ms
>
> Why are we keeping these timeouts in a per device struct? I can't
> see
> anywhere where we change these values.
>
These are overwritten in a later patch, which I've missed! thanks for
pointing this out.
These initial values will be overwritten by card specific timeouts.
> > +
> > pci_set_drvdata(pdev, ocxlpmem);
> >
> > ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> > @@ -501,6 +561,11 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (setup_command_metadata(ocxlpmem)) {
> > + dev_err(>dev, "Could not read OCXL command
> > matada\n");
>
> metadata
Wow, not sure how that happened.
>
> Also, "OCXL command metadata" is misleading, this is a pmem specific
> thing, not an OpenCAPI thing, I would prefer just "command metadata".
>
Ok
> > + goto err;
> > + }
> > +
> > elapsed = 0;
> > timeout = ocxlpmem->readiness_timeout + ocxlpmem-
> > >memory_available_timeout;
> > while (!is_usable(ocxlpmem, false)) {
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > index 617ca943b1b8..583f48023025 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > @@ -17,3 +17,156 @@ int ocxlpmem_chi(const struct ocxlpmem
> > *ocxlpmem, u64 *chi)
> >
> > return 0;
> > }
> > +
> > +#define COMMAND_REQUEST_SIZE (8 * sizeof(u64))
> > +static int
[Bug 206695] kmemleak reports leaks in drivers/macintosh/windfarm
https://bugzilla.kernel.org/show_bug.cgi?id=206695 --- Comment #2 from Erhard F. (erhar...@mailbox.org) --- Created attachment 287691 --> https://bugzilla.kernel.org/attachment.cgi?id=287691=edit kernel .config (kernel 5.6-rc3, PowerMac G5 11,2) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 206695] kmemleak reports leaks in drivers/macintosh/windfarm
https://bugzilla.kernel.org/show_bug.cgi?id=206695 --- Comment #1 from Erhard F. (erhar...@mailbox.org) --- Created attachment 287689 --> https://bugzilla.kernel.org/attachment.cgi?id=287689=edit dmesg (kernel 5.6-rc3, PowerMac G5 11,2) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 206695] New: kmemleak reports leaks in drivers/macintosh/windfarm
https://bugzilla.kernel.org/show_bug.cgi?id=206695 Bug ID: 206695 Summary: kmemleak reports leaks in drivers/macintosh/windfarm Product: Platform Specific/Hardware Version: 2.5 Kernel Version: 5.6-rc3 Hardware: PPC-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: PPC-64 Assignee: platform_ppc...@kernel-bugs.osdl.org Reporter: erhar...@mailbox.org Regression: No Created attachment 287687 --> https://bugzilla.kernel.org/attachment.cgi?id=287687=edit kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2) kmemleak reports leaks from the windfarm module of my PowerMac G5 11,2: [...] unreferenced object 0xc0047081f840 (size 32): comm "kwindfarm", pid 203, jiffies 4294880630 (age 5552.877s) hex dump (first 32 bytes): c8 06 02 7f ff 02 ff 01 fb bf 00 41 00 20 00 00 ...A. .. 00 07 89 37 00 a0 00 00 00 00 00 00 00 00 00 00 ...7 backtrace: [<83f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0 [windfarm_smu_sat] [<3010fcb7>] .pm112_wf_notify+0x104c/0x13bc [windfarm_pm112] [] .notifier_call_chain+0xa8/0x180 [<70490868>] .blocking_notifier_call_chain+0x64/0x90 [<131d8149>] .wf_thread_func+0x114/0x1a0 [<0d54838d>] .kthread+0x13c/0x190 [<669b72bc>] .ret_from_kernel_thread+0x58/0x64 unreferenced object 0xc004737089f0 (size 16): comm "kwindfarm", pid 203, jiffies 4294880879 (age 5552.050s) hex dump (first 16 bytes): c4 04 01 7f 22 11 e0 e6 ff 55 7b 12 ec 11 00 00 "U{. backtrace: [<83f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0 [windfarm_smu_sat] [ ] .pm112_wf_notify+0x1294/0x13bc [windfarm_pm112] [ ] .notifier_call_chain+0xa8/0x180 [<70490868>] .blocking_notifier_call_chain+0x64/0x90 [<131d8149>] .wf_thread_func+0x114/0x1a0 [<0d54838d>] .kthread+0x13c/0x190 [<669b72bc>] .ret_from_kernel_thread+0x58/0x64 unreferenced object 0xc0047081fdc0 (size 32): comm "kwindfarm", pid 203, jiffies 4294881067 (age 5551.427s) hex dump (first 32 bytes): c9 06 02 7f ff 02 ff 01 fb bf 00 41 00 20 00 00 ...A. .. 00 07 89 37 00 a0 00 00 00 00 00 00 00 00 00 00 ...7 backtrace: [<83f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0 [windfarm_smu_sat] [<3010fcb7>] .pm112_wf_notify+0x104c/0x13bc [windfarm_pm112] [ ] .notifier_call_chain+0xa8/0x180 [<70490868>] .blocking_notifier_call_chain+0x64/0x90 [<131d8149>] .wf_thread_func+0x114/0x1a0 [<0d54838d>] .kthread+0x13c/0x190 [<669b72bc>] .ret_from_kernel_thread+0x58/0x64 unreferenced object 0xc00473708b60 (size 16): comm "kwindfarm", pid 203, jiffies 4294881320 (age 5550.587s) hex dump (first 16 bytes): c5 04 01 7f 22 11 e0 e6 ff 55 7b 12 ec 11 00 00 "U{. backtrace: [<83f0a65c>] .smu_sat_get_sdb_partition+0xc4/0x2d0 [windfarm_smu_sat] [ ] .pm112_wf_notify+0x1294/0x13bc [windfarm_pm112] [ ] .notifier_call_chain+0xa8/0x180 [<70490868>] .blocking_notifier_call_chain+0x64/0x90 [<131d8149>] .wf_thread_func+0x114/0x1a0 [<0d54838d>] .kthread+0x13c/0x190 [<669b72bc>] .ret_from_kernel_thread+0x58/0x64 -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
https://bugzilla.kernel.org/show_bug.cgi?id=206203 --- Comment #7 from Erhard F. (erhar...@mailbox.org) --- (In reply to mpe from comment #6) > Can you attach the /sys/kernel/debug/kmemleak output please. > > cheers I already did: "kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2) (91.35 KB, text/plain)" -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
Christophe Leroy writes: > Russel, > > Le 27/02/2020 à 12:49, Christophe Leroy a écrit : >> ptrace_triggered() is declared in asm/hw_breakpoint.h and >> only needed when CONFIG_HW_BREAKPOINT is set, so move it >> into hw_breakpoint.c > > My series v4 is definitely buggy (I included ptrace_decl.h instead > instead of ptrace-decl.h), how can Snowpatch say build succeeded > (https://patchwork.ozlabs.org/patch/1245807/) ? Which links to: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895//artifact/linux/report.txt The actual build log of which is: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895/artifact/linux/build_new.log Which contains: scripts/Makefile.build:267: recipe for target 'arch/powerpc/kernel/ptrace/ptrace-altivec.o' failed make[3]: *** [arch/powerpc/kernel/ptrace/ptrace-altivec.o] Error 1 make[3]: *** Waiting for unfinished jobs scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel/ptrace' failed make[2]: *** [arch/powerpc/kernel/ptrace] Error 2 make[2]: *** Waiting for unfinished jobs scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel' failed make[1]: *** [arch/powerpc/kernel] Error 2 make[1]: *** Waiting for unfinished jobs Makefile:1681: recipe for target 'arch/powerpc' failed make: *** [arch/powerpc] Error 2 make: *** Waiting for unfinished jobs Same for ppc64le: https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15896/artifact/linux/build_new.log So it seems like snowpatch always reports the build as succeeded even when it fails. cheers
[Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
https://bugzilla.kernel.org/show_bug.cgi?id=206203 --- Comment #6 from m...@ellerman.id.au --- bugzilla-dae...@bugzilla.kernel.org writes: > https://bugzilla.kernel.org/show_bug.cgi?id=206203 > > --- Comment #3 from Erhard F. (erhar...@mailbox.org) --- > Created attachment 287671 > --> https://bugzilla.kernel.org/attachment.cgi?id=287671=edit > kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2) > > Same on a PowerMac G5 11,2 (kernel 5.6-rc3). Can you attach the /sys/kernel/debug/kmemleak output please. cheers -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
bugzilla-dae...@bugzilla.kernel.org writes: > https://bugzilla.kernel.org/show_bug.cgi?id=206203 > > --- Comment #3 from Erhard F. (erhar...@mailbox.org) --- > Created attachment 287671 > --> https://bugzilla.kernel.org/attachment.cgi?id=287671=edit > kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2) > > Same on a PowerMac G5 11,2 (kernel 5.6-rc3). Can you attach the /sys/kernel/debug/kmemleak output please. cheers
[Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
https://bugzilla.kernel.org/show_bug.cgi?id=206203 --- Comment #5 from Erhard F. (erhar...@mailbox.org) --- Created attachment 287675 --> https://bugzilla.kernel.org/attachment.cgi?id=287675=edit kernel .config (kernel 5.6-rc3, PowerMac G5 11,2) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
https://bugzilla.kernel.org/show_bug.cgi?id=206203 --- Comment #4 from Erhard F. (erhar...@mailbox.org) --- Created attachment 287673 --> https://bugzilla.kernel.org/attachment.cgi?id=287673=edit dmesg (kernel 5.6-rc3, PowerMac G5 11,2) -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
https://bugzilla.kernel.org/show_bug.cgi?id=206203 --- Comment #3 from Erhard F. (erhar...@mailbox.org) --- Created attachment 287671 --> https://bugzilla.kernel.org/attachment.cgi?id=287671=edit kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2) Same on a PowerMac G5 11,2 (kernel 5.6-rc3). -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v3 10/27] powerpc: Add driver for OpenCAPI Persistent Memory
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
From: Alastair D'Silva
This driver exposes LPC memory on OpenCAPI pmem cards
as an NVDIMM, allowing the existing nvram infrastructure
to be used.
Namespace metadata is stored on the media itself, so
scm_reserve_metadata() maps 1 section's worth of PMEM storage
at the start to hold this. The rest of the PMEM range is registered
with libnvdimm as an nvdimm. scm_ndctl_config_read/write/size() provide
callbacks to libnvdimm to access the metadata.
Signed-off-by: Alastair D'Silva
---
arch/powerpc/platforms/powernv/Kconfig| 3 +
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/pmem/Kconfig | 15 +
arch/powerpc/platforms/powernv/pmem/Makefile | 7 +
arch/powerpc/platforms/powernv/pmem/ocxl.c| 473 ++
.../platforms/powernv/pmem/ocxl_internal.h| 28 ++
6 files changed, 527 insertions(+)
create mode 100644 arch/powerpc/platforms/powernv/pmem/Kconfig
create mode 100644 arch/powerpc/platforms/powernv/pmem/Makefile
create mode 100644 arch/powerpc/platforms/powernv/pmem/ocxl.c
create mode 100644 arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
diff --git a/arch/powerpc/platforms/powernv/Kconfig
b/arch/powerpc/platforms/powernv/Kconfig
index 938803eab0ad..fc8976af0e52 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -50,3 +50,6 @@ config PPC_VAS
config SCOM_DEBUGFS
bool "Expose SCOM controllers via debugfs"
depends on DEBUG_FS
+
+source "arch/powerpc/platforms/powernv/pmem/Kconfig"
+
diff --git a/arch/powerpc/platforms/powernv/Makefile
b/arch/powerpc/platforms/powernv/Makefile
index c0f8120045c3..0bbd72988b6f 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -21,3 +21,4 @@ obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
obj-$(CONFIG_OCXL_BASE) += ocxl.o
obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
+obj-$(CONFIG_LIBNVDIMM) += pmem/
diff --git a/arch/powerpc/platforms/powernv/pmem/Kconfig
b/arch/powerpc/platforms/powernv/pmem/Kconfig
new file mode 100644
index ..c5d927520920
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/pmem/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+if LIBNVDIMM
+
+config OCXL_PMEM
+ tristate "OpenCAPI Persistent Memory"
+ depends on LIBNVDIMM && PPC_POWERNV && PCI && EEH && ZONE_DEVICE && OCXL
+ help
+ Exposes devices that implement the OpenCAPI Storage Class Memory
+ specification as persistent memory regions. You may also want
+ DEV_DAX, DEV_DAX_PMEM & FS_DAX if you plan on using DAX devices
+ stacked on top of this driver.
+
+ Select N if unsure.
+
+endif
diff --git a/arch/powerpc/platforms/powernv/pmem/Makefile
b/arch/powerpc/platforms/powernv/pmem/Makefile
new file mode 100644
index ..1c55c4193175
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/pmem/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+
+ccflags-$(CONFIG_PPC_WERROR) += -Werror
+
+obj-$(CONFIG_OCXL_PMEM) += ocxlpmem.o
+
+ocxlpmem-y := ocxl.o
diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
b/arch/powerpc/platforms/powernv/pmem/ocxl.c
new file mode 100644
index ..3c4eeb5dcc0f
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
@@ -0,0 +1,473 @@
+// SPDX-License-Id
+// Copyright 2019 IBM Corp.
+
+/*
+ * A driver for OpenCAPI devices that implement the Storage Class
+ * Memory specification.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include "ocxl_internal.h"
+
+
+static const struct pci_device_id ocxlpmem_pci_tbl[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0625), },
+ { }
+};
+
+MODULE_DEVICE_TABLE(pci, ocxlpmem_pci_tbl);
+
+#define NUM_MINORS 256 // Total to reserve
+
+static dev_t ocxlpmem_dev;
+static struct class *ocxlpmem_class;
+static struct mutex minors_idr_lock;
+static struct idr minors_idr;
+
+/**
+ * ndctl_config_write() - Handle a ND_CMD_SET_CONFIG_DATA command from ndctl
+ * @ocxlpmem: the device metadata
+ * @command: the incoming data to write
+ * Return: 0 on success, negative on failure
+ */
+static int ndctl_config_write(struct ocxlpmem *ocxlpmem,
+ struct nd_cmd_set_config_hdr *command)
+{
+ if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
+ return -EINVAL;
+
+ memcpy_flushcache(ocxlpmem->metadata_addr + command->in_offset,
command->in_buf,
+ command->in_length);
+
+ return 0;
+}
+
+/**
+ * ndctl_config_read() - Handle a ND_CMD_GET_CONFIG_DATA command from ndctl
+ * @ocxlpmem: the device metadata
+ * @command: the read request
+ * Return: 0 on success, negative on failure
+ */
+static int ndctl_config_read(struct ocxlpmem *ocxlpmem,
+struct nd_cmd_get_config_data_hdr
Re: [PATCH] ima: add a new CONFIG for loading arch-specific policies
On Wed, 2020-02-26 at 15:36 -0500, Mimi Zohar wrote: > On Wed, 2020-02-26 at 11:21 -0800, Lakshmi Ramasubramanian wrote: > > Hi Nayna, > > > > > + > > > +config IMA_SECURE_AND_OR_TRUSTED_BOOT > > > + bool > > > + depends on IMA > > > + depends on IMA_ARCH_POLICY > > > + default n > > > + help > > > +This option is selected by architectures to enable secure and/or > > > +trusted boot based on IMA runtime policies. > > > > > > > Why is the default for this new config "n"? > > Is there any reason to not turn on this config if both IMA and > > IMA_ARCH_POLICY are set to y? > > Good catch. Having "IMA_SECURE_AND_OR_TRUSTED_BOOT" depend on > "IMA_ARCH_POLICY" doesn't make sense. "IMA_ARCH_POLICY" needs to be > selected. After discussing this some more with Nayna, the new Kconfig indicates that the architecture defines the arch_ima_get_secureboot() and arch_get_ima_policy() functions, but doesn't automatically enable IMA_ARCH_POLICY. The decision to enable IMA_ARCH_POLICY is left up to whoever is building the kernel. The patch, at least this aspect of it, is correct. Mimi
Re: [PATCH v3 2/4] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
On Thu, Feb 27, 2020 at 10:41:56AM +0800, Shengjiu Wang wrote:
> There is a new ASRC included in i.MX serial platform, there
> are some common definition can be shared with each other.
> So move the common definition to a separate header file.
>
> And add fsl_asrc_pair_internal and fsl_asrc_internal for
> the variable specific for the module, which can be used
> internally.
I think we can just call it "priv", instead of "internal", since
it's passed by the "void *private" pointer.
And it'd be nicer to have an extra preparational patch to rename
existing "struct fsl_asrc *asrc_priv" to "struct fsl_asrc *asrc".
Something like:
struct fsl_asrc *asrc = ;
struct fsl_asrc_pair *pair = ;
struct fsl_asrc_priv *asrc_priv = asrc->private;
struct fsl_asrc_pair_priv *pair_priv = pair->private;
Thanks
--
> Signed-off-by: Shengjiu Wang
> ---
> sound/soc/fsl/fsl_asrc.c| 81 +++-
> sound/soc/fsl/fsl_asrc.h| 74 ++
> sound/soc/fsl/fsl_asrc_common.h | 105
> sound/soc/fsl/fsl_asrc_dma.c| 25
> 4 files changed, 176 insertions(+), 109 deletions(-)
> create mode 100644 sound/soc/fsl/fsl_asrc_common.h
>
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 2b6a1643573c..7f862d220a8e 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -308,8 +308,10 @@ static int fsl_asrc_set_ideal_ratio(struct fsl_asrc_pair
> *pair,
> */
> static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool
> use_ideal_rate)
> {
> - struct asrc_config *config = pair->config;
> + struct fsl_asrc_pair_internal *pair_internal = pair->private;
> + struct asrc_config *config = pair_internal->config;
> struct fsl_asrc *asrc_priv = pair->asrc_priv;
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
> enum asrc_pair_index index = pair->index;
> enum asrc_word_width input_word_width;
> enum asrc_word_width output_word_width;
> @@ -392,11 +394,11 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
> *pair, bool use_ideal_rate)
> }
>
> /* Validate input and output clock sources */
> - clk_index[IN] = asrc_priv->clk_map[IN][config->inclk];
> - clk_index[OUT] = asrc_priv->clk_map[OUT][config->outclk];
> + clk_index[IN] = asrc_internal->clk_map[IN][config->inclk];
> + clk_index[OUT] = asrc_internal->clk_map[OUT][config->outclk];
>
> /* We only have output clock for ideal ratio mode */
> - clk = asrc_priv->asrck_clk[clk_index[ideal ? OUT : IN]];
> + clk = asrc_internal->asrck_clk[clk_index[ideal ? OUT : IN]];
>
> clk_rate = clk_get_rate(clk);
> rem[IN] = do_div(clk_rate, inrate);
> @@ -417,7 +419,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
> *pair, bool use_ideal_rate)
>
> div[IN] = min_t(u32, 1024, div[IN]);
>
> - clk = asrc_priv->asrck_clk[clk_index[OUT]];
> + clk = asrc_internal->asrck_clk[clk_index[OUT]];
> clk_rate = clk_get_rate(clk);
> if (ideal && use_ideal_rate)
> rem[OUT] = do_div(clk_rate, IDEAL_RATIO_RATE);
> @@ -437,13 +439,13 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
> *pair, bool use_ideal_rate)
> /* Set the channel number */
> channels = config->channel_num;
>
> - if (asrc_priv->soc->channel_bits < 4)
> + if (asrc_internal->soc->channel_bits < 4)
> channels /= 2;
>
> /* Update channels for current pair */
> regmap_update_bits(asrc_priv->regmap, REG_ASRCNCR,
> -ASRCNCR_ANCi_MASK(index,
> asrc_priv->soc->channel_bits),
> -ASRCNCR_ANCi(index, channels,
> asrc_priv->soc->channel_bits));
> +ASRCNCR_ANCi_MASK(index,
> asrc_internal->soc->channel_bits),
> +ASRCNCR_ANCi(index, channels,
> asrc_internal->soc->channel_bits));
>
> /* Default setting: Automatic selection for processing mode */
> regmap_update_bits(asrc_priv->regmap, REG_ASRCTR,
> @@ -568,9 +570,10 @@ static int fsl_asrc_dai_startup(struct snd_pcm_substream
> *substream,
> struct snd_soc_dai *dai)
> {
> struct fsl_asrc *asrc_priv = snd_soc_dai_get_drvdata(dai);
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
>
> /* Odd channel number is not valid for older ASRC (channel_bits==3) */
> - if (asrc_priv->soc->channel_bits == 3)
> + if (asrc_internal->soc->channel_bits == 3)
> snd_pcm_hw_constraint_step(substream->runtime, 0,
> SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>
> @@ -586,6 +589,7 @@ static int fsl_asrc_dai_hw_params(struct
> snd_pcm_substream *substream,
> struct fsl_asrc *asrc_priv = snd_soc_dai_get_drvdata(dai);
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct fsl_asrc_pair *pair =
Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
[Cc ppc maintainers]
On Thu 27-02-20 17:16:41, Vlastimil Babka wrote:
> On 2/27/20 5:00 PM, Sachin Sant wrote:
> >
> >
> >> On 27-Feb-2020, at 5:42 PM, Michal Hocko wrote:
> >>
> >> A very good hint indeed. I would do this
> >> diff --git a/include/linux/topology.h b/include/linux/topology.h
> >> index eb2fe6edd73c..d9f1b6737e4d 100644
> >> --- a/include/linux/topology.h
> >> +++ b/include/linux/topology.h
> >> @@ -137,6 +137,8 @@ static inline void set_numa_mem(int node)
> >> {
> >>this_cpu_write(_numa_mem_, node);
> >>_node_numa_mem_[numa_node_id()] = node;
> >> + pr_info("%s %d -> %d\n", __FUNCTION__, numa_node_id(), node);
> >> + dump_stack();
> >> }
> >> #endif
> >>
> >> Btw. it would be also helpful to get
> >> `faddr2line ___slab_alloc+0x334' from your kernel Sachin.
> >
> > [linux-next]# ./scripts/faddr2line ./vmlinux ___slab_alloc+0x334
> > ___slab_alloc+0x334/0x760:
> > new_slab_objects at mm/slub.c:2478
> > (inlined by) ___slab_alloc at mm/slub.c:2628
> > [linux-next]#
>
> Hmm that doesn't look relevant, but that address was marked as unreliable, no?
> Don't we actually need this one?
>
> [8.768727] NIP [c03d55f4] ___slab_alloc+0x1f4/0x760
>
> > I have also attached boot log with a kernel that include about change.
> > I see the following o/p during boot:
> >
> > [0.005269] set_numa_mem 1 -> 1
>
> So there's no "set_numa_mem 0 -> X", specifically not
> "set_numa_mem 0 -> 1" which I would have expected. That seems to confirm my
> suspicion that the arch code doesn't set up the memoryless node 0 properly.
Please have a look at
http://lkml.kernel.org/r/52ef4673-7292-4c4c-b459-af583951b...@linux.vnet.ibm.com
for the boot log with the debugging patch which tracks set_numa_mem.
This seems to lead to a crash in the slab allocator bebcause
node_to_mem_node(0) for memory less node resolves to the memory less
node http://lkml.kernel.org/r/dd450314-d428-6776-af07-f92c04c7b...@suse.cz.
The original report is
http://lkml.kernel.org/r/3381cd91-ab3d-4773-ba04-e7a072a63...@linux.vnet.ibm.com
>
> > [0.005270] CPU: 12 PID: 0 Comm: swapper/12 Not tainted
> > 5.6.0-rc3-next-20200227-autotest+ #6
> > [0.005271] Call Trace:
> > [0.005272] [c008b37dfe80] [c0b5d948] dump_stack+0xbc/0x104
> > (unreliable)
> > [0.005274] [c008b37dfec0] [c0059320]
> > start_secondary+0x600/0x6e0
> > [0.005277] [c008b37dff90] [c000ac54]
> > start_secondary_prolog+0x10/0x14
> >
> > Thanks
> > -Sachin
> >
--
Michal Hocko
SUSE Labs
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On Thu, Feb 27, 2020 at 10:03 AM Jason Gunthorpe wrote: > > On Thu, Feb 27, 2020 at 09:55:04AM -0800, Dan Williams wrote: > > On Thu, Feb 27, 2020 at 9:43 AM Jason Gunthorpe wrote: > > > > > > On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote: > > > > > > > > > > > > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote: > > > > >> Instead of this, this series proposes a change to arch_add_memory() > > > > >> to take the pgprot required by the mapping which allows us to > > > > >> explicitly set pagetable entries for P2PDMA memory to WC. > > > > > > > > > > Is there a particular reason why WC was selected here? I thought for > > > > > the p2pdma cases there was no kernel user that touched the memory? > > > > > > > > Yes, that's correct. I choose WC here because the existing users are > > > > registering memory blocks without side effects which fit the WC > > > > semantics well. > > > > > > Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in > > > Linux, so while it is true the memory has no side effects, there would > > > be surprising concurrency risks if anything in the kernel tried to > > > write to it. > > > > > > Not compatible means the locks don't contain stores to WC memory the > > > way you would expect. AFAIK on many CPUs extra barriers are required > > > to keep WC stores ordered, the same way ARM already has extra barriers > > > to keep UC stores ordered with locking.. > > > > > > The spinlocks are defined to contain UC stores though. > > > > How are spinlocks and mutexes getting into p2pdma ranges in the first > > instance? Even with UC, the system has bigger problems if it's trying > > to send bus locks targeting PCI, see the flurry of activity of trying > > to trigger faults on split locks [1]. > > This is not what I was trying to explain. > > Consider > > static spinlock lock; // CPU DRAM > static idx = 0; > u64 *wc_memory = [..]; > > spin_lock(); > wc_memory[0] = idx++; > spin_unlock(); > > You'd expect that the PCI device will observe stores where idx is > strictly increasing, but this is not guarenteed. idx may decrease, idx > may skip. It just won't duplicate. > > Or perhaps > > wc_memory[0] = foo; > writel(doorbell) > > foo is not guarenteed observable by the device before doorbell reaches > the device. > > All of these are things that do not happen with UC or NC memory, and > are surprising violations of our programming model. > > Generic kernel code should never touch WC memory unless the code is > specifically designed to handle it. Ah, yes, agree.
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On Thu, Feb 27, 2020 at 09:55:04AM -0800, Dan Williams wrote: > On Thu, Feb 27, 2020 at 9:43 AM Jason Gunthorpe wrote: > > > > On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote: > > > > > > > > > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote: > > > >> Instead of this, this series proposes a change to arch_add_memory() > > > >> to take the pgprot required by the mapping which allows us to > > > >> explicitly set pagetable entries for P2PDMA memory to WC. > > > > > > > > Is there a particular reason why WC was selected here? I thought for > > > > the p2pdma cases there was no kernel user that touched the memory? > > > > > > Yes, that's correct. I choose WC here because the existing users are > > > registering memory blocks without side effects which fit the WC > > > semantics well. > > > > Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in > > Linux, so while it is true the memory has no side effects, there would > > be surprising concurrency risks if anything in the kernel tried to > > write to it. > > > > Not compatible means the locks don't contain stores to WC memory the > > way you would expect. AFAIK on many CPUs extra barriers are required > > to keep WC stores ordered, the same way ARM already has extra barriers > > to keep UC stores ordered with locking.. > > > > The spinlocks are defined to contain UC stores though. > > How are spinlocks and mutexes getting into p2pdma ranges in the first > instance? Even with UC, the system has bigger problems if it's trying > to send bus locks targeting PCI, see the flurry of activity of trying > to trigger faults on split locks [1]. This is not what I was trying to explain. Consider static spinlock lock; // CPU DRAM static idx = 0; u64 *wc_memory = [..]; spin_lock(); wc_memory[0] = idx++; spin_unlock(); You'd expect that the PCI device will observe stores where idx is strictly increasing, but this is not guarenteed. idx may decrease, idx may skip. It just won't duplicate. Or perhaps wc_memory[0] = foo; writel(doorbell) foo is not guarenteed observable by the device before doorbell reaches the device. All of these are things that do not happen with UC or NC memory, and are surprising violations of our programming model. Generic kernel code should never touch WC memory unless the code is specifically designed to handle it. Jason
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On Thu, Feb 27, 2020 at 9:43 AM Jason Gunthorpe wrote: > > On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote: > > > > > > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote: > > >> Instead of this, this series proposes a change to arch_add_memory() > > >> to take the pgprot required by the mapping which allows us to > > >> explicitly set pagetable entries for P2PDMA memory to WC. > > > > > > Is there a particular reason why WC was selected here? I thought for > > > the p2pdma cases there was no kernel user that touched the memory? > > > > Yes, that's correct. I choose WC here because the existing users are > > registering memory blocks without side effects which fit the WC > > semantics well. > > Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in > Linux, so while it is true the memory has no side effects, there would > be surprising concurrency risks if anything in the kernel tried to > write to it. > > Not compatible means the locks don't contain stores to WC memory the > way you would expect. AFAIK on many CPUs extra barriers are required > to keep WC stores ordered, the same way ARM already has extra barriers > to keep UC stores ordered with locking.. > > The spinlocks are defined to contain UC stores though. How are spinlocks and mutexes getting into p2pdma ranges in the first instance? Even with UC, the system has bigger problems if it's trying to send bus locks targeting PCI, see the flurry of activity of trying to trigger faults on split locks [1]. This does raise a question about separating the cacheability of the 'struct page' memmap from the BAR range. You get this for free if the memmap is dynamically allocated from "System RAM", but perhaps memremap_pages() should explicitly prevent altmap configurations that try to place the map in PCI space? > If there is no actual need today for WC I would suggest using UC as > the default. That's reasonable, but it still seems to be making a broken configuration marginally less broken. I'd be more interested in safeguards that prevent p2pdma mappings from being used for any cpu atomic cycles. [1]: https://lwn.net/Articles/784864/
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On 2020-02-27 10:43 a.m., Jason Gunthorpe wrote: > Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in > Linux, so while it is true the memory has no side effects, there would > be surprising concurrency risks if anything in the kernel tried to > write to it. > > Not compatible means the locks don't contain stores to WC memory the > way you would expect. AFAIK on many CPUs extra barriers are required > to keep WC stores ordered, the same way ARM already has extra barriers > to keep UC stores ordered with locking.. > > The spinlocks are defined to contain UC stores though. > > If there is no actual need today for WC I would suggest using UC as > the default. Ok, that sounds sensible. I'll do that in the next revision. Thanks, Logan
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote: >> Instead of this, this series proposes a change to arch_add_memory() >> to take the pgprot required by the mapping which allows us to >> explicitly set pagetable entries for P2PDMA memory to WC. > > Is there a particular reason why WC was selected here? I thought for > the p2pdma cases there was no kernel user that touched the memory? Yes, that's correct. I choose WC here because the existing users are registering memory blocks without side effects which fit the WC semantics well. > I definitely forsee devices where we want UC instead. Yes. My expectation is that once we have a kernel user that needs this, we'd wire the option through struct dev_pagemap so the caller can choose the mapping that makes sense. Logan
Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote: > On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen wrote: > > > > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote: > > > asrc_format is more inteligent variable, which is align > > > with the alsa definition snd_pcm_format_t. > > > > > > Signed-off-by: Shengjiu Wang > > > --- > > > sound/soc/fsl/fsl_asrc.c | 23 +++ > > > sound/soc/fsl/fsl_asrc.h | 4 ++-- > > > sound/soc/fsl/fsl_asrc_dma.c | 2 +- > > > 3 files changed, 14 insertions(+), 15 deletions(-) > > > > > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c > > > index 0dcebc24c312..2b6a1643573c 100644 > > > --- a/sound/soc/fsl/fsl_asrc.c > > > +++ b/sound/soc/fsl/fsl_asrc.c > > > > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct > > > snd_pcm_substream *substream, > > > > > > pair->config = > > > > > > - if (asrc_priv->asrc_width == 16) > > > - format = SNDRV_PCM_FORMAT_S16_LE; > > > - else > > > - format = SNDRV_PCM_FORMAT_S24_LE; > > > > It feels better to me that we have format settings in hw_params(). > > > > Why not let fsl_easrc align with this? Any reason that I'm missing? > > because the asrc_width is not formal, in the future we can direct Hmm..that's our DT binding. And I don't feel it is a problem to be ASoC irrelative. > input the format from the dts. format involve the info about width. Is there such any formal ASoC binding? I don't see those PCM formats under include/dt-bindings/ folder. How are we going to involve those formats in DT?
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote: > > > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote: > >> Instead of this, this series proposes a change to arch_add_memory() > >> to take the pgprot required by the mapping which allows us to > >> explicitly set pagetable entries for P2PDMA memory to WC. > > > > Is there a particular reason why WC was selected here? I thought for > > the p2pdma cases there was no kernel user that touched the memory? > > Yes, that's correct. I choose WC here because the existing users are > registering memory blocks without side effects which fit the WC > semantics well. Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in Linux, so while it is true the memory has no side effects, there would be surprising concurrency risks if anything in the kernel tried to write to it. Not compatible means the locks don't contain stores to WC memory the way you would expect. AFAIK on many CPUs extra barriers are required to keep WC stores ordered, the same way ARM already has extra barriers to keep UC stores ordered with locking.. The spinlocks are defined to contain UC stores though. If there is no actual need today for WC I would suggest using UC as the default. Jason
Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
On Fri, Feb 21, 2020 at 11:24:56AM -0700, Logan Gunthorpe wrote: > Hi, > > This is v3 of the patchset which cleans up a number of minor issues > from the feedback of v2 and rebases onto v5.6-rc2. Additional feedback > is welcome. > > Thanks, > > Logan > > -- > > Changes in v3: > * Rebased onto v5.6-rc2 > * Rename mhp_modifiers to mhp_params per David with an updated kernel >doc per Dan > * Drop support for s390 per David seeing it does not support >ZONE_DEVICE yet and there was a potential problem with huge pages. > * Added WARN_ON_ONCE in cases where arches recieve non PAGE_KERNEL >parameters > * Collected David and Micheal's Reviewed-By and Acked-by Tags > > Changes in v2: > * Rebased onto v5.5-rc5 > * Renamed mhp_restrictions to mhp_modifiers and added the pgprot field >to that structure instead of using an argument for >arch_add_memory(). > * Add patch to drop the unused flags field in mhp_restrictions > > A git branch is available here: > > https://github.com/sbates130272/linux-p2pmem remap_pages_cache_v3 > > -- > > Currently, the page tables created using memremap_pages() are always > created with the PAGE_KERNEL cacheing mode. However, the P2PDMA code > is creating pages for PCI BAR memory which should never be accessed > through the cache and instead use either WC or UC. This still works in > most cases, on x86, because the MTRR registers typically override the > caching settings in the page tables for all of the IO memory to be > UC-. However, this tends not to work so well on other arches or > some rare x86 machines that have firmware which does not setup the > MTRR registers in this way. > > Instead of this, this series proposes a change to arch_add_memory() > to take the pgprot required by the mapping which allows us to > explicitly set pagetable entries for P2PDMA memory to WC. Is there a particular reason why WC was selected here? I thought for the p2pdma cases there was no kernel user that touched the memory? I definitely forsee devices where we want UC instead. Even so, the whole idea looks like the right direction to me. Jason
Re: [PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
Russel,
Le 27/02/2020 à 12:49, Christophe Leroy a écrit :
ptrace_triggered() is declared in asm/hw_breakpoint.h and
only needed when CONFIG_HW_BREAKPOINT is set, so move it
into hw_breakpoint.c
My series v4 is definitely buggy (I included ptrace_decl.h instead
instead of ptrace-decl.h), how can Snowpatch say build succeeded
(https://patchwork.ozlabs.org/patch/1245807/) ?
It fails at least on pmac32_defconfig and ppc64_defconfig, see:
http://kisskb.ellerman.id.au/kisskb/head/d45c91cf5f83424b8f3989b7ead28c50d8d765a9/
Christophe
Signed-off-by: Christophe Leroy
---
v4: removing inclusing of hw_breakpoint.h now. Previously it was done too early.
---
arch/powerpc/kernel/hw_breakpoint.c | 16
arch/powerpc/kernel/ptrace/ptrace.c | 19 ---
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 2462cd7c565c..2c0be9d941cf 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -427,3 +427,19 @@ void hw_breakpoint_pmu_read(struct perf_event *bp)
{
/* TODO */
}
+
+void ptrace_triggered(struct perf_event *bp,
+ struct perf_sample_data *data, struct pt_regs *regs)
+{
+ struct perf_event_attr attr;
+
+ /*
+* Disable the breakpoint request here since ptrace has defined a
+* one-shot behaviour for breakpoint exceptions in PPC64.
+* The SIGTRAP signal is generated automatically for us in do_dabr().
+* We don't have to do anything about that here
+*/
+ attr = bp->attr;
+ attr.disabled = true;
+ modify_user_hw_breakpoint(bp, );
+}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index a44f6e5e05ff..f6e51be47c6e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include
@@ -31,24 +30,6 @@
#include "ptrace-decl.h"
-#ifdef CONFIG_HAVE_HW_BREAKPOINT
-void ptrace_triggered(struct perf_event *bp,
- struct perf_sample_data *data, struct pt_regs *regs)
-{
- struct perf_event_attr attr;
-
- /*
-* Disable the breakpoint request here since ptrace has defined a
-* one-shot behaviour for breakpoint exceptions in PPC64.
-* The SIGTRAP signal is generated automatically for us in do_dabr().
-* We don't have to do anything about that here
-*/
- attr = bp->attr;
- attr.disabled = true;
- modify_user_hw_breakpoint(bp, );
-}
-#endif /* CONFIG_HAVE_HW_BREAKPOINT */
-
/*
* Called by kernel/ptrace.c when detaching..
*
Re: [PATCH] selftests/vm: Fix map_hugetlb length used for testing read and write
Shuah,
Le 06/02/2020 à 09:42, Christophe Leroy a écrit :
Commit fa7b9a805c79 ("tools/selftest/vm: allow choosing mem size and
page size in map_hugetlb") added the possibility to change the size
of memory mapped for the test, but left the read and write test using
the default value. This is unnoticed when mapping a length greater
than the default one, but segfaults otherwise.
Fix read_bytes() and write_bytes() by giving them the real length.
Also fix the call to munmap().
Fixes: fa7b9a805c79 ("tools/selftest/vm: allow choosing mem size and page size in
map_hugetlb")
Cc: sta...@vger.kernel.org
Can you also consider this one for next rc ?
Thanks
Christophe
Signed-off-by: Christophe Leroy
---
tools/testing/selftests/vm/map_hugetlb.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/vm/map_hugetlb.c
b/tools/testing/selftests/vm/map_hugetlb.c
index 5a2d7b8efc40..6af951900aa3 100644
--- a/tools/testing/selftests/vm/map_hugetlb.c
+++ b/tools/testing/selftests/vm/map_hugetlb.c
@@ -45,20 +45,20 @@ static void check_bytes(char *addr)
printf("First hex is %x\n", *((unsigned int *)addr));
}
-static void write_bytes(char *addr)
+static void write_bytes(char *addr, size_t length)
{
unsigned long i;
- for (i = 0; i < LENGTH; i++)
+ for (i = 0; i < length; i++)
*(addr + i) = (char)i;
}
-static int read_bytes(char *addr)
+static int read_bytes(char *addr, size_t length)
{
unsigned long i;
check_bytes(addr);
- for (i = 0; i < LENGTH; i++)
+ for (i = 0; i < length; i++)
if (*(addr + i) != (char)i) {
printf("Mismatch at %lu\n", i);
return 1;
@@ -96,11 +96,11 @@ int main(int argc, char **argv)
printf("Returned address is %p\n", addr);
check_bytes(addr);
- write_bytes(addr);
- ret = read_bytes(addr);
+ write_bytes(addr, length);
+ ret = read_bytes(addr, length);
/* munmap() length of MAP_HUGETLB memory must be hugepage aligned */
- if (munmap(addr, LENGTH)) {
+ if (munmap(addr, length)) {
perror("munmap");
exit(1);
}
Re: [PATCH v3 15/27] powerpc/powernv/pmem: Add support for near storage commands
On Thu, Feb 20, 2020 at 7:28 PM Alastair D'Silva wrote: > > From: Alastair D'Silva > > Similar to the previous patch, this adds support for near storage commands. Similar comment as the last patch. This changelog does not give the reviewer any frame of reference to review the patch.
Re: [PATCH v3 14/27] powerpc/powernv/pmem: Add support for Admin commands
On Thu, Feb 20, 2020 at 7:28 PM Alastair D'Silva wrote: > > From: Alastair D'Silva > > This patch requests the metadata required to issue admin commands, as well > as some helper functions to construct and check the completion of the > commands. What are the admin commands? Any pointer to a spec? Why does Linux need to support these commands?
Re: [PATCH] selftest/lkdtm: Use local .gitignore
Le 27/02/2020 à 17:45, Shuah Khan a écrit :
On 2/27/20 9:17 AM, Kees Cook wrote:
On Thu, Feb 27, 2020 at 02:07:10PM +, Christophe Leroy wrote:
Commit 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
introduced patterns for git to ignore files generated in
tools/testing/selftests/lkdtm/
Use local .gitignore file instead of using the root one.
Fixes: 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
Signed-off-by: Christophe Leroy
Yeah, that's better. Thanks!
Acked-by: Kees Cook
I will apply it for next rc.
Thanks. I should have noticed the problem in the previous version.
It slipped by me. :(
My fault, I didn't even know we could have .gitignore in subdirectories.
Christophe
Re: [PATCH] selftest/lkdtm: Use local .gitignore
On 2/27/20 9:17 AM, Kees Cook wrote:
On Thu, Feb 27, 2020 at 02:07:10PM +, Christophe Leroy wrote:
Commit 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
introduced patterns for git to ignore files generated in
tools/testing/selftests/lkdtm/
Use local .gitignore file instead of using the root one.
Fixes: 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
Signed-off-by: Christophe Leroy
Yeah, that's better. Thanks!
Acked-by: Kees Cook
I will apply it for next rc.
Thanks. I should have noticed the problem in the previous version.
It slipped by me. :(
thanks,
-- Shuah
Re: [PATCH] selftest/lkdtm: Use local .gitignore
On Thu, Feb 27, 2020 at 02:07:10PM +, Christophe Leroy wrote:
> Commit 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
> introduced patterns for git to ignore files generated in
> tools/testing/selftests/lkdtm/
>
> Use local .gitignore file instead of using the root one.
>
> Fixes: 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
> Signed-off-by: Christophe Leroy
Yeah, that's better. Thanks!
Acked-by: Kees Cook
-Kees
> ---
> .gitignore | 4
> tools/testing/selftests/lkdtm/.gitignore | 2 ++
> 2 files changed, 2 insertions(+), 4 deletions(-)
> create mode 100644 tools/testing/selftests/lkdtm/.gitignore
>
> diff --git a/.gitignore b/.gitignore
> index bb05dce58f8e..b849a72d69d5 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -100,10 +100,6 @@ modules.order
> /include/ksym/
> /arch/*/include/generated/
>
> -# Generated lkdtm tests
> -/tools/testing/selftests/lkdtm/*.sh
> -!/tools/testing/selftests/lkdtm/run.sh
> -
> # stgit generated dirs
> patches-*
>
> diff --git a/tools/testing/selftests/lkdtm/.gitignore
> b/tools/testing/selftests/lkdtm/.gitignore
> new file mode 100644
> index ..f26212605b6b
> --- /dev/null
> +++ b/tools/testing/selftests/lkdtm/.gitignore
> @@ -0,0 +1,2 @@
> +*.sh
> +!run.sh
> --
> 2.25.0
>
--
Kees Cook
Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
On 2/27/20 5:00 PM, Sachin Sant wrote:
>
>
>> On 27-Feb-2020, at 5:42 PM, Michal Hocko wrote:
>>
>> A very good hint indeed. I would do this
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index eb2fe6edd73c..d9f1b6737e4d 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -137,6 +137,8 @@ static inline void set_numa_mem(int node)
>> {
>> this_cpu_write(_numa_mem_, node);
>> _node_numa_mem_[numa_node_id()] = node;
>> +pr_info("%s %d -> %d\n", __FUNCTION__, numa_node_id(), node);
>> +dump_stack();
>> }
>> #endif
>>
>> Btw. it would be also helpful to get
>> `faddr2line ___slab_alloc+0x334' from your kernel Sachin.
>
> [linux-next]# ./scripts/faddr2line ./vmlinux ___slab_alloc+0x334
> ___slab_alloc+0x334/0x760:
> new_slab_objects at mm/slub.c:2478
> (inlined by) ___slab_alloc at mm/slub.c:2628
> [linux-next]#
Hmm that doesn't look relevant, but that address was marked as unreliable, no?
Don't we actually need this one?
[8.768727] NIP [c03d55f4] ___slab_alloc+0x1f4/0x760
> I have also attached boot log with a kernel that include about change.
> I see the following o/p during boot:
>
> [0.005269] set_numa_mem 1 -> 1
So there's no "set_numa_mem 0 -> X", specifically not
"set_numa_mem 0 -> 1" which I would have expected. That seems to confirm my
suspicion that the arch code doesn't set up the memoryless node 0 properly.
> [0.005270] CPU: 12 PID: 0 Comm: swapper/12 Not tainted
> 5.6.0-rc3-next-20200227-autotest+ #6
> [0.005271] Call Trace:
> [0.005272] [c008b37dfe80] [c0b5d948] dump_stack+0xbc/0x104
> (unreliable)
> [0.005274] [c008b37dfec0] [c0059320]
> start_secondary+0x600/0x6e0
> [0.005277] [c008b37dff90] [c000ac54]
> start_secondary_prolog+0x10/0x14
>
> Thanks
> -Sachin
>
[Bug 206669] Little-endian kernel crashing on POWER8 on heavy big-endian PowerKVM load
https://bugzilla.kernel.org/show_bug.cgi?id=206669 --- Comment #7 from John Paul Adrian Glaubitz (glaub...@physik.fu-berlin.de) --- I have set /sys/kernel/debug/tracing/tracing_on to "0" and /sys/kernel/debug/tracing/free_buffer to "1" and it seems I can no longer reproduce the issue. I will have to do more testing to see if that's just an artifact or really related. -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
> On 27-Feb-2020, at 5:42 PM, Michal Hocko wrote:
>
> A very good hint indeed. I would do this
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index eb2fe6edd73c..d9f1b6737e4d 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -137,6 +137,8 @@ static inline void set_numa_mem(int node)
> {
> this_cpu_write(_numa_mem_, node);
> _node_numa_mem_[numa_node_id()] = node;
> + pr_info("%s %d -> %d\n", __FUNCTION__, numa_node_id(), node);
> + dump_stack();
> }
> #endif
>
> Btw. it would be also helpful to get
> `faddr2line ___slab_alloc+0x334' from your kernel Sachin.
[linux-next]# ./scripts/faddr2line ./vmlinux ___slab_alloc+0x334
___slab_alloc+0x334/0x760:
new_slab_objects at mm/slub.c:2478
(inlined by) ___slab_alloc at mm/slub.c:2628
[linux-next]#
I have also attached boot log with a kernel that include about change.
I see the following o/p during boot:
[0.005269] set_numa_mem 1 -> 1
[0.005270] CPU: 12 PID: 0 Comm: swapper/12 Not tainted
5.6.0-rc3-next-20200227-autotest+ #6
[0.005271] Call Trace:
[0.005272] [c008b37dfe80] [c0b5d948] dump_stack+0xbc/0x104
(unreliable)
[0.005274] [c008b37dfec0] [c0059320] start_secondary+0x600/0x6e0
[0.005277] [c008b37dff90] [c000ac54]
start_secondary_prolog+0x10/0x14
Thanks
-Sachin
boot.log
Description: Binary data
Re: [PATCH, v2] powerpc: fix hardware PMU exception bug on PowerVM compatibility mode systems
On Thu, 2020-02-27 at 10:47 -0300, Desnes A. Nunes do Rosario wrote:
> PowerVM systems running compatibility mode on a few Power8 revisions are
> still vulnerable to the hardware defect that loses PMU exceptions arriving
> prior to a context switch.
>
> The software fix for this issue is enabled through the CPU_FTR_PMAO_BUG
> cpu_feature bit, nevertheless this bit also needs to be set for PowerVM
> compatibility mode systems.
>
> Fixes: 68f2f0d431d9ea4 ("powerpc: Add a cpu feature CPU_FTR_PMAO_BUG")
> Signed-off-by: Desnes A. Nunes do Rosario
> ---
> arch/powerpc/kernel/cputable.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index e745abc5457a..245be4fafe13 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -2193,11 +2193,13 @@ static struct cpu_spec * __init
> setup_cpu_spec(unsigned long offset,
>* oprofile_cpu_type already has a value, then we are
>* possibly overriding a real PVR with a logical one,
>* and, in that case, keep the current value for
> - * oprofile_cpu_type.
> + * oprofile_cpu_type. Futhermore, let's ensure that the
> + * fix for the PMAO bug is enabled on compatibility mode.
>*/
> if (old.oprofile_cpu_type != NULL) {
> t->oprofile_cpu_type = old.oprofile_cpu_type;
> t->oprofile_type = old.oprofile_type;
> + t->cpu_features |= old.cpu_features & CPU_FTR_PMAO_BUG;
> }
> }
>
Looks good to me.
Reviewed-by: Leonardo Bras
signature.asc
Description: This is a digitally signed message part
[PATCH] selftest/lkdtm: Use local .gitignore
Commit 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
introduced patterns for git to ignore files generated in
tools/testing/selftests/lkdtm/
Use local .gitignore file instead of using the root one.
Fixes: 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
Signed-off-by: Christophe Leroy
---
.gitignore | 4
tools/testing/selftests/lkdtm/.gitignore | 2 ++
2 files changed, 2 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/lkdtm/.gitignore
diff --git a/.gitignore b/.gitignore
index bb05dce58f8e..b849a72d69d5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -100,10 +100,6 @@ modules.order
/include/ksym/
/arch/*/include/generated/
-# Generated lkdtm tests
-/tools/testing/selftests/lkdtm/*.sh
-!/tools/testing/selftests/lkdtm/run.sh
-
# stgit generated dirs
patches-*
diff --git a/tools/testing/selftests/lkdtm/.gitignore
b/tools/testing/selftests/lkdtm/.gitignore
new file mode 100644
index ..f26212605b6b
--- /dev/null
+++ b/tools/testing/selftests/lkdtm/.gitignore
@@ -0,0 +1,2 @@
+*.sh
+!run.sh
--
2.25.0
[PATCH, v2] powerpc: fix hardware PMU exception bug on PowerVM compatibility mode systems
PowerVM systems running compatibility mode on a few Power8 revisions are
still vulnerable to the hardware defect that loses PMU exceptions arriving
prior to a context switch.
The software fix for this issue is enabled through the CPU_FTR_PMAO_BUG
cpu_feature bit, nevertheless this bit also needs to be set for PowerVM
compatibility mode systems.
Fixes: 68f2f0d431d9ea4 ("powerpc: Add a cpu feature CPU_FTR_PMAO_BUG")
Signed-off-by: Desnes A. Nunes do Rosario
---
arch/powerpc/kernel/cputable.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index e745abc5457a..245be4fafe13 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -2193,11 +2193,13 @@ static struct cpu_spec * __init setup_cpu_spec(unsigned
long offset,
* oprofile_cpu_type already has a value, then we are
* possibly overriding a real PVR with a logical one,
* and, in that case, keep the current value for
-* oprofile_cpu_type.
+* oprofile_cpu_type. Futhermore, let's ensure that the
+* fix for the PMAO bug is enabled on compatibility mode.
*/
if (old.oprofile_cpu_type != NULL) {
t->oprofile_cpu_type = old.oprofile_cpu_type;
t->oprofile_type = old.oprofile_type;
+ t->cpu_features |= old.cpu_features & CPU_FTR_PMAO_BUG;
}
}
--
2.21.1
Re: [PATCH] powerpc: fix hardware PMU exception bug on PowerVM compatibility mode systems
Hello Leonardo,
On 2/15/20 2:39 AM, Leonardo Bras wrote:
Hello Desnes, thanks for the patch.
"Desnes A. Nunes do Rosario" writes:
PowerVM systems running compatibility mode on a few Power8 revisions are
still vulnerable to the hardware defect that loses PMU exceptions arriving
prior to a context switch.
The software fix for this issue is enabled through the CPU_FTR_PMAO_BUG
cpu_feature bit, nevertheless this bit also needs to be set for PowerVM
compatibility mode systems.
Fixes: 68f2f0d431d9ea4 ("powerpc: Add a cpu feature CPU_FTR_PMAO_BUG")
Signed-off-by: Desnes A. Nunes do Rosario
---
arch/powerpc/kernel/cputable.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index e745abc5457a..5bfef6263dfb 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -2198,6 +2198,8 @@ static struct cpu_spec * __init setup_cpu_spec(unsigned
long offset,
if (old.oprofile_cpu_type != NULL) {
t->oprofile_cpu_type = old.oprofile_cpu_type;
t->oprofile_type = old.oprofile_type;
+ if (old.cpu_features & CPU_FTR_PMAO_BUG)
+ t->cpu_features |= CPU_FTR_PMAO_BUG;
What are your thoughts about doing:
t->cpu_features |= old.cpu_features & CPU_FTR_PMAO_BUG;
Looks good to me.
Also, I would recommend adding a short comment on top of the added
lines explaining why it is needed.
Sure, I'll send a second version.
Thanks for the review.
Best regards,
Leonardo Bras
--
Desnes A. Nunes do Rosario
Advisory Software Engineer - IBM
Virtual Onsite Engineer - Red Hat
Re: [PATCH] powerpc/watchpoint: Don't call dar_within_range() for Book3S
On Sat, 2020-02-22 at 08:20:49 UTC, Ravi Bangoria wrote:
> DAR is set to the first byte of overlap between actual access and
> watched range at DSI on Book3S processor. But actual access range
> might or might not be within user asked range. So for Book3S, it
> must not call dar_within_range().
>
> This revert portion of commit 39413ae00967 ("powerpc/hw_breakpoints:
> Rewrite 8xx breakpoints to allow any address range size.").
>
> Before patch:
> # ./tools/testing/selftests/powerpc/ptrace/perf-hwbreak
> ...
> TESTED: No overlap
> FAILED: Partial overlap: 0 != 2
> TESTED: Partial overlap
> TESTED: No overlap
> FAILED: Full overlap: 0 != 2
> failure: perf_hwbreak
>
> After patch:
> TESTED: No overlap
> TESTED: Partial overlap
> TESTED: Partial overlap
> TESTED: No overlap
> TESTED: Full overlap
> success: perf_hwbreak
>
> Fixes: 39413ae00967 ("powerpc/hw_breakpoints: Rewrite 8xx breakpoints to
> allow any address range size.")
> Reported-by: Michael Ellerman
> Signed-off-by: Ravi Bangoria
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/e08658a657f974590809290c62e889f0fd420200
cheers
Re: [PATCH] powerpc: Include .BTF section
On Thu, 2020-02-20 at 11:31:32 UTC, "Naveen N. Rao" wrote: > Selecting CONFIG_DEBUG_INFO_BTF results in the below warning from ld: > ld: warning: orphan section `.BTF' from `.btf.vmlinux.bin.o' being placed > in section `.BTF' > > Include .BTF section in vmlinux explicitly to fix the same. > > Signed-off-by: Naveen N. Rao Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/cb0cc635c7a9fa8a3a0f75d4d896721819c63add cheers
Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
On Wed 26-02-20 23:29:24, Vlastimil Babka wrote:
> On 2/26/20 10:45 PM, Vlastimil Babka wrote:
> >
> >
> > if (node == NUMA_NO_NODE)
> > page = alloc_pages(flags, order);
> > else
> > page = __alloc_pages_node(node, flags, order);
> >
> > So yeah looks like SLUB's kmalloc_node() is supposed to behave like the
> > page allocator's __alloc_pages_node() and respect __GFP_THISNODE but not
> > enforce it by itself. There's probably just some missing data structure
> > initialization somewhere right now for memoryless nodes.
>
> Upon more digging, I think the problem could manifest if
> node_to_mem_node(0) (_node_numa_mem_[0]) returned 0 instead of 1,
> because it wasn't initialized properly for a memoryless node. Can you
> e.g. print it somewhere?
A very good hint indeed. I would do this
diff --git a/include/linux/topology.h b/include/linux/topology.h
index eb2fe6edd73c..d9f1b6737e4d 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -137,6 +137,8 @@ static inline void set_numa_mem(int node)
{
this_cpu_write(_numa_mem_, node);
_node_numa_mem_[numa_node_id()] = node;
+ pr_info("%s %d -> %d\n", __FUNCTION__, numa_node_id(), node);
+ dump_stack();
}
#endif
Btw. it would be also helpful to get
`faddr2line ___slab_alloc+0x334' from your kernel Sachin.
--
Michal Hocko
SUSE Labs
Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
On Wed 26-02-20 22:45:52, Vlastimil Babka wrote:
> On 2/26/20 7:41 PM, Michal Hocko wrote:
> > On Wed 26-02-20 18:25:28, Cristopher Lameter wrote:
> >> On Mon, 24 Feb 2020, Michal Hocko wrote:
> >>
> >>> Hmm, nasty. Is there any reason why kmalloc_node behaves differently
> >>> from the page allocator?
> >>
> >> The page allocator will do the same thing if you pass GFP_THISNODE and
> >> insist on allocating memory from a node that does not exist.
> >
> > I do not think that the page allocator would blow up even with
> > GFP_THISNODE. The allocation would just fail on memory less node.
> >
> > Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
> > semantic right? At least I do not see anything like that documented
> > anywhere.
>
> Seems like SLAB at least behaves like the page allocator. See
> cache_alloc_node() where it basically does:
>
> page = cache_grow_begin(cachep, gfp_exact_node(flags), nodeid);
> ...
> if (!page)
> fallback_alloc(cachep, flags)
>
> gfp_exact_node() adds __GFP_THISNODE among other things, so the initial
> attempt does try to stick only to the given node. But fallback_alloc()
> doesn't. In fact, even if kmalloc_node() was called with __GFP_THISNODE
> then it wouldn't work as intended, as fallback_alloc() doesn't get the
> nodeid, but instead will use numa_mem_id(). That part could probably be
> improved.
>
> SLUB's ___slab_alloc() has for example this:
> if (node != NUMA_NO_NODE && !node_present_pages(node))
Hmm, just a quick note. Shouldn't this be node_managed_pages? In most
cases the difference is negligible but I can imagine crazy setups where
all present pages are simply consumed.
> searchnode = node_to_mem_node(node);
>
> That's from Joonsoo's 2014 commit a561ce00b09e ("slub: fall back to
> node_to_mem_node() node if allocating on memoryless node"), suggesting
> that the scenario in this bug report should work. Perhaps it just got
> broken unintentionally later.
A very good reference. Thanks!
> And AFAICS the whole path leading to alloc_slab_page() also doesn't add
> __GFP_THISNODE, but will keep it if caller passed it, and ultimately it
> does:
>
>
> if (node == NUMA_NO_NODE)
> page = alloc_pages(flags, order);
> else
> page = __alloc_pages_node(node, flags, order);
>
> So yeah looks like SLUB's kmalloc_node() is supposed to behave like the
> page allocator's __alloc_pages_node() and respect __GFP_THISNODE but not
> enforce it by itself. There's probably just some missing data structure
> initialization somewhere right now for memoryless nodes.
Thanks for the confirmation!
--
Michal Hocko
SUSE Labs
[PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
ptrace_triggered() is declared in asm/hw_breakpoint.h and
only needed when CONFIG_HW_BREAKPOINT is set, so move it
into hw_breakpoint.c
Signed-off-by: Christophe Leroy
---
v4: removing inclusing of hw_breakpoint.h now. Previously it was done too early.
---
arch/powerpc/kernel/hw_breakpoint.c | 16
arch/powerpc/kernel/ptrace/ptrace.c | 19 ---
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 2462cd7c565c..2c0be9d941cf 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -427,3 +427,19 @@ void hw_breakpoint_pmu_read(struct perf_event *bp)
{
/* TODO */
}
+
+void ptrace_triggered(struct perf_event *bp,
+ struct perf_sample_data *data, struct pt_regs *regs)
+{
+ struct perf_event_attr attr;
+
+ /*
+* Disable the breakpoint request here since ptrace has defined a
+* one-shot behaviour for breakpoint exceptions in PPC64.
+* The SIGTRAP signal is generated automatically for us in do_dabr().
+* We don't have to do anything about that here
+*/
+ attr = bp->attr;
+ attr.disabled = true;
+ modify_user_hw_breakpoint(bp, );
+}
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index a44f6e5e05ff..f6e51be47c6e 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include
@@ -31,24 +30,6 @@
#include "ptrace-decl.h"
-#ifdef CONFIG_HAVE_HW_BREAKPOINT
-void ptrace_triggered(struct perf_event *bp,
- struct perf_sample_data *data, struct pt_regs *regs)
-{
- struct perf_event_attr attr;
-
- /*
-* Disable the breakpoint request here since ptrace has defined a
-* one-shot behaviour for breakpoint exceptions in PPC64.
-* The SIGTRAP signal is generated automatically for us in do_dabr().
-* We don't have to do anything about that here
-*/
- attr = bp->attr;
- attr.disabled = true;
- modify_user_hw_breakpoint(bp, );
-}
-#endif /* CONFIG_HAVE_HW_BREAKPOINT */
-
/*
* Called by kernel/ptrace.c when detaching..
*
--
2.25.0
[PATCH v4 12/13] powerpc/ptrace: create ppc_gethwdinfo()
Create ippc_gethwdinfo() to handle PPC_PTRACE_GETHWDBGINFO and
reduce ifdef mess
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/ptrace/ptrace-adv.c | 15 +++
arch/powerpc/kernel/ptrace/ptrace-decl.h | 1 +
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 20 ++
arch/powerpc/kernel/ptrace/ptrace.c | 32 +--
4 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c
b/arch/powerpc/kernel/ptrace/ptrace-adv.c
index c71bedd54a8b..3990c01ef8cf 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
@@ -56,6 +56,21 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
+{
+ dbginfo->version = 1;
+ dbginfo->num_instruction_bps = CONFIG_PPC_ADV_DEBUG_IACS;
+ dbginfo->num_data_bps = CONFIG_PPC_ADV_DEBUG_DACS;
+ dbginfo->num_condition_regs = CONFIG_PPC_ADV_DEBUG_DVCS;
+ dbginfo->data_bp_alignment = 4;
+ dbginfo->sizeof_condition = 4;
+ dbginfo->features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
+ PPC_DEBUG_FEATURE_INSN_BP_MASK;
+ if (IS_ENABLED(CONFIG_PPC_ADV_DEBUG_DAC_RANGE))
+ dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_RANGE |
+PPC_DEBUG_FEATURE_DATA_BP_MASK;
+}
+
int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
unsigned long __user *datalp)
{
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index 4b4b6a1d508a..3c8a81999292 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -176,6 +176,7 @@ int tm_cgpr32_set(struct task_struct *target, const struct
user_regset *regset,
extern const struct user_regset_view user_ppc_native_view;
/* ptrace-(no)adv */
+void ppc_gethwdinfo(struct ppc_debug_info *dbginfo);
int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
unsigned long __user *datalp);
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned
long data);
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 9c1ef9c2ffd4..8a616d87fffb 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -39,6 +39,26 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
+{
+ dbginfo->version = 1;
+ dbginfo->num_instruction_bps = 0;
+ if (ppc_breakpoint_available())
+ dbginfo->num_data_bps = 1;
+ else
+ dbginfo->num_data_bps = 0;
+ dbginfo->num_condition_regs = 0;
+ dbginfo->data_bp_alignment = sizeof(long);
+ dbginfo->sizeof_condition = 0;
+ if (IS_ENABLED(CONFIG_HAVE_HW_BREAKPOINT)) {
+ dbginfo->features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
+ if (dawr_enabled())
+ dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_DAWR;
+ } else {
+ dbginfo->features = 0;
+ }
+}
+
int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
unsigned long __user *datalp)
{
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
b/arch/powerpc/kernel/ptrace/ptrace.c
index d6e1a301d20e..a44f6e5e05ff 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -139,37 +139,7 @@ long arch_ptrace(struct task_struct *child, long request,
case PPC_PTRACE_GETHWDBGINFO: {
struct ppc_debug_info dbginfo;
- dbginfo.version = 1;
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- dbginfo.num_instruction_bps = CONFIG_PPC_ADV_DEBUG_IACS;
- dbginfo.num_data_bps = CONFIG_PPC_ADV_DEBUG_DACS;
- dbginfo.num_condition_regs = CONFIG_PPC_ADV_DEBUG_DVCS;
- dbginfo.data_bp_alignment = 4;
- dbginfo.sizeof_condition = 4;
- dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
- PPC_DEBUG_FEATURE_INSN_BP_MASK;
-#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
- dbginfo.features |=
- PPC_DEBUG_FEATURE_DATA_BP_RANGE |
- PPC_DEBUG_FEATURE_DATA_BP_MASK;
-#endif
-#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
- dbginfo.num_instruction_bps = 0;
- if (ppc_breakpoint_available())
- dbginfo.num_data_bps = 1;
- else
- dbginfo.num_data_bps = 0;
- dbginfo.num_condition_regs = 0;
- dbginfo.data_bp_alignment = sizeof(long);
- dbginfo.sizeof_condition = 0;
