* [PATCH AUTOSEL 5.4 70/78] pci/hotplug/pnv-php: Remove erroneous warning
From: Sasha Levin @ 2020-04-18 14:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Andrew Donnellan, linux-pci, Alastair D'Silva,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20200418144047.9013-1-sashal@kernel.org>
From: Frederic Barrat <fbarrat@linux.ibm.com>
[ Upstream commit 658ab186dd22060408d94f5c5a6d02d809baba44 ]
On powernv, when removing a device through hotplug, the following
warning is logged:
Invalid refcount <.> on <...>
It may be incorrect, the refcount may be set to a higher value than 1
and be valid. of_detach_node() can drop more than one reference. As it
doesn't seem trivial to assert the correct value, let's remove the
warning.
Reviewed-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-7-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/pci/hotplug/pnv_php.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index d7b2b47bc33eb..6037983c6e46b 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -151,17 +151,11 @@ static void pnv_php_rmv_pdns(struct device_node *dn)
static void pnv_php_detach_device_nodes(struct device_node *parent)
{
struct device_node *dn;
- int refcount;
for_each_child_of_node(parent, dn) {
pnv_php_detach_device_nodes(dn);
of_node_put(dn);
- refcount = kref_read(&dn->kobj.kref);
- if (refcount != 1)
- pr_warn("Invalid refcount %d on <%pOF>\n",
- refcount, dn);
-
of_detach_node(dn);
}
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 69/78] powerpc/powernv/ioda: Fix ref count for devices with their own PE
From: Sasha Levin @ 2020-04-18 14:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Frederic Barrat, linuxppc-dev, Andrew Donnellan, Sasha Levin
In-Reply-To: <20200418144047.9013-1-sashal@kernel.org>
From: Frederic Barrat <fbarrat@linux.ibm.com>
[ Upstream commit 05dd7da76986937fb288b4213b1fa10dbe0d1b33 ]
The pci_dn structure used to store a pointer to the struct pci_dev, so
taking a reference on the device was required. However, the pci_dev
pointer was later removed from the pci_dn structure, but the reference
was kept for the npu device.
See commit 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary
pcidev from pci_dn").
We don't need to take a reference on the device when assigning the PE
as the struct pnv_ioda_pe is cleaned up at the same time as
the (physical) device is released. Doing so prevents the device from
being released, which is a problem for opencapi devices, since we want
to be able to remove them through PCI hotplug.
Now the ugly part: nvlink npu devices are not meant to be
released. Because of the above, we've always leaked a reference and
simply removing it now is dangerous and would likely require more
work. There's currently no release device callback for nvlink devices
for example. So to be safe, this patch leaks a reference on the npu
device, but only for nvlink and not opencapi.
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-2-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 058223233088e..e9cda7e316a50 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1062,14 +1062,13 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
return NULL;
}
- /* NOTE: We get only one ref to the pci_dev for the pdn, not for the
- * pointer in the PE data structure, both should be destroyed at the
- * same time. However, this needs to be looked at more closely again
- * once we actually start removing things (Hotplug, SR-IOV, ...)
+ /* NOTE: We don't get a reference for the pointer in the PE
+ * data structure, both the device and PE structures should be
+ * destroyed at the same time. However, removing nvlink
+ * devices will need some work.
*
* At some point we want to remove the PDN completely anyways
*/
- pci_dev_get(dev);
pdn->pe_number = pe->pe_number;
pe->flags = PNV_IODA_PE_DEV;
pe->pdev = dev;
@@ -1084,7 +1083,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
pnv_ioda_free_pe(pe);
pdn->pe_number = IODA_INVALID_PE;
pe->pdev = NULL;
- pci_dev_put(dev);
return NULL;
}
@@ -1205,6 +1203,14 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
struct pci_controller *hose = pci_bus_to_host(npu_pdev->bus);
struct pnv_phb *phb = hose->private_data;
+ /*
+ * Intentionally leak a reference on the npu device (for
+ * nvlink only; this is not an opencapi path) to make sure it
+ * never goes away, as it's been the case all along and some
+ * work is needed otherwise.
+ */
+ pci_dev_get(npu_pdev);
+
/*
* Due to a hardware errata PE#0 on the NPU is reserved for
* error handling. This means we only have three PEs remaining
@@ -1228,7 +1234,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
*/
dev_info(&npu_pdev->dev,
"Associating to existing PE %x\n", pe_num);
- pci_dev_get(npu_pdev);
npu_pdn = pci_get_pdn(npu_pdev);
rid = npu_pdev->bus->number << 8 | npu_pdn->devfn;
npu_pdn->pe_number = pe_num;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 36/78] powerpc/pseries: Fix MCE handling on pseries
From: Sasha Levin @ 2020-04-18 14:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Mahesh Salgaonkar, Nicholas Piggin, Ganesh Goudar,
linuxppc-dev
In-Reply-To: <20200418144047.9013-1-sashal@kernel.org>
From: Ganesh Goudar <ganeshgr@linux.ibm.com>
[ Upstream commit a95a0a1654f16366360399574e10efd87e867b39 ]
MCE handling on pSeries platform fails as recent rework to use common
code for pSeries and PowerNV in machine check error handling tries to
access per-cpu variables in realmode. The per-cpu variables may be
outside the RMO region on pSeries platform and needs translation to be
enabled for access. Just moving these per-cpu variable into RMO region
did'nt help because we queue some work to workqueues in real mode, which
again tries to touch per-cpu variables. Also fwnmi_release_errinfo()
cannot be called when translation is not enabled.
This patch fixes this by enabling translation in the exception handler
when all required real mode handling is done. This change only affects
the pSeries platform.
Without this fix below kernel crash is seen on injecting
SLB multihit:
BUG: Unable to handle kernel data access on read at 0xc00000027b205950
Faulting instruction address: 0xc00000000003b7e0
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: mcetest_slb(OE+) af_packet(E) xt_tcpudp(E) ip6t_rpfilter(E) ip6t_REJECT(E) ipt_REJECT(E) xt_conntrack(E) ip_set(E) nfnetlink(E) ebtable_nat(E) ebtable_broute(E) ip6table_nat(E) ip6table_mangle(E) ip6table_raw(E) ip6table_security(E) iptable_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) iptable_mangle(E) iptable_raw(E) iptable_security(E) ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) xfs(E) ibmveth(E) vmx_crypto(E) gf128mul(E) uio_pdrv_genirq(E) uio(E) crct10dif_vpmsum(E) rtc_generic(E) btrfs(E) libcrc32c(E) xor(E) zstd_decompress(E) zstd_compress(E) raid6_pq(E) sr_mod(E) sd_mod(E) cdrom(E) ibmvscsi(E) scsi_transport_srp(E) crc32c_vpmsum(E) dm_mod(E) sg(E) scsi_mod(E)
CPU: 34 PID: 8154 Comm: insmod Kdump: loaded Tainted: G OE 5.5.0-mahesh #1
NIP: c00000000003b7e0 LR: c0000000000f2218 CTR: 0000000000000000
REGS: c000000007dcb960 TRAP: 0300 Tainted: G OE (5.5.0-mahesh)
MSR: 8000000000001003 <SF,ME,RI,LE> CR: 28002428 XER: 20040000
CFAR: c0000000000f2214 DAR: c00000027b205950 DSISR: 40000000 IRQMASK: 0
GPR00: c0000000000f2218 c000000007dcbbf0 c000000001544800 c000000007dcbd70
GPR04: 0000000000000001 c000000007dcbc98 c008000000d00258 c0080000011c0000
GPR08: 0000000000000000 0000000300000003 c000000001035950 0000000003000048
GPR12: 000000027a1d0000 c000000007f9c000 0000000000000558 0000000000000000
GPR16: 0000000000000540 c008000001110000 c008000001110540 0000000000000000
GPR20: c00000000022af10 c00000025480fd70 c008000001280000 c00000004bfbb300
GPR24: c000000001442330 c00800000800000d c008000008000000 4009287a77000510
GPR28: 0000000000000000 0000000000000002 c000000001033d30 0000000000000001
NIP [c00000000003b7e0] save_mce_event+0x30/0x240
LR [c0000000000f2218] pseries_machine_check_realmode+0x2c8/0x4f0
Call Trace:
Instruction dump:
3c4c0151 38429050 7c0802a6 60000000 fbc1fff0 fbe1fff8 f821ffd1 3d42ffaf
3fc2ffaf e98d0030 394a1150 3bdef530 <7d6a62aa> 1d2b0048 2f8b0063 380b0001
---[ end trace 46fd63f36bbdd940 ]---
Fixes: 9ca766f9891d ("powerpc/64s/pseries: machine check convert to use common event code")
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200320110119.10207-1-ganeshgr@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/pseries/ras.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 3acdcc3bb908c..753adeb624f23 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -683,6 +683,17 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
#endif
out:
+ /*
+ * Enable translation as we will be accessing per-cpu variables
+ * in save_mce_event() which may fall outside RMO region, also
+ * leave it enabled because subsequently we will be queuing work
+ * to workqueues where again per-cpu variables accessed, besides
+ * fwnmi_release_errinfo() crashes when called in realmode on
+ * pseries.
+ * Note: All the realmode handling like flushing SLB entries for
+ * SLB multihit is done by now.
+ */
+ mtmsr(mfmsr() | MSR_IR | MSR_DR);
save_mce_event(regs, disposition == RTAS_DISP_FULLY_RECOVERED,
&mce_err, regs->nip, eaddr, paddr);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 35/78] Revert "powerpc/64: irq_work avoid interrupt when called with hardware irqs enabled"
From: Sasha Levin @ 2020-04-18 14:40 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20200418144047.9013-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit abc3fce76adbdfa8f87272c784b388cd20b46049 ]
This reverts commit ebb37cf3ffd39fdb6ec5b07111f8bb2f11d92c5f.
That commit does not play well with soft-masked irq state
manipulations in idle, interrupt replay, and possibly others due to
tracing code sometimes using irq_work_queue (e.g., in
trace_hardirqs_on()). That can cause PACA_IRQ_DEC to become set when
it is not expected, and be ignored or cleared or cause warnings.
The net result seems to be missing an irq_work until the next timer
interrupt in the worst case which is usually not going to be noticed,
however it could be a long time if the tick is disabled, which is
against the spirit of irq_work and might cause real problems.
The idea is still solid, but it would need more work. It's not really
clear if it would be worth added complexity, so revert this for
now (not a straight revert, but replace with a comment explaining why
we might see interrupts happening, and gives git blame something to
find).
Fixes: ebb37cf3ffd3 ("powerpc/64: irq_work avoid interrupt when called with hardware irqs enabled")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200402120401.1115883-1-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/time.c | 44 +++++++++++---------------------------
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 11301a1187f33..0e0a2227af7d7 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -522,35 +522,6 @@ static inline void clear_irq_work_pending(void)
"i" (offsetof(struct paca_struct, irq_work_pending)));
}
-void arch_irq_work_raise(void)
-{
- preempt_disable();
- set_irq_work_pending_flag();
- /*
- * Non-nmi code running with interrupts disabled will replay
- * irq_happened before it re-enables interrupts, so setthe
- * decrementer there instead of causing a hardware exception
- * which would immediately hit the masked interrupt handler
- * and have the net effect of setting the decrementer in
- * irq_happened.
- *
- * NMI interrupts can not check this when they return, so the
- * decrementer hardware exception is raised, which will fire
- * when interrupts are next enabled.
- *
- * BookE does not support this yet, it must audit all NMI
- * interrupt handlers to ensure they call nmi_enter() so this
- * check would be correct.
- */
- if (IS_ENABLED(CONFIG_BOOKE) || !irqs_disabled() || in_nmi()) {
- set_dec(1);
- } else {
- hard_irq_disable();
- local_paca->irq_happened |= PACA_IRQ_DEC;
- }
- preempt_enable();
-}
-
#else /* 32-bit */
DEFINE_PER_CPU(u8, irq_work_pending);
@@ -559,16 +530,27 @@ DEFINE_PER_CPU(u8, irq_work_pending);
#define test_irq_work_pending() __this_cpu_read(irq_work_pending)
#define clear_irq_work_pending() __this_cpu_write(irq_work_pending, 0)
+#endif /* 32 vs 64 bit */
+
void arch_irq_work_raise(void)
{
+ /*
+ * 64-bit code that uses irq soft-mask can just cause an immediate
+ * interrupt here that gets soft masked, if this is called under
+ * local_irq_disable(). It might be possible to prevent that happening
+ * by noticing interrupts are disabled and setting decrementer pending
+ * to be replayed when irqs are enabled. The problem there is that
+ * tracing can call irq_work_raise, including in code that does low
+ * level manipulations of irq soft-mask state (e.g., trace_hardirqs_on)
+ * which could get tangled up if we're messing with the same state
+ * here.
+ */
preempt_disable();
set_irq_work_pending_flag();
set_dec(1);
preempt_enable();
}
-#endif /* 32 vs 64 bit */
-
#else /* CONFIG_IRQ_WORK */
#define test_irq_work_pending() 0
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 73/75] ocxl: Add PCI hotplug dependency to Kconfig
From: Sasha Levin @ 2020-04-18 14:09 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Andrew Donnellan, Alastair D'Silva,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20200418140910.8280-1-sashal@kernel.org>
From: Frederic Barrat <fbarrat@linux.ibm.com>
[ Upstream commit 49ce94b8677c7d7a15c4d7cbbb9ff1cd8387827b ]
The PCI hotplug framework is used to update the devices when a new
image is written to the FPGA.
Reviewed-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-12-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/misc/ocxl/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/misc/ocxl/Kconfig b/drivers/misc/ocxl/Kconfig
index 1916fa65f2f2a..2d2266c1439ef 100644
--- a/drivers/misc/ocxl/Kconfig
+++ b/drivers/misc/ocxl/Kconfig
@@ -11,6 +11,7 @@ config OCXL
tristate "OpenCAPI coherent accelerator support"
depends on PPC_POWERNV && PCI && EEH
select OCXL_BASE
+ select HOTPLUG_PCI_POWERNV
default m
help
Select this option to enable the ocxl driver for Open
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 71/75] powerpc/powernv/ioda: Fix ref count for devices with their own PE
From: Sasha Levin @ 2020-04-18 14:09 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Frederic Barrat, linuxppc-dev, Andrew Donnellan, Sasha Levin
In-Reply-To: <20200418140910.8280-1-sashal@kernel.org>
From: Frederic Barrat <fbarrat@linux.ibm.com>
[ Upstream commit 05dd7da76986937fb288b4213b1fa10dbe0d1b33 ]
The pci_dn structure used to store a pointer to the struct pci_dev, so
taking a reference on the device was required. However, the pci_dev
pointer was later removed from the pci_dn structure, but the reference
was kept for the npu device.
See commit 902bdc57451c ("powerpc/powernv/idoa: Remove unnecessary
pcidev from pci_dn").
We don't need to take a reference on the device when assigning the PE
as the struct pnv_ioda_pe is cleaned up at the same time as
the (physical) device is released. Doing so prevents the device from
being released, which is a problem for opencapi devices, since we want
to be able to remove them through PCI hotplug.
Now the ugly part: nvlink npu devices are not meant to be
released. Because of the above, we've always leaked a reference and
simply removing it now is dangerous and would likely require more
work. There's currently no release device callback for nvlink devices
for example. So to be safe, this patch leaks a reference on the npu
device, but only for nvlink and not opencapi.
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-2-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 67e4628dd5274..b4afabe20744a 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1062,14 +1062,13 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
return NULL;
}
- /* NOTE: We get only one ref to the pci_dev for the pdn, not for the
- * pointer in the PE data structure, both should be destroyed at the
- * same time. However, this needs to be looked at more closely again
- * once we actually start removing things (Hotplug, SR-IOV, ...)
+ /* NOTE: We don't get a reference for the pointer in the PE
+ * data structure, both the device and PE structures should be
+ * destroyed at the same time. However, removing nvlink
+ * devices will need some work.
*
* At some point we want to remove the PDN completely anyways
*/
- pci_dev_get(dev);
pdn->pe_number = pe->pe_number;
pe->flags = PNV_IODA_PE_DEV;
pe->pdev = dev;
@@ -1084,7 +1083,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
pnv_ioda_free_pe(pe);
pdn->pe_number = IODA_INVALID_PE;
pe->pdev = NULL;
- pci_dev_put(dev);
return NULL;
}
@@ -1205,6 +1203,14 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
struct pci_controller *hose = pci_bus_to_host(npu_pdev->bus);
struct pnv_phb *phb = hose->private_data;
+ /*
+ * Intentionally leak a reference on the npu device (for
+ * nvlink only; this is not an opencapi path) to make sure it
+ * never goes away, as it's been the case all along and some
+ * work is needed otherwise.
+ */
+ pci_dev_get(npu_pdev);
+
/*
* Due to a hardware errata PE#0 on the NPU is reserved for
* error handling. This means we only have three PEs remaining
@@ -1228,7 +1234,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_npu_PE(struct pci_dev *npu_pdev)
*/
dev_info(&npu_pdev->dev,
"Associating to existing PE %x\n", pe_num);
- pci_dev_get(npu_pdev);
npu_pdn = pci_get_pdn(npu_pdev);
rid = npu_pdev->bus->number << 8 | npu_pdn->devfn;
npu_pdn->pe_number = pe_num;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 72/75] pci/hotplug/pnv-php: Remove erroneous warning
From: Sasha Levin @ 2020-04-18 14:09 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Andrew Donnellan, linux-pci, Alastair D'Silva,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20200418140910.8280-1-sashal@kernel.org>
From: Frederic Barrat <fbarrat@linux.ibm.com>
[ Upstream commit 658ab186dd22060408d94f5c5a6d02d809baba44 ]
On powernv, when removing a device through hotplug, the following
warning is logged:
Invalid refcount <.> on <...>
It may be incorrect, the refcount may be set to a higher value than 1
and be valid. of_detach_node() can drop more than one reference. As it
doesn't seem trivial to assert the correct value, let's remove the
warning.
Reviewed-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191121134918.7155-7-fbarrat@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/pci/hotplug/pnv_php.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index d7b2b47bc33eb..6037983c6e46b 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -151,17 +151,11 @@ static void pnv_php_rmv_pdns(struct device_node *dn)
static void pnv_php_detach_device_nodes(struct device_node *parent)
{
struct device_node *dn;
- int refcount;
for_each_child_of_node(parent, dn) {
pnv_php_detach_device_nodes(dn);
of_node_put(dn);
- refcount = kref_read(&dn->kobj.kref);
- if (refcount != 1)
- pr_warn("Invalid refcount %d on <%pOF>\n",
- refcount, dn);
-
of_detach_node(dn);
}
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 38/75] powerpc/pseries: Fix MCE handling on pseries
From: Sasha Levin @ 2020-04-18 14:08 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Mahesh Salgaonkar, Nicholas Piggin, Ganesh Goudar,
linuxppc-dev
In-Reply-To: <20200418140910.8280-1-sashal@kernel.org>
From: Ganesh Goudar <ganeshgr@linux.ibm.com>
[ Upstream commit a95a0a1654f16366360399574e10efd87e867b39 ]
MCE handling on pSeries platform fails as recent rework to use common
code for pSeries and PowerNV in machine check error handling tries to
access per-cpu variables in realmode. The per-cpu variables may be
outside the RMO region on pSeries platform and needs translation to be
enabled for access. Just moving these per-cpu variable into RMO region
did'nt help because we queue some work to workqueues in real mode, which
again tries to touch per-cpu variables. Also fwnmi_release_errinfo()
cannot be called when translation is not enabled.
This patch fixes this by enabling translation in the exception handler
when all required real mode handling is done. This change only affects
the pSeries platform.
Without this fix below kernel crash is seen on injecting
SLB multihit:
BUG: Unable to handle kernel data access on read at 0xc00000027b205950
Faulting instruction address: 0xc00000000003b7e0
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: mcetest_slb(OE+) af_packet(E) xt_tcpudp(E) ip6t_rpfilter(E) ip6t_REJECT(E) ipt_REJECT(E) xt_conntrack(E) ip_set(E) nfnetlink(E) ebtable_nat(E) ebtable_broute(E) ip6table_nat(E) ip6table_mangle(E) ip6table_raw(E) ip6table_security(E) iptable_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) iptable_mangle(E) iptable_raw(E) iptable_security(E) ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) xfs(E) ibmveth(E) vmx_crypto(E) gf128mul(E) uio_pdrv_genirq(E) uio(E) crct10dif_vpmsum(E) rtc_generic(E) btrfs(E) libcrc32c(E) xor(E) zstd_decompress(E) zstd_compress(E) raid6_pq(E) sr_mod(E) sd_mod(E) cdrom(E) ibmvscsi(E) scsi_transport_srp(E) crc32c_vpmsum(E) dm_mod(E) sg(E) scsi_mod(E)
CPU: 34 PID: 8154 Comm: insmod Kdump: loaded Tainted: G OE 5.5.0-mahesh #1
NIP: c00000000003b7e0 LR: c0000000000f2218 CTR: 0000000000000000
REGS: c000000007dcb960 TRAP: 0300 Tainted: G OE (5.5.0-mahesh)
MSR: 8000000000001003 <SF,ME,RI,LE> CR: 28002428 XER: 20040000
CFAR: c0000000000f2214 DAR: c00000027b205950 DSISR: 40000000 IRQMASK: 0
GPR00: c0000000000f2218 c000000007dcbbf0 c000000001544800 c000000007dcbd70
GPR04: 0000000000000001 c000000007dcbc98 c008000000d00258 c0080000011c0000
GPR08: 0000000000000000 0000000300000003 c000000001035950 0000000003000048
GPR12: 000000027a1d0000 c000000007f9c000 0000000000000558 0000000000000000
GPR16: 0000000000000540 c008000001110000 c008000001110540 0000000000000000
GPR20: c00000000022af10 c00000025480fd70 c008000001280000 c00000004bfbb300
GPR24: c000000001442330 c00800000800000d c008000008000000 4009287a77000510
GPR28: 0000000000000000 0000000000000002 c000000001033d30 0000000000000001
NIP [c00000000003b7e0] save_mce_event+0x30/0x240
LR [c0000000000f2218] pseries_machine_check_realmode+0x2c8/0x4f0
Call Trace:
Instruction dump:
3c4c0151 38429050 7c0802a6 60000000 fbc1fff0 fbe1fff8 f821ffd1 3d42ffaf
3fc2ffaf e98d0030 394a1150 3bdef530 <7d6a62aa> 1d2b0048 2f8b0063 380b0001
---[ end trace 46fd63f36bbdd940 ]---
Fixes: 9ca766f9891d ("powerpc/64s/pseries: machine check convert to use common event code")
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200320110119.10207-1-ganeshgr@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/pseries/ras.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 1d7f973c647b3..43710b69e09eb 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -683,6 +683,17 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
#endif
out:
+ /*
+ * Enable translation as we will be accessing per-cpu variables
+ * in save_mce_event() which may fall outside RMO region, also
+ * leave it enabled because subsequently we will be queuing work
+ * to workqueues where again per-cpu variables accessed, besides
+ * fwnmi_release_errinfo() crashes when called in realmode on
+ * pseries.
+ * Note: All the realmode handling like flushing SLB entries for
+ * SLB multihit is done by now.
+ */
+ mtmsr(mfmsr() | MSR_IR | MSR_DR);
save_mce_event(regs, disposition == RTAS_DISP_FULLY_RECOVERED,
&mce_err, regs->nip, eaddr, paddr);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 37/75] Revert "powerpc/64: irq_work avoid interrupt when called with hardware irqs enabled"
From: Sasha Levin @ 2020-04-18 14:08 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20200418140910.8280-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit abc3fce76adbdfa8f87272c784b388cd20b46049 ]
This reverts commit ebb37cf3ffd39fdb6ec5b07111f8bb2f11d92c5f.
That commit does not play well with soft-masked irq state
manipulations in idle, interrupt replay, and possibly others due to
tracing code sometimes using irq_work_queue (e.g., in
trace_hardirqs_on()). That can cause PACA_IRQ_DEC to become set when
it is not expected, and be ignored or cleared or cause warnings.
The net result seems to be missing an irq_work until the next timer
interrupt in the worst case which is usually not going to be noticed,
however it could be a long time if the tick is disabled, which is
against the spirit of irq_work and might cause real problems.
The idea is still solid, but it would need more work. It's not really
clear if it would be worth added complexity, so revert this for
now (not a straight revert, but replace with a comment explaining why
we might see interrupts happening, and gives git blame something to
find).
Fixes: ebb37cf3ffd3 ("powerpc/64: irq_work avoid interrupt when called with hardware irqs enabled")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200402120401.1115883-1-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/time.c | 44 +++++++++++---------------------------
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 1168e8b37e306..716f8d0960a7b 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -522,35 +522,6 @@ static inline void clear_irq_work_pending(void)
"i" (offsetof(struct paca_struct, irq_work_pending)));
}
-void arch_irq_work_raise(void)
-{
- preempt_disable();
- set_irq_work_pending_flag();
- /*
- * Non-nmi code running with interrupts disabled will replay
- * irq_happened before it re-enables interrupts, so setthe
- * decrementer there instead of causing a hardware exception
- * which would immediately hit the masked interrupt handler
- * and have the net effect of setting the decrementer in
- * irq_happened.
- *
- * NMI interrupts can not check this when they return, so the
- * decrementer hardware exception is raised, which will fire
- * when interrupts are next enabled.
- *
- * BookE does not support this yet, it must audit all NMI
- * interrupt handlers to ensure they call nmi_enter() so this
- * check would be correct.
- */
- if (IS_ENABLED(CONFIG_BOOKE) || !irqs_disabled() || in_nmi()) {
- set_dec(1);
- } else {
- hard_irq_disable();
- local_paca->irq_happened |= PACA_IRQ_DEC;
- }
- preempt_enable();
-}
-
#else /* 32-bit */
DEFINE_PER_CPU(u8, irq_work_pending);
@@ -559,16 +530,27 @@ DEFINE_PER_CPU(u8, irq_work_pending);
#define test_irq_work_pending() __this_cpu_read(irq_work_pending)
#define clear_irq_work_pending() __this_cpu_write(irq_work_pending, 0)
+#endif /* 32 vs 64 bit */
+
void arch_irq_work_raise(void)
{
+ /*
+ * 64-bit code that uses irq soft-mask can just cause an immediate
+ * interrupt here that gets soft masked, if this is called under
+ * local_irq_disable(). It might be possible to prevent that happening
+ * by noticing interrupts are disabled and setting decrementer pending
+ * to be replayed when irqs are enabled. The problem there is that
+ * tracing can call irq_work_raise, including in code that does low
+ * level manipulations of irq soft-mask state (e.g., trace_hardirqs_on)
+ * which could get tangled up if we're messing with the same state
+ * here.
+ */
preempt_disable();
set_irq_work_pending_flag();
set_dec(1);
preempt_enable();
}
-#endif /* 32 vs 64 bit */
-
#else /* CONFIG_IRQ_WORK */
#define test_irq_work_pending() 0
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 39/73] Revert "powerpc/64: irq_work avoid interrupt when called with hardware irqs enabled"
From: Sasha Levin @ 2020-04-18 13:47 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20200418134815.6519-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit abc3fce76adbdfa8f87272c784b388cd20b46049 ]
This reverts commit ebb37cf3ffd39fdb6ec5b07111f8bb2f11d92c5f.
That commit does not play well with soft-masked irq state
manipulations in idle, interrupt replay, and possibly others due to
tracing code sometimes using irq_work_queue (e.g., in
trace_hardirqs_on()). That can cause PACA_IRQ_DEC to become set when
it is not expected, and be ignored or cleared or cause warnings.
The net result seems to be missing an irq_work until the next timer
interrupt in the worst case which is usually not going to be noticed,
however it could be a long time if the tick is disabled, which is
against the spirit of irq_work and might cause real problems.
The idea is still solid, but it would need more work. It's not really
clear if it would be worth added complexity, so revert this for
now (not a straight revert, but replace with a comment explaining why
we might see interrupts happening, and gives git blame something to
find).
Fixes: ebb37cf3ffd3 ("powerpc/64: irq_work avoid interrupt when called with hardware irqs enabled")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200402120401.1115883-1-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/time.c | 44 +++++++++++---------------------------
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 1168e8b37e306..716f8d0960a7b 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -522,35 +522,6 @@ static inline void clear_irq_work_pending(void)
"i" (offsetof(struct paca_struct, irq_work_pending)));
}
-void arch_irq_work_raise(void)
-{
- preempt_disable();
- set_irq_work_pending_flag();
- /*
- * Non-nmi code running with interrupts disabled will replay
- * irq_happened before it re-enables interrupts, so setthe
- * decrementer there instead of causing a hardware exception
- * which would immediately hit the masked interrupt handler
- * and have the net effect of setting the decrementer in
- * irq_happened.
- *
- * NMI interrupts can not check this when they return, so the
- * decrementer hardware exception is raised, which will fire
- * when interrupts are next enabled.
- *
- * BookE does not support this yet, it must audit all NMI
- * interrupt handlers to ensure they call nmi_enter() so this
- * check would be correct.
- */
- if (IS_ENABLED(CONFIG_BOOKE) || !irqs_disabled() || in_nmi()) {
- set_dec(1);
- } else {
- hard_irq_disable();
- local_paca->irq_happened |= PACA_IRQ_DEC;
- }
- preempt_enable();
-}
-
#else /* 32-bit */
DEFINE_PER_CPU(u8, irq_work_pending);
@@ -559,16 +530,27 @@ DEFINE_PER_CPU(u8, irq_work_pending);
#define test_irq_work_pending() __this_cpu_read(irq_work_pending)
#define clear_irq_work_pending() __this_cpu_write(irq_work_pending, 0)
+#endif /* 32 vs 64 bit */
+
void arch_irq_work_raise(void)
{
+ /*
+ * 64-bit code that uses irq soft-mask can just cause an immediate
+ * interrupt here that gets soft masked, if this is called under
+ * local_irq_disable(). It might be possible to prevent that happening
+ * by noticing interrupts are disabled and setting decrementer pending
+ * to be replayed when irqs are enabled. The problem there is that
+ * tracing can call irq_work_raise, including in code that does low
+ * level manipulations of irq soft-mask state (e.g., trace_hardirqs_on)
+ * which could get tangled up if we're messing with the same state
+ * here.
+ */
preempt_disable();
set_irq_work_pending_flag();
set_dec(1);
preempt_enable();
}
-#endif /* 32 vs 64 bit */
-
#else /* CONFIG_IRQ_WORK */
#define test_irq_work_pending() 0
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.6 40/73] powerpc/pseries: Fix MCE handling on pseries
From: Sasha Levin @ 2020-04-18 13:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, Mahesh Salgaonkar, Nicholas Piggin, Ganesh Goudar,
linuxppc-dev
In-Reply-To: <20200418134815.6519-1-sashal@kernel.org>
From: Ganesh Goudar <ganeshgr@linux.ibm.com>
[ Upstream commit a95a0a1654f16366360399574e10efd87e867b39 ]
MCE handling on pSeries platform fails as recent rework to use common
code for pSeries and PowerNV in machine check error handling tries to
access per-cpu variables in realmode. The per-cpu variables may be
outside the RMO region on pSeries platform and needs translation to be
enabled for access. Just moving these per-cpu variable into RMO region
did'nt help because we queue some work to workqueues in real mode, which
again tries to touch per-cpu variables. Also fwnmi_release_errinfo()
cannot be called when translation is not enabled.
This patch fixes this by enabling translation in the exception handler
when all required real mode handling is done. This change only affects
the pSeries platform.
Without this fix below kernel crash is seen on injecting
SLB multihit:
BUG: Unable to handle kernel data access on read at 0xc00000027b205950
Faulting instruction address: 0xc00000000003b7e0
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: mcetest_slb(OE+) af_packet(E) xt_tcpudp(E) ip6t_rpfilter(E) ip6t_REJECT(E) ipt_REJECT(E) xt_conntrack(E) ip_set(E) nfnetlink(E) ebtable_nat(E) ebtable_broute(E) ip6table_nat(E) ip6table_mangle(E) ip6table_raw(E) ip6table_security(E) iptable_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) iptable_mangle(E) iptable_raw(E) iptable_security(E) ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) xfs(E) ibmveth(E) vmx_crypto(E) gf128mul(E) uio_pdrv_genirq(E) uio(E) crct10dif_vpmsum(E) rtc_generic(E) btrfs(E) libcrc32c(E) xor(E) zstd_decompress(E) zstd_compress(E) raid6_pq(E) sr_mod(E) sd_mod(E) cdrom(E) ibmvscsi(E) scsi_transport_srp(E) crc32c_vpmsum(E) dm_mod(E) sg(E) scsi_mod(E)
CPU: 34 PID: 8154 Comm: insmod Kdump: loaded Tainted: G OE 5.5.0-mahesh #1
NIP: c00000000003b7e0 LR: c0000000000f2218 CTR: 0000000000000000
REGS: c000000007dcb960 TRAP: 0300 Tainted: G OE (5.5.0-mahesh)
MSR: 8000000000001003 <SF,ME,RI,LE> CR: 28002428 XER: 20040000
CFAR: c0000000000f2214 DAR: c00000027b205950 DSISR: 40000000 IRQMASK: 0
GPR00: c0000000000f2218 c000000007dcbbf0 c000000001544800 c000000007dcbd70
GPR04: 0000000000000001 c000000007dcbc98 c008000000d00258 c0080000011c0000
GPR08: 0000000000000000 0000000300000003 c000000001035950 0000000003000048
GPR12: 000000027a1d0000 c000000007f9c000 0000000000000558 0000000000000000
GPR16: 0000000000000540 c008000001110000 c008000001110540 0000000000000000
GPR20: c00000000022af10 c00000025480fd70 c008000001280000 c00000004bfbb300
GPR24: c000000001442330 c00800000800000d c008000008000000 4009287a77000510
GPR28: 0000000000000000 0000000000000002 c000000001033d30 0000000000000001
NIP [c00000000003b7e0] save_mce_event+0x30/0x240
LR [c0000000000f2218] pseries_machine_check_realmode+0x2c8/0x4f0
Call Trace:
Instruction dump:
3c4c0151 38429050 7c0802a6 60000000 fbc1fff0 fbe1fff8 f821ffd1 3d42ffaf
3fc2ffaf e98d0030 394a1150 3bdef530 <7d6a62aa> 1d2b0048 2f8b0063 380b0001
---[ end trace 46fd63f36bbdd940 ]---
Fixes: 9ca766f9891d ("powerpc/64s/pseries: machine check convert to use common event code")
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200320110119.10207-1-ganeshgr@linux.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/platforms/pseries/ras.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 1d7f973c647b3..43710b69e09eb 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -683,6 +683,17 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
#endif
out:
+ /*
+ * Enable translation as we will be accessing per-cpu variables
+ * in save_mce_event() which may fall outside RMO region, also
+ * leave it enabled because subsequently we will be queuing work
+ * to workqueues where again per-cpu variables accessed, besides
+ * fwnmi_release_errinfo() crashes when called in realmode on
+ * pseries.
+ * Note: All the realmode handling like flushing SLB entries for
+ * SLB multihit is done by now.
+ */
+ mtmsr(mfmsr() | MSR_IR | MSR_DR);
save_mce_event(regs, disposition == RTAS_DISP_FULLY_RECOVERED,
&mce_err, regs->nip, eaddr, paddr);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 3/4] dma-mapping: add a dma_ops_bypass flag to struct device
From: Joerg Roedel @ 2020-04-18 12:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Alexey Kardashevskiy, Greg Kroah-Hartman, Robin Murphy,
linux-kernel, iommu, linuxppc-dev, Lu Baolu
In-Reply-To: <20200414122506.438134-4-hch@lst.de>
Hi Christoph,
On Tue, Apr 14, 2020 at 02:25:05PM +0200, Christoph Hellwig wrote:
> +static inline bool dma_map_direct(struct device *dev,
> + const struct dma_map_ops *ops)
> +{
> + if (likely(!ops))
> + return true;
> + if (!dev->dma_ops_bypass)
> + return false;
> +
> + return min_not_zero(*dev->dma_mask, dev->bus_dma_limit) >=
> + dma_direct_get_required_mask(dev);
Why is the dma-mask check done here? The dma-direct code handles memory
outside of the devices dma-mask with swiotlb, no?
I also don't quite get what the difference between setting the
dma_ops_bypass flag non-zero and setting ops to NULL is.
Joerg
^ permalink raw reply
* Re: [PATCH] iommu: spapr_tce: Disable compile testing to fix build on book3s_32 config
From: Joerg Roedel @ 2020-04-18 12:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-kernel, virtualization, iommu, Geert Uytterhoeven, netdev,
linuxppc-dev
In-Reply-To: <20200414142630.21153-1-krzk@kernel.org>
On Tue, Apr 14, 2020 at 04:26:30PM +0200, Krzysztof Kozlowski wrote:
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Fixes: e93a1695d7fb ("iommu: Enable compile testing for some of drivers")
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
> drivers/iommu/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/2] signal: Factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Eric W. Biederman @ 2020-04-18 11:55 UTC (permalink / raw)
To: Christophe Leroy
Cc: Arnd Bergmann, x86, linux-kernel, Alexander Viro, linux-fsdevel,
Andrew Morton, linuxppc-dev, Christoph Hellwig, Jeremy Kerr
In-Reply-To: <c51c6192-2ea4-62d8-dd22-305f7a1e0dd3@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 17/04/2020 à 23:09, Eric W. Biederman a écrit :
>>
>> To remove the use of set_fs in the coredump code there needs to be a
>> way to convert a kernel siginfo to a userspace compat siginfo.
>>
>> Call that function copy_siginfo_to_compat and factor it out of
>> copy_siginfo_to_user32.
>
> I find it a pitty to do that.
>
> The existing function could have been easily converted to using
> user_access_begin() + user_access_end() and use unsafe_put_user() to copy to
> userspace to avoid copying through a temporary structure on the stack.
>
> With your change, it becomes impossible to do that.
I don't follow. You don't like temporary structures in the coredump
code or temporary structures in copy_siginfo_to_user32?
A temporary structure in copy_siginfo_to_user is pretty much required
so that it can be zeroed to guarantee we don't pass a structure with
holes to userspace.
The implementation of copy_siginfo_to_user32 used to use the equivalent
of user_access_begin() and user_access_end() and the code was a mess
that was very difficult to reason about. I recall their being holes
in the structure that were being copied to userspace.
Meanwhile if you are going to set all of the bytes a cache hot temporary
structure is quite cheap.
> Is that really an issue to use that set_fs() in the coredump code ?
Using set_fs() is pretty bad and something that we would like to remove
from the kernel entirely. The fewer instances of set_fs() we have the
better.
I forget all of the details but set_fs() is both a type violation and an
attack point when people are attacking the kernel. The existence of
set_fs() requires somethings that should be constants to be variables.
Something about that means that our current code is difficult to protect
from spectre style vulnerabilities.
There was a very good thread about it all in I think 2018 but
unfortunately I can't find it now.
Eric
^ permalink raw reply
* Re: [RFC PATCH] powerpc/64/signal: balance return predictor stack in signal trampoline
From: Nicholas Piggin @ 2020-04-18 10:44 UTC (permalink / raw)
To: Alan Modra; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <20200417234026.GB29913@bubble.grove.modra.org>
Excerpts from Alan Modra's message of April 18, 2020 9:40 am:
> On Fri, Apr 17, 2020 at 07:17:47PM +1000, Nicholas Piggin wrote:
>> I don't know much about dwarf, gdb still seems to recognize the signal
>> frame and unwind properly if I break inside a signal handler.
>
> Yes, the dwarf unwind info still looks good. The commented out dwarf
> near the end of sigtramp.S should probably go. At least if you really
> can't take an async signal in the trampoline (a kernel question, not
> anything to do with gcc support of async signals as the comment
> wrongly says). If you *can* take an async signal at some point past
> the trampoline addi, then delete the comment and uncomment the code.
I don't think the kernel has anything that holds off signals from being
raised in the tramp area, so it looks like we could get a signal there.
> Note that the advance_loc there bitrotted ever since the nop was added
> before the trampoline, so you'd need to change that to an advance_loc
> that moves from .Lsigrt_start to immediately after the addi, ie. 0x42.
Okay, would you do the honors of fixing it for upstream kernel? I'd just
be repeating what you wrote without understand it if I write a patch.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH 1/3] powerpc/mm: Introduce temporary mm
From: Christophe Leroy @ 2020-04-18 10:36 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev
In-Reply-To: <20200323045205.20314-2-cmr@informatik.wtf>
Le 23/03/2020 à 05:52, Christopher M. Riedl a écrit :
> x86 supports the notion of a temporary mm which restricts access to
> temporary PTEs to a single CPU. A temporary mm is useful for situations
> where a CPU needs to perform sensitive operations (such as patching a
> STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing
> said mappings to other CPUs. A side benefit is that other CPU TLBs do
> not need to be flushed when the temporary mm is torn down.
>
> Mappings in the temporary mm can be set in the userspace portion of the
> address-space.
>
> Interrupts must be disabled while the temporary mm is in use. HW
> breakpoints, which may have been set by userspace as watchpoints on
> addresses now within the temporary mm, are saved and disabled when
> loading the temporary mm. The HW breakpoints are restored when unloading
> the temporary mm. All HW breakpoints are indiscriminately disabled while
> the temporary mm is in use.
>
> Based on x86 implementation:
>
> commit cefa929c034e
> ("x86/mm: Introduce temporary mm structs")
>
> Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
> ---
> arch/powerpc/include/asm/debug.h | 1 +
> arch/powerpc/include/asm/mmu_context.h | 56 +++++++++++++++++++++++++-
> arch/powerpc/kernel/process.c | 5 +++
> 3 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
> index 7756026b95ca..b945bc16c932 100644
> --- a/arch/powerpc/include/asm/debug.h
> +++ b/arch/powerpc/include/asm/debug.h
> @@ -45,6 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs) { return 0; }
> static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
> #endif
>
> +void __get_breakpoint(struct arch_hw_breakpoint *brk);
> void __set_breakpoint(struct arch_hw_breakpoint *brk);
> bool ppc_breakpoint_available(void);
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 360367c579de..3e6381d04c28 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -7,9 +7,10 @@
> #include <linux/mm.h>
> #include <linux/sched.h>
> #include <linux/spinlock.h>
> -#include <asm/mmu.h>
> +#include <asm/mmu.h>
> #include <asm/cputable.h>
> #include <asm/cputhreads.h>
> +#include <asm/hw_breakpoint.h>
>
> /*
> * Most if the context management is out of line
> @@ -270,5 +271,58 @@ static inline int arch_dup_mmap(struct mm_struct *oldmm,
> return 0;
> }
>
> +struct temp_mm {
> + struct mm_struct *temp;
> + struct mm_struct *prev;
> + bool is_kernel_thread;
> + struct arch_hw_breakpoint brk;
> +};
> +
> +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm)
> +{
> + temp_mm->temp = mm;
> + temp_mm->prev = NULL;
> + temp_mm->is_kernel_thread = false;
> + memset(&temp_mm->brk, 0, sizeof(temp_mm->brk));
> +}
> +
> +static inline void use_temporary_mm(struct temp_mm *temp_mm)
> +{
> + lockdep_assert_irqs_disabled();
> +
> + temp_mm->is_kernel_thread = current->mm == NULL;
> + if (temp_mm->is_kernel_thread)
> + temp_mm->prev = current->active_mm;
> + else
> + temp_mm->prev = current->mm;
Could be:
temp_mm->prev = current->mm ? : current->active_mm;
> +
> + /*
> + * Hash requires a non-NULL current->mm to allocate a userspace address
> + * when handling a page fault. Does not appear to hurt in Radix either.
> + */
> + current->mm = temp_mm->temp;
> + switch_mm_irqs_off(NULL, temp_mm->temp, current);
> +
> + if (ppc_breakpoint_available()) {
> + __get_breakpoint(&temp_mm->brk);
> + if (temp_mm->brk.type != 0)
> + hw_breakpoint_disable();
> + }
There is a series for having more than one breakpoint, see
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=170073&state=*
> +}
> +
> +static inline void unuse_temporary_mm(struct temp_mm *temp_mm)
> +{
> + lockdep_assert_irqs_disabled();
> +
> + if (temp_mm->is_kernel_thread)
> + current->mm = NULL;
> + else
> + current->mm = temp_mm->prev;
Could be:
current->mm = !temp_mm->is_kernel_thread ? temp_mm->prev : NULL;
> + switch_mm_irqs_off(NULL, temp_mm->prev, current);
> +
> + if (ppc_breakpoint_available() && temp_mm->brk.type != 0)
> + __set_breakpoint(&temp_mm->brk);
> +}
> +
> #endif /* __KERNEL__ */
> #endif /* __ASM_POWERPC_MMU_CONTEXT_H */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index fad50db9dcf2..5e5cf33fc358 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -793,6 +793,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk)
> return 0;
> }
>
> +void __get_breakpoint(struct arch_hw_breakpoint *brk)
> +{
> + memcpy(brk, this_cpu_ptr(¤t_brk), sizeof(*brk));
> +}
> +
> void __set_breakpoint(struct arch_hw_breakpoint *brk)
> {
> memcpy(this_cpu_ptr(¤t_brk), brk, sizeof(*brk));
>
Christophe
^ permalink raw reply
* Re: [RFC PATCH] powerpc/lib: Fixing use a temporary mm for code patching
From: Christophe Leroy @ 2020-04-18 10:27 UTC (permalink / raw)
To: Christopher M Riedl; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1418874364.198277.1586967776509@privateemail.com>
Le 15/04/2020 à 18:22, Christopher M Riedl a écrit :
>> On April 15, 2020 4:12 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>>
>> Le 15/04/2020 à 07:16, Christopher M Riedl a écrit :
>>>> On March 26, 2020 9:42 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>>
>>>>
>>>> This patch fixes the RFC series identified below.
>>>> It fixes three points:
>>>> - Failure with CONFIG_PPC_KUAP
>>>> - Failure to write do to lack of DIRTY bit set on the 8xx
>>>> - Inadequaly complex WARN post verification
>>>>
>>>> However, it has an impact on the CPU load. Here is the time
>>>> needed on an 8xx to run the ftrace selftests without and
>>>> with this series:
>>>> - Without CONFIG_STRICT_KERNEL_RWX ==> 38 seconds
>>>> - With CONFIG_STRICT_KERNEL_RWX ==> 40 seconds
>>>> - With CONFIG_STRICT_KERNEL_RWX + this series ==> 43 seconds
>>>>
>>>> Link: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=166003
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> ---
>>>> arch/powerpc/lib/code-patching.c | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>>>> index f156132e8975..4ccff427592e 100644
>>>> --- a/arch/powerpc/lib/code-patching.c
>>>> +++ b/arch/powerpc/lib/code-patching.c
>>>> @@ -97,6 +97,7 @@ static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
>>>> }
>>>>
>>>> pte = mk_pte(page, pgprot);
>>>> + pte = pte_mkdirty(pte);
>>>> set_pte_at(patching_mm, patching_addr, ptep, pte);
>>>>
>>>> init_temp_mm(&patch_mapping->temp_mm, patching_mm);
>>>> @@ -168,7 +169,9 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
>>>> (offset_in_page((unsigned long)addr) /
>>>> sizeof(unsigned int));
>>>>
>>>> + allow_write_to_user(patch_addr, sizeof(instr));
>>>> __patch_instruction(addr, instr, patch_addr);
>>>> + prevent_write_to_user(patch_addr, sizeof(instr));
>>>>
>>>
>>> On radix we can map the page with PAGE_KERNEL protection which ends up
>>> setting EAA[0] in the radix PTE. This means the KUAP (AMR) protection is
>>> ignored (ISA v3.0b Fig. 35) since we are accessing the page from MSR[PR]=0.
>>>
>>> Can we employ a similar approach on the 8xx? I would prefer *not* to wrap
>>> the __patch_instruction() with the allow_/prevent_write_to_user() KUAP things
>>> because this is a temporary kernel mapping which really isn't userspace in
>>> the usual sense.
>>
>> On the 8xx, that's pretty different.
>>
>> The PTE doesn't control whether a page is user page or a kernel page.
>> The only thing that is set in the PTE is whether a page is linked to a
>> given PID or not.
>> PAGE_KERNEL tells that the page can be addressed with any PID.
>>
>> The user access right is given by a kind of zone, which is in the PGD
>> entry. Every pages above PAGE_OFFSET are defined as belonging to zone 0.
>> Every pages below PAGE_OFFSET are defined as belonging to zone 1.
>>
>> By default, zone 0 can only be accessed by kernel, and zone 1 can only
>> be accessed by user. When kernel wants to access zone 1, it temporarily
>> changes properties of zone 1 to allow both kernel and user accesses.
>>
>> So, if your mapping is below PAGE_OFFSET, it is in zone 1 and kernel
>> must unlock it to access it.
>>
>>
>> And this is more or less the same on hash/32. This is managed by segment
>> registers. One segment register corresponds to a 256Mbytes area. Every
>> pages below PAGE_OFFSET can only be read by default by kernel. Only user
>> can write if the PTE allows it. When the kernel needs to write at an
>> address below PAGE_OFFSET, it must change the segment properties in the
>> corresponding segment register.
>>
>> So, for both cases, if we want to have it local to a task while still
>> allowing kernel access, it means we have to define a new special area
>> between TASK_SIZE and PAGE_OFFSET which belongs to kernel zone.
>>
>> That looks complex to me for a small benefit, especially as 8xx is not
>> SMP and neither are most of the hash/32 targets.
>>
>
> Agreed. So I guess the solution is to differentiate between radix/non-radix
> and use PAGE_SHARED for non-radix along with the KUAP functions when KUAP
> is enabled. Hmm, I need to think about this some more, especially if it's
> acceptable to temporarily map kernel text as PAGE_SHARED for patching. Do
> you see any obvious problems on 8xx and hash/32 w/ using PAGE_SHARED?
No it shouldn't be a problem AFAICS, except maybe the CPU overhead it
brings as I mentioned previously (ftrace selftests going from 40 to 43
seconds ie 8% overhead.
>
> I don't necessarily want to drop the local mm patching idea for non-radix
> platforms since that means we would have to maintain two implementations.
>
What's the problem with RADIX, why can't PAGE_SHARED be used on radix ?
Christophe
^ permalink raw reply
* How to use "y" constraint in GCC inline powerpc assembly ?
From: Christophe Leroy @ 2020-04-18 8:28 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev@lists.ozlabs.org
Hi Segher,
I'd like to use cr instead of gpr to return error condition from
__get_user().
I saw in GCC doc
(https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html) that it is
possible to use "y" as constraint to refer to "Any condition register
field, cr0…cr7".
I tried the test below, but it fails with "error: impossible register
constraint in 'asm'"
How does "y" has to be used ?
int test(char *p)
{
struct {
int r:1;
} res = {0};
asm("crnot %0 * 4 + eq, %0 * 4 + eq": "=&y"(res.r));
if (res.r)
return -14;
return 0;
}
Christophe
^ permalink raw reply
* Re: [PATCH 8/8] exec: open code copy_string_kernel
From: Christophe Leroy @ 2020-04-18 8:15 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Eric W . Biederman, linux-fsdevel,
linuxppc-dev, Jeremy Kerr
In-Reply-To: <20200414070142.288696-9-hch@lst.de>
Le 14/04/2020 à 09:01, Christoph Hellwig a écrit :
> Currently copy_string_kernel is just a wrapper around copy_strings that
> simplifies the calling conventions and uses set_fs to allow passing a
> kernel pointer. But due to the fact the we only need to handle a single
> kernel argument pointer, the logic can be sigificantly simplified while
> getting rid of the set_fs.
Instead of duplicating almost identical code, can you write a function
that takes whether the source is from user or from kernel, then you just
do things like:
if (from_user)
len = strnlen_user(str, MAX_ARG_STRLEN);
else
len = strnlen(str, MAX_ARG_STRLEN);
if (from_user)
copy_from_user(kaddr+offset, str, bytes_to_copy);
else
memcpy(kaddr+offset, str, bytes_to_copy);
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/exec.c | 43 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index b2a77d5acede..ea90af1fb236 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -592,17 +592,42 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
> */
> int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
> {
> - int r;
> - mm_segment_t oldfs = get_fs();
> - struct user_arg_ptr argv = {
> - .ptr.native = (const char __user *const __user *)&arg,
> - };
> + int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating NUL */;
> + unsigned long pos = bprm->p;
> +
> + if (len == 0)
> + return -EFAULT;
> + if (!valid_arg_len(bprm, len))
> + return -E2BIG;
> +
> + /* We're going to work our way backwards. */
> + arg += len;
> + bprm->p -= len;
> + if (IS_ENABLED(CONFIG_MMU) && bprm->p < bprm->argmin)
> + return -E2BIG;
> +
> + while (len > 0) {
> + unsigned int bytes_to_copy = min_t(unsigned int, len,
> + min_not_zero(offset_in_page(pos), PAGE_SIZE));
> + struct page *page;
> + char *kaddr;
>
> - set_fs(KERNEL_DS);
> - r = copy_strings(1, argv, bprm);
> - set_fs(oldfs);
> + pos -= bytes_to_copy;
> + arg -= bytes_to_copy;
> + len -= bytes_to_copy;
>
> - return r;
> + page = get_arg_page(bprm, pos, 1);
> + if (!page)
> + return -E2BIG;
> + kaddr = kmap_atomic(page);
> + flush_arg_page(bprm, pos & PAGE_MASK, page);
> + memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
> + flush_kernel_dcache_page(page);
> + kunmap_atomic(kaddr);
> + put_arg_page(page);
> + }
> +
> + return 0;
> }
> EXPORT_SYMBOL(copy_string_kernel);
>
>
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] signal: Factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Christophe Leroy @ 2020-04-18 8:05 UTC (permalink / raw)
To: Eric W. Biederman, Christoph Hellwig
Cc: Arnd Bergmann, x86, linux-kernel, Alexander Viro, linux-fsdevel,
Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <87k12dakfx.fsf_-_@x220.int.ebiederm.org>
Le 17/04/2020 à 23:09, Eric W. Biederman a écrit :
>
> To remove the use of set_fs in the coredump code there needs to be a
> way to convert a kernel siginfo to a userspace compat siginfo.
>
> Call that function copy_siginfo_to_compat and factor it out of
> copy_siginfo_to_user32.
I find it a pitty to do that.
The existing function could have been easily converted to using
user_access_begin() + user_access_end() and use unsafe_put_user() to
copy to userspace to avoid copying through a temporary structure on the
stack.
With your change, it becomes impossible to do that.
Is that really an issue to use that set_fs() in the coredump code ?
Christophe
>
> The existence of x32 complicates this code. On x32 SIGCHLD uses 64bit
> times for utime and stime. As only SIGCHLD is affected and SIGCHLD
> never causes a coredump I have avoided handling that case.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> include/linux/compat.h | 1 +
> kernel/signal.c | 108 +++++++++++++++++++++++------------------
> 2 files changed, 63 insertions(+), 46 deletions(-)
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 0480ba4db592..4962b254e550 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -402,6 +402,7 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
> unsigned long bitmap_size);
> long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
> unsigned long bitmap_size);
> +void copy_siginfo_to_external32(struct compat_siginfo *to, const struct kernel_siginfo *from);
> int copy_siginfo_from_user32(kernel_siginfo_t *to, const struct compat_siginfo __user *from);
> int copy_siginfo_to_user32(struct compat_siginfo __user *to, const kernel_siginfo_t *from);
> int get_compat_sigevent(struct sigevent *event,
> diff --git a/kernel/signal.c b/kernel/signal.c
> index e58a6c619824..578f196898cb 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3235,90 +3235,106 @@ int copy_siginfo_from_user(kernel_siginfo_t *to, const siginfo_t __user *from)
> }
>
> #ifdef CONFIG_COMPAT
> -int copy_siginfo_to_user32(struct compat_siginfo __user *to,
> - const struct kernel_siginfo *from)
> -#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
> +void copy_siginfo_to_external32(struct compat_siginfo *to,
> + const struct kernel_siginfo *from)
> {
> - return __copy_siginfo_to_user32(to, from, in_x32_syscall());
> -}
> -int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
> - const struct kernel_siginfo *from, bool x32_ABI)
> -#endif
> -{
> - struct compat_siginfo new;
> - memset(&new, 0, sizeof(new));
> + /*
> + * This function does not work properly for SIGCHLD on x32,
> + * but it does not need to as SIGCHLD never causes a coredump.
> + */
> + memset(to, 0, sizeof(*to));
>
> - new.si_signo = from->si_signo;
> - new.si_errno = from->si_errno;
> - new.si_code = from->si_code;
> + to->si_signo = from->si_signo;
> + to->si_errno = from->si_errno;
> + to->si_code = from->si_code;
> switch(siginfo_layout(from->si_signo, from->si_code)) {
> case SIL_KILL:
> - new.si_pid = from->si_pid;
> - new.si_uid = from->si_uid;
> + to->si_pid = from->si_pid;
> + to->si_uid = from->si_uid;
> break;
> case SIL_TIMER:
> - new.si_tid = from->si_tid;
> - new.si_overrun = from->si_overrun;
> - new.si_int = from->si_int;
> + to->si_tid = from->si_tid;
> + to->si_overrun = from->si_overrun;
> + to->si_int = from->si_int;
> break;
> case SIL_POLL:
> - new.si_band = from->si_band;
> - new.si_fd = from->si_fd;
> + to->si_band = from->si_band;
> + to->si_fd = from->si_fd;
> break;
> case SIL_FAULT:
> - new.si_addr = ptr_to_compat(from->si_addr);
> + to->si_addr = ptr_to_compat(from->si_addr);
> #ifdef __ARCH_SI_TRAPNO
> - new.si_trapno = from->si_trapno;
> + to->si_trapno = from->si_trapno;
> #endif
> break;
> case SIL_FAULT_MCEERR:
> - new.si_addr = ptr_to_compat(from->si_addr);
> + to->si_addr = ptr_to_compat(from->si_addr);
> #ifdef __ARCH_SI_TRAPNO
> - new.si_trapno = from->si_trapno;
> + to->si_trapno = from->si_trapno;
> #endif
> - new.si_addr_lsb = from->si_addr_lsb;
> + to->si_addr_lsb = from->si_addr_lsb;
> break;
> case SIL_FAULT_BNDERR:
> - new.si_addr = ptr_to_compat(from->si_addr);
> + to->si_addr = ptr_to_compat(from->si_addr);
> #ifdef __ARCH_SI_TRAPNO
> - new.si_trapno = from->si_trapno;
> + to->si_trapno = from->si_trapno;
> #endif
> - new.si_lower = ptr_to_compat(from->si_lower);
> - new.si_upper = ptr_to_compat(from->si_upper);
> + to->si_lower = ptr_to_compat(from->si_lower);
> + to->si_upper = ptr_to_compat(from->si_upper);
> break;
> case SIL_FAULT_PKUERR:
> - new.si_addr = ptr_to_compat(from->si_addr);
> + to->si_addr = ptr_to_compat(from->si_addr);
> #ifdef __ARCH_SI_TRAPNO
> - new.si_trapno = from->si_trapno;
> + to->si_trapno = from->si_trapno;
> #endif
> - new.si_pkey = from->si_pkey;
> + to->si_pkey = from->si_pkey;
> break;
> case SIL_CHLD:
> - new.si_pid = from->si_pid;
> - new.si_uid = from->si_uid;
> - new.si_status = from->si_status;
> + to->si_pid = from->si_pid;
> + to->si_uid = from->si_uid;
> + to->si_status = from->si_status;
> + to->si_utime = from->si_utime;
> + to->si_stime = from->si_stime;
> #ifdef CONFIG_X86_X32_ABI
> if (x32_ABI) {
> - new._sifields._sigchld_x32._utime = from->si_utime;
> - new._sifields._sigchld_x32._stime = from->si_stime;
> + to->_sifields._sigchld_x32._utime = from->si_utime;
> + to->_sifields._sigchld_x32._stime = from->si_stime;
> } else
> #endif
> {
> - new.si_utime = from->si_utime;
> - new.si_stime = from->si_stime;
> }
> break;
> case SIL_RT:
> - new.si_pid = from->si_pid;
> - new.si_uid = from->si_uid;
> - new.si_int = from->si_int;
> + to->si_pid = from->si_pid;
> + to->si_uid = from->si_uid;
> + to->si_int = from->si_int;
> break;
> case SIL_SYS:
> - new.si_call_addr = ptr_to_compat(from->si_call_addr);
> - new.si_syscall = from->si_syscall;
> - new.si_arch = from->si_arch;
> + to->si_call_addr = ptr_to_compat(from->si_call_addr);
> + to->si_syscall = from->si_syscall;
> + to->si_arch = from->si_arch;
> break;
> }
> +}
> +
> +int copy_siginfo_to_user32(struct compat_siginfo __user *to,
> + const struct kernel_siginfo *from)
> +#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
> +{
> + return __copy_siginfo_to_user32(to, from, in_x32_syscall());
> +}
> +int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
> + const struct kernel_siginfo *from, bool x32_ABI)
> +#endif
> +{
> + struct compat_siginfo new;
> + copy_siginfo_to_external32(&new, from);
> +#ifdef CONFIG_X86_X32_ABI
> + if (x32_ABI && from->si_signo == SIGCHLD) {
> + new._sifields._sigchld_x32._utime = from->si_utime;
> + new._sifields._sigchld_x32._stime = from->si_stime;
> + }
> +#endif
>
> if (copy_to_user(to, &new, sizeof(struct compat_siginfo)))
> return -EFAULT;
>
^ permalink raw reply
* [PATCH] soc: fsl: guts: remove unneeded semicolon in fsl_soc_die_match()
From: Jason Yan @ 2020-04-18 8:18 UTC (permalink / raw)
To: leoyang.li, tglx, linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Hulk Robot, Jason Yan
Fix the following coccicheck warning:
drivers/soc/fsl/guts.c:120:2-3: Unneeded semicolon
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
drivers/soc/fsl/guts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 34810f9bb2ee..d5e9a5f2c087 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -117,7 +117,7 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
if (matches->svr == (svr & matches->mask))
return matches;
matches++;
- };
+ }
return NULL;
}
--
2.21.1
^ permalink raw reply related
* Re: [PATCH] powerpc/pseries: Make vio and ibmebus initcalls pseries specific
From: Tyrel Datwyler @ 2020-04-18 1:04 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20200417040749.25800-1-oohall@gmail.com>
On 4/16/20 9:07 PM, Oliver O'Halloran wrote:
> The vio and ibmebus buses are used for pseries specific paravirtualised
> devices and currently they're initialised by the generic initcall types.
> This is mostly fine, but it can result in some nuisance errors in dmesg
> when booting on PowerNV on some OSes, e.g.
>
> [ 2.984439] synth uevent: /devices/vio: failed to send uevent
> [ 2.984442] vio vio: uevent: failed to send synthetic uevent
> [ 17.968551] synth uevent: /devices/vio: failed to send uevent
> [ 17.968554] vio vio: uevent: failed to send synthetic uevent
>
> We don't see anything similar for the ibmebus because that depends on
> !CONFIG_LITTLE_ENDIAN.
>
> This patch squashes those by switching to using machine_*_initcall() so the bus
> type is only registered when the kernel is running on a pseries machine.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
^ permalink raw reply
* Re: [RFC PATCH] powerpc/64/signal: balance return predictor stack in signal trampoline
From: Alan Modra @ 2020-04-17 23:40 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, Anton Blanchard
In-Reply-To: <20200417091747.316707-1-npiggin@gmail.com>
On Fri, Apr 17, 2020 at 07:17:47PM +1000, Nicholas Piggin wrote:
> I don't know much about dwarf, gdb still seems to recognize the signal
> frame and unwind properly if I break inside a signal handler.
Yes, the dwarf unwind info still looks good. The commented out dwarf
near the end of sigtramp.S should probably go. At least if you really
can't take an async signal in the trampoline (a kernel question, not
anything to do with gcc support of async signals as the comment
wrongly says). If you *can* take an async signal at some point past
the trampoline addi, then delete the comment and uncomment the code.
Note that the advance_loc there bitrotted ever since the nop was added
before the trampoline, so you'd need to change that to an advance_loc
that moves from .Lsigrt_start to immediately after the addi, ie. 0x42.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply
* Re: [PATCH] drivers: uio: new driver uio_fsl_85xx_cache_sram
From: Scott Wood @ 2020-04-17 23:02 UTC (permalink / raw)
To: Wang Wenhu, gregkh, linux-kernel, linuxppc-dev; +Cc: kernel, robh
In-Reply-To: <20200417172130.14287-1-wenhu.wang@vivo.com>
On Fri, 2020-04-17 at 10:21 -0700, Wang Wenhu wrote:
> Implements a new uio driver for freescale 85xx platforms to access
> the Cache-Sram form user level. It is extremely helpful for the user
> space applications that require high performance memory accesses.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Wang Wenhu <wenhu.wang@vivo.com>
> ---
> drivers/uio/Kconfig | 8 +
> drivers/uio/Makefile | 1 +
> drivers/uio/uio_fsl_85xx_cache_sram.c | 407 ++++++++++++++++++++++++++
> 3 files changed, 416 insertions(+)
> create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
NACK, we don't need two copies of this code in the kernel. Please just wait a
bit and I'll send a patch to have the existing driver expose a dynamic
allocation interface to userspace.
-Scott
^ permalink raw reply
* Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
From: Scott Wood @ 2020-04-17 22:50 UTC (permalink / raw)
To: 王文虎
Cc: Rob Herring, Greg KH, linux-kernel, kernel, linuxppc-dev
In-Reply-To: <APIAAAABCKquIIhDNOkcHqp9.3.1587132987353.Hmail.wenhu.wang@vivo.com>
On Fri, 2020-04-17 at 22:16 +0800, 王文虎 wrote:
> > On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:>> On Thu, Apr 16, 2020
> > at 11:58:29PM -0500, Scott Wood wrote:
> > > > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > > > Sounds it is. And does the modification below fit well?
> > > > > ---
> > > > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > > - { .compatible = "uio,mpc85xx-cache-sram", },
> > > > > - {},
> > > > > +#ifdef CONFIG_OF
> > > > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > > > + { /* This is filled with module_parm */ },
> > > > > + { /* Sentinel */ },
> > > > > };
> > > > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > > > +module_param_string(of_id,
> > > > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > > > + sizeof(uio_fsl_85xx_cache_sram_of_match[
> > > > > 0].c
> > > > > ompa
> > > > > tible), 0);
> > > > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > > > sram-
> > > > > uio");
> > > > > +#endif
> > > >
> > > > No. The point is that you wouldn't be configuring this with the
> > > > device
> > > > tree
> > > > at all.
> > >
> > > Wait, why not? Don't force people to use module parameters, that is
> > > crazy. DT describes the hardware involved, if someone wants to bind to
> > > a specific range of memory, as described by DT, why can't they do so?
> >
> > Yes, DT describes the hardware, and as I've said a couple times already,
> > this
> > isn't hardware description.
> >
> > I'm not forcing people to use module parameters. That was a least-effort
> > suggestion to avoid abusing the DT. I later said I'd try to come up with
> > a
> > patch that allocates regions dynamically (and most likely doesn't use UIO
> > at
> > all).
> >
> > > I can understand not liking the name "uio" in a dt tree, but there's no
> > > reason that DT can not describe what a driver binds to here.
> >
> > The DT already describes this hardware, and there is already code that
> > binds
> > to it. This patch is trying to add a second node for it with
> > configuration.
> >
>
> Hi, Scott, Greg,
> Seems like no balance here. How about I implement a driver of uio including
> the l2ctrl and cache_sram related implementations?
> And this way, the driver would be a hardware level driver and targeted for
> uio.
No, duplicating the code makes no sense whatsoever. Please just wait a bit
and I'll send a patch to have the existing driver expose a dynamic allocation
interface to userspace.
-Scott
^ 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