* [PATCH v6 2/3] powerpc/powernv: Introduce support and parsing for self-save API
From: Pratik Rajesh Sampat @ 2020-03-26 7:10 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
psampat, pratik.r.sampat
In-Reply-To: <20200326071034.12838-1-psampat@linux.ibm.com>
This commit introduces and leverages the Self save API. The difference
between self-save and self-restore is that the value to be saved for the
SPR does not need to be passed to the call.
Add the new Self Save OPAL API call in the list of OPAL calls.
Implement the self saving of the SPRs based on the support populated.
This commit imposes the self-save over self-restore in case both are
supported for a particular SPR.
Along with support for self-save, kernel supported save restore is also
populated in the list. This property is only populated for those SPRs
which encapsulate support from the kernel and hav ethe possibility to
garner support from a firmware mode too.
In addition, the commit also parses the device tree for nodes self-save,
self-restore and populate support for the preferred SPRs based on what
was advertised by the device tree.
In the case a SPR is supported by the firmware self-save, self-restore
and kernel save restore then the preference of execution is also in the
same order as above.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
.../bindings/powerpc/opal/power-mgt.txt | 18 +++
arch/powerpc/include/asm/opal-api.h | 3 +-
arch/powerpc/include/asm/opal.h | 1 +
arch/powerpc/platforms/powernv/idle.c | 131 +++++++++++++++++-
arch/powerpc/platforms/powernv/opal-call.c | 1 +
5 files changed, 146 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
index 9d619e955576..5fb03c6d7de9 100644
--- a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
+++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
@@ -116,3 +116,21 @@ otherwise. The length of all the property arrays must be the same.
which of the fields of the PMICR are set in the corresponding
entries in ibm,cpu-idle-state-pmicr. This is an optional
property on POWER8 and is absent on POWER9.
+
+- self-restore:
+ Array of unsigned 64-bit values containing a property for sprn-mask
+ with each bit indicating the index of the supported SPR for the
+ functionality. This is an optional property for both Power8 and Power9
+
+- self-save:
+ Array of unsigned 64-bit values containing a property for sprn-mask
+ with each bit indicating the index of the supported SPR for the
+ functionality. This is an optional property for both Power8 and Power9
+
+Example of arrangement of self-restore and self-save arrays:
+For instance if PSSCR is supported, the value is 0x357 = 855.
+Since the array is of 64 bit values, the index of the array is determined by
+855 / 64 = 13th element. Within that index, the bit number is determined by
+855 % 64 = 23rd bit.
+This means that if the 23rd bit in array[13] is set, then that SPR is supported
+by the corresponding self-save or self-restore API.
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index c1f25a760eb1..1b6e1a68d431 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -214,7 +214,8 @@
#define OPAL_SECVAR_GET 176
#define OPAL_SECVAR_GET_NEXT 177
#define OPAL_SECVAR_ENQUEUE_UPDATE 178
-#define OPAL_LAST 178
+#define OPAL_SLW_SELF_SAVE_REG 181
+#define OPAL_LAST 181
#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..a370b0e8d899 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -204,6 +204,7 @@ int64_t opal_handle_hmi2(__be64 *out_flags);
int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
int64_t opal_unregister_dump_region(uint32_t id);
int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
+int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
int64_t opal_pci_get_pbcq_tunnel_bar(uint64_t phb_id, uint64_t *addr);
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 858ceb86394d..e77b31621081 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -35,13 +35,20 @@
/*
* Type of support for each SPR
* FIRMWARE_RESTORE: firmware restoration supported: calls self-restore OPAL API
+ * FIRMWARE_SELF_SAVE: firmware save and restore: calls self-save OPAL API
+ * KERNEL_SAVE_RESTORE: kernel handles the saving and restoring of SPR
*/
#define UNSUPPORTED 0x0
#define FIRMWARE_RESTORE 0x1
+#define FIRMWARE_SELF_SAVE 0x2
+#define KERNEL_SAVE_RESTORE 0x4
static u32 supported_cpuidle_states;
struct pnv_idle_states_t *pnv_idle_states;
int nr_pnv_idle_states;
+/* Caching the lpcr & ptcr support to use later */
+static bool is_lpcr_self_save;
+static bool is_ptcr_self_save;
struct preferred_sprs {
u64 spr;
@@ -51,6 +58,10 @@ struct preferred_sprs {
/*
* Supported mode: Default support. Can be overwritten during system
* initialization
+ * Note: SPRs with support for KERNEL_SAVE_RESTORE in this list are only those
+ * which have a possibility of support from another firmware mode (i.e self-save
+ * or self-restore)
+ * SPRs with exclusive kernel save support are implicit.
*/
struct preferred_sprs preferred_sprs[] = {
{
@@ -61,6 +72,10 @@ struct preferred_sprs preferred_sprs[] = {
.spr = SPRN_LPCR,
.supported_mode = FIRMWARE_RESTORE,
},
+ {
+ .spr = SPRN_PTCR,
+ .supported_mode = KERNEL_SAVE_RESTORE,
+ },
{
.spr = SPRN_HMEER,
.supported_mode = FIRMWARE_RESTORE,
@@ -219,11 +234,33 @@ static int pnv_self_save_restore_sprs(void)
curr_spr.spr == SPRN_HID4 ||
curr_spr.spr == SPRN_HID5))
continue;
- if (curr_spr.supported_mode & FIRMWARE_RESTORE) {
+
+ if (curr_spr.supported_mode & FIRMWARE_SELF_SAVE) {
+ rc = opal_slw_self_save_reg(pir,
+ curr_spr.spr);
+ if (rc != 0)
+ return rc;
+ switch (curr_spr.spr) {
+ case SPRN_LPCR:
+ is_lpcr_self_save = true;
+ break;
+ case SPRN_PTCR:
+ is_ptcr_self_save = true;
+ break;
+ }
+ } else if (curr_spr.supported_mode & FIRMWARE_RESTORE) {
rc = pnv_self_restore_sprs(pir, cpu,
curr_spr.spr);
if (rc != 0)
return rc;
+ } else {
+ if (curr_spr.supported_mode & KERNEL_SAVE_RESTORE ||
+ (cpu_has_feature(CPU_FTR_ARCH_300) &&
+ (curr_spr.spr == SPRN_HID1 ||
+ curr_spr.spr == SPRN_HID4 ||
+ curr_spr.spr == SPRN_HID5)))
+ continue;
+ return OPAL_UNSUPPORTED;
}
}
}
@@ -762,7 +799,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
mmcr0 = mfspr(SPRN_MMCR0);
}
if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
- sprs.lpcr = mfspr(SPRN_LPCR);
+ if (!is_lpcr_self_save)
+ sprs.lpcr = mfspr(SPRN_LPCR);
sprs.hfscr = mfspr(SPRN_HFSCR);
sprs.fscr = mfspr(SPRN_FSCR);
sprs.pid = mfspr(SPRN_PID);
@@ -776,7 +814,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
sprs.mmcr1 = mfspr(SPRN_MMCR1);
sprs.mmcr2 = mfspr(SPRN_MMCR2);
- sprs.ptcr = mfspr(SPRN_PTCR);
+ if (!is_ptcr_self_save)
+ sprs.ptcr = mfspr(SPRN_PTCR);
sprs.rpr = mfspr(SPRN_RPR);
sprs.tscr = mfspr(SPRN_TSCR);
if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
@@ -860,7 +899,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
goto core_woken;
/* Per-core SPRs */
- mtspr(SPRN_PTCR, sprs.ptcr);
+ if (!is_ptcr_self_save)
+ mtspr(SPRN_PTCR, sprs.ptcr);
mtspr(SPRN_RPR, sprs.rpr);
mtspr(SPRN_TSCR, sprs.tscr);
@@ -881,7 +921,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
atomic_unlock_and_stop_thread_idle();
/* Per-thread SPRs */
- mtspr(SPRN_LPCR, sprs.lpcr);
+ if (!is_lpcr_self_save)
+ mtspr(SPRN_LPCR, sprs.lpcr);
mtspr(SPRN_HFSCR, sprs.hfscr);
mtspr(SPRN_FSCR, sprs.fscr);
mtspr(SPRN_PID, sprs.pid);
@@ -1060,8 +1101,10 @@ void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
* Program the LPCR via stop-api only if the deepest stop state
* can lose hypervisor context.
*/
- if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT)
- opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+ if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT) {
+ if (!is_lpcr_self_save)
+ opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
+ }
}
/*
@@ -1316,6 +1359,77 @@ static void __init pnv_probe_idle_states(void)
supported_cpuidle_states |= pnv_idle_states[i].flags;
}
+/*
+ * Extracts and populates the self save or restore capabilities
+ * passed from the device tree node
+ */
+static int extract_save_restore_state_dt(struct device_node *np, u32 support)
+{
+ int nr_sprns = 0, i, bitmask_index;
+ u64 *temp_u64;
+ u64 bit_pos;
+
+ nr_sprns = of_property_count_u64_elems(np, "sprn-bitmask");
+ if (nr_sprns <= 0)
+ return -EINVAL;
+ temp_u64 = kcalloc(nr_sprns, sizeof(u64), GFP_KERNEL);
+ if (of_property_read_u64_array(np, "sprn-bitmask",
+ temp_u64, nr_sprns)) {
+ pr_warn("cpuidle-powernv: failed to find registers in DT\n");
+ kfree(temp_u64);
+ return -EINVAL;
+ }
+ /*
+ * Populate acknowledgment of support for the sprs in the global vector
+ * gotten by the registers supplied by the firmware.
+ * The registers are in a bitmask, bit index within
+ * that specifies the SPR
+ */
+ for (i = 0; i < nr_preferred_sprs; i++) {
+ bitmask_index = BIT_ULL_WORD(preferred_sprs[i].spr);
+ bit_pos = BIT_ULL_MASK(preferred_sprs[i].spr);
+ if ((temp_u64[bitmask_index] & bit_pos) == 0) {
+ preferred_sprs[i].supported_mode &= ~support;
+ continue;
+ }
+ preferred_sprs[i].supported_mode |= support;
+ }
+
+ kfree(temp_u64);
+ return 0;
+}
+
+static int pnv_parse_deepstate_dt(void)
+{
+ struct device_node *np;
+ int rc = 0, i;
+
+ /*
+ * Self restore register population
+ * In the case the node is not found, the support for self-restore for
+ * already populated SPRs is *not* cut. This is because self-restore
+ * assumes legacy support. In an event, self-restore is actually not
+ * supported then the call to the firmware fails and deep stop states
+ * will be cut.
+ */
+ np = of_find_compatible_node(NULL, NULL, "ibm,opal-self-restore");
+ if (np) {
+ rc = extract_save_restore_state_dt(np, FIRMWARE_RESTORE);
+ if (rc != 0)
+ return rc;
+ }
+ /* Self save register population */
+ np = of_find_compatible_node(NULL, NULL, "ibm,opal-self-save");
+ if (!np) {
+ for (i = 0; i < nr_preferred_sprs; i++)
+ preferred_sprs[i].supported_mode &= ~FIRMWARE_SELF_SAVE;
+ } else {
+ rc = extract_save_restore_state_dt(np, FIRMWARE_SELF_SAVE);
+ }
+ of_node_put(np);
+ return rc;
+}
+
/*
* This function parses device-tree and populates all the information
* into pnv_idle_states structure. It also sets up nr_pnv_idle_states
@@ -1464,6 +1578,9 @@ static int __init pnv_init_idle_states(void)
return rc;
pnv_probe_idle_states();
+ rc = pnv_parse_deepstate_dt();
+ if (rc)
+ return rc;
if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) {
power7_fastsleep_workaround_entry = false;
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 5cd0f52d258f..11e0ceb90de0 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -223,6 +223,7 @@ OPAL_CALL(opal_handle_hmi, OPAL_HANDLE_HMI);
OPAL_CALL(opal_handle_hmi2, OPAL_HANDLE_HMI2);
OPAL_CALL(opal_config_cpu_idle_state, OPAL_CONFIG_CPU_IDLE_STATE);
OPAL_CALL(opal_slw_set_reg, OPAL_SLW_SET_REG);
+OPAL_CALL(opal_slw_self_save_reg, OPAL_SLW_SELF_SAVE_REG);
OPAL_CALL(opal_register_dump_region, OPAL_REGISTER_DUMP_REGION);
OPAL_CALL(opal_unregister_dump_region, OPAL_UNREGISTER_DUMP_REGION);
OPAL_CALL(opal_pci_set_phb_cxl_mode, OPAL_PCI_SET_PHB_CAPI_MODE);
--
2.17.1
^ permalink raw reply related
* [PATCH v6 3/3] powerpc/powernv: Preference optimization for SPRs with constant values
From: Pratik Rajesh Sampat @ 2020-03-26 7:10 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, mpe, skiboot, oohall, ego, linuxram,
psampat, pratik.r.sampat
In-Reply-To: <20200326071034.12838-1-psampat@linux.ibm.com>
There are SPRs whose values don't tend to change over time and invoking
self-save on them, where the values are gotten each time may turn out to
be inefficient. In that case calling a self-restore where passing the
value makes more sense as, if the value is same the memory location
is not updated.
SPRs that dont change are as follows:
SPRN_HSPRG0,
SPRN_LPCR,
SPRN_PTCR,
SPRN_HMEER,
SPRN_HID0,
There are also SPRs whose values change and/or their value may not be
correcty determinable in the kernel. Eg: MSR and PSSCR
The value of LPCR is dynamic based on if the CPU is entered a stop
state during cpu idle versus cpu hotplug.
Therefore in this optimization patch, introducing the concept of
preference for each SPR to choose from in the case both self-save and
self-restore is supported.
The preference bitmask is shown as below:
----------------------------
|... | 2nd pref | 1st pref |
----------------------------
MSB LSB
The preference from higher to lower is from LSB to MSB with a shift of 8
bits.
Example:
Prefer self save first, if not available then prefer self
restore
The preference mask for this scenario will be seen as below.
((FIRMWARE_RESTORE << PREFERENCE_SHIFT) | FIRMWARE_SELF_SAVE)
---------------------------------
|... | Self restore | Self save |
---------------------------------
MSB LSB
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 88 +++++++++++++++++++++------
1 file changed, 70 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index e77b31621081..4d896df51582 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -43,6 +43,31 @@
#define FIRMWARE_SELF_SAVE 0x2
#define KERNEL_SAVE_RESTORE 0x4
+#define NR_PREFERENCES 2
+#define PREFERENCE_SHIFT 4
+#define PREFERENCE_MASK 0xf
+/*
+ * Bitmask defining the kind of preferences available.
+ * Note : The higher to lower preference is from LSB to MSB, with a shift of
+ * 4 bits.
+ * ----------------------------
+ * | | 2nd pref | 1st pref |
+ * ----------------------------
+ * MSB LSB
+ */
+/* Prefer Restore if available, otherwise unsupported */
+#define PREFER_SELF_RESTORE_ONLY FIRMWARE_RESTORE
+/* Prefer Save if available, otherwise unsupported */
+#define PREFER_SELF_SAVE_ONLY FIRMWARE_SELF_SAVE
+/* Prefer Restore when available, otherwise prefer Save */
+#define PREFER_RESTORE_SAVE ((FIRMWARE_SELF_SAVE << \
+ PREFERENCE_SHIFT)\
+ | FIRMWARE_RESTORE)
+/* Prefer Save when available, otherwise prefer Restore*/
+#define PREFER_SAVE_RESTORE ((FIRMWARE_RESTORE <<\
+ PREFERENCE_SHIFT)\
+ | FIRMWARE_SELF_SAVE)
+
static u32 supported_cpuidle_states;
struct pnv_idle_states_t *pnv_idle_states;
int nr_pnv_idle_states;
@@ -52,6 +77,7 @@ static bool is_ptcr_self_save;
struct preferred_sprs {
u64 spr;
+ u32 preferred_mode;
u32 supported_mode;
};
@@ -66,42 +92,52 @@ struct preferred_sprs {
struct preferred_sprs preferred_sprs[] = {
{
.spr = SPRN_HSPRG0,
+ .preferred_mode = PREFER_RESTORE_SAVE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = SPRN_LPCR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = SPRN_PTCR,
+ .preferred_mode = PREFER_RESTORE_SAVE,
.supported_mode = KERNEL_SAVE_RESTORE,
},
{
.spr = SPRN_HMEER,
+ .preferred_mode = PREFER_RESTORE_SAVE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = SPRN_HID0,
+ .preferred_mode = PREFER_RESTORE_SAVE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = P9_STOP_SPR_MSR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = P9_STOP_SPR_PSSCR,
+ .preferred_mode = PREFER_SAVE_RESTORE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = SPRN_HID1,
+ .preferred_mode = PREFER_RESTORE_SAVE,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = SPRN_HID4,
+ .preferred_mode = PREFER_SELF_RESTORE_ONLY,
.supported_mode = FIRMWARE_RESTORE,
},
{
.spr = SPRN_HID5,
+ .preferred_mode = PREFER_SELF_RESTORE_ONLY,
.supported_mode = FIRMWARE_RESTORE,
}
};
@@ -218,7 +254,9 @@ static int pnv_self_restore_sprs(u64 pir, int cpu, u64 spr)
static int pnv_self_save_restore_sprs(void)
{
- int rc, index, cpu;
+ int rc, index, cpu, k;
+ bool is_initialized;
+ u32 preferred;
u64 pir;
struct preferred_sprs curr_spr;
@@ -234,26 +272,40 @@ static int pnv_self_save_restore_sprs(void)
curr_spr.spr == SPRN_HID4 ||
curr_spr.spr == SPRN_HID5))
continue;
-
- if (curr_spr.supported_mode & FIRMWARE_SELF_SAVE) {
- rc = opal_slw_self_save_reg(pir,
- curr_spr.spr);
- if (rc != 0)
- return rc;
- switch (curr_spr.spr) {
- case SPRN_LPCR:
- is_lpcr_self_save = true;
+ for (k = 0; k < NR_PREFERENCES; k++) {
+ preferred = curr_spr.preferred_mode
+ & PREFERENCE_MASK;
+ if (preferred & curr_spr.supported_mode &
+ FIRMWARE_SELF_SAVE) {
+ is_initialized = true;
+ rc = opal_slw_self_save_reg(pir,
+ curr_spr.spr);
+ if (rc != 0)
+ return rc;
+ switch (curr_spr.spr) {
+ case SPRN_LPCR:
+ is_lpcr_self_save = true;
+ break;
+ case SPRN_PTCR:
+ is_ptcr_self_save = true;
+ break;
+ }
break;
- case SPRN_PTCR:
- is_ptcr_self_save = true;
+ } else if (preferred & curr_spr.supported_mode &
+ FIRMWARE_RESTORE) {
+ is_initialized = true;
+ rc = pnv_self_restore_sprs(pir, cpu,
+ curr_spr.spr);
+ if (rc != 0)
+ return rc;
break;
}
- } else if (curr_spr.supported_mode & FIRMWARE_RESTORE) {
- rc = pnv_self_restore_sprs(pir, cpu,
- curr_spr.spr);
- if (rc != 0)
- return rc;
- } else {
+ preferred_sprs[index].preferred_mode =
+ preferred_sprs[index].preferred_mode >>
+ PREFERENCE_SHIFT;
+ curr_spr = preferred_sprs[index];
+ }
+ if (!is_initialized) {
if (curr_spr.supported_mode & KERNEL_SAVE_RESTORE ||
(cpu_has_feature(CPU_FTR_ARCH_300) &&
(curr_spr.spr == SPRN_HID1 ||
--
2.17.1
^ permalink raw reply related
* [PATCH -next] usb: gadget: fsl: remove unused variable 'driver_desc'
From: YueHaibing @ 2020-03-26 7:14 UTC (permalink / raw)
To: leoyang.li, balbi, gregkh
Cc: YueHaibing, linux-usb, linuxppc-dev, linux-kernel
drivers/usb/gadget/udc/fsl_udc_core.c:56:19:
warning: 'driver_desc' defined but not used [-Wunused-const-variable=]
It is never used, so remove it.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/usb/gadget/udc/fsl_udc_core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index ec6eda426223..febabde62f71 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -53,7 +53,6 @@
#define DMA_ADDR_INVALID (~(dma_addr_t)0)
static const char driver_name[] = "fsl-usb2-udc";
-static const char driver_desc[] = DRIVER_DESC;
static struct usb_dr_device __iomem *dr_regs;
--
2.17.1
^ permalink raw reply related
* Re: [PATCHv3] powerpc/crashkernel: take "mem=" option into account
From: Hari Bathini @ 2020-03-26 7:48 UTC (permalink / raw)
To: Pingfan Liu, linuxppc-dev; +Cc: kexec
In-Reply-To: <1582121897-24336-1-git-send-email-kernelfans@gmail.com>
Hello Pingfan,
Thanks for the patch..
On 19/02/20 7:48 PM, Pingfan Liu wrote:
> 'mem=" option is an easy way to put high pressure on memory during some
> test. Hence after applying the memory limit, instead of total mem, the
> actual usable memory should be considered when reserving mem for
> crashkernel. Otherwise the boot up may experience OOM issue.
>
> E.g. it would reserve 4G prior to the change and 512M afterward, if passing
> crashkernel="2G-4G:384M,4G-16G:512M,16G-64G:1G,64G-128G:2G,128G-:4G", and
> mem=5G on a 256G machine.
>
> This issue is powerpc specific because it puts higher priority on fadump
> and kdump reservation than on "mem=". Referring the following code:
> if (fadump_reserve_mem() == 0)
> reserve_crashkernel();
> ...
> /* Ensure that total memory size is page-aligned. */
> limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE);
> memblock_enforce_memory_limit(limit);
>
> While on other arches, the effect of "mem=" takes a higher priority and pass
> through memblock_phys_mem_size() before calling reserve_crashkernel().
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: kexec@lists.infradead.org
> ---
> v2 -> v3: improve commit log
> arch/powerpc/kernel/machine_kexec.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index c4ed328..eec96dc 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -114,11 +114,12 @@ void machine_kexec(struct kimage *image)
>
> void __init reserve_crashkernel(void)
> {
> - unsigned long long crash_size, crash_base;
> + unsigned long long crash_size, crash_base, total_mem_sz;
> int ret;
>
> + total_mem_sz = memory_limit ? memory_limit : memblock_phys_mem_size();
> /* use common parsing */
> - ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> + ret = parse_crashkernel(boot_command_line, total_mem_sz,
> &crash_size, &crash_base);
> if (ret == 0 && crash_size > 0) {
> crashk_res.start = crash_base;
memory_limit is adjusted after this with the below snippet:
/* Crash kernel trumps memory limit */
if (memory_limit && memory_limit <= crashk_res.end) {
memory_limit = crashk_res.end + 1;
printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
memory_limit);
}
So, either the above snippet must be dropped or the print below should use an updated total_mem_sz
based on adjusted memory_limit. I would prefer the latter..
> @@ -185,7 +186,7 @@ void __init reserve_crashkernel(void)
> "for crashkernel (System RAM: %ldMB)\n",
> (unsigned long)(crash_size >> 20),
> (unsigned long)(crashk_res.start >> 20),
> - (unsigned long)(memblock_phys_mem_size() >> 20));
> + (unsigned long)(total_mem_sz >> 20));
>
> if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
> memblock_reserve(crashk_res.start, crash_size)) {
>
--
- Hari
^ permalink raw reply
* [powerpc] Intermittent crashes ( link_path_walk) with linux-next
From: Sachin Sant @ 2020-03-26 8:27 UTC (permalink / raw)
To: linuxppc-dev, linux-fsdevel; +Cc: Linux Next Mailing List, Al Viro
[-- Attachment #1: Type: text/plain, Size: 4052 bytes --]
I am running into intermittent crashes with linux-next on POWER 9 PowerVM LPAR
First it was against next-20200324 while running LTP tests. With next-20200325
I ran into similar crash (a different stack trace but same failure point — link_path_walk)
while running sosreport command.
BUG: Kernel NULL pointer dereference on read at 0x00000000
Faulting instruction address: 0xc00000000043f278
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in: loop iscsi_target_mod target_core_mod macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc overlay dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod sd_mod cdrom t10_pi ibmvscsi scsi_transport_srp ibmveth
CPU: 26 PID: 7771 Comm: avocado Not tainted 5.6.0-rc7-next-20200324-autotest #1
NIP: c00000000043f278 LR: c00000000043f330 CTR: 00000000000001fb
REGS: c00000082802f830 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200324-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 28248442 XER: 20040000
CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
GPR00: c00000000043f330 c00000082802fac0 c00000000155e900 0000000000000000
GPR04: 0000000000000002 0000000000000000 0000000000000002 c0000008b3400000
GPR08: 0000000000031419 0000000000000000 0000000000000000 ffffffffffff0000
GPR12: 0000000000008000 c00000001ec48600 00007fffa08a53f8 0000000000000001
GPR16: 00007fff9faf9a63 00000100073bec00 00007fff9f2493b0 0000000000000000
GPR20: 00007fffa1143bf8 00007fffa1103b18 c00000087f547cb3 2f2f2f2f2f2f2f2f
GPR24: 0000000000000003 0000000000000000 c00000082802fbc8 fffffffffffff000
GPR28: 0000000000200000 ffffffffffffffff 61c8864680b583eb 0000000000000000
NIP [c00000000043f278] link_path_walk.part.49+0x228/0x400
LR [c00000000043f330] link_path_walk.part.49+0x2e0/0x400
Call Trace:
[c00000082802fac0] [c00000000043f330] link_path_walk.part.49+0x2e0/0x400 (unreliable)
[c00000082802fb50] [c00000000043f5a4] path_lookupat.isra.51+0x64/0x1f0
[c00000082802fba0] [c000000000441c00] filename_lookup.part.69+0xa0/0x1b0
[c00000082802fce0] [c00000000042ff38] vfs_statx+0xa8/0x190
[c00000082802fd60] [c0000000004302a0] __do_sys_newstat+0x40/0x90
[c00000082802fe20] [c00000000000b278] system_call+0x5c/0x68
Instruction dump:
3bffffff e93a0058 38800000 7f43d378 7fff07b4 1d5f0030 7d295214 eac90020
4bfffb21 2fa30000 409e00c8 e93a0008 <81290000> 55290256 7f89e000 419efecc
---[ end trace 34abf29ebd56e423 ]—
Relevant snippet from obj dump:
6dc4: 20 00 c9 ea ld r22,32(r9)
link = walk_component(nd, 0);
6db4: 78 d3 43 7f mr r3,r26
name = nd->stack[--depth].name;
6db8: b4 07 ff 7f extsw r31,r31
6dbc: 30 00 5f 1d mulli r10,r31,48
6dc0: 14 52 29 7d add r9,r9,r10
6dc4: 20 00 c9 ea ld r22,32(r9)
link = walk_component(nd, 0);
6dc8: 01 00 00 48 bl 6dc8 <link_path_walk.part.49+0x218>
if (unlikely(link)) {
6dcc: 00 00 a3 2f cmpdi cr7,r3,0
6dd0: c8 00 9e 40 bne cr7,6e98 <link_path_walk.part.49+0x2e8>
return dentry->d_flags & DCACHE_ENTRY_TYPE;
6dd4: 08 00 3a e9 ld r9,8(r26)
6dd8: 00 00 29 81 lwz r9,0(r9) <<=== crashes here ??
6ddc: 56 02 29 55 rlwinm r9,r9,0,9,11
if (unlikely(!d_can_lookup(nd->path.dentry))) {
6de0: 00 e0 89 7f cmpw cr7,r9,r28
The code in question (link_path_walk() in fs/namei.c ) was recently changed by
following commit:
commit 881386f7e46a:
link_path_walk(): sample parent's i_uid and i_mode for the last component
Thanks
-Sachin
[-- Attachment #2: next-20200325.log --]
[-- Type: application/octet-stream, Size: 18482 bytes --]
Crash while running sosreport command:
[ 1917.926113] BUG: Kernel NULL pointer dereference on read at 0x00000000
[ 1917.926126] Faulting instruction address: 0xc00000000043f638
[ 1917.926131] Oops: Kernel access of bad area, sig: 11 [#1]
[ 1917.926136] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 1917.926147] Dumping ftrace buffer:
[ 1917.926157] (ftrace buffer empty)
[ 1917.926161] Modules linked in: iscsi_target_mod target_core_mod macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc overlay dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod cdrom sd_mod t10_pi ibmvscsi scsi_transport_srp ibmveth
[ 1917.926198] CPU: 12 PID: 20734 Comm: busctl Not tainted 5.6.0-rc7-next-20200325-autotest #1
[ 1917.926203] NIP: c00000000043f638 LR: c00000000043f6f0 CTR: 0000000000000000
[ 1917.926209] REGS: c00000082bc1f720 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200325-autotest)
[ 1917.926215] MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 28022422 XER: 20040000
[ 1917.926222] CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
[ 1917.926222] GPR00: c00000000043f6f0 c00000082bc1f9b0 c00000000155e800 0000000000000000
[ 1917.926222] GPR04: 0000000000000002 0000000000000000 0000000000000002 c0000008b3400000
[ 1917.926222] GPR08: 000000000000febb 0000000000000000 0000000000000000 ffffffffffff0000
[ 1917.926222] GPR12: 0000000000002000 c00000001ec5d600 0000000135ed11e0 c0000008879046d0
[ 1917.926222] GPR16: 7fffffffffffffff fffffffffffffe00 fffffffffffffffc fffffffffffffff5
[ 1917.926222] GPR20: c00000089fa27200 0000000000000000 c00000008609bf33 2f2f2f2f2f2f2f2f
[ 1917.926222] GPR24: 0000000000000003 0000000000000000 c00000082bc1fab8 fffffffffffff000
[ 1917.926222] GPR28: 0000000000200000 ffffffffffffffff 61c8864680b583eb 0000000000000001
[ 1917.926270] NIP [c00000000043f638] link_path_walk.part.49+0x228/0x400
[ 1917.926275] LR [c00000000043f6f0] link_path_walk.part.49+0x2e0/0x400
[ 1917.926280] Call Trace:
[ 1917.926284] [c00000082bc1f9b0] [c00000000043f6f0] link_path_walk.part.49+0x2e0/0x400 (unreliable)
[ 1917.926291] [c00000082bc1fa40] [c00000000043f964] path_lookupat.isra.51+0x64/0x1f0
[ 1917.926296] [c00000082bc1fa90] [c000000000441fc0] filename_lookup.part.69+0xa0/0x1b0
[ 1917.926303] [c00000082bc1fbd0] [c000000000ae0784] unix_find_other+0x64/0x3f0
[ 1917.926309] [c00000082bc1fc60] [c000000000ae1b78] unix_stream_connect+0x148/0x930
[ 1917.926316] [c00000082bc1fd30] [c000000000933010] __sys_connect+0x140/0x170
[ 1917.926322] [c00000082bc1fe00] [c000000000933068] sys_connect+0x28/0x40
[ 1917.926328] [c00000082bc1fe20] [c00000000000b278] system_call+0x5c/0x68
[ 1917.926332] Instruction dump:
[ 1917.926336] 3bffffff e93a0058 38800000 7f43d378 7fff07b4 1d5f0030 7d295214 eac90020
[ 1917.926343] 4bfffb21 2fa30000 409e00c8 e93a0008 <81290000> 55290256 7f89e000 419efecc
[ 1917.926351] ---[ end trace 1b673f0e7295c08b ]---
Objdump o/p
===========
fs/namei.o: file format elf64-powerpcle
Disassembly of section .text:
0000000000000000 <full_name_hash>:
* In particular, we must end by hashing a final word containing 0..7
* payload bytes, to match the way that hash_name() iterates until it
* finds the delimiter after the name.
*/
unsigned int full_name_hash(const void *salt, const char *name, unsigned int len)
{
0: a6 02 08 7c mflr r0
4: 01 00 00 48 bl 4 <full_name_hash+0x4>
unsigned long a, x = 0, y = (unsigned long)salt;
for (;;) {
if (!len)
.........
.........
0000000000006bb0 <link_path_walk.part.49>:
static int link_path_walk(const char *name, struct nameidata *nd)
6bb0: 00 00 4c 3c addis r2,r12,0
6bb4: 00 00 42 38 addi r2,r2,0
6bb8: a6 02 08 7c mflr r0
6bbc: 01 00 00 48 bl 6bbc <link_path_walk.part.49+0xc>
6bc0: b0 ff c1 fa std r22,-80(r1)
6bc4: d0 ff 41 fb std r26,-48(r1)
6bc8: c0 ff 01 fb std r24,-64(r1)
6bcc: 78 1b 76 7c mr r22,r3
6bd0: 71 ff 21 f8 stdu r1,-144(r1)
6bd4: 78 23 9a 7c mr r26,r4
6bd8: 18 00 41 f8 std r2,24(r1)
6bdc: 78 11 2d e9 ld r9,4472(r13)
6be0: 38 00 21 f9 std r9,56(r1)
6be4: 00 00 20 39 li r9,0
while (*name=='/')
6be8: 00 00 23 89 lbz r9,0(r3)
6bec: 2f 00 89 2b cmplwi cr7,r9,47
6bf0: 1c 00 9e 40 bne cr7,6c0c <link_path_walk.part.49+0x5c>
6bf4: 00 00 00 60 nop
6bf8: 00 00 00 60 nop
6bfc: 00 00 00 60 nop
6c00: 01 00 36 8d lbzu r9,1(r22)
6c04: 2f 00 89 2b cmplwi cr7,r9,47
6c08: f8 ff 9e 41 beq cr7,6c00 <link_path_walk.part.49+0x50>
if (!*name)
6c0c: 00 00 a9 2f cmpdi cr7,r9,0
return 0;
6c10: 00 00 00 3b li r24,0
if (!*name)
6c14: 3c 00 9e 40 bne cr7,6c50 <link_path_walk.part.49+0xa0>
}
6c18: 38 00 21 e9 ld r9,56(r1)
6c1c: 78 11 4d e9 ld r10,4472(r13)
6c20: 79 52 29 7d xor. r9,r9,r10
6c24: 00 00 40 39 li r10,0
6c28: 78 c3 03 7f mr r3,r24
6c2c: 54 03 82 40 bne 6f80 <link_path_walk.part.49+0x3d0>
6c30: 90 00 21 38 addi r1,r1,144
6c34: b0 ff c1 ea ld r22,-80(r1)
6c38: c0 ff 01 eb ld r24,-64(r1)
6c3c: d0 ff 41 eb ld r26,-48(r1)
6c40: 20 00 80 4e blr
6c44: 00 00 00 60 nop
6c48: 00 00 00 60 nop
6c4c: 00 00 00 60 nop
6c50: 48 00 e1 fa std r23,72(r1)
6c54: 80 00 c1 fb std r30,128(r1)
b = a ^ REPEAT_BYTE('/');
6c58: 2f 2f e0 3e lis r23,12079
6c5c: 2f 2f f7 62 ori r23,r23,12079
6c60: a6 02 08 7c mflr r0
y ^= x * GOLDEN_RATIO_64;
6c64: c8 61 c0 3f lis r30,25032
6c68: 46 86 de 63 ori r30,r30,34374
6c6c: 58 00 21 fb std r25,88(r1)
6c70: 68 00 61 fb std r27,104(r1)
unsigned long a = 0, b, x = 0, y = (unsigned long)salt;
6c74: 00 00 20 3b li r25,0
6c78: 70 00 81 fb std r28,112(r1)
6c7c: 78 00 a1 fb std r29,120(r1)
6c80: 88 00 e1 fb std r31,136(r1)
x ^= a & zero_bytemask(mask);
6c84: ff ff a0 3b li r29,-1
int depth = 0; // depth <= nd->depth
6c88: 00 00 e0 3b li r31,0
if (unlikely(!d_can_lookup(nd->path.dentry))) {
6c8c: 20 00 80 3f lis r28,32
if (IS_ERR(link))
6c90: 00 f0 60 3b li r27,-4096
b = a ^ REPEAT_BYTE('/');
6c94: c6 07 f7 7a rldicr r23,r23,32,31
y ^= x * GOLDEN_RATIO_64;
6c98: c6 07 de 7b rldicr r30,r30,32,31
b = a ^ REPEAT_BYTE('/');
6c9c: 2f 2f f7 66 oris r23,r23,12079
6ca0: a0 00 01 f8 std r0,160(r1)
y ^= x * GOLDEN_RATIO_64;
6ca4: b5 80 de 67 oris r30,r30,32949
b = a ^ REPEAT_BYTE('/');
6ca8: 2f 2f f7 62 ori r23,r23,12079
y ^= x * GOLDEN_RATIO_64;
6cac: eb 83 de 63 ori r30,r30,33771
if (nd->flags & LOOKUP_RCU) {
6cb0: 38 00 3a 81 lwz r9,56(r26)
6cb4: 30 00 7a e8 ld r3,48(r26)
6cb8: 40 00 29 71 andi. r9,r9,64
6cbc: 08 02 82 41 beq 6ec4 <link_path_walk.part.49+0x314>
int err = inode_permission(nd->inode, MAY_EXEC|MAY_NOT_BLOCK);
6cc0: 81 00 80 38 li r4,129
6cc4: 01 00 00 48 bl 6cc4 <link_path_walk.part.49+0x114>
if (err != -ECHILD)
6cc8: f6 ff 83 2f cmpwi cr7,r3,-10
int err = inode_permission(nd->inode, MAY_EXEC|MAY_NOT_BLOCK);
6ccc: 78 1b 78 7c mr r24,r3
if (err != -ECHILD)
6cd0: e0 01 9e 41 beq cr7,6eb0 <link_path_walk.part.49+0x300>
if (err)
6cd4: 00 00 b8 2f cmpdi cr7,r24,0
6cd8: 30 01 9e 40 bne cr7,6e08 <link_path_walk.part.49+0x258>
hash_len = hash_name(nd->path.dentry, name);
6cdc: 08 00 7a e8 ld r3,8(r26)
len = 0;
6ce0: 00 00 a0 38 li r5,0
unsigned long a = 0, b, x = 0, y = (unsigned long)salt;
6ce4: 00 00 c0 38 li r6,0
6ce8: 78 1b 67 7c mr r7,r3
goto inside;
6cec: 24 00 00 48 b 6d10 <link_path_walk.part.49+0x160>
HASH_MIX(x, y, a);
6cf0: 78 4a c9 7c xor r9,r6,r9
len += sizeof(unsigned long);
6cf4: 08 00 a5 38 addi r5,r5,8
HASH_MIX(x, y, a);
6cf8: 78 3a 26 7d xor r6,r9,r7
6cfc: 00 60 29 79 rotldi r9,r9,12
6d00: 02 68 c7 78 rotldi r7,r6,45
6d04: 14 4a c6 7c add r6,r6,r9
6d08: 24 1f e9 78 rldicr r9,r7,3,60
6d0c: 14 3a e9 7c add r7,r9,r7
a = load_unaligned_zeropad(name+len);
6d10: 14 2a 56 7d add r10,r22,r5
asm(
6d14: 00 00 2a e9 ld r9,0(r10)
b = a ^ REPEAT_BYTE('/');
6d18: 78 ba 2a 7d xor r10,r9,r23
asm("cmpb %0,%1,%2" : "=r" (ret) : "r" (a), "r" (zero));
6d1c: f8 cb 28 7d cmpb r8,r9,r25
6d20: f8 cb 4a 7d cmpb r10,r10,r25
} while (!(has_zero(a, &adata, &constants) | has_zero(b, &bdata, &constants)));
6d24: 79 53 0a 7d or. r10,r8,r10
6d28: c8 ff 82 41 beq 6cf0 <link_path_walk.part.49+0x140>
asm("addi %1,%2,-1\n\t"
6d2c: ff ff 0a 39 addi r8,r10,-1
6d30: 78 50 08 7d andc r8,r8,r10
6d34: f4 03 0a 7d popcntd r10,r8
x ^= a & zero_bytemask(mask);
6d38: 36 50 ab 7f sld r11,r29,r10
return mask >> 3;
6d3c: c2 e8 48 79 rldicl r8,r10,61,3
if (name[0] == '.') switch (hashlen_len(hash_len)) {
6d40: 00 00 96 88 lbz r4,0(r22)
x ^= a & zero_bytemask(mask);
6d44: 78 58 2a 7d andc r10,r9,r11
return hashlen_create(fold_hash(x, y), len + find_zero(mask));
6d48: 14 2a 08 7d add r8,r8,r5
x ^= a & zero_bytemask(mask);
6d4c: 78 32 49 7d xor r9,r10,r6
if (name[0] == '.') switch (hashlen_len(hash_len)) {
6d50: 2e 00 84 2f cmpwi cr7,r4,46
y ^= x * GOLDEN_RATIO_64;
6d54: d2 f1 29 7d mulld r9,r9,r30
6d58: 78 3a 29 7d xor r9,r9,r7
y *= GOLDEN_RATIO_64;
6d5c: d2 f1 29 7d mulld r9,r9,r30
return y >> 32;
6d60: 22 00 29 79 rldicl r9,r9,32,32
return hashlen_create(fold_hash(x, y), len + find_zero(mask));
6d64: 0e 00 09 79 rldimi r9,r8,32,0
if (name[0] == '.') switch (hashlen_len(hash_len)) {
6d68: c8 00 9e 41 beq cr7,6e30 <link_path_walk.part.49+0x280>
6d6c: 38 00 5a 81 lwz r10,56(r26)
nd->flags &= ~LOOKUP_JUMPED;
6d70: 24 05 4a 55 rlwinm r10,r10,0,20,18
6d74: 38 00 5a 91 stw r10,56(r26)
if (unlikely(parent->d_flags & DCACHE_OP_HASH)) {
6d78: 00 00 43 81 lwz r10,0(r3)
6d7c: 01 00 4a 71 andi. r10,r10,1
6d80: 70 01 82 40 bne 6ef0 <link_path_walk.part.49+0x340>
6d84: 22 00 28 79 rldicl r8,r9,32,32
nd->last.hash_len = hash_len;
6d88: 10 00 3a f9 std r9,16(r26)
nd->last.name = name;
6d8c: 18 00 da fa std r22,24(r26)
nd->last_type = type;
6d90: 48 00 1a 93 stw r24,72(r26)
if (!*name)
6d94: ee 40 36 7d lbzux r9,r22,r8
6d98: 00 00 89 2f cmpwi cr7,r9,0
6d9c: d4 00 9e 40 bne cr7,6e70 <link_path_walk.part.49+0x2c0>
if (!depth) {
6da0: 00 00 bf 2f cmpdi cr7,r31,0
6da4: ac 01 9e 41 beq cr7,6f50 <link_path_walk.part.49+0x3a0>
name = nd->stack[--depth].name;
6da8: ff ff ff 3b addi r31,r31,-1
6dac: 58 00 3a e9 ld r9,88(r26)
link = walk_component(nd, 0);
6db0: 00 00 80 38 li r4,0
6db4: 78 d3 43 7f mr r3,r26
name = nd->stack[--depth].name;
6db8: b4 07 ff 7f extsw r31,r31
6dbc: 30 00 5f 1d mulli r10,r31,48
6dc0: 14 52 29 7d add r9,r9,r10
6dc4: 20 00 c9 ea ld r22,32(r9)
link = walk_component(nd, 0);
6dc8: 01 00 00 48 bl 6dc8 <link_path_walk.part.49+0x218>
if (unlikely(link)) {
6dcc: 00 00 a3 2f cmpdi cr7,r3,0
6dd0: c8 00 9e 40 bne cr7,6e98 <link_path_walk.part.49+0x2e8>
return dentry->d_flags & DCACHE_ENTRY_TYPE;
6dd4: 08 00 3a e9 ld r9,8(r26)
6dd8: 00 00 29 81 lwz r9,0(r9)
6ddc: 56 02 29 55 rlwinm r9,r9,0,9,11
if (unlikely(!d_can_lookup(nd->path.dentry))) {
6de0: 00 e0 89 7f cmpw cr7,r9,r28
6de4: cc fe 9e 41 beq cr7,6cb0 <link_path_walk.part.49+0x100>
if (nd->flags & LOOKUP_RCU) {
6de8: 38 00 3a 81 lwz r9,56(r26)
6dec: 40 00 29 71 andi. r9,r9,64
6df0: 38 01 82 41 beq 6f28 <link_path_walk.part.49+0x378>
if (unlazy_walk(nd))
6df4: 78 d3 43 7f mr r3,r26
6df8: 01 00 00 48 bl 6df8 <link_path_walk.part.49+0x248>
6dfc: 00 00 a3 2f cmpdi cr7,r3,0
6e00: 28 01 9e 41 beq cr7,6f28 <link_path_walk.part.49+0x378>
return -ECHILD;
6e04: f6 ff 00 3b li r24,-10
6e08: a0 00 01 e8 ld r0,160(r1)
6e0c: 48 00 e1 ea ld r23,72(r1)
6e10: 58 00 21 eb ld r25,88(r1)
6e14: 68 00 61 eb ld r27,104(r1)
6e18: 70 00 81 eb ld r28,112(r1)
6e1c: 78 00 a1 eb ld r29,120(r1)
6e20: 80 00 c1 eb ld r30,128(r1)
6e24: 88 00 e1 eb ld r31,136(r1)
6e28: a6 03 08 7c mtlr r0
6e2c: ec fd ff 4b b 6c18 <link_path_walk.part.49+0x68>
if (name[0] == '.') switch (hashlen_len(hash_len)) {
6e30: 22 00 28 79 rldicl r8,r9,32,32
6e34: 20 00 0a 79 clrldi r10,r8,32
6e38: 01 00 8a 2b cmplwi cr7,r10,1
6e3c: a4 00 9e 41 beq cr7,6ee0 <link_path_walk.part.49+0x330>
6e40: 02 00 8a 2b cmplwi cr7,r10,2
6e44: 28 ff 9e 40 bne cr7,6d6c <link_path_walk.part.49+0x1bc>
if (name[1] == '.') {
6e48: 01 00 f6 88 lbz r7,1(r22)
6e4c: 38 00 5a 81 lwz r10,56(r26)
6e50: 2e 00 87 2f cmpwi cr7,r7,46
6e54: 1c ff 9e 40 bne cr7,6d70 <link_path_walk.part.49+0x1c0>
nd->flags |= LOOKUP_JUMPED;
6e58: 00 10 4a 61 ori r10,r10,4096
type = LAST_DOTDOT;
6e5c: 03 00 00 3b li r24,3
nd->flags |= LOOKUP_JUMPED;
6e60: 38 00 5a 91 stw r10,56(r26)
if (likely(type == LAST_NORM)) {
6e64: 24 ff ff 4b b 6d88 <link_path_walk.part.49+0x1d8>
6e68: 00 00 00 60 nop
6e6c: 00 00 00 60 nop
} while (unlikely(*name == '/'));
6e70: 01 00 36 8d lbzu r9,1(r22)
6e74: 2f 00 89 2b cmplwi cr7,r9,47
6e78: f8 ff 9e 41 beq cr7,6e70 <link_path_walk.part.49+0x2c0>
if (unlikely(!*name)) {
6e7c: 00 00 a9 2f cmpdi cr7,r9,0
6e80: 20 ff 9e 41 beq cr7,6da0 <link_path_walk.part.49+0x1f0>
link = walk_component(nd, WALK_MORE);
6e84: 02 00 80 38 li r4,2
6e88: 78 d3 43 7f mr r3,r26
6e8c: 01 00 00 48 bl 6e8c <link_path_walk.part.49+0x2dc>
if (unlikely(link)) {
6e90: 00 00 a3 2f cmpdi cr7,r3,0
6e94: 40 ff 9e 41 beq cr7,6dd4 <link_path_walk.part.49+0x224>
if (IS_ERR(link))
6e98: 40 d8 a3 7f cmpld cr7,r3,r27
6e9c: 94 00 9d 40 ble cr7,6f30 <link_path_walk.part.49+0x380>
return PTR_ERR(link);
6ea0: b4 07 78 7c extsw r24,r3
6ea4: 64 ff ff 4b b 6e08 <link_path_walk.part.49+0x258>
6ea8: 00 00 00 60 nop
6eac: 00 00 00 60 nop
if (unlazy_walk(nd))
6eb0: 78 d3 43 7f mr r3,r26
6eb4: 01 00 00 48 bl 6eb4 <link_path_walk.part.49+0x304>
6eb8: 00 00 a3 2f cmpdi cr7,r3,0
6ebc: 48 ff 9e 40 bne cr7,6e04 <link_path_walk.part.49+0x254>
6ec0: 30 00 7a e8 ld r3,48(r26)
return inode_permission(nd->inode, MAY_EXEC);
6ec4: 01 00 80 38 li r4,1
6ec8: 01 00 00 48 bl 6ec8 <link_path_walk.part.49+0x318>
6ecc: 78 1b 78 7c mr r24,r3
6ed0: 04 fe ff 4b b 6cd4 <link_path_walk.part.49+0x124>
6ed4: 00 00 00 60 nop
6ed8: 00 00 00 60 nop
6edc: 00 00 00 60 nop
type = LAST_DOT;
6ee0: 02 00 00 3b li r24,2
6ee4: a4 fe ff 4b b 6d88 <link_path_walk.part.49+0x1d8>
6ee8: 00 00 00 60 nop
6eec: 00 00 00 60 nop
err = parent->d_op->d_hash(parent, &this);
6ef0: 60 00 43 e9 ld r10,96(r3)
struct qstr this = { { .hash_len = hash_len }, .name = name };
6ef4: 28 00 21 f9 std r9,40(r1)
6ef8: 30 00 c1 fa std r22,48(r1)
err = parent->d_op->d_hash(parent, &this);
6efc: 28 00 81 38 addi r4,r1,40
6f00: 10 00 2a e9 ld r9,16(r10)
6f04: 78 4b 2c 7d mr r12,r9
6f08: a6 03 29 7d mtctr r9
6f0c: 21 04 80 4e bctrl
6f10: 18 00 41 e8 ld r2,24(r1)
if (err < 0)
6f14: 00 00 83 2f cmpwi cr7,r3,0
6f18: 60 00 9c 41 blt cr7,6f78 <link_path_walk.part.49+0x3c8>
hash_len = this.hash_len;
6f1c: 28 00 21 e9 ld r9,40(r1)
name = this.name;
6f20: 30 00 c1 ea ld r22,48(r1)
6f24: 60 fe ff 4b b 6d84 <link_path_walk.part.49+0x1d4>
return -ENOTDIR;
6f28: ec ff 00 3b li r24,-20
6f2c: dc fe ff 4b b 6e08 <link_path_walk.part.49+0x258>
nd->stack[depth++].name = name;
6f30: 30 00 5f 1d mulli r10,r31,48
6f34: 58 00 3a e9 ld r9,88(r26)
6f38: 01 00 ff 3b addi r31,r31,1
6f3c: b4 07 ff 7f extsw r31,r31
6f40: 14 52 29 7d add r9,r9,r10
6f44: 20 00 c9 fa std r22,32(r9)
6f48: 78 1b 76 7c mr r22,r3
6f4c: 64 fd ff 4b b 6cb0 <link_path_walk.part.49+0x100>
nd->dir_uid = nd->inode->i_uid;
6f50: 30 00 5a e9 ld r10,48(r26)
nd->flags &= ~LOOKUP_PARENT;
6f54: 38 00 3a 81 lwz r9,56(r26)
return 0;
6f58: 00 00 00 3b li r24,0
nd->dir_uid = nd->inode->i_uid;
6f5c: 04 00 0a 81 lwz r8,4(r10)
nd->flags &= ~LOOKUP_PARENT;
6f60: 34 07 29 55 rlwinm r9,r9,0,28,26
nd->dir_uid = nd->inode->i_uid;
6f64: d8 00 1a 91 stw r8,216(r26)
nd->dir_mode = nd->inode->i_mode;
6f68: 00 00 4a a1 lhz r10,0(r10)
nd->flags &= ~LOOKUP_PARENT;
6f6c: 38 00 3a 91 stw r9,56(r26)
nd->dir_mode = nd->inode->i_mode;
6f70: dc 00 5a b1 sth r10,220(r26)
return 0;
6f74: 94 fe ff 4b b 6e08 <link_path_walk.part.49+0x258>
err = parent->d_op->d_hash(parent, &this);
6f78: 78 1b 78 7c mr r24,r3
6f7c: 8c fe ff 4b b 6e08 <link_path_walk.part.49+0x258>
6f80: a6 02 08 7c mflr r0
6f84: 48 00 e1 fa std r23,72(r1)
6f88: 58 00 21 fb std r25,88(r1)
6f8c: 68 00 61 fb std r27,104(r1)
6f90: 70 00 81 fb std r28,112(r1)
6f94: 78 00 a1 fb std r29,120(r1)
6f98: 80 00 c1 fb std r30,128(r1)
6f9c: 88 00 e1 fb std r31,136(r1)
6fa0: a0 00 01 f8 std r0,160(r1)
}
6fa4: 01 00 00 48 bl 6fa4 <link_path_walk.part.49+0x3f4>
6fa8: 00 00 00 60 nop
6fac: 00 00 00 60 nop
^ permalink raw reply
* [PATCH v2] powerpc/XIVE: SVM: share the event-queue page with the Hypervisor.
From: Ram Pai @ 2020-03-26 8:38 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: aik, andmike, linuxram, groug, clg, sukadev, bauerman, david
XIVE interrupt controller use an Event Queue (EQ) to enqueue event
notifications when an exception occurs. The EQ is a single memory page
provided by the O/S defining a circular buffer, one per server and
priority couple.
On baremetal, the EQ page is configured with an OPAL call. On pseries,
an extra hop is necessary and the guest OS uses the hcall
H_INT_SET_QUEUE_CONFIG to configure the XIVE interrupt controller.
The XIVE controller being Hypervisor privileged, it will not be allowed
to enqueue event notifications for a Secure VM unless the EQ pages are
shared by the Secure VM.
Hypervisor/Ultravisor still requires support for the TIMA and ESB page
fault handlers. Until this is complete, QEMU can use the emulated XIVE
device for Secure VMs, option "kernel_irqchip=off" on the QEMU pseries
machine.
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: Michael Anderson <andmike@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Greg Kurz <groug@kaod.org>
Cc: Cedric Le Goater <clg@fr.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
v2: better description of the patch from Cedric.
---
arch/powerpc/sysdev/xive/spapr.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
index 55dc61c..608b52f 100644
--- a/arch/powerpc/sysdev/xive/spapr.c
+++ b/arch/powerpc/sysdev/xive/spapr.c
@@ -26,6 +26,8 @@
#include <asm/xive.h>
#include <asm/xive-regs.h>
#include <asm/hvcall.h>
+#include <asm/svm.h>
+#include <asm/ultravisor.h>
#include "xive-internal.h"
@@ -501,6 +503,9 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio,
rc = -EIO;
} else {
q->qpage = qpage;
+ if (is_secure_guest())
+ uv_share_page(PHYS_PFN(qpage_phys),
+ 1 << xive_alloc_order(order));
}
fail:
return rc;
@@ -534,6 +539,8 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc,
hw_cpu, prio);
alloc_order = xive_alloc_order(xive_queue_shift);
+ if (is_secure_guest())
+ uv_unshare_page(PHYS_PFN(__pa(q->qpage)), 1 << alloc_order);
free_pages((unsigned long)q->qpage, alloc_order);
q->qpage = NULL;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/XIVE: SVM: share the event-queue page with the Hypervisor.
From: Cédric Le Goater @ 2020-03-26 8:45 UTC (permalink / raw)
To: Ram Pai, kvm-ppc, linuxppc-dev
Cc: aik, andmike, groug, sukadev, bauerman, david
In-Reply-To: <1585211927-784-1-git-send-email-linuxram@us.ibm.com>
On 3/26/20 9:38 AM, Ram Pai wrote:
> XIVE interrupt controller use an Event Queue (EQ) to enqueue event
The XIVE interrupt controller uses ... (my bad)
> notifications when an exception occurs. The EQ is a single memory page
> provided by the O/S defining a circular buffer, one per server and
> priority couple.
>
> On baremetal, the EQ page is configured with an OPAL call. On pseries,
> an extra hop is necessary and the guest OS uses the hcall
> H_INT_SET_QUEUE_CONFIG to configure the XIVE interrupt controller.
>
> The XIVE controller being Hypervisor privileged, it will not be allowed
> to enqueue event notifications for a Secure VM unless the EQ pages are
> shared by the Secure VM.
>
> Hypervisor/Ultravisor still requires support for the TIMA and ESB page
> fault handlers. Until this is complete, QEMU can use the emulated XIVE
> device for Secure VMs, option "kernel_irqchip=off" on the QEMU pseries
> machine.
>
> Cc: kvm-ppc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Cc: Michael Anderson <andmike@linux.ibm.com>
> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Cedric Le Goater <clg@fr.ibm.com>
clg@fr.ibm.com is insecure. Please use clg@kaod.org.
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Cedric Le Goater <clg@kaod.org>
Thanks,
C.
>
> v2: better description of the patch from Cedric.
> ---
> arch/powerpc/sysdev/xive/spapr.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> index 55dc61c..608b52f 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -26,6 +26,8 @@
> #include <asm/xive.h>
> #include <asm/xive-regs.h>
> #include <asm/hvcall.h>
> +#include <asm/svm.h>
> +#include <asm/ultravisor.h>
>
> #include "xive-internal.h"
>
> @@ -501,6 +503,9 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio,
> rc = -EIO;
> } else {
> q->qpage = qpage;
> + if (is_secure_guest())
> + uv_share_page(PHYS_PFN(qpage_phys),
> + 1 << xive_alloc_order(order));
> }
> fail:
> return rc;
> @@ -534,6 +539,8 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc,
> hw_cpu, prio);
>
> alloc_order = xive_alloc_order(xive_queue_shift);
> + if (is_secure_guest())
> + uv_unshare_page(PHYS_PFN(__pa(q->qpage)), 1 << alloc_order);
> free_pages((unsigned long)q->qpage, alloc_order);
> q->qpage = NULL;
> }
>
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Michal Hocko @ 2020-03-26 9:40 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, Baoquan He, linux-kernel, linux-mm, akpm,
linuxppc-dev
In-Reply-To: <20200325031914.107660-1-aneesh.kumar@linux.ibm.com>
On Wed 25-03-20 08:49:14, Aneesh Kumar K.V wrote:
> Fixes the below crash
>
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc000000000c3447c
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> ...
> NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> LR [c000000000088354] vmemmap_free+0x144/0x320
> Call Trace:
> section_deactivate+0x220/0x240
It would be great to match this to the specific source code.
> __remove_pages+0x118/0x170
> arch_remove_memory+0x3c/0x150
> memunmap_pages+0x1cc/0x2f0
> devm_action_release+0x30/0x50
> release_nodes+0x2f8/0x3e0
> device_release_driver_internal+0x168/0x270
> unbind_store+0x130/0x170
> drv_attr_store+0x44/0x60
> sysfs_kf_write+0x68/0x80
> kernfs_fop_write+0x100/0x290
> __vfs_write+0x3c/0x70
> vfs_write+0xcc/0x240
> ksys_write+0x7c/0x140
> system_call+0x5c/0x68
>
> With commit: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> section_mem_map is set to NULL after depopulate_section_mem(). This
> was done so that pfn_page() can work correctly with kernel config that disables
> SPARSEMEM_VMEMMAP. With that config pfn_to_page does
>
> __section_mem_map_addr(__sec) + __pfn;
> where
>
> static inline struct page *__section_mem_map_addr(struct mem_section *section)
> {
> unsigned long map = section->section_mem_map;
> map &= SECTION_MAP_MASK;
> return (struct page *)map;
> }
>
> Now with SPASEMEM_VMEMAP enabled, mem_section->usage->subsection_map is used to
> check the pfn validity (pfn_valid()). Since section_deactivate release
> mem_section->usage if a section is fully deactivated, pfn_valid() check after
> a subsection_deactivate cause a kernel crash.
>
> static inline int pfn_valid(unsigned long pfn)
> {
> ...
> return early_section(ms) || pfn_section_valid(ms, pfn);
> }
>
> where
>
> static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> {
> int idx = subsection_map_index(pfn);
>
> return test_bit(idx, ms->usage->subsection_map);
> }
>
> Avoid this by clearing SECTION_HAS_MEM_MAP when mem_section->usage is freed.
I am sorry, I haven't noticed that during the review of the commit
mentioned above. This is all subtle as hell, I have to say.
Why do we have to free usage before deactivaing section memmap? Now that
we have a late section_mem_map reset shouldn't we tear down the usage in
the same branch?
> Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> Cc: Baoquan He <bhe@redhat.com>
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/sparse.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index aadb7298dcef..3012d1f3771a 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> ms->usage = NULL;
> }
> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> + /* Mark the section invalid */
> + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
Btw. this comment is not really helping at all.
/*
* section->usage is gone and VMEMMAP's pfn_valid depens
* on it (see pfn_section_valid)
*/
> }
>
> if (section_is_early && memmap)
> --
> 2.25.1
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Aneesh Kumar K.V @ 2020-03-26 9:56 UTC (permalink / raw)
To: Michal Hocko
Cc: Sachin Sant, Baoquan He, linux-kernel, linux-mm, akpm,
linuxppc-dev
In-Reply-To: <20200326094023.GG27965@dhcp22.suse.cz>
On 3/26/20 3:10 PM, Michal Hocko wrote:
> On Wed 25-03-20 08:49:14, Aneesh Kumar K.V wrote:
>> Fixes the below crash
>>
>> BUG: Kernel NULL pointer dereference on read at 0x00000000
>> Faulting instruction address: 0xc000000000c3447c
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
>> CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
>> ...
>> NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
>> LR [c000000000088354] vmemmap_free+0x144/0x320
>> Call Trace:
>> section_deactivate+0x220/0x240
>
> It would be great to match this to the specific source code.
The crash is due to NULL dereference at
test_bit(idx, ms->usage->subsection_map); due to ms->usage = NULL;
that is explained in later part of the commit.
>
>> __remove_pages+0x118/0x170
>> arch_remove_memory+0x3c/0x150
>> memunmap_pages+0x1cc/0x2f0
>> devm_action_release+0x30/0x50
>> release_nodes+0x2f8/0x3e0
>> device_release_driver_internal+0x168/0x270
>> unbind_store+0x130/0x170
>> drv_attr_store+0x44/0x60
>> sysfs_kf_write+0x68/0x80
>> kernfs_fop_write+0x100/0x290
>> __vfs_write+0x3c/0x70
>> vfs_write+0xcc/0x240
>> ksys_write+0x7c/0x140
>> system_call+0x5c/0x68
>>
>> With commit: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
>> section_mem_map is set to NULL after depopulate_section_mem(). This
>> was done so that pfn_page() can work correctly with kernel config that disables
>> SPARSEMEM_VMEMMAP. With that config pfn_to_page does
>>
>> __section_mem_map_addr(__sec) + __pfn;
>> where
>>
>> static inline struct page *__section_mem_map_addr(struct mem_section *section)
>> {
>> unsigned long map = section->section_mem_map;
>> map &= SECTION_MAP_MASK;
>> return (struct page *)map;
>> }
>>
>> Now with SPASEMEM_VMEMAP enabled, mem_section->usage->subsection_map is used to
>> check the pfn validity (pfn_valid()). Since section_deactivate release
>> mem_section->usage if a section is fully deactivated, pfn_valid() check after
>> a subsection_deactivate cause a kernel crash.
>>
>> static inline int pfn_valid(unsigned long pfn)
>> {
>> ...
>> return early_section(ms) || pfn_section_valid(ms, pfn);
>> }
>>
>> where
>>
>> static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
>> {
>
>> int idx = subsection_map_index(pfn);
>>
>> return test_bit(idx, ms->usage->subsection_map);
>> }
>>
>> Avoid this by clearing SECTION_HAS_MEM_MAP when mem_section->usage is freed.
>
> I am sorry, I haven't noticed that during the review of the commit
> mentioned above. This is all subtle as hell, I have to say.
>
> Why do we have to free usage before deactivaing section memmap? Now that
> we have a late section_mem_map reset shouldn't we tear down the usage in
> the same branch?
>
We still need to make the section invalid before we call into
depopulate_section_memmap(). Because architecture like powerpc can share
vmemmap area across sections (16MB mapping of vmemmap area) and we use
vmemmap_popluated() to make that decision.
>> Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
>> Cc: Baoquan He <bhe@redhat.com>
>> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> mm/sparse.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/sparse.c b/mm/sparse.c
>> index aadb7298dcef..3012d1f3771a 100644
>> --- a/mm/sparse.c
>> +++ b/mm/sparse.c
>> @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
>> ms->usage = NULL;
>> }
>> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
>> + /* Mark the section invalid */
>> + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
>
> Btw. this comment is not really helping at all.
That is marking the section invalid so that
static inline int valid_section(struct mem_section *section)
{
return (section && (section->section_mem_map & SECTION_HAS_MEM_MAP));
}
returns false.
> /*
> * section->usage is gone and VMEMMAP's pfn_valid depens
> * on it (see pfn_section_valid)
> */
>> }
>>
>> if (section_is_early && memmap)
>> --
>> 2.25.1
>>
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/XIVE: SVM: share the event-queue page with the Hypervisor.
From: Greg Kurz @ 2020-03-26 10:04 UTC (permalink / raw)
To: Ram Pai; +Cc: aik, andmike, kvm-ppc, clg, sukadev, linuxppc-dev, bauerman,
david
In-Reply-To: <1585211927-784-1-git-send-email-linuxram@us.ibm.com>
On Thu, 26 Mar 2020 01:38:47 -0700
Ram Pai <linuxram@us.ibm.com> wrote:
> XIVE interrupt controller use an Event Queue (EQ) to enqueue event
> notifications when an exception occurs. The EQ is a single memory page
> provided by the O/S defining a circular buffer, one per server and
> priority couple.
>
> On baremetal, the EQ page is configured with an OPAL call. On pseries,
> an extra hop is necessary and the guest OS uses the hcall
> H_INT_SET_QUEUE_CONFIG to configure the XIVE interrupt controller.
>
> The XIVE controller being Hypervisor privileged, it will not be allowed
> to enqueue event notifications for a Secure VM unless the EQ pages are
> shared by the Secure VM.
>
> Hypervisor/Ultravisor still requires support for the TIMA and ESB page
> fault handlers. Until this is complete, QEMU can use the emulated XIVE
> device for Secure VMs, option "kernel_irqchip=off" on the QEMU pseries
> machine.
>
> Cc: kvm-ppc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Cc: Michael Anderson <andmike@linux.ibm.com>
> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Cedric Le Goater <clg@fr.ibm.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>
> v2: better description of the patch from Cedric.
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/sysdev/xive/spapr.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> index 55dc61c..608b52f 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -26,6 +26,8 @@
> #include <asm/xive.h>
> #include <asm/xive-regs.h>
> #include <asm/hvcall.h>
> +#include <asm/svm.h>
> +#include <asm/ultravisor.h>
>
> #include "xive-internal.h"
>
> @@ -501,6 +503,9 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio,
> rc = -EIO;
> } else {
> q->qpage = qpage;
> + if (is_secure_guest())
> + uv_share_page(PHYS_PFN(qpage_phys),
> + 1 << xive_alloc_order(order));
> }
> fail:
> return rc;
> @@ -534,6 +539,8 @@ static void xive_spapr_cleanup_queue(unsigned int cpu, struct xive_cpu *xc,
> hw_cpu, prio);
>
> alloc_order = xive_alloc_order(xive_queue_shift);
> + if (is_secure_guest())
> + uv_unshare_page(PHYS_PFN(__pa(q->qpage)), 1 << alloc_order);
> free_pages((unsigned long)q->qpage, alloc_order);
> q->qpage = NULL;
> }
^ permalink raw reply
* Re: [PATCH 2/3] soc: fsl: dpio: QMAN performance improvement. Function pointer indirection.
From: Arnd Bergmann @ 2020-03-26 10:07 UTC (permalink / raw)
To: Youri Querry
Cc: Roy Pledge, Alexandru Marginean, linux-kernel@vger.kernel.org,
Leo Li, Ioana Ciornei, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1576170032-3124-3-git-send-email-youri.querry_1@nxp.com>
On Thu, Dec 12, 2019 at 6:02 PM Youri Querry <youri.querry_1@nxp.com> wrote:
>
> We are making the access decision in the initialization and
> setting the function pointers accordingly.
>
> Signed-off-by: Youri Querry <youri.querry_1@nxp.com>
> ---
> drivers/soc/fsl/dpio/qbman-portal.c | 451 +++++++++++++++++++++++++++++++-----
> drivers/soc/fsl/dpio/qbman-portal.h | 129 ++++++++++-
> 2 files changed, 507 insertions(+), 73 deletions(-)
While merging pull requests, I came across some style issues with this driver.
I'm pulling it anyway, but please have another look and fix these for the next
release, or send a follow-up patch for the coming merge window.
>
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
> index 5a37ac8..0ffe018 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -83,6 +83,82 @@ enum qbman_sdqcr_fc {
> qbman_sdqcr_fc_up_to_3 = 1
> };
>
> +/* Internal Function declaration */
> +static int qbman_swp_enqueue_direct(struct qbman_swp *s,
> + const struct qbman_eq_desc *d,
> + const struct dpaa2_fd *fd);
> +static int qbman_swp_enqueue_mem_back(struct qbman_swp *s,
> + const struct qbman_eq_desc *d,
> + const struct dpaa2_fd *fd);
> +static int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
> + const struct qbman_eq_desc *d,
> + const struct dpaa2_fd *fd,
> + uint32_t *flags,
> + int num_frames);
> +static int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp *s,
> + const struct qbman_eq_desc *d,
> + const struct dpaa2_fd *fd,
> + uint32_t *flags,
> + int num_frames);
Please try to avoid all static forward declarations. The coding style for the
kernel generally mandates that you define the functions in the order they
are used in, and have no such declarations, unless there is a recursion
that requires it. If you do have recursion, then please add a comment that
explains how you limit it to avoid overrunning the kernel stack.
> +const struct dpaa2_dq *qbman_swp_dqrr_next_direct(struct qbman_swp *s);
> +const struct dpaa2_dq *qbman_swp_dqrr_next_mem_back(struct qbman_swp *s);
Forward declarations for non-static functions in C code are much
worse, you should
never need these. If a function is shared between files, then put the
declaration
into a header file that is included by both, to ensure the prototypes match, and
if it's only used here, then make it 'static'.
> +/* Function pointers */
> +int (*qbman_swp_enqueue_ptr)(struct qbman_swp *s,
> + const struct qbman_eq_desc *d,
> + const struct dpaa2_fd *fd)
> + = qbman_swp_enqueue_direct;
> +
> +int (*qbman_swp_enqueue_multiple_ptr)(struct qbman_swp *s,
> + const struct qbman_eq_desc *d,
> + const struct dpaa2_fd *fd,
> + uint32_t *flags,
> + int num_frames)
> + = qbman_swp_enqueue_multiple_direct;
This looks like you just have an indirect function pointer with a
single possible
implementation. This is less of a problem, but until you have a way to safely
override these at runtime, it may be better to simplify this by using direct
function calls.
Arnd
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Michal Hocko @ 2020-03-26 10:16 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, Baoquan He, linux-kernel, linux-mm, akpm,
linuxppc-dev
In-Reply-To: <6ef554a6-313d-2b17-cee0-14078ed225f6@linux.ibm.com>
On Thu 26-03-20 15:26:22, Aneesh Kumar K.V wrote:
> On 3/26/20 3:10 PM, Michal Hocko wrote:
> > On Wed 25-03-20 08:49:14, Aneesh Kumar K.V wrote:
> > > Fixes the below crash
> > >
> > > BUG: Kernel NULL pointer dereference on read at 0x00000000
> > > Faulting instruction address: 0xc000000000c3447c
> > > Oops: Kernel access of bad area, sig: 11 [#1]
> > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> > > CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> > > ...
> > > NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> > > LR [c000000000088354] vmemmap_free+0x144/0x320
> > > Call Trace:
> > > section_deactivate+0x220/0x240
> >
> > It would be great to match this to the specific source code.
>
> The crash is due to NULL dereference at
>
> test_bit(idx, ms->usage->subsection_map); due to ms->usage = NULL;
It would be nice to call that out here as well
[...]
> > Why do we have to free usage before deactivaing section memmap? Now that
> > we have a late section_mem_map reset shouldn't we tear down the usage in
> > the same branch?
> >
>
> We still need to make the section invalid before we call into
> depopulate_section_memmap(). Because architecture like powerpc can share
> vmemmap area across sections (16MB mapping of vmemmap area) and we use
> vmemmap_popluated() to make that decision.
This should be noted in a comment as well.
> > > Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> > > Cc: Baoquan He <bhe@redhat.com>
> > > Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > > ---
> > > mm/sparse.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/mm/sparse.c b/mm/sparse.c
> > > index aadb7298dcef..3012d1f3771a 100644
> > > --- a/mm/sparse.c
> > > +++ b/mm/sparse.c
> > > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> > > ms->usage = NULL;
> > > }
> > > memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > > + /* Mark the section invalid */
> > > + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> >
> > Btw. this comment is not really helping at all.
>
> That is marking the section invalid so that
>
> static inline int valid_section(struct mem_section *section)
> {
> return (section && (section->section_mem_map & SECTION_HAS_MEM_MAP));
> }
>
>
> returns false.
Yes that is obvious once you are clear where to look. I was really
hoping for a comment that would simply point you to the right
direcection without chasing SECTION_HAS_MEM_MAP usage. This code is
subtle and useful comments, even when they state something that is
obvious to you _right_now_, can be really helpful.
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: maddy @ 2020-03-26 10:19 UTC (permalink / raw)
To: Kim Phillips, Ravi Bangoria
Cc: Mark Rutland, Andi Kleen, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <8803550e-5d6d-2eda-39f5-e4594052188c@amd.com>
On 3/18/20 11:05 PM, Kim Phillips wrote:
> Hi Maddy,
>
> On 3/17/20 1:50 AM, maddy wrote:
>> On 3/13/20 4:08 AM, Kim Phillips wrote:
>>> On 3/11/20 11:00 AM, Ravi Bangoria wrote:
>>>> On 3/6/20 3:36 AM, Kim Phillips wrote:
>>>>>> On 3/3/20 3:55 AM, Kim Phillips wrote:
>>>>>>> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>>>>>>>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>>>>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>>>>>>>> Modern processors export such hazard data in Performance
>>>>>>>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>>>>>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>>>>>>>> AMD[3] provides similar information.
>>>>>>>>>>
>>>>>>>>>> Implementation detail:
>>>>>>>>>>
>>>>>>>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>>>>>>>> If it's set, kernel converts arch specific hazard information
>>>>>>>>>> into generic format:
>>>>>>>>>>
>>>>>>>>>> struct perf_pipeline_haz_data {
>>>>>>>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>>>>>>>> __u8 itype;
>>>>>>>>>> /* Instruction Cache source */
>>>>>>>>>> __u8 icache;
>>>>>>>>>> /* Instruction suffered hazard in pipeline stage */
>>>>>>>>>> __u8 hazard_stage;
>>>>>>>>>> /* Hazard reason */
>>>>>>>>>> __u8 hazard_reason;
>>>>>>>>>> /* Instruction suffered stall in pipeline stage */
>>>>>>>>>> __u8 stall_stage;
>>>>>>>>>> /* Stall reason */
>>>>>>>>>> __u8 stall_reason;
>>>>>>>>>> __u16 pad;
>>>>>>>>>> };
>>>>>>>>> Kim, does this format indeed work for AMD IBS?
>>>>>>> It's not really 1:1, we don't have these separations of stages
>>>>>>> and reasons, for example: we have missed in L2 cache, for example.
>>>>>>> So IBS output is flatter, with more cycle latency figures than
>>>>>>> IBM's AFAICT.
>>>>>> AMD IBS captures pipeline latency data incase Fetch sampling like the
>>>>>> Fetch latency, tag to retire latency, completion to retire latency and
>>>>>> so on. Yes, Ops sampling do provide more data on load/store centric
>>>>>> information. But it also captures more detailed data for Branch instructions.
>>>>>> And we also looked at ARM SPE, which also captures more details pipeline
>>>>>> data and latency information.
>>>>>>
>>>>>>>> Personally, I don't like the term hazard. This is too IBM Power
>>>>>>>> specific. We need to find a better term, maybe stall or penalty.
>>>>>>> Right, IBS doesn't have a filter to only count stalled or otherwise
>>>>>>> bad events. IBS' PPR descriptions has one occurrence of the
>>>>>>> word stall, and no penalty. The way I read IBS is it's just
>>>>>>> reporting more sample data than just the precise IP: things like
>>>>>>> hits, misses, cycle latencies, addresses, types, etc., so words
>>>>>>> like 'extended', or the 'auxiliary' already used today even
>>>>>>> are more appropriate for IBS, although I'm the last person to
>>>>>>> bikeshed.
>>>>>> We are thinking of using "pipeline" word instead of Hazard.
>>>>> Hm, the word 'pipeline' occurs 0 times in IBS documentation.
>>>> NP. We thought pipeline is generic hw term so we proposed "pipeline"
>>>> word. We are open to term which can be generic enough.
>>>>
>>>>> I realize there are a couple of core pipeline-specific pieces
>>>>> of information coming out of it, but the vast majority
>>>>> are addresses, latencies of various components in the memory
>>>>> hierarchy, and various component hit/miss bits.
>>>> Yes. we should capture core pipeline specific details. For example,
>>>> IBS generates Branch unit information(IbsOpData1) and Icahce related
>>>> data(IbsFetchCtl) which is something that shouldn't be extended as
>>>> part of perf-mem, IMO.
>>> Sure, IBS Op-side output is more 'perf mem' friendly, and so it
>>> should populate perf_mem_data_src fields, just like POWER9 can:
>>>
>>> union perf_mem_data_src {
>>> ...
>>> __u64 mem_rsvd:24,
>>> mem_snoopx:2, /* snoop mode, ext */
>>> mem_remote:1, /* remote */
>>> mem_lvl_num:4, /* memory hierarchy level number */
>>> mem_dtlb:7, /* tlb access */
>>> mem_lock:2, /* lock instr */
>>> mem_snoop:5, /* snoop mode */
>>> mem_lvl:14, /* memory hierarchy level */
>>> mem_op:5; /* type of opcode */
>>>
>>>
>>> E.g., SIER[LDST] SIER[A_XLATE_SRC] can be used to populate
>>> mem_lvl[_num], SIER_TYPE can be used to populate 'mem_op',
>>> 'mem_lock', and the Reload Bus Source Encoding bits can
>>> be used to populate mem_snoop, right?
>> Hi Kim,
>>
>> Yes. We do expose these data as part of perf-mem for POWER.
> OK, I see relevant PERF_MEM_S bits in arch/powerpc/perf/isa207-common.c:
> isa207_find_source now, thanks.
>
>>> For IBS, I see PERF_SAMPLE_ADDR and PERF_SAMPLE_PHYS_ADDR can be
>>> used for the ld/st target addresses, too.
>>>
>>>>> What's needed here is a vendor-specific extended
>>>>> sample information that all these technologies gather,
>>>>> of which things like e.g., 'L1 TLB cycle latency' we
>>>>> all should have in common.
>>>> Yes. We will include fields to capture the latency cycles (like Issue
>>>> latency, Instruction completion latency etc..) along with other pipeline
>>>> details in the proposed structure.
>>> Latency figures are just an example, and from what I
>>> can tell, struct perf_sample_data already has a 'weight' member,
>>> used with PERF_SAMPLE_WEIGHT, that is used by intel-pt to
>>> transfer memory access latency figures. Granted, that's
>>> a bad name given all other vendors don't call latency
>>> 'weight'.
>>>
>>> I didn't see any latency figures coming out of POWER9,
>>> and do not expect this patchseries to implement those
>>> of other vendors, e.g., AMD's IBS; leave each vendor
>>> to amend perf to suit their own h/w output please.
>> Reference structure proposed in this patchset did not have members
>> to capture latency info for that exact reason. But idea here is to
>> abstract as vendor specific as possible. So if we include u16 array,
>> then this format can also capture data from IBS since it provides
>> few latency details.
> OK, that sounds a bit different from the 6 x u8's + 1 u16 padded
> struct presented in this patchset.
>
> IBS Ops can report e.g.:
>
> 15 tag-to-retire cycles bits,
> 15 completion to retire count bits,
> 15 L1 DTLB refill latency bits,
> 15 DC miss latency bits,
> 5 outstanding memory requests on mem refill bits, and so on.
>
> IBS Fetch reports 15 bits of fetch latency, and another 16
> for iTLB latency, among others.
>
> Some of these may/may not be valid simultaneously, and
> there are IBS specific rules to establish validity.
>
>>> My main point there, however, was that each vendor should
>>> use streamlined record-level code to just copy the data
>>> in the proprietary format that their hardware produces,
>>> and then then perf tooling can synthesize the events
>>> from the raw data at report/script/etc. time.
>>>
>>>>> I'm not sure why a new PERF_SAMPLE_PIPELINE_HAZ is needed
>>>>> either. Can we use PERF_SAMPLE_AUX instead?
>>>> We took a look at PERF_SAMPLE_AUX. IIUC, PERF_SAMPLE_AUX is intended when
>>>> large volume of data needs to be captured as part of perf.data without
>>>> frequent PMIs. But proposed type is to address the capture of pipeline
>>> SAMPLE_AUX shouldn't care whether the volume is large, or how frequent
>>> PMIs are, even though it may be used in those environments.
>>>
>>>> information on each sample using PMI at periodic intervals. Hence proposing
>>>> PERF_SAMPLE_PIPELINE_HAZ.
>>> And that's fine for any extra bits that POWER9 has to convey
>>> to its users beyond things already represented by other sample
>>> types like PERF_SAMPLE_DATA_SRC, but the capturing of both POWER9
>>> and other vendor e.g., AMD IBS data can be made vendor-independent
>>> at record time by using SAMPLE_AUX, or SAMPLE_RAW even, which is
>>> what IBS currently uses.
>> My bad. Not sure what you mean by this. We are trying to abstract
>> as much vendor specific data as possible with this (like perf-mem).
> Perhaps if I say it this way: instead of doing all the
> isa207_get_phazard_data() work past the mfspr(SPRN_SIER)
> in patch 4/11, rather/instead just put the raw sier value in a
> PERF_SAMPLE_RAW or _AUX event, and call perf_event_update_userpage.
> Specific SIER capabilities can be written as part of the perf.data
> header. Then synthesize the true pipe events from the raw SIER
> values later, and in userspace.
Hi Kim,
Would like to stay away from SAMPLE_RAW type for these comments in
perf_events.h
* #
* # The RAW record below is opaque data wrt the ABI
* #
* # That is, the ABI doesn't make any promises wrt to
* # the stability of its content, it may vary depending
* # on event, hardware, kernel version and phase of
* # the moon.
* #
* # In other words, PERF_SAMPLE_RAW contents are not an ABI.
* #
Secondly, sorry I didn't understand your suggestion about using
PERF_SAMPLE_AUX.
IIUC, SAMPLE_AUX will go to AUX ring buffer, which is more memory and more
challenging when correlating and presenting the pipeline details for
each IP.
IMO, having a new sample type can be useful to capture the pipeline data
both in perf_sample_data and if _AUX is enabled, can be made to push to
AUX buffer.
Maddy
>
> I guess it's technically optional, but I think that's how
> I'd do it in IBS, since it minimizes the record-time overhead.
>
> Thanks,
>
> Kim
>
>> Maddy
>>>>> Take a look at
>>>>> commit 98dcf14d7f9c "perf tools: Add kernel AUX area sampling
>>>>> definitions". The sample identifier can be used to determine
>>>>> which vendor's sampling IP's data is in it, and events can
>>>>> be recorded just by copying the content of the SIER, etc.
>>>>> registers, and then events get synthesized from the aux
>>>>> sample at report/inject/annotate etc. time. This allows
>>>>> for less sample recording overhead, and moves all the vendor
>>>>> specific decoding and common event conversions for userspace
>>>>> to figure out.
>>>> When AUX buffer data is structured, tool side changes added to present the
>>>> pipeline data can be re-used.
>>> Not sure I understand: AUX data would be structured on
>>> each vendor's raw h/w register formats.
>>>
>>> Thanks,
>>>
>>> Kim
>>>
>>>>>>>> Also worth considering is the support of ARM SPE (Statistical
>>>>>>>> Profiling Extension) which is their version of IBS.
>>>>>>>> Whatever gets added need to cover all three with no limitations.
>>>>>>> I thought Intel's various LBR, PEBS, and PT supported providing
>>>>>>> similar sample data in perf already, like with perf mem/c2c?
>>>>>> perf-mem is more of data centric in my opinion. It is more towards
>>>>>> memory profiling. So proposal here is to expose pipeline related
>>>>>> details like stalls and latencies.
>>>>> Like I said, I don't see it that way, I see it as "any particular
>>>>> vendor's event's extended details', and these pipeline details
>>>>> have overlap with existing infrastructure within perf, e.g., L2
>>>>> cache misses.
>>>>>
>>>>> Kim
>>>>>
^ permalink raw reply
* Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Michal Hocko @ 2020-03-26 10:50 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, Baoquan He, linux-kernel, linux-mm, akpm,
linuxppc-dev
In-Reply-To: <20200326101631.GJ27965@dhcp22.suse.cz>
On Thu 26-03-20 11:16:33, Michal Hocko wrote:
> On Thu 26-03-20 15:26:22, Aneesh Kumar K.V wrote:
> > On 3/26/20 3:10 PM, Michal Hocko wrote:
> > > On Wed 25-03-20 08:49:14, Aneesh Kumar K.V wrote:
> > > > Fixes the below crash
> > > >
> > > > BUG: Kernel NULL pointer dereference on read at 0x00000000
> > > > Faulting instruction address: 0xc000000000c3447c
> > > > Oops: Kernel access of bad area, sig: 11 [#1]
> > > > LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> > > > CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> > > > ...
> > > > NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> > > > LR [c000000000088354] vmemmap_free+0x144/0x320
> > > > Call Trace:
> > > > section_deactivate+0x220/0x240
> > >
> > > It would be great to match this to the specific source code.
> >
> > The crash is due to NULL dereference at
> >
> > test_bit(idx, ms->usage->subsection_map); due to ms->usage = NULL;
>
> It would be nice to call that out here as well
>
> [...]
> > > Why do we have to free usage before deactivaing section memmap? Now that
> > > we have a late section_mem_map reset shouldn't we tear down the usage in
> > > the same branch?
> > >
> >
> > We still need to make the section invalid before we call into
> > depopulate_section_memmap(). Because architecture like powerpc can share
> > vmemmap area across sections (16MB mapping of vmemmap area) and we use
> > vmemmap_popluated() to make that decision.
>
> This should be noted in a comment as well.
>
> > > > Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> > > > Cc: Baoquan He <bhe@redhat.com>
> > > > Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> > > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > > > ---
> > > > mm/sparse.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/mm/sparse.c b/mm/sparse.c
> > > > index aadb7298dcef..3012d1f3771a 100644
> > > > --- a/mm/sparse.c
> > > > +++ b/mm/sparse.c
> > > > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> > > > ms->usage = NULL;
> > > > }
> > > > memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > > > + /* Mark the section invalid */
> > > > + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> > >
> > > Btw. this comment is not really helping at all.
> >
> > That is marking the section invalid so that
> >
> > static inline int valid_section(struct mem_section *section)
> > {
> > return (section && (section->section_mem_map & SECTION_HAS_MEM_MAP));
> > }
> >
> >
> > returns false.
>
> Yes that is obvious once you are clear where to look. I was really
> hoping for a comment that would simply point you to the right
> direcection without chasing SECTION_HAS_MEM_MAP usage. This code is
> subtle and useful comments, even when they state something that is
> obvious to you _right_now_, can be really helpful.
Btw. forgot to add. With the improved comment feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH -next] usb: gadget: fsl: remove unused variable 'driver_desc'
From: Peter Chen @ 2020-03-26 11:27 UTC (permalink / raw)
To: YueHaibing
Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20200326071419.19240-1-yuehaibing@huawei.com>
On 20-03-26 15:14:19, YueHaibing wrote:
> drivers/usb/gadget/udc/fsl_udc_core.c:56:19:
> warning: 'driver_desc' defined but not used [-Wunused-const-variable=]
>
> It is never used, so remove it.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/usb/gadget/udc/fsl_udc_core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
> index ec6eda426223..febabde62f71 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -53,7 +53,6 @@
> #define DMA_ADDR_INVALID (~(dma_addr_t)0)
>
> static const char driver_name[] = "fsl-usb2-udc";
> -static const char driver_desc[] = DRIVER_DESC;
>
> static struct usb_dr_device __iomem *dr_regs;
>
> --
> 2.17.1
>
>
Reviewed-by: Peter Chen <peter.chen@nxp.com>
--
Thanks,
Peter Chen
^ permalink raw reply
* [PATCH v2] macintosh: convert to i2c_new_scanned_device
From: Wolfram Sang @ 2020-03-26 11:38 UTC (permalink / raw)
To: linux-i2c; +Cc: Wolfram Sang, linuxppc-dev
Move from the deprecated i2c_new_probed_device() to the new
i2c_new_scanned_device(). No functional change for this driver because
it doesn't check the return code anyhow.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
---
Change since v1: rebased on top of v5.6-rc7
drivers/macintosh/therm_windtunnel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c b/drivers/macintosh/therm_windtunnel.c
index a0d87ed9da69..f55f6adf5e5f 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -323,7 +323,7 @@ static void do_attach(struct i2c_adapter *adapter)
of_node_put(np);
} else {
strlcpy(info.type, "MAC,ds1775", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_ds1775, NULL);
+ i2c_new_scanned_device(adapter, &info, scan_ds1775, NULL);
}
np = of_find_compatible_node(adapter->dev.of_node, NULL, "MAC,adm1030");
@@ -331,7 +331,7 @@ static void do_attach(struct i2c_adapter *adapter)
of_node_put(np);
} else {
strlcpy(info.type, "MAC,adm1030", I2C_NAME_SIZE);
- i2c_new_probed_device(adapter, &info, scan_adm1030, NULL);
+ i2c_new_scanned_device(adapter, &info, scan_adm1030, NULL);
}
}
--
2.20.1
^ permalink raw reply related
* Re: [powerpc] Intermittent crashes ( link_path_walk) with linux-next
From: Michael Ellerman @ 2020-03-26 11:40 UTC (permalink / raw)
To: Sachin Sant, linuxppc-dev, linux-fsdevel; +Cc: Linux Next Mailing List, Al Viro
In-Reply-To: <1CB4E533-FD97-4C39-87ED-4857F3AB9097@linux.vnet.ibm.com>
Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
> I am running into intermittent crashes with linux-next on POWER 9 PowerVM LPAR
> First it was against next-20200324 while running LTP tests. With next-20200325
> I ran into similar crash (a different stack trace but same failure point — link_path_walk)
> while running sosreport command.
>
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc00000000043f278
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in: loop iscsi_target_mod target_core_mod macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc overlay dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod sd_mod cdrom t10_pi ibmvscsi scsi_transport_srp ibmveth
> CPU: 26 PID: 7771 Comm: avocado Not tainted 5.6.0-rc7-next-20200324-autotest #1
> NIP: c00000000043f278 LR: c00000000043f330 CTR: 00000000000001fb
> REGS: c00000082802f830 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200324-autotest)
> MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 28248442 XER: 20040000
> CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
> GPR00: c00000000043f330 c00000082802fac0 c00000000155e900 0000000000000000
> GPR04: 0000000000000002 0000000000000000 0000000000000002 c0000008b3400000
> GPR08: 0000000000031419 0000000000000000 0000000000000000 ffffffffffff0000
> GPR12: 0000000000008000 c00000001ec48600 00007fffa08a53f8 0000000000000001
> GPR16: 00007fff9faf9a63 00000100073bec00 00007fff9f2493b0 0000000000000000
> GPR20: 00007fffa1143bf8 00007fffa1103b18 c00000087f547cb3 2f2f2f2f2f2f2f2f
> GPR24: 0000000000000003 0000000000000000 c00000082802fbc8 fffffffffffff000
> GPR28: 0000000000200000 ffffffffffffffff 61c8864680b583eb 0000000000000000
> NIP [c00000000043f278] link_path_walk.part.49+0x228/0x400
> LR [c00000000043f330] link_path_walk.part.49+0x2e0/0x400
> Call Trace:
> [c00000082802fac0] [c00000000043f330] link_path_walk.part.49+0x2e0/0x400 (unreliable)
> [c00000082802fb50] [c00000000043f5a4] path_lookupat.isra.51+0x64/0x1f0
> [c00000082802fba0] [c000000000441c00] filename_lookup.part.69+0xa0/0x1b0
> [c00000082802fce0] [c00000000042ff38] vfs_statx+0xa8/0x190
> [c00000082802fd60] [c0000000004302a0] __do_sys_newstat+0x40/0x90
> [c00000082802fe20] [c00000000000b278] system_call+0x5c/0x68
> Instruction dump:
> 3bffffff e93a0058 38800000 7f43d378 7fff07b4 1d5f0030 7d295214 eac90020
> 4bfffb21 2fa30000 409e00c8 e93a0008 <81290000> 55290256 7f89e000 419efecc
> ---[ end trace 34abf29ebd56e423 ]—
>
> Relevant snippet from obj dump:
>
> 6dc4: 20 00 c9 ea ld r22,32(r9)
> link = walk_component(nd, 0);
> 6db4: 78 d3 43 7f mr r3,r26
> name = nd->stack[--depth].name;
> 6db8: b4 07 ff 7f extsw r31,r31
> 6dbc: 30 00 5f 1d mulli r10,r31,48
> 6dc0: 14 52 29 7d add r9,r9,r10
> 6dc4: 20 00 c9 ea ld r22,32(r9)
> link = walk_component(nd, 0);
> 6dc8: 01 00 00 48 bl 6dc8 <link_path_walk.part.49+0x218>
> if (unlikely(link)) {
> 6dcc: 00 00 a3 2f cmpdi cr7,r3,0
> 6dd0: c8 00 9e 40 bne cr7,6e98 <link_path_walk.part.49+0x2e8>
> return dentry->d_flags & DCACHE_ENTRY_TYPE;
> 6dd4: 08 00 3a e9 ld r9,8(r26)
> 6dd8: 00 00 29 81 lwz r9,0(r9) <<=== crashes here ??
Yeah.
And r9 is NULL.
It came from r26 + 8, r26 is c00000082802fbc8 which looks OK.
r26 is nd, the nameidata:
struct nameidata {
struct path path;
struct path {
struct vfsmount *mnt;
struct dentry *dentry;
} __randomize_layout;
r26 + 8 should be the dentry in the struct path.
So we have an nd.path with a NULL dentry.
> 6ddc: 56 02 29 55 rlwinm r9,r9,0,9,11
> if (unlikely(!d_can_lookup(nd->path.dentry))) {
> 6de0: 00 e0 89 7f cmpw cr7,r9,r28
>
> The code in question (link_path_walk() in fs/namei.c ) was recently changed by
> following commit:
>
> commit 881386f7e46a:
> link_path_walk(): sample parent's i_uid and i_mode for the last component
That and about 10 other commits.
Unless Al can give us a clue we'll need to bisect.
cheers
> Crash while running sosreport command:
>
> [ 1917.926113] BUG: Kernel NULL pointer dereference on read at 0x00000000
> [ 1917.926126] Faulting instruction address: 0xc00000000043f638
> [ 1917.926131] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 1917.926136] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [ 1917.926147] Dumping ftrace buffer:
> [ 1917.926157] (ftrace buffer empty)
> [ 1917.926161] Modules linked in: iscsi_target_mod target_core_mod macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc overlay dm_mod nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c ip6_tables nft_compat ip_set rfkill nf_tables nfnetlink sunrpc sg pseries_rng uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod cdrom sd_mod t10_pi ibmvscsi scsi_transport_srp ibmveth
> [ 1917.926198] CPU: 12 PID: 20734 Comm: busctl Not tainted 5.6.0-rc7-next-20200325-autotest #1
> [ 1917.926203] NIP: c00000000043f638 LR: c00000000043f6f0 CTR: 0000000000000000
> [ 1917.926209] REGS: c00000082bc1f720 TRAP: 0300 Not tainted (5.6.0-rc7-next-20200325-autotest)
> [ 1917.926215] MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 28022422 XER: 20040000
> [ 1917.926222] CFAR: c00000000000dec4 DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
> [ 1917.926222] GPR00: c00000000043f6f0 c00000082bc1f9b0 c00000000155e800 0000000000000000
> [ 1917.926222] GPR04: 0000000000000002 0000000000000000 0000000000000002 c0000008b3400000
> [ 1917.926222] GPR08: 000000000000febb 0000000000000000 0000000000000000 ffffffffffff0000
> [ 1917.926222] GPR12: 0000000000002000 c00000001ec5d600 0000000135ed11e0 c0000008879046d0
> [ 1917.926222] GPR16: 7fffffffffffffff fffffffffffffe00 fffffffffffffffc fffffffffffffff5
> [ 1917.926222] GPR20: c00000089fa27200 0000000000000000 c00000008609bf33 2f2f2f2f2f2f2f2f
> [ 1917.926222] GPR24: 0000000000000003 0000000000000000 c00000082bc1fab8 fffffffffffff000
> [ 1917.926222] GPR28: 0000000000200000 ffffffffffffffff 61c8864680b583eb 0000000000000001
> [ 1917.926270] NIP [c00000000043f638] link_path_walk.part.49+0x228/0x400
> [ 1917.926275] LR [c00000000043f6f0] link_path_walk.part.49+0x2e0/0x400
> [ 1917.926280] Call Trace:
> [ 1917.926284] [c00000082bc1f9b0] [c00000000043f6f0] link_path_walk.part.49+0x2e0/0x400 (unreliable)
> [ 1917.926291] [c00000082bc1fa40] [c00000000043f964] path_lookupat.isra.51+0x64/0x1f0
> [ 1917.926296] [c00000082bc1fa90] [c000000000441fc0] filename_lookup.part.69+0xa0/0x1b0
> [ 1917.926303] [c00000082bc1fbd0] [c000000000ae0784] unix_find_other+0x64/0x3f0
> [ 1917.926309] [c00000082bc1fc60] [c000000000ae1b78] unix_stream_connect+0x148/0x930
> [ 1917.926316] [c00000082bc1fd30] [c000000000933010] __sys_connect+0x140/0x170
> [ 1917.926322] [c00000082bc1fe00] [c000000000933068] sys_connect+0x28/0x40
> [ 1917.926328] [c00000082bc1fe20] [c00000000000b278] system_call+0x5c/0x68
> [ 1917.926332] Instruction dump:
> [ 1917.926336] 3bffffff e93a0058 38800000 7f43d378 7fff07b4 1d5f0030 7d295214 eac90020
> [ 1917.926343] 4bfffb21 2fa30000 409e00c8 e93a0008 <81290000> 55290256 7f89e000 419efecc
> [ 1917.926351] ---[ end trace 1b673f0e7295c08b ]---
>
> Objdump o/p
> ===========
>
>
> fs/namei.o: file format elf64-powerpcle
>
>
> Disassembly of section .text:
>
> 0000000000000000 <full_name_hash>:
> * In particular, we must end by hashing a final word containing 0..7
> * payload bytes, to match the way that hash_name() iterates until it
> * finds the delimiter after the name.
> */
> unsigned int full_name_hash(const void *salt, const char *name, unsigned int len)
> {
> 0: a6 02 08 7c mflr r0
> 4: 01 00 00 48 bl 4 <full_name_hash+0x4>
> unsigned long a, x = 0, y = (unsigned long)salt;
>
> for (;;) {
> if (!len)
> .........
> .........
>
> 0000000000006bb0 <link_path_walk.part.49>:
> static int link_path_walk(const char *name, struct nameidata *nd)
> 6bb0: 00 00 4c 3c addis r2,r12,0
> 6bb4: 00 00 42 38 addi r2,r2,0
> 6bb8: a6 02 08 7c mflr r0
> 6bbc: 01 00 00 48 bl 6bbc <link_path_walk.part.49+0xc>
> 6bc0: b0 ff c1 fa std r22,-80(r1)
> 6bc4: d0 ff 41 fb std r26,-48(r1)
> 6bc8: c0 ff 01 fb std r24,-64(r1)
> 6bcc: 78 1b 76 7c mr r22,r3
> 6bd0: 71 ff 21 f8 stdu r1,-144(r1)
> 6bd4: 78 23 9a 7c mr r26,r4
> 6bd8: 18 00 41 f8 std r2,24(r1)
> 6bdc: 78 11 2d e9 ld r9,4472(r13)
> 6be0: 38 00 21 f9 std r9,56(r1)
> 6be4: 00 00 20 39 li r9,0
> while (*name=='/')
> 6be8: 00 00 23 89 lbz r9,0(r3)
> 6bec: 2f 00 89 2b cmplwi cr7,r9,47
> 6bf0: 1c 00 9e 40 bne cr7,6c0c <link_path_walk.part.49+0x5c>
> 6bf4: 00 00 00 60 nop
> 6bf8: 00 00 00 60 nop
> 6bfc: 00 00 00 60 nop
> 6c00: 01 00 36 8d lbzu r9,1(r22)
> 6c04: 2f 00 89 2b cmplwi cr7,r9,47
> 6c08: f8 ff 9e 41 beq cr7,6c00 <link_path_walk.part.49+0x50>
> if (!*name)
> 6c0c: 00 00 a9 2f cmpdi cr7,r9,0
> return 0;
> 6c10: 00 00 00 3b li r24,0
> if (!*name)
> 6c14: 3c 00 9e 40 bne cr7,6c50 <link_path_walk.part.49+0xa0>
> }
> 6c18: 38 00 21 e9 ld r9,56(r1)
> 6c1c: 78 11 4d e9 ld r10,4472(r13)
> 6c20: 79 52 29 7d xor. r9,r9,r10
> 6c24: 00 00 40 39 li r10,0
> 6c28: 78 c3 03 7f mr r3,r24
> 6c2c: 54 03 82 40 bne 6f80 <link_path_walk.part.49+0x3d0>
> 6c30: 90 00 21 38 addi r1,r1,144
> 6c34: b0 ff c1 ea ld r22,-80(r1)
> 6c38: c0 ff 01 eb ld r24,-64(r1)
> 6c3c: d0 ff 41 eb ld r26,-48(r1)
> 6c40: 20 00 80 4e blr
> 6c44: 00 00 00 60 nop
> 6c48: 00 00 00 60 nop
> 6c4c: 00 00 00 60 nop
> 6c50: 48 00 e1 fa std r23,72(r1)
> 6c54: 80 00 c1 fb std r30,128(r1)
> b = a ^ REPEAT_BYTE('/');
> 6c58: 2f 2f e0 3e lis r23,12079
> 6c5c: 2f 2f f7 62 ori r23,r23,12079
> 6c60: a6 02 08 7c mflr r0
> y ^= x * GOLDEN_RATIO_64;
> 6c64: c8 61 c0 3f lis r30,25032
> 6c68: 46 86 de 63 ori r30,r30,34374
> 6c6c: 58 00 21 fb std r25,88(r1)
> 6c70: 68 00 61 fb std r27,104(r1)
> unsigned long a = 0, b, x = 0, y = (unsigned long)salt;
> 6c74: 00 00 20 3b li r25,0
> 6c78: 70 00 81 fb std r28,112(r1)
> 6c7c: 78 00 a1 fb std r29,120(r1)
> 6c80: 88 00 e1 fb std r31,136(r1)
> x ^= a & zero_bytemask(mask);
> 6c84: ff ff a0 3b li r29,-1
> int depth = 0; // depth <= nd->depth
> 6c88: 00 00 e0 3b li r31,0
> if (unlikely(!d_can_lookup(nd->path.dentry))) {
> 6c8c: 20 00 80 3f lis r28,32
> if (IS_ERR(link))
> 6c90: 00 f0 60 3b li r27,-4096
> b = a ^ REPEAT_BYTE('/');
> 6c94: c6 07 f7 7a rldicr r23,r23,32,31
> y ^= x * GOLDEN_RATIO_64;
> 6c98: c6 07 de 7b rldicr r30,r30,32,31
> b = a ^ REPEAT_BYTE('/');
> 6c9c: 2f 2f f7 66 oris r23,r23,12079
> 6ca0: a0 00 01 f8 std r0,160(r1)
> y ^= x * GOLDEN_RATIO_64;
> 6ca4: b5 80 de 67 oris r30,r30,32949
> b = a ^ REPEAT_BYTE('/');
> 6ca8: 2f 2f f7 62 ori r23,r23,12079
> y ^= x * GOLDEN_RATIO_64;
> 6cac: eb 83 de 63 ori r30,r30,33771
> if (nd->flags & LOOKUP_RCU) {
> 6cb0: 38 00 3a 81 lwz r9,56(r26)
> 6cb4: 30 00 7a e8 ld r3,48(r26)
> 6cb8: 40 00 29 71 andi. r9,r9,64
> 6cbc: 08 02 82 41 beq 6ec4 <link_path_walk.part.49+0x314>
> int err = inode_permission(nd->inode, MAY_EXEC|MAY_NOT_BLOCK);
> 6cc0: 81 00 80 38 li r4,129
> 6cc4: 01 00 00 48 bl 6cc4 <link_path_walk.part.49+0x114>
> if (err != -ECHILD)
> 6cc8: f6 ff 83 2f cmpwi cr7,r3,-10
> int err = inode_permission(nd->inode, MAY_EXEC|MAY_NOT_BLOCK);
> 6ccc: 78 1b 78 7c mr r24,r3
> if (err != -ECHILD)
> 6cd0: e0 01 9e 41 beq cr7,6eb0 <link_path_walk.part.49+0x300>
> if (err)
> 6cd4: 00 00 b8 2f cmpdi cr7,r24,0
> 6cd8: 30 01 9e 40 bne cr7,6e08 <link_path_walk.part.49+0x258>
> hash_len = hash_name(nd->path.dentry, name);
> 6cdc: 08 00 7a e8 ld r3,8(r26)
> len = 0;
> 6ce0: 00 00 a0 38 li r5,0
> unsigned long a = 0, b, x = 0, y = (unsigned long)salt;
> 6ce4: 00 00 c0 38 li r6,0
> 6ce8: 78 1b 67 7c mr r7,r3
> goto inside;
> 6cec: 24 00 00 48 b 6d10 <link_path_walk.part.49+0x160>
> HASH_MIX(x, y, a);
> 6cf0: 78 4a c9 7c xor r9,r6,r9
> len += sizeof(unsigned long);
> 6cf4: 08 00 a5 38 addi r5,r5,8
> HASH_MIX(x, y, a);
> 6cf8: 78 3a 26 7d xor r6,r9,r7
> 6cfc: 00 60 29 79 rotldi r9,r9,12
> 6d00: 02 68 c7 78 rotldi r7,r6,45
> 6d04: 14 4a c6 7c add r6,r6,r9
> 6d08: 24 1f e9 78 rldicr r9,r7,3,60
> 6d0c: 14 3a e9 7c add r7,r9,r7
> a = load_unaligned_zeropad(name+len);
> 6d10: 14 2a 56 7d add r10,r22,r5
> asm(
> 6d14: 00 00 2a e9 ld r9,0(r10)
> b = a ^ REPEAT_BYTE('/');
> 6d18: 78 ba 2a 7d xor r10,r9,r23
> asm("cmpb %0,%1,%2" : "=r" (ret) : "r" (a), "r" (zero));
> 6d1c: f8 cb 28 7d cmpb r8,r9,r25
> 6d20: f8 cb 4a 7d cmpb r10,r10,r25
> } while (!(has_zero(a, &adata, &constants) | has_zero(b, &bdata, &constants)));
> 6d24: 79 53 0a 7d or. r10,r8,r10
> 6d28: c8 ff 82 41 beq 6cf0 <link_path_walk.part.49+0x140>
> asm("addi %1,%2,-1\n\t"
> 6d2c: ff ff 0a 39 addi r8,r10,-1
> 6d30: 78 50 08 7d andc r8,r8,r10
> 6d34: f4 03 0a 7d popcntd r10,r8
> x ^= a & zero_bytemask(mask);
> 6d38: 36 50 ab 7f sld r11,r29,r10
> return mask >> 3;
> 6d3c: c2 e8 48 79 rldicl r8,r10,61,3
> if (name[0] == '.') switch (hashlen_len(hash_len)) {
> 6d40: 00 00 96 88 lbz r4,0(r22)
> x ^= a & zero_bytemask(mask);
> 6d44: 78 58 2a 7d andc r10,r9,r11
> return hashlen_create(fold_hash(x, y), len + find_zero(mask));
> 6d48: 14 2a 08 7d add r8,r8,r5
> x ^= a & zero_bytemask(mask);
> 6d4c: 78 32 49 7d xor r9,r10,r6
> if (name[0] == '.') switch (hashlen_len(hash_len)) {
> 6d50: 2e 00 84 2f cmpwi cr7,r4,46
> y ^= x * GOLDEN_RATIO_64;
> 6d54: d2 f1 29 7d mulld r9,r9,r30
> 6d58: 78 3a 29 7d xor r9,r9,r7
> y *= GOLDEN_RATIO_64;
> 6d5c: d2 f1 29 7d mulld r9,r9,r30
> return y >> 32;
> 6d60: 22 00 29 79 rldicl r9,r9,32,32
> return hashlen_create(fold_hash(x, y), len + find_zero(mask));
> 6d64: 0e 00 09 79 rldimi r9,r8,32,0
> if (name[0] == '.') switch (hashlen_len(hash_len)) {
> 6d68: c8 00 9e 41 beq cr7,6e30 <link_path_walk.part.49+0x280>
> 6d6c: 38 00 5a 81 lwz r10,56(r26)
> nd->flags &= ~LOOKUP_JUMPED;
> 6d70: 24 05 4a 55 rlwinm r10,r10,0,20,18
> 6d74: 38 00 5a 91 stw r10,56(r26)
> if (unlikely(parent->d_flags & DCACHE_OP_HASH)) {
> 6d78: 00 00 43 81 lwz r10,0(r3)
> 6d7c: 01 00 4a 71 andi. r10,r10,1
> 6d80: 70 01 82 40 bne 6ef0 <link_path_walk.part.49+0x340>
> 6d84: 22 00 28 79 rldicl r8,r9,32,32
> nd->last.hash_len = hash_len;
> 6d88: 10 00 3a f9 std r9,16(r26)
> nd->last.name = name;
> 6d8c: 18 00 da fa std r22,24(r26)
> nd->last_type = type;
> 6d90: 48 00 1a 93 stw r24,72(r26)
> if (!*name)
> 6d94: ee 40 36 7d lbzux r9,r22,r8
> 6d98: 00 00 89 2f cmpwi cr7,r9,0
> 6d9c: d4 00 9e 40 bne cr7,6e70 <link_path_walk.part.49+0x2c0>
> if (!depth) {
> 6da0: 00 00 bf 2f cmpdi cr7,r31,0
> 6da4: ac 01 9e 41 beq cr7,6f50 <link_path_walk.part.49+0x3a0>
> name = nd->stack[--depth].name;
> 6da8: ff ff ff 3b addi r31,r31,-1
> 6dac: 58 00 3a e9 ld r9,88(r26)
> link = walk_component(nd, 0);
> 6db0: 00 00 80 38 li r4,0
> 6db4: 78 d3 43 7f mr r3,r26
> name = nd->stack[--depth].name;
> 6db8: b4 07 ff 7f extsw r31,r31
> 6dbc: 30 00 5f 1d mulli r10,r31,48
> 6dc0: 14 52 29 7d add r9,r9,r10
> 6dc4: 20 00 c9 ea ld r22,32(r9)
> link = walk_component(nd, 0);
> 6dc8: 01 00 00 48 bl 6dc8 <link_path_walk.part.49+0x218>
> if (unlikely(link)) {
> 6dcc: 00 00 a3 2f cmpdi cr7,r3,0
> 6dd0: c8 00 9e 40 bne cr7,6e98 <link_path_walk.part.49+0x2e8>
> return dentry->d_flags & DCACHE_ENTRY_TYPE;
> 6dd4: 08 00 3a e9 ld r9,8(r26)
> 6dd8: 00 00 29 81 lwz r9,0(r9)
> 6ddc: 56 02 29 55 rlwinm r9,r9,0,9,11
> if (unlikely(!d_can_lookup(nd->path.dentry))) {
> 6de0: 00 e0 89 7f cmpw cr7,r9,r28
> 6de4: cc fe 9e 41 beq cr7,6cb0 <link_path_walk.part.49+0x100>
> if (nd->flags & LOOKUP_RCU) {
> 6de8: 38 00 3a 81 lwz r9,56(r26)
> 6dec: 40 00 29 71 andi. r9,r9,64
> 6df0: 38 01 82 41 beq 6f28 <link_path_walk.part.49+0x378>
> if (unlazy_walk(nd))
> 6df4: 78 d3 43 7f mr r3,r26
> 6df8: 01 00 00 48 bl 6df8 <link_path_walk.part.49+0x248>
> 6dfc: 00 00 a3 2f cmpdi cr7,r3,0
> 6e00: 28 01 9e 41 beq cr7,6f28 <link_path_walk.part.49+0x378>
> return -ECHILD;
> 6e04: f6 ff 00 3b li r24,-10
> 6e08: a0 00 01 e8 ld r0,160(r1)
> 6e0c: 48 00 e1 ea ld r23,72(r1)
> 6e10: 58 00 21 eb ld r25,88(r1)
> 6e14: 68 00 61 eb ld r27,104(r1)
> 6e18: 70 00 81 eb ld r28,112(r1)
> 6e1c: 78 00 a1 eb ld r29,120(r1)
> 6e20: 80 00 c1 eb ld r30,128(r1)
> 6e24: 88 00 e1 eb ld r31,136(r1)
> 6e28: a6 03 08 7c mtlr r0
> 6e2c: ec fd ff 4b b 6c18 <link_path_walk.part.49+0x68>
> if (name[0] == '.') switch (hashlen_len(hash_len)) {
> 6e30: 22 00 28 79 rldicl r8,r9,32,32
> 6e34: 20 00 0a 79 clrldi r10,r8,32
> 6e38: 01 00 8a 2b cmplwi cr7,r10,1
> 6e3c: a4 00 9e 41 beq cr7,6ee0 <link_path_walk.part.49+0x330>
> 6e40: 02 00 8a 2b cmplwi cr7,r10,2
> 6e44: 28 ff 9e 40 bne cr7,6d6c <link_path_walk.part.49+0x1bc>
> if (name[1] == '.') {
> 6e48: 01 00 f6 88 lbz r7,1(r22)
> 6e4c: 38 00 5a 81 lwz r10,56(r26)
> 6e50: 2e 00 87 2f cmpwi cr7,r7,46
> 6e54: 1c ff 9e 40 bne cr7,6d70 <link_path_walk.part.49+0x1c0>
> nd->flags |= LOOKUP_JUMPED;
> 6e58: 00 10 4a 61 ori r10,r10,4096
> type = LAST_DOTDOT;
> 6e5c: 03 00 00 3b li r24,3
> nd->flags |= LOOKUP_JUMPED;
> 6e60: 38 00 5a 91 stw r10,56(r26)
> if (likely(type == LAST_NORM)) {
> 6e64: 24 ff ff 4b b 6d88 <link_path_walk.part.49+0x1d8>
> 6e68: 00 00 00 60 nop
> 6e6c: 00 00 00 60 nop
> } while (unlikely(*name == '/'));
> 6e70: 01 00 36 8d lbzu r9,1(r22)
> 6e74: 2f 00 89 2b cmplwi cr7,r9,47
> 6e78: f8 ff 9e 41 beq cr7,6e70 <link_path_walk.part.49+0x2c0>
> if (unlikely(!*name)) {
> 6e7c: 00 00 a9 2f cmpdi cr7,r9,0
> 6e80: 20 ff 9e 41 beq cr7,6da0 <link_path_walk.part.49+0x1f0>
> link = walk_component(nd, WALK_MORE);
> 6e84: 02 00 80 38 li r4,2
> 6e88: 78 d3 43 7f mr r3,r26
> 6e8c: 01 00 00 48 bl 6e8c <link_path_walk.part.49+0x2dc>
> if (unlikely(link)) {
> 6e90: 00 00 a3 2f cmpdi cr7,r3,0
> 6e94: 40 ff 9e 41 beq cr7,6dd4 <link_path_walk.part.49+0x224>
> if (IS_ERR(link))
> 6e98: 40 d8 a3 7f cmpld cr7,r3,r27
> 6e9c: 94 00 9d 40 ble cr7,6f30 <link_path_walk.part.49+0x380>
> return PTR_ERR(link);
> 6ea0: b4 07 78 7c extsw r24,r3
> 6ea4: 64 ff ff 4b b 6e08 <link_path_walk.part.49+0x258>
> 6ea8: 00 00 00 60 nop
> 6eac: 00 00 00 60 nop
> if (unlazy_walk(nd))
> 6eb0: 78 d3 43 7f mr r3,r26
> 6eb4: 01 00 00 48 bl 6eb4 <link_path_walk.part.49+0x304>
> 6eb8: 00 00 a3 2f cmpdi cr7,r3,0
> 6ebc: 48 ff 9e 40 bne cr7,6e04 <link_path_walk.part.49+0x254>
> 6ec0: 30 00 7a e8 ld r3,48(r26)
> return inode_permission(nd->inode, MAY_EXEC);
> 6ec4: 01 00 80 38 li r4,1
> 6ec8: 01 00 00 48 bl 6ec8 <link_path_walk.part.49+0x318>
> 6ecc: 78 1b 78 7c mr r24,r3
> 6ed0: 04 fe ff 4b b 6cd4 <link_path_walk.part.49+0x124>
> 6ed4: 00 00 00 60 nop
> 6ed8: 00 00 00 60 nop
> 6edc: 00 00 00 60 nop
> type = LAST_DOT;
> 6ee0: 02 00 00 3b li r24,2
> 6ee4: a4 fe ff 4b b 6d88 <link_path_walk.part.49+0x1d8>
> 6ee8: 00 00 00 60 nop
> 6eec: 00 00 00 60 nop
> err = parent->d_op->d_hash(parent, &this);
> 6ef0: 60 00 43 e9 ld r10,96(r3)
> struct qstr this = { { .hash_len = hash_len }, .name = name };
> 6ef4: 28 00 21 f9 std r9,40(r1)
> 6ef8: 30 00 c1 fa std r22,48(r1)
> err = parent->d_op->d_hash(parent, &this);
> 6efc: 28 00 81 38 addi r4,r1,40
> 6f00: 10 00 2a e9 ld r9,16(r10)
> 6f04: 78 4b 2c 7d mr r12,r9
> 6f08: a6 03 29 7d mtctr r9
> 6f0c: 21 04 80 4e bctrl
> 6f10: 18 00 41 e8 ld r2,24(r1)
> if (err < 0)
> 6f14: 00 00 83 2f cmpwi cr7,r3,0
> 6f18: 60 00 9c 41 blt cr7,6f78 <link_path_walk.part.49+0x3c8>
> hash_len = this.hash_len;
> 6f1c: 28 00 21 e9 ld r9,40(r1)
> name = this.name;
> 6f20: 30 00 c1 ea ld r22,48(r1)
> 6f24: 60 fe ff 4b b 6d84 <link_path_walk.part.49+0x1d4>
> return -ENOTDIR;
> 6f28: ec ff 00 3b li r24,-20
> 6f2c: dc fe ff 4b b 6e08 <link_path_walk.part.49+0x258>
> nd->stack[depth++].name = name;
> 6f30: 30 00 5f 1d mulli r10,r31,48
> 6f34: 58 00 3a e9 ld r9,88(r26)
> 6f38: 01 00 ff 3b addi r31,r31,1
> 6f3c: b4 07 ff 7f extsw r31,r31
> 6f40: 14 52 29 7d add r9,r9,r10
> 6f44: 20 00 c9 fa std r22,32(r9)
> 6f48: 78 1b 76 7c mr r22,r3
> 6f4c: 64 fd ff 4b b 6cb0 <link_path_walk.part.49+0x100>
> nd->dir_uid = nd->inode->i_uid;
> 6f50: 30 00 5a e9 ld r10,48(r26)
> nd->flags &= ~LOOKUP_PARENT;
> 6f54: 38 00 3a 81 lwz r9,56(r26)
> return 0;
> 6f58: 00 00 00 3b li r24,0
> nd->dir_uid = nd->inode->i_uid;
> 6f5c: 04 00 0a 81 lwz r8,4(r10)
> nd->flags &= ~LOOKUP_PARENT;
> 6f60: 34 07 29 55 rlwinm r9,r9,0,28,26
> nd->dir_uid = nd->inode->i_uid;
> 6f64: d8 00 1a 91 stw r8,216(r26)
> nd->dir_mode = nd->inode->i_mode;
> 6f68: 00 00 4a a1 lhz r10,0(r10)
> nd->flags &= ~LOOKUP_PARENT;
> 6f6c: 38 00 3a 91 stw r9,56(r26)
> nd->dir_mode = nd->inode->i_mode;
> 6f70: dc 00 5a b1 sth r10,220(r26)
> return 0;
> 6f74: 94 fe ff 4b b 6e08 <link_path_walk.part.49+0x258>
> err = parent->d_op->d_hash(parent, &this);
> 6f78: 78 1b 78 7c mr r24,r3
> 6f7c: 8c fe ff 4b b 6e08 <link_path_walk.part.49+0x258>
> 6f80: a6 02 08 7c mflr r0
> 6f84: 48 00 e1 fa std r23,72(r1)
> 6f88: 58 00 21 fb std r25,88(r1)
> 6f8c: 68 00 61 fb std r27,104(r1)
> 6f90: 70 00 81 fb std r28,112(r1)
> 6f94: 78 00 a1 fb std r29,120(r1)
> 6f98: 80 00 c1 fb std r30,128(r1)
> 6f9c: 88 00 e1 fb std r31,136(r1)
> 6fa0: a0 00 01 f8 std r0,160(r1)
> }
> 6fa4: 01 00 00 48 bl 6fa4 <link_path_walk.part.49+0x3f4>
> 6fa8: 00 00 00 60 nop
> 6fac: 00 00 00 60 nop
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Add a test of sigreturn vs VDSO
From: Michael Ellerman @ 2020-03-26 12:04 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev
In-Reply-To: <87k13xjpxr.fsf@linux.ibm.com>
Nathan Lynch <nathanl@linux.ibm.com> writes:
> Nathan Lynch <nathanl@linux.ibm.com> writes:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> +static int search_proc_maps(char *needle, unsigned long *low, unsigned long *high)
>>
>> ^^ const?
Sorry I meant to do this but then forgot.
>>> +{
>>> + unsigned long start, end;
>>> + static char buf[4096];
>>> + char name[128];
>>> + FILE *f;
>>> + int rc = -1;
>>> +
>>> + f = fopen("/proc/self/maps", "r");
>>> + if (!f) {
>>> + perror("fopen");
>>> + return -1;
>>> + }
>>> +
>>> + while (fgets(buf, sizeof(buf), f)) {
>>> + rc = sscanf(buf, "%lx-%lx %*c%*c%*c%*c %*x %*d:%*d %*d %127s\n",
>>> + &start, &end, name);
>>
>> I suspect it doesn't matter in practice for this particular test, but
>> since this looks like a generally useful function that could gain users
>> in the future: does this spuriously fail if the matching line straddles
>> a 4096-byte boundary? Maybe fscanf(3) should be used instead?
>
> Or maybe I should read the fgets man page more closely :-)
>
> "Reading stops after an EOF or a newline."
>
> Sorry for the noise.
No worries, thanks for reviewing.
cheers
^ permalink raw reply
* Re: [PATCH v3 2/3] selftests/powerpc: Add tm-signal-pagefault test
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Gustavo Luiz Duarte, linuxppc-dev; +Cc: mikey, gromero, Gustavo Luiz Duarte
In-Reply-To: <20200211033831.11165-2-gustavold@linux.ibm.com>
On Tue, 2020-02-11 at 03:38:30 UTC, Gustavo Luiz Duarte wrote:
> This test triggers a TM Bad Thing by raising a signal in transactional state
> and forcing a pagefault to happen in kernelspace when the kernel signal
> handling code first touches the user signal stack.
>
> This is inspired by the test tm-signal-context-force-tm but uses userfaultfd to
> make the test deterministic. While this test always triggers the bug in one
> run, I had to execute tm-signal-context-force-tm several times (the test runs
> 5000 times each execution) to trigger the same bug.
>
> tm-signal-context-force-tm is kept instead of replaced because, while this test
> is more reliable and triggers the same bug, tm-signal-context-force-tm has a
> better coverage, in the sense that by running the test several times it might
> trigger the pagefault and/or be preempted at different places.
>
> v3: skip test if userfaultfd is unavailable.
>
> Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/915b7f6f9a5e232c138bb36743a1fdb0fcf2c432
cheers
^ permalink raw reply
* Re: [PATCH v3 3/3] selftests/powerpc: Don't rely on segfault to rerun the test
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Gustavo Luiz Duarte, linuxppc-dev; +Cc: mikey, gromero, Gustavo Luiz Duarte
In-Reply-To: <20200211033831.11165-3-gustavold@linux.ibm.com>
On Tue, 2020-02-11 at 03:38:31 UTC, Gustavo Luiz Duarte wrote:
> The test case tm-signal-context-force-tm expects a segfault to happen on
> returning from signal handler, and then does a setcontext() to run the test
> again. However, the test doesn't always segfault, causing the test to run a
> single time.
>
> This patch fixes the test by putting it within a loop and jumping, via
> setcontext, just prior to the loop in case it segfaults. This way we get the
> desired behavior (run the test COUNT_MAX times) regardless if it segfaults or
> not. This also reduces the use of setcontext for control flow logic, keeping it
> only in the segfault handler.
>
> Also, since 'count' is changed within the signal handler, it is declared as
> volatile to prevent any compiler optimization getting confused with
> asynchronous changes.
>
> Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/0f8f554e5244f56f496b4ce30ada1126fe290345
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/kprobes: Remove redundant code
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Naveen N. Rao
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <07a17425743600460ce35fa9432d42487a825583.1582099499.git.christophe.leroy@c-s.fr>
On Wed, 2020-02-19 at 08:05:57 UTC, Christophe Leroy wrote:
> At the time being we have something like
>
> if (something) {
> p = get();
> if (p) {
> if (something_wrong)
> goto out;
> ...
> return;
> } else if (a != b) {
> if (some_error)
> goto out;
> ...
> }
> goto out;
> }
> p = get();
> if (!p) {
> if (a != b) {
> if (some_error)
> goto out;
> ...
> }
> goto out;
> }
>
> This is similar to
>
> p = get();
> if (!p) {
> if (a != b) {
> if (some_error)
> goto out;
> ...
> }
> goto out;
> }
> if (something) {
> if (something_wrong)
> goto out;
> ...
> return;
> }
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/eb4f8e259acc37b91b62ca57e0d3c8960c357843
cheers
^ permalink raw reply
* Re: [PATCH v3] powerpc/kprobes: Ignore traps that happened in real mode
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Larry Finger, Naveen N. Rao, Masami Hiramatsu
Cc: linuxppc-dev, stable, linux-kernel, Anil S Keshavamurthy,
David S. Miller
In-Reply-To: <424331e2006e7291a1bfe40e7f3fa58825f565e1.1582054578.git.christophe.leroy@c-s.fr>
On Tue, 2020-02-18 at 19:38:27 UTC, Christophe Leroy wrote:
> When a program check exception happens while MMU translation is
> disabled, following Oops happens in kprobe_handler() in the following
> code:
>
> } else if (*addr != BREAKPOINT_INSTRUCTION) {
>
> [ 33.098554] BUG: Unable to handle kernel data access on read at 0x0000e268
> [ 33.105091] Faulting instruction address: 0xc000ec34
> [ 33.110010] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 33.115348] BE PAGE_SIZE=16K PREEMPT CMPC885
> [ 33.119540] Modules linked in:
> [ 33.122591] CPU: 0 PID: 429 Comm: cat Not tainted 5.6.0-rc1-s3k-dev-00824-g84195dc6c58a #3267
> [ 33.131005] NIP: c000ec34 LR: c000ecd8 CTR: c019cab8
> [ 33.136002] REGS: ca4d3b58 TRAP: 0300 Not tainted (5.6.0-rc1-s3k-dev-00824-g84195dc6c58a)
> [ 33.144324] MSR: 00001032 <ME,IR,DR,RI> CR: 2a4d3c52 XER: 00000000
> [ 33.150699] DAR: 0000e268 DSISR: c0000000
> [ 33.150699] GPR00: c000b09c ca4d3c10 c66d0620 00000000 ca4d3c60 00000000 00009032 00000000
> [ 33.150699] GPR08: 00020000 00000000 c087de44 c000afe0 c66d0ad0 100d3dd6 fffffff3 00000000
> [ 33.150699] GPR16: 00000000 00000041 00000000 ca4d3d70 00000000 00000000 0000416d 00000000
> [ 33.150699] GPR24: 00000004 c53b6128 00000000 0000e268 00000000 c07c0000 c07bb6fc ca4d3c60
> [ 33.188015] NIP [c000ec34] kprobe_handler+0x128/0x290
> [ 33.192989] LR [c000ecd8] kprobe_handler+0x1cc/0x290
> [ 33.197854] Call Trace:
> [ 33.200340] [ca4d3c30] [c000b09c] program_check_exception+0xbc/0x6fc
> [ 33.206590] [ca4d3c50] [c000e43c] ret_from_except_full+0x0/0x4
> [ 33.212392] --- interrupt: 700 at 0xe268
> [ 33.270401] Instruction dump:
> [ 33.273335] 913e0008 81220000 38600001 3929ffff 91220000 80010024 bb410008 7c0803a6
> [ 33.280992] 38210020 4e800020 38600000 4e800020 <813b0000> 6d2a7fe0 2f8a0008 419e0154
> [ 33.288841] ---[ end trace 5b9152d4cdadd06d ]---
>
> kprobe is not prepared to handle events in real mode and functions
> running in real mode should have been blacklisted, so kprobe_handler()
> can safely bail out telling 'this trap is not mine' for any trap that
> happened while in real-mode.
>
> If the trap happened with MSR_IR or MSR_DR cleared, return 0 immediately.
>
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Fixes: 6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
> Cc: stable@vger.kernel.org
> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/21f8b2fa3ca5b01f7a2b51b89ce97a3705a15aa0
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/xmon: Lower limits on nidump and ndump
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20200219110007.31195-1-mpe@ellerman.id.au>
On Wed, 2020-02-19 at 11:00:07 UTC, Michael Ellerman wrote:
> In xmon we have two variables that are used by the dump commands.
> There's ndump which is the number of bytes to dump using 'd', and
> nidump which is the number of instructions to dump using 'di'.
>
> ndump starts as 64 and nidump starts as 16, but both can be set by the
> user.
>
> It's fairly common to be pasting addresses into xmon when trying to
> debug something, and if you inadvertently double paste an address like
> so:
>
> 0:mon> di c000000002101f6c c000000002101f6c
>
> The second value is interpreted as the number of instructions to dump.
>
> Luckily it doesn't dump 13 quintrillion instructions, the value is
> limited to MAX_DUMP (128K). But as each instruction is dumped on a
> single line, that's still a lot of output. If you're on a slow console
> that can take multiple minutes to print. If you were "just popping in
> and out of xmon quickly before the RCU/hardlockup detector fires" you
> are now having a bad day.
>
> Things are not as bad with 'd' because we print 16 bytes per line, so
> it's fewer lines. But it's still quite a lot.
>
> So shrink the maximum for 'd' to 64K (one page), which is 4096 lines.
> For 'di' add a new limit which is the above / 4 - because instructions
> are 4 bytes, meaning again we can dump one page.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Applied to powerpc next.
https://git.kernel.org/powerpc/c/d64c7dbb4d98306b794401ca924ad053f84b59f8
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/64s: Fix section mismatch warnings from boot code
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20200225031328.14676-1-mpe@ellerman.id.au>
On Tue, 2020-02-25 at 03:13:28 UTC, Michael Ellerman wrote:
> We currently have two section mismatch warnings:
>
> The function __boot_from_prom() references
> the function __init prom_init().
>
> The function start_here_common() references
> the function __init start_kernel().
>
> The warnings are correct, we do have branches from non-init code into
> init code, which is freed after boot. But we don't expect to ever
> execute any of that early boot code after boot, if we did that would
> be a bug. In particular calling into OF after boot would be fatal
> because OF is no longer resident.
>
> So for now fix the warnings by marking the relevant functions as
> __REF, which puts them in the ".ref.text" section.
>
> This causes some reordering of the functions in the final link:
>
> @@ -217,10 +217,9 @@
> c00000000000b088 t generic_secondary_common_init
> c00000000000b124 t __mmu_off
> c00000000000b14c t __start_initialization_multiplatform
> -c00000000000b1ac t __boot_from_prom
> -c00000000000b1ec t __after_prom_start
> -c00000000000b260 t p_end
> -c00000000000b27c T copy_and_flush
> +c00000000000b1ac t __after_prom_start
> +c00000000000b220 t p_end
> +c00000000000b23c T copy_and_flush
> c00000000000b300 T __secondary_start
> c00000000000b300 t copy_to_here
> c00000000000b344 t start_secondary_prolog
> @@ -228,8 +227,9 @@
> c00000000000b36c t enable_64b_mode
> c00000000000b388 T relative_toc
> c00000000000b3a8 t p_toc
> -c00000000000b3b0 t start_here_common
> -c00000000000b3d0 t start_here_multiplatform
> +c00000000000b3b0 t __boot_from_prom
> +c00000000000b3f0 t start_here_multiplatform
> +c00000000000b480 t start_here_common
> c00000000000b880 T system_call_common
> c00000000000b974 t system_call
> c00000000000b9dc t system_call_exit
>
> In particular __boot_from_prom moves after copy_to_here, which means
> it's not copied to zero in the first stage of copy of the kernel to
> zero.
>
> But that's OK, because we only call __boot_from_prom before we do the
> copy, so it makes no difference when it's copied. The call sequence
> is:
> __start
> -> __start_initialization_multiplatform
> -> __boot_from_prom
> -> __start
> -> __start_initialization_multiplatform
> -> __after_prom_start
> -> copy_and_flush
> -> copy_and_flush (relocated to 0)
> -> start_here_multiplatform
> -> early_setup
>
> Reported-by: Mauricio Faria de Oliveira <mauricfo@linux.ibm.com>
> Reported-by: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Applied to powerpc next.
https://git.kernel.org/powerpc/c/6eeb9b3b9ce588f14a697737a30d0702b5a20293
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/kuap: PPC_KUAP_DEBUG should depend on PPC_KUAP
From: Michael Ellerman @ 2020-03-26 12:06 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20200301111738.22497-1-mpe@ellerman.id.au>
On Sun, 2020-03-01 at 11:17:38 UTC, Michael Ellerman wrote:
> Currently you can enable PPC_KUAP_DEBUG when PPC_KUAP is disabled,
> even though the former has not effect without the latter.
>
> Fix it so that PPC_KUAP_DEBUG can only be enabled when PPC_KUAP is
> enabled, not when the platform could support KUAP (PPC_HAVE_KUAP).
>
> Fixes: 890274c2dc4c ("powerpc/64s: Implement KUAP for Radix MMU")
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Applied to powerpc next.
https://git.kernel.org/powerpc/c/61da50b76b62fd815aa82d853bf82bf4f69568f5
cheers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox