* [PATCH 05/13] powerpc/xive: Fix allocation of pages donated to the XIVE controller
From: Cédric Le Goater @ 2020-12-08 15:11 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Cédric Le Goater
In-Reply-To: <20201208151124.1329942-1-clg@kaod.org>
The XIVE interrupt controller uses a set of internal tables to handle
interrupt routing. The small tables (SBE, EAT) are direct tables and
allocated by OPAL. The bigger ones (NVT, ENDT) are indirect, i.e., the
first table entries point to a page which contains the XIVE structures
used by HW. For these, OPAL only allocates the first page level and
requests pages to be allocated by Linux when a new entry is inserted.
Make sure that these pages can not be reclaimed. The problem can be
seen while stressing a system with 4K KVM guests, 16 vCPUs each.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/sysdev/xive/xive-internal.h | 2 ++
arch/powerpc/sysdev/xive/common.c | 2 +-
arch/powerpc/sysdev/xive/native.c | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
index d701af7fb48c..1eacc90f4dcf 100644
--- a/arch/powerpc/sysdev/xive/xive-internal.h
+++ b/arch/powerpc/sysdev/xive/xive-internal.h
@@ -73,6 +73,8 @@ static inline u32 xive_alloc_order(u32 queue_shift)
return (queue_shift > PAGE_SHIFT) ? (queue_shift - PAGE_SHIFT) : 0;
}
+#define XIVE_GFP (__GFP_NORETRY | __GFP_NOWARN | __GFP_NOMEMALLOC)
+
extern bool xive_cmdline_disabled;
#endif /* __XIVE_INTERNAL_H */
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 605238ca65e4..80fd97d764ab 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1544,7 +1544,7 @@ __be32 *xive_queue_page_alloc(unsigned int cpu, u32 queue_shift)
__be32 *qpage;
alloc_order = xive_alloc_order(queue_shift);
- pages = alloc_pages_node(cpu_to_node(cpu), GFP_KERNEL, alloc_order);
+ pages = alloc_pages_node(cpu_to_node(cpu), XIVE_GFP, alloc_order);
if (!pages)
return ERR_PTR(-ENOMEM);
qpage = (__be32 *)page_address(pages);
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index cb58ec7ce77a..6afb44d0d816 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -643,7 +643,7 @@ static bool xive_native_provision_pages(void)
* XXX TODO: Try to make the allocation local to the node where
* the chip resides.
*/
- p = kmem_cache_alloc(xive_provision_cache, GFP_KERNEL);
+ p = kmem_cache_alloc(xive_provision_cache, XIVE_GFP);
if (!p) {
pr_err("Failed to allocate provisioning page\n");
return false;
--
2.26.2
^ permalink raw reply related
* Re: [PATCH 15/20] ethernet: ucc_geth: use UCC_GETH_{RX, TX}_BD_RING_ALIGNMENT macros directly
From: Christophe Leroy @ 2020-12-08 15:14 UTC (permalink / raw)
To: Rasmus Villemoes, Li Yang, David S. Miller, Jakub Kicinski
Cc: Vladimir Oltean, Zhao Qiang, linuxppc-dev, linux-kernel, netdev
In-Reply-To: <20201205191744.7847-16-rasmus.villemoes@prevas.dk>
Le 05/12/2020 à 20:17, Rasmus Villemoes a écrit :
> These macros both have the value 32, there's no point first
> initializing align to a lower value.
>
> If anything, one could throw in a
> BUILD_BUG_ON(UCC_GETH_TX_BD_RING_ALIGNMENT < 4), but it's not worth it
> - lots of code depends on named constants having sensible values.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> drivers/net/ethernet/freescale/ucc_geth.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index 273342233bba..ccde42f547b8 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -2196,9 +2196,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private *ugeth)
> UCC_GETH_TX_BD_RING_SIZE_MEMORY_ALIGNMENT)
> length += UCC_GETH_TX_BD_RING_SIZE_MEMORY_ALIGNMENT;
> if (uf_info->bd_mem_part == MEM_PART_SYSTEM) {
> - u32 align = 4;
> - if (UCC_GETH_TX_BD_RING_ALIGNMENT > 4)
> - align = UCC_GETH_TX_BD_RING_ALIGNMENT;
> + u32 align = UCC_GETH_TX_BD_RING_ALIGNMENT;
I think checkpatch.pl will expect an empty line here in addition.
> ugeth->tx_bd_ring_offset[j] =
> (u32) kmalloc((u32) (length + align), GFP_KERNEL);
>
> @@ -2274,9 +2272,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private *ugeth)
> for (j = 0; j < ug_info->numQueuesRx; j++) {
> length = ug_info->bdRingLenRx[j] * sizeof(struct qe_bd);
> if (uf_info->bd_mem_part == MEM_PART_SYSTEM) {
> - u32 align = 4;
> - if (UCC_GETH_RX_BD_RING_ALIGNMENT > 4)
> - align = UCC_GETH_RX_BD_RING_ALIGNMENT;
> + u32 align = UCC_GETH_RX_BD_RING_ALIGNMENT;
Same
> ugeth->rx_bd_ring_offset[j] =
> (u32) kmalloc((u32) (length + align), GFP_KERNEL);
> if (ugeth->rx_bd_ring_offset[j] != 0)
>
^ permalink raw reply
* Re: [PATCH 18/20] ethernet: ucc_geth: add helper to replace repeated switch statements
From: Christophe Leroy @ 2020-12-08 15:21 UTC (permalink / raw)
To: Rasmus Villemoes, Li Yang, David S. Miller, Jakub Kicinski
Cc: Vladimir Oltean, Zhao Qiang, linuxppc-dev, linux-kernel, netdev
In-Reply-To: <20201205191744.7847-19-rasmus.villemoes@prevas.dk>
Le 05/12/2020 à 20:17, Rasmus Villemoes a écrit :
> The translation from the ucc_geth_num_of_threads enum value to the
> actual count can be written somewhat more compactly with a small
> lookup table, allowing us to replace the four switch statements.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> drivers/net/ethernet/freescale/ucc_geth.c | 100 +++++-----------------
> 1 file changed, 22 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index 3aebea191b52..a26d1feb7dab 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -70,6 +70,20 @@ static struct {
> module_param_named(debug, debug.msg_enable, int, 0);
> MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 0xffff=all)");
>
> +static int ucc_geth_thread_count(enum ucc_geth_num_of_threads idx)
> +{
> + static const u8 count[] = {
> + [UCC_GETH_NUM_OF_THREADS_1] = 1,
> + [UCC_GETH_NUM_OF_THREADS_2] = 2,
> + [UCC_GETH_NUM_OF_THREADS_4] = 4,
> + [UCC_GETH_NUM_OF_THREADS_6] = 6,
> + [UCC_GETH_NUM_OF_THREADS_8] = 8,
> + };
> + if (idx >= ARRAY_SIZE(count))
> + return 0;
> + return count[idx];
> +}
I think you would allow GCC to provide a much better optimisation with something like:
static int ucc_geth_thread_count(enum ucc_geth_num_of_threads idx)
{
if (idx == UCC_GETH_NUM_OF_THREADS_1)
return 1;
if (idx == UCC_GETH_NUM_OF_THREADS_2)
return 2;
if (idx == UCC_GETH_NUM_OF_THREADS_4)
return 4;
if (idx == UCC_GETH_NUM_OF_THREADS_6)
return 6;
if (idx == UCC_GETH_NUM_OF_THREADS_8)
return 8;
return 0;
}
> +
> static const struct ucc_geth_info ugeth_primary_info = {
> .uf_info = {
> .rtsm = UCC_FAST_SEND_IDLES_BETWEEN_FRAMES,
> @@ -668,32 +682,12 @@ static void dump_regs(struct ucc_geth_private *ugeth)
> in_be32(&ugeth->ug_regs->scam));
>
> if (ugeth->p_thread_data_tx) {
> - int numThreadsTxNumerical;
> - switch (ugeth->ug_info->numThreadsTx) {
> - case UCC_GETH_NUM_OF_THREADS_1:
> - numThreadsTxNumerical = 1;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_2:
> - numThreadsTxNumerical = 2;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_4:
> - numThreadsTxNumerical = 4;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_6:
> - numThreadsTxNumerical = 6;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_8:
> - numThreadsTxNumerical = 8;
> - break;
> - default:
> - numThreadsTxNumerical = 0;
> - break;
> - }
> + int count = ucc_geth_thread_count(ugeth->ug_info->numThreadsTx);
>
> pr_info("Thread data TXs:\n");
> pr_info("Base address: 0x%08x\n",
> (u32)ugeth->p_thread_data_tx);
> - for (i = 0; i < numThreadsTxNumerical; i++) {
> + for (i = 0; i < count; i++) {
> pr_info("Thread data TX[%d]:\n", i);
> pr_info("Base address: 0x%08x\n",
> (u32)&ugeth->p_thread_data_tx[i]);
> @@ -702,32 +696,12 @@ static void dump_regs(struct ucc_geth_private *ugeth)
> }
> }
> if (ugeth->p_thread_data_rx) {
> - int numThreadsRxNumerical;
> - switch (ugeth->ug_info->numThreadsRx) {
> - case UCC_GETH_NUM_OF_THREADS_1:
> - numThreadsRxNumerical = 1;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_2:
> - numThreadsRxNumerical = 2;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_4:
> - numThreadsRxNumerical = 4;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_6:
> - numThreadsRxNumerical = 6;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_8:
> - numThreadsRxNumerical = 8;
> - break;
> - default:
> - numThreadsRxNumerical = 0;
> - break;
> - }
> + int count = ucc_geth_thread_count(ugeth->ug_info->numThreadsRx);
>
> pr_info("Thread data RX:\n");
> pr_info("Base address: 0x%08x\n",
> (u32)ugeth->p_thread_data_rx);
> - for (i = 0; i < numThreadsRxNumerical; i++) {
> + for (i = 0; i < count; i++) {
> pr_info("Thread data RX[%d]:\n", i);
> pr_info("Base address: 0x%08x\n",
> (u32)&ugeth->p_thread_data_rx[i]);
> @@ -2315,45 +2289,15 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> uf_regs = uccf->uf_regs;
> ug_regs = ugeth->ug_regs;
>
> - switch (ug_info->numThreadsRx) {
> - case UCC_GETH_NUM_OF_THREADS_1:
> - numThreadsRxNumerical = 1;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_2:
> - numThreadsRxNumerical = 2;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_4:
> - numThreadsRxNumerical = 4;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_6:
> - numThreadsRxNumerical = 6;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_8:
> - numThreadsRxNumerical = 8;
> - break;
> - default:
> + numThreadsRxNumerical = ucc_geth_thread_count(ug_info->numThreadsRx);
> + if (!numThreadsRxNumerical) {
> if (netif_msg_ifup(ugeth))
> pr_err("Bad number of Rx threads value\n");
> return -EINVAL;
> }
>
> - switch (ug_info->numThreadsTx) {
> - case UCC_GETH_NUM_OF_THREADS_1:
> - numThreadsTxNumerical = 1;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_2:
> - numThreadsTxNumerical = 2;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_4:
> - numThreadsTxNumerical = 4;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_6:
> - numThreadsTxNumerical = 6;
> - break;
> - case UCC_GETH_NUM_OF_THREADS_8:
> - numThreadsTxNumerical = 8;
> - break;
> - default:
> + numThreadsTxNumerical = ucc_geth_thread_count(ug_info->numThreadsTx);
> + if (!numThreadsTxNumerical) {
> if (netif_msg_ifup(ugeth))
> pr_err("Bad number of Tx threads value\n");
> return -EINVAL;
>
^ permalink raw reply
* Re: [PATCH v5 10/19] dt-bindings: usb: Convert DWC USB3 bindings to DT schema
From: Rob Herring @ 2020-12-08 15:21 UTC (permalink / raw)
To: Serge Semin
Cc: Neil Armstrong, Bjorn Andersson, Pavel Parkhomenko, Kevin Hilman,
Krzysztof Kozlowski, Ahmad Zainie, Andy Gross, Chunfeng Yun,
linux-snps-arc, devicetree, Mathias Nyman, Martin Blumenstingl,
Lad Prabhakar, Alexey Malahov, Rob Herring, linux-arm-kernel,
Roger Quadros, Felipe Balbi, linux-mips, Greg Kroah-Hartman,
Yoshihiro Shimoda, linux-usb, linux-kernel, Serge Semin,
Manu Gautam, linuxppc-dev
In-Reply-To: <20201205152427.29537-11-Sergey.Semin@baikalelectronics.ru>
On Sat, 05 Dec 2020 18:24:17 +0300, Serge Semin wrote:
> DWC USB3 DT node is supposed to be compliant with the Generic xHCI
> Controller schema, but with additional vendor-specific properties, the
> controller-specific reference clocks and PHYs. So let's convert the
> currently available legacy text-based DWC USB3 bindings to the DT schema
> and make sure the DWC USB3 nodes are also validated against the
> usb-xhci.yaml schema.
>
> Note 1. we have to discard the nodename restriction of being prefixed with
> "dwc3@" string, since in accordance with the usb-hcd.yaml schema USB nodes
> are supposed to be named as "^usb(@.*)".
>
> Note 2. The clock-related properties are marked as optional to match the
> DWC USB3 driver expectation and to improve the bindings mainainability
> so in case if there is a glue-node it would the responsible for the
> clocks initialization.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> ---
>
> Changelog v2:
> - Discard '|' from the descriptions, since we don't need to preserve
> the text formatting in any of them.
> - Drop quotes from around the string constants.
> - Fix the "clock-names" prop description to be referring the enumerated
> clock-names instead of the ones from the Databook.
>
> Changelog v3:
> - Apply usb-xhci.yaml# schema only if the controller is supposed to work
> as either host or otg.
>
> Changelog v4:
> - Apply usb-drd.yaml schema first. If the controller is configured
> to work in a gadget mode only, then apply the usb.yaml schema too,
> otherwise apply the usb-xhci.yaml schema.
> - Discard the Rob'es Reviewed-by tag. Please review the patch one more
> time.
>
> Changelog v5:
> - Add "snps,dis-split-quirk" property to the DWC USB3 DT schema.
> - Add a commit log text about the clock-related property changes.
> - Make sure dr_mode exist to apply the USB-gadget-only schema.
> ---
> .../devicetree/bindings/usb/dwc3.txt | 128 -------
> .../devicetree/bindings/usb/snps,dwc3.yaml | 312 ++++++++++++++++++
> 2 files changed, 312 insertions(+), 128 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt
> create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Michael Walle @ 2020-12-08 15:41 UTC (permalink / raw)
To: lorenzo.pieralisi
Cc: kw, heiko, shawn.lin, paulus, Michael Walle, thomas.petazzoni,
jonnyc, toan, will, robh, f.fainelli, michal.simek,
linux-rockchip, bcm-kernel-feedback-list, jonathan.derrick,
linux-pci, rjui, linux-rpi-kernel, Jonathan.Cameron, bhelgaas,
linux-arm-kernel, sbranden, wangzhou1, rrichter, linuxppc-dev,
nsaenzjulienne
In-Reply-To: <160683676668.20365.13565676178464743008.b4-ty@arm.com>
Hi Lorenzo, Krzysztof,
>On Sun, 29 Nov 2020 23:07:38 +0000, Krzysztof Wilczyński wrote:
>> Unify ECAM-related constants into a single set of standard constants
>> defining memory address shift values for the byte-level address that can
>> be used when accessing the PCI Express Configuration Space, and then
>> move native PCI Express controller drivers to use newly introduced
>> definitions retiring any driver-specific ones.
>>
>> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
>> PCI Express specification (see PCI Express Base Specification, Revision
>> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
>> implement it the same way.
>>
>> [...]
>
>Applied to pci/ecam, thanks!
>
>[1/5] PCI: Unify ECAM constants in native PCI Express drivers
> https://git.kernel.org/lpieralisi/pci/c/f3c07cf692
>[2/5] PCI: thunder-pem: Add constant for custom ".bus_shift" initialiser
> https://git.kernel.org/lpieralisi/pci/c/3c38579263
>[3/5] PCI: iproc: Convert to use the new ECAM constants
> https://git.kernel.org/lpieralisi/pci/c/333ec9d3cc
>[4/5] PCI: vmd: Update type of the __iomem pointers
> https://git.kernel.org/lpieralisi/pci/c/89094c12ea
>[5/5] PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
> https://git.kernel.org/lpieralisi/pci/c/3dc62532a5
Patch 1/5 breaks LS1028A boards:
[..]
[ 1.144426] pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
[ 1.152276] pci-host-generic 1f0000000.pcie: MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
[ 1.161161] pci-host-generic 1f0000000.pcie: MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
[ 1.170043] pci-host-generic 1f0000000.pcie: MEM 0x01f81d0000..0x01f81effff -> 0x0000000000
[ 1.178924] pci-host-generic 1f0000000.pcie: MEM 0x01f81f0000..0x01f820ffff -> 0x0000000000
[ 1.187805] pci-host-generic 1f0000000.pcie: MEM 0x01f8210000..0x01f822ffff -> 0x0000000000
[ 1.196686] pci-host-generic 1f0000000.pcie: MEM 0x01f8230000..0x01f824ffff -> 0x0000000000
[ 1.205562] pci-host-generic 1f0000000.pcie: MEM 0x01fc000000..0x01fc3fffff -> 0x0000000000
[ 1.214465] pci-host-generic 1f0000000.pcie: ECAM at [mem 0x1f0000000-0x1f00fffff] for [bus 00]
[ 1.223318] pci-host-generic 1f0000000.pcie: PCI host bridge to bus 0000:00
[ 1.230350] pci_bus 0000:00: root bus resource [bus 00]
[ 1.235625] pci_bus 0000:00: root bus resource [mem 0x1f8000000-0x1f815ffff] (bus address [0x00000000-0x0015ffff])
[ 1.246077] pci_bus 0000:00: root bus resource [mem 0x1f8160000-0x1f81cffff pref] (bus address [0x00000000-0x0006ffff])
[ 1.256969] pci_bus 0000:00: root bus resource [mem 0x1f81d0000-0x1f81effff] (bus address [0x00000000-0x0001ffff])
[ 1.267427] pci_bus 0000:00: root bus resource [mem 0x1f81f0000-0x1f820ffff pref] (bus address [0x00000000-0x0001ffff])
[ 1.278326] pci_bus 0000:00: root bus resource [mem 0x1f8210000-0x1f822ffff] (bus address [0x00000000-0x0001ffff])
[ 1.288779] pci_bus 0000:00: root bus resource [mem 0x1f8230000-0x1f824ffff pref] (bus address [0x00000000-0x0001ffff])
[ 1.299669] pci_bus 0000:00: root bus resource [mem 0x1fc000000-0x1fc3fffff] (bus address [0x00000000-0x003fffff])
[ 1.310138] pci 0000:00:00.0: [1957:e100] type 00 class 0x020001
[ 1.316234] pci 0000:00:00.0: BAR 0: [mem 0x1f8000000-0x1f803ffff 64bit] (from Enhanced Allocation, properties 0x0)
[ 1.326776] pci 0000:00:00.0: BAR 2: [mem 0x1f8160000-0x1f816ffff 64bit pref] (from Enhanced Allocation, properties 0x1)
[ 1.337759] pci 0000:00:00.0: VF BAR 0: [mem 0x1f81d0000-0x1f81dffff 64bit] (from Enhanced Allocation, properties 0x4)
[ 1.348563] pci 0000:00:00.0: VF BAR 2: [mem 0x1f81f0000-0x1f81fffff 64bit pref] (from Enhanced Allocation, properties 0x3)
[ 1.359821] pci 0000:00:00.0: PME# supported from D0 D3hot
[ 1.365368] pci 0000:00:00.0: VF(n) BAR0 space: [mem 0x1f81d0000-0x1f81effff 64bit] (contains BAR0 for 2 VFs)
[ 1.375381] pci 0000:00:00.0: VF(n) BAR2 space: [mem 0x1f81f0000-0x1f820ffff 64bit pref] (contains BAR2 for 2 VFs)
[ 1.385983] Unable to handle kernel paging request at virtual address ffff800012132000
[ 1.393972] Mem abort info:
[ 1.396783] ESR = 0x96000007
[ 1.399859] EC = 0x25: DABT (current EL), IL = 32 bits
[ 1.405215] SET = 0, FnV = 0
[ 1.408290] EA = 0, S1PTW = 0
[ 1.411453] Data abort info:
[ 1.414352] ISV = 0, ISS = 0x00000007
[ 1.418216] CM = 0, WnR = 0
[ 1.421205] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000008369c000
[ 1.427966] [ffff800012132000] pgd=00000020fffff003, p4d=00000020fffff003, pud=00000020ffffe003, pmd=00000020ffffa003, pte=0000000000000000
[ 1.440618] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[ 1.446239] Modules linked in:
[ 1.449320] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc3-00101-g2f378db5c89 #191
[ 1.457484] Hardware name: Kontron SMARC-sAL28 (Single PHY) on SMARC Eval 2.0 carrier (DT)
[ 1.465827] pstate: 20000085 (nzCv daIf -PAN -UAO -TCO BTYPE=--)
[ 1.471892] pc : pci_generic_config_read+0x38/0xe0
[ 1.476723] lr : pci_generic_config_read+0x24/0xe0
[ 1.481553] sp : ffff80001211b920
[ 1.484891] x29: ffff80001211b920 x28: 0000000000000000
[ 1.490252] x27: ffff8000116a04bc x26: 0000000000000000
[ 1.495612] x25: 0000000000000001 x24: ffff80001211ba54
[ 1.500972] x23: ffff0020009c3800 x22: 0000000000000000
[ 1.506332] x21: 0000000000000087 x20: ffff80001211b994
[ 1.511692] x19: 0000000000000004 x18: 0000000000000000
[ 1.517052] x17: 0000000000000000 x16: 00000000d5edfbc1
[ 1.522412] x15: ffffffffffffffff x14: ffff800011cf9948
[ 1.527772] x13: ffff002000305a1c x12: 0000000000000030
[ 1.533132] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
[ 1.538491] x9 : 2c6b7173626d686f x8 : 000000000000ea60
[ 1.543851] x7 : ffff80001211ba54 x6 : 0000000000000000
[ 1.549211] x5 : 0000000000000000 x4 : ffff800012131000
[ 1.554570] x3 : 0000000000000000 x2 : 0000000000000000
[ 1.559930] x1 : 0000000000001000 x0 : ffff800012132000
[ 1.565290] Call trace:
[ 1.567752] pci_generic_config_read+0x38/0xe0
[ 1.572233] pci_bus_read_config_dword+0x84/0xd8
[ 1.576890] pci_bus_generic_read_dev_vendor_id+0x34/0x1b0
[ 1.582423] pci_bus_read_dev_vendor_id+0x4c/0x70
[ 1.587167] pci_scan_single_device+0x84/0xe0
[ 1.591559] pci_scan_slot+0x6c/0x120
[ 1.595250] pci_scan_child_bus_extend+0x54/0x298
[ 1.599994] pci_scan_root_bus_bridge+0xd4/0xf0
[ 1.604562] pci_host_probe+0x18/0xb0
[ 1.608254] pci_host_common_probe+0x13c/0x1a0
[ 1.612735] platform_drv_probe+0x54/0xa8
[ 1.616777] really_probe+0xe4/0x3b8
[ 1.620380] driver_probe_device+0x58/0xb8
[ 1.624509] device_driver_attach+0x74/0x80
[ 1.628725] __driver_attach+0x58/0xe0
[ 1.632503] bus_for_each_dev+0x74/0xc8
[ 1.636369] driver_attach+0x24/0x30
[ 1.639972] bus_add_driver+0x18c/0x1f0
[ 1.643838] driver_register+0x64/0x120
[ 1.647704] __platform_driver_register+0x48/0x58
[ 1.652449] gen_pci_driver_init+0x1c/0x28
[ 1.656580] do_one_initcall+0x4c/0x2c0
[ 1.660447] kernel_init_freeable+0x1e4/0x250
[ 1.664840] kernel_init+0x14/0x118
[ 1.668355] ret_from_fork+0x10/0x34
[ 1.671961] Code: 7100067f 540001c0 71000a7f 54000300 (b9400001)
[ 1.678114] ---[ end trace 0aca1b048661e8b3 ]---
[ 1.682770] note: swapper/0[1] exited with preempt_count 1
[ 1.688305] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 1.696031] SMP: stopping secondary CPUs
[ 1.699989] Kernel Offset: disabled
[ 1.703503] CPU features: 0x0240022,61006008
[ 1.707806] Memory Limit: none
[ 1.710884] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
There is a LS1028A eval board in kernelci here:
https://lavalab.nxp.com/scheduler/job/170566
I actually have this board which also have a LS1028A SoC:
https://lavalab.kontron.com/scheduler/job/1771
But in the latter you won't see much because earlycon isn't active. [I'm
about to fix that.]
By reverting patch 1/5, the board will work again.
-michael
^ permalink raw reply
* Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Qian Cai @ 2020-12-08 16:07 UTC (permalink / raw)
To: Krzysztof Wilczyński, Bjorn Helgaas
Cc: Heiko Stuebner, Shawn Lin, Paul Mackerras, Thomas Petazzoni,
Jonathan Chocron, Toan Le, Will Deacon, Rob Herring,
Lorenzo Pieralisi, Michal Simek, linux-rockchip,
Linux Next Mailing List, bcm-kernel-feedback-list,
Jonathan Derrick, linux-pci, Stephen Rothwell, Ray Jui,
Florian Fainelli, linux-rpi-kernel, Jonathan Cameron,
linux-arm-kernel, Scott Branden, Zhou Wang, Robert Richter,
linuxppc-dev, Nicolas Saenz Julienne
In-Reply-To: <20201129230743.3006978-1-kw@linux.com>
On Sun, 2020-11-29 at 23:07 +0000, Krzysztof Wilczyński wrote:
> Unify ECAM-related constants into a single set of standard constants
> defining memory address shift values for the byte-level address that can
> be used when accessing the PCI Express Configuration Space, and then
> move native PCI Express controller drivers to use newly introduced
> definitions retiring any driver-specific ones.
>
> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
> PCI Express specification (see PCI Express Base Specification, Revision
> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
> implement it the same way.
>
> Most of the native PCI Express controller drivers define their ECAM-related
> constants, many of these could be shared, or use open-coded values when
> setting the ".bus_shift" field of the "struct pci_ecam_ops".
>
> All of the newly added constants should remove ambiguity and reduce the
> number of open-coded values, and also correlate more strongly with the
> descriptions in the aforementioned specification (see Table 7-1
> "Enhanced Configuration Address Mapping", p. 677).
>
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reverting this series on the top of today's linux-next fixed a boot crash on
arm64 Thunder X2 server.
.config: https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config
[ 186.285957][ T1] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-7f])
[ 186.293127][ T1] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig Segments MSI HPX-Type3]
[ 186.317072][ T1] acpi PNP0A08:00: _OSC: not requesting OS control; OS requires [ExtendedConfig ASPM ClockPM MSI]
[ 186.330336][ T1] acpi PNP0A08:00: ECAM area [mem 0x30000000-0x37ffffff] reserved by PNP0C02:00
[ 186.339538][ T1] acpi PNP0A08:00: ECAM at [mem 0x30000000-0x37ffffff] for [bus 00-7f]
[ 186.353258][ T1] PCI host bridge to bus 0000:00
[ 186.358162][ T1] pci_bus 0000:00: root bus resource [mem 0x40000000-0x5fffffff window]
[ 186.366509][ T1] pci_bus 0000:00: root bus resource [mem 0x10000000000-0x13fffffffff window]
[ 186.375366][ T1] pci_bus 0000:00: root bus resource [bus 00-7f]
[ 186.382652][ T1] pci 0000:00:00.0: [177d:af00] type 00 class 0x060000
[ 186.395174][ T1] pci 0000:00:01.0: [177d:af84] type 01 class 0x060400
[ 186.402433][ T1] pci 0000:00:01.0: PME# supported from D0 D3hot D3cold
[ 186.415652][ T1] Unable to handle kernel paging request at virtual address ffff800029370000
[ 186.424398][ T1] Mem abort info:
[ 186.427930][ T1] ESR = 0x96000007
[ 186.431725][ T1] EC = 0x25: DABT (current EL), IL = 32 bits
[ 186.437805][ T1] SET = 0, FnV = 0
[ 186.441599][ T1] EA = 0, S1PTW = 0
[ 186.445485][ T1] Data abort info:
[ 186.449104][ T1] ISV = 0, ISS = 0x00000007
[ 186.453687][ T1] CM = 0, WnR = 0
[ 186.457396][ T1] swapper pgtable: 64k pages, 48-bit VAs, pgdp=00000000da920000
[ 186.464979][ T1] [ffff800029370000] pgd=0000008ffcff0003, p4d=0000008ffcff0003, pud=0000008ffcff0003, pmd=00000000811d0003, pte=0000000000000000
[ 186.478424][ T1] Internal error: Oops: 96000007 [#1] SMP
[ 186.484059][ T1] Modules linked in:
[ 186.487854][ T1] CPU: 38 PID: 1 Comm: swapper/0 Tainted: G W L 5.10.0-rc7-next-20201208 #3
[ 186.497617][ T1] Hardware name: HPE Apollo 70 /C01_APACHE_MB , BIOS L50_5.13_1.16 07/29/2020
[ 186.508174][ T1] pstate: 20400089 (nzCv daIf +PAN -UAO -TCO BTYPE=--)
[ 186.514954][ T1] pc : pci_generic_config_read+0x78/0x1d0
[ 186.520587][ T1] lr : pci_generic_config_read+0x64/0x1d0
pci_generic_config_read at drivers/pci/access.c:83
[ 186.526223][ T1] sp : ffff000005f0ef30
[ 186.530278][ T1] x29: ffff000005f0ef30 x28: 0000000000000010
[ 186.536359][ T1] x27: 0000000000000000 x26: 0000000000000087
[ 186.542441][ T1] x25: 0000000000000000 x24: ffff00084a3a5000
[ 186.548517][ T1] x23: ffff000005f0f150 x22: 0000000000000004
[ 186.554593][ T1] x21: ffff800011404588 x20: ffff000005f0eff0
[ 186.560669][ T1] x19: ffff00084a3a5000 x18: 1fffe001cf0d53ed
[ 186.566750][ T1] x17: 0000000000000000 x16: 0000000000000003
[ 186.572831][ T1] x15: 0000000000000000 x14: 0000000000000003
[ 186.578908][ T1] x13: ffff600000be1ddf x12: 1fffe00000be1dde
[ 186.584983][ T1] x11: 1fffe00000be1dde x10: ffff600000be1dde
[ 186.591059][ T1] x9 : ffff800010c4f59c x8 : ffff000005f0eef3
[ 186.597139][ T1] x7 : 0000000000000001 x6 : 0000000000000001
[ 186.603222][ T1] x5 : 1fffe00109474a1c x4 : 1fffe010fd074cb2
[ 186.609298][ T1] x3 : 0000000000000000 x2 : 0000000000000000
[ 186.615374][ T1] x1 : 0000000000010000 x0 : ffff800029370000
[ 186.621451][ T1] Call trace:
[ 186.624623][ T1] pci_generic_config_read+0x78/0x1d0
__raw_readl at arch/arm64/include/asm/io.h:75
(inlined by) pci_generic_config_read at drivers/pci/access.c:93
[ 186.629905][ T1] pci_bus_read_config_dword+0xfc/0x198
pci_bus_read_config_dword at drivers/pci/access.c:65 (discriminator 2)
[ 186.635362][ T1] pci_bus_generic_read_dev_vendor_id+0x3c/0x310
pci_bus_generic_read_dev_vendor_id at drivers/pci/probe.c:2300
[ 186.641611][ T1] pci_bus_read_dev_vendor_id+0x7c/0xd0
pci_bus_read_dev_vendor_id at drivers/pci/probe.c:2329
[ 186.647067][ T1] pci_scan_single_device+0xe0/0x1f8
pci_scan_device at drivers/pci/probe.c:2342
(inlined by) pci_scan_single_device at drivers/pci/probe.c:2511
(inlined by) pci_scan_single_device at drivers/pci/probe.c:2501
[ 186.652261][ T1] pci_scan_slot+0x88/0x258
pci_scan_slot at drivers/pci/probe.c:2590
[ 186.656663][ T1] pci_scan_child_bus_extend+0x88/0x608
pci_scan_child_bus_extend at drivers/pci/probe.c:2807
[ 186.662120][ T1] pci_scan_child_bus+0x18/0x20
pci_scan_child_bus at drivers/pci/probe.c:2938
[ 186.666875][ T1] acpi_pci_root_create+0x518/0x7a8
acpi_pci_root_create at drivers/acpi/pci_root.c:938
[ 186.671982][ T1] pci_acpi_scan_root+0x2bc/0x4c0
pci_acpi_scan_root at arch/arm64/kernel/pci.c:189
[ 186.676911][ T1] acpi_pci_root_add+0x45c/0x920
acpi_pci_root_add at drivers/acpi/pci_root.c:609
[ 186.681754][ T1] acpi_bus_attach+0x270/0x6d8
acpi_scan_attach_handler at drivers/acpi/scan.c:1969
(inlined by) acpi_bus_attach at drivers/acpi/scan.c:2013
[ 186.686419][ T1] acpi_bus_attach+0x140/0x6d8
[ 186.691085][ T1] acpi_bus_attach+0x140/0x6d8
acpi_bus_attach at drivers/acpi/scan.c:2033 (discriminator 9)
[ 186.695751][ T1] acpi_bus_scan+0x98/0xf8
acpi_bus_scan at drivers/acpi/scan.c:2087
[ 186.700067][ T1] acpi_scan_init+0x220/0x524
[ 186.704644][ T1] acpi_init+0x460/0x4e8
[ 186.708790][ T1] do_one_initcall+0x170/0xb70
[ 186.713461][ T1] kernel_init_freeable+0x6a8/0x734
[ 186.718574][ T1] kernel_init+0x18/0x12c
[ 186.722806][ T1] ret_from_fork+0x10/0x1c
[ 186.727123][ T1] Code: 710006df 540002e0 71000adf 540004e0 (b9400013)
[ 186.733988][ T1] ---[ end trace bad65ebbc8c09fe0 ]---
[ 186.739359][ T1] Kernel panic - not syncing: Oops: Fatal exception
[ 186.746115][ T1] SMP: stopping secondary CPUs
[ 186.750968][ T1] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---
>
> ---
> Changed in v6:
> Converted single patch into a series.
> Dropped changes related to PPC 4xx platform.
> Refactored pci_ecam_map_bus() so that bus, device function and offset
> are correctly masked, limiting offset to 4K as per the PCI Express
> Specification. After the refactor this function will now use sensible
> defaults allowing for removal of the ".bus_shit" initialiser from all
> the users of the "ecam_ops" structure who do not use a non-standard
> ECAM bus shit values.
>
> Changed in v5:
> Removed unused constant "PCIE_ECAM_DEV_SHIFT".
> Refactored ppc4xx_pciex_get_config_base() so that the "offset"
> parameter can be passed to so that the PCIE_ECAM_OFFSET() macro
> can be used.
> Used the ALIGN_DOWN() macro where 32 bit alignment is required
> instead using the 0xffc mask.
> Added CFG_ADDR_CFG_TYPE_1 macro to pci/controller/pcie-iproc.c to
> denote that this is a configuration type 1 address and access type.
> Refactored boundary check in pci/controller/vmd.c as used by the
> vmd_cfg_addr() function following addition of the PCIE_ECAM_OFFSET()
> macro.
> Changed the "bus->number" to simply pass the "bus" argument in the
> PCIE_ECAM_OFFSET() macro.
>
> Changed in v4:
> Removed constants related to "CAM".
> Added more platforms and devices that can use new ECAM macros and
> constants.
> Removed unused ".bus_shift" initialisers from pci-xgene.c as
> xgene_pcie_map_bus() did not use these.
>
> Changes in v3:
> Updated commit message wording.
> Updated regarding custom ECAM bus shift values and concerning PCI base
> configuration space access for Type 1 access.
> Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf()
> and removed the "busdev" variable.
> Removed surplus "relbus" variable from nwl_pcie_map_bus() and
> xilinx_pcie_map_bus().
> Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET().
>
> Changes in v2:
> Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop
> PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour
> of using a single value for the device/function.
>
> Krzysztof Wilczyński (5):
> PCI: Unify ECAM constants in native PCI Express drivers
> PCI: thunder-pem: Add constant for custom ".bus_shit" initialiser
> PCI: iproc: Convert to use the new ECAM constants
> PCI: vmd: Update type of the __iomem pointers
> PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
>
> drivers/pci/controller/dwc/pcie-al.c | 12 ++------
> drivers/pci/controller/dwc/pcie-hisi.c | 2 --
> drivers/pci/controller/pci-aardvark.c | 13 ++-------
> drivers/pci/controller/pci-host-generic.c | 1 -
> drivers/pci/controller/pci-thunder-ecam.c | 1 -
> drivers/pci/controller/pci-thunder-pem.c | 13 +++++++--
> drivers/pci/controller/pci-xgene.c | 2 --
> drivers/pci/controller/pcie-brcmstb.c | 16 ++---------
> drivers/pci/controller/pcie-iproc.c | 31 ++++++---------------
> drivers/pci/controller/pcie-rockchip-host.c | 27 +++++++++---------
> drivers/pci/controller/pcie-rockchip.h | 8 +-----
> drivers/pci/controller/pcie-tango.c | 1 -
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ++----
> drivers/pci/controller/pcie-xilinx.c | 11 ++------
> drivers/pci/controller/vmd.c | 19 ++++++-------
> drivers/pci/ecam.c | 23 ++++++++++-----
> include/linux/pci-ecam.h | 27 ++++++++++++++++++
> 17 files changed, 96 insertions(+), 120 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v2 0/5] drop unused BACKLIGHT_GENERIC option
From: Arnd Bergmann @ 2020-12-08 16:34 UTC (permalink / raw)
To: jonathanh, linux-kernel, thierry.reding, krzk, mripard, benh,
emil.l.velikov, alexandre.belloni, mpe, linux-parisc, paulus,
nicolas.ferre, tony, sam, linux, ludovic.desroches, soc,
James.Bottomley, will, linuxppc-dev, linux-tegra, linux-omap,
daniel.thompson, jernej.skrabec, linux-arm-kernel, deller,
tsbogend, catalin.marinas, wens, linux-mips, lee.jones,
Andrey Zhizhikin
Cc: Arnd Bergmann
In-Reply-To: <20201201222922.3183-1-andrey.zhizhikin@leica-geosystems.com>
From: Arnd Bergmann <arnd@arndb.de>
On Tue, 1 Dec 2020 22:29:17 +0000, Andrey Zhizhikin wrote:
> Since the removal of generic_bl driver from the source tree in commit
> 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") BACKLIGHT_GENERIC config option became obsolete as well and
> therefore subject to clean-up from all configuration files.
>
> This series introduces patches to address this removal, separated by
> architectures in the kernel tree.
>
> [...]
While my plan was to only take the arm specific patches, it seems
nobody else has applied the other architecture specific ones,
but there have been a lot of Acks. Also, b4 makes it easy to
merge the entire branch, so I'll just take all of these.
Applied to arm/defconfig, thanks!
[1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option
commit: 0437141b4e2233ae0109a9584e7a003cd05b0a20
[2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option
commit: 717c4c8336486781630893508b3347ae18953fae
[3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
commit: 2257682282531de45929c6006152f6e2ee881b42
[4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option
commit: 4e9c44b128d3eb5da129e53c7312240f838c2dbf
[5/5] powerpc/configs: drop unused BACKLIGHT_GENERIC option
commit: 4985c506303fb6a41a885d503a6e1f3d3126431d
Arnd
^ permalink raw reply
* Re: [PATCH v2 0/5] drop unused BACKLIGHT_GENERIC option
From: Alexandre Belloni @ 2020-12-08 16:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: sam, tony, linux-kernel, James.Bottomley, wens, thierry.reding,
paulus, will, daniel.thompson, deller, linux, krzk, jonathanh,
ludovic.desroches, catalin.marinas, linux-mips, Arnd Bergmann,
mripard, Andrey Zhizhikin, soc, linux-tegra, lee.jones,
linux-omap, linux-arm-kernel, jernej.skrabec, tsbogend,
linux-parisc, emil.l.velikov, nicolas.ferre, linuxppc-dev
In-Reply-To: <160744514229.359082.11487352663734358657.b4-ty@arndb.de>
Hi,
On 08/12/2020 17:34:46+0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> On Tue, 1 Dec 2020 22:29:17 +0000, Andrey Zhizhikin wrote:
> > Since the removal of generic_bl driver from the source tree in commit
> > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > therefore subject to clean-up from all configuration files.
> >
> > This series introduces patches to address this removal, separated by
> > architectures in the kernel tree.
> >
> > [...]
>
> While my plan was to only take the arm specific patches, it seems
> nobody else has applied the other architecture specific ones,
> but there have been a lot of Acks. Also, b4 makes it easy to
> merge the entire branch, so I'll just take all of these.
>
> Applied to arm/defconfig, thanks!
>
> [1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option
> commit: 0437141b4e2233ae0109a9584e7a003cd05b0a20
> [2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> commit: 717c4c8336486781630893508b3347ae18953fae
> [3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
> commit: 2257682282531de45929c6006152f6e2ee881b42
Thomas did applied this one:
https://lore.kernel.org/linux-arm-kernel/20201204120632.GA10011@alpha.franken.de/
> [4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option
> commit: 4e9c44b128d3eb5da129e53c7312240f838c2dbf
> [5/5] powerpc/configs: drop unused BACKLIGHT_GENERIC option
> commit: 4985c506303fb6a41a885d503a6e1f3d3126431d
>
> Arnd
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 02/13] powerpc/xive: Rename XIVE_IRQ_NO_EOI to show its a flag
From: Greg Kurz @ 2020-12-08 16:59 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20201208151124.1329942-3-clg@kaod.org>
On Tue, 8 Dec 2020 16:11:13 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> This is a simple cleanup to identify easily all flags of the XIVE
> interrupt structure. The interrupts flagged with XIVE_IRQ_FLAG_NO_EOI
> are the escalations used to wake up vCPUs in KVM. They are handled
> very differently from the rest.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/include/asm/xive.h | 2 +-
> arch/powerpc/kvm/book3s_xive.c | 4 ++--
> arch/powerpc/sysdev/xive/common.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index 309b4d65b74f..d332dd9a18de 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -66,7 +66,7 @@ struct xive_irq_data {
> #define XIVE_IRQ_FLAG_H_INT_ESB 0x20
>
> /* Special flag set by KVM for excalation interrupts */
> -#define XIVE_IRQ_NO_EOI 0x80
> +#define XIVE_IRQ_FLAG_NO_EOI 0x80
>
> #define XIVE_INVALID_CHIP_ID -1
>
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index 18a6b75a3bfd..fae1c2e8da29 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -219,7 +219,7 @@ int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
> /* In single escalation mode, we grab the ESB MMIO of the
> * interrupt and mask it. Also populate the VCPU v/raddr
> * of the ESB page for use by asm entry/exit code. Finally
> - * set the XIVE_IRQ_NO_EOI flag which will prevent the
> + * set the XIVE_IRQ_FLAG_NO_EOI flag which will prevent the
> * core code from performing an EOI on the escalation
> * interrupt, thus leaving it effectively masked after
> * it fires once.
> @@ -231,7 +231,7 @@ int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
> xive_vm_esb_load(xd, XIVE_ESB_SET_PQ_01);
> vcpu->arch.xive_esc_raddr = xd->eoi_page;
> vcpu->arch.xive_esc_vaddr = (__force u64)xd->eoi_mmio;
> - xd->flags |= XIVE_IRQ_NO_EOI;
> + xd->flags |= XIVE_IRQ_FLAG_NO_EOI;
> }
>
> return 0;
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index a80440af491a..65af34ac1fa2 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -416,7 +416,7 @@ static void xive_irq_eoi(struct irq_data *d)
> * been passed-through to a KVM guest
> */
> if (!irqd_irq_disabled(d) && !irqd_is_forwarded_to_vcpu(d) &&
> - !(xd->flags & XIVE_IRQ_NO_EOI))
> + !(xd->flags & XIVE_IRQ_FLAG_NO_EOI))
> xive_do_source_eoi(irqd_to_hwirq(d), xd);
> else
> xd->stale_p = true;
^ permalink raw reply
* Re: [PATCH v2 0/5] drop unused BACKLIGHT_GENERIC option
From: Thomas Bogendoerfer @ 2020-12-08 17:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: alexandre.belloni, sam, tony, linux-kernel, James.Bottomley, wens,
thierry.reding, paulus, will, daniel.thompson, deller, linux,
krzk, jonathanh, ludovic.desroches, catalin.marinas, linux-mips,
Arnd Bergmann, mripard, Andrey Zhizhikin, soc, linux-tegra,
lee.jones, linux-omap, linux-arm-kernel, jernej.skrabec,
linux-parisc, emil.l.velikov, nicolas.ferre, linuxppc-dev
In-Reply-To: <160744514229.359082.11487352663734358657.b4-ty@arndb.de>
On Tue, Dec 08, 2020 at 05:34:46PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> On Tue, 1 Dec 2020 22:29:17 +0000, Andrey Zhizhikin wrote:
> > Since the removal of generic_bl driver from the source tree in commit
> > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > therefore subject to clean-up from all configuration files.
> >
> > This series introduces patches to address this removal, separated by
> > architectures in the kernel tree.
> >
> > [...]
>
> While my plan was to only take the arm specific patches, it seems
> nobody else has applied the other architecture specific ones,
> but there have been a lot of Acks. Also, b4 makes it easy to
> merge the entire branch, so I'll just take all of these.
>
> Applied to arm/defconfig, thanks!
>
> [1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option
> commit: 0437141b4e2233ae0109a9584e7a003cd05b0a20
> [2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> commit: 717c4c8336486781630893508b3347ae18953fae
> [3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
> commit: 2257682282531de45929c6006152f6e2ee881b42
this one is already in mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH 03/13] powerpc/xive: Introduce XIVE_IPI_HW_IRQ
From: Greg Kurz @ 2020-12-08 17:06 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20201208151124.1329942-4-clg@kaod.org>
On Tue, 8 Dec 2020 16:11:14 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> The XIVE driver deals with CPU IPIs in a peculiar way. Each CPU has
> its own XIVE IPI interrupt allocated at the HW level, for PowerNV, or
> at the hypervisor level for pSeries. In practice, these interrupts are
> not always used. pSeries/PowerVM prefers local doorbells for local
> threads since they are faster. On PowerNV, global doorbells are also
> preferred for the same reason.
>
> The mapping in the Linux is reduced to a single interrupt using HW
> interrupt number 0 and a custom irq_chip to handle EOI. This can cause
> performance issues in some benchmark (ipistorm) on multichip systems.
>
> Clarify the use of the 0 value, it will help in improving multichip
> support.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/sysdev/xive/xive-internal.h | 2 ++
> arch/powerpc/sysdev/xive/common.c | 10 +++++-----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
> index b7b901da2168..d701af7fb48c 100644
> --- a/arch/powerpc/sysdev/xive/xive-internal.h
> +++ b/arch/powerpc/sysdev/xive/xive-internal.h
> @@ -5,6 +5,8 @@
> #ifndef __XIVE_INTERNAL_H
> #define __XIVE_INTERNAL_H
>
> +#define XIVE_IPI_HW_IRQ 0 /* interrupt source # for IPIs */
> +
> /*
> * A "disabled" interrupt should never fire, to catch problems
> * we set its logical number to this
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 65af34ac1fa2..ee375daf8114 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1142,7 +1142,7 @@ static void __init xive_request_ipi(void)
> return;
>
> /* Initialize it */
> - virq = irq_create_mapping(xive_irq_domain, 0);
> + virq = irq_create_mapping(xive_irq_domain, XIVE_IPI_HW_IRQ);
> xive_ipi_irq = virq;
>
> WARN_ON(request_irq(virq, xive_muxed_ipi_action,
> @@ -1242,7 +1242,7 @@ static int xive_irq_domain_map(struct irq_domain *h, unsigned int virq,
>
> #ifdef CONFIG_SMP
> /* IPIs are special and come up with HW number 0 */
> - if (hw == 0) {
> + if (hw == XIVE_IPI_HW_IRQ) {
> /*
> * IPIs are marked per-cpu. We use separate HW interrupts under
> * the hood but associated with the same "linux" interrupt
> @@ -1271,7 +1271,7 @@ static void xive_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
> if (!data)
> return;
> hw_irq = (unsigned int)irqd_to_hwirq(data);
> - if (hw_irq)
> + if (hw_irq != XIVE_IPI_HW_IRQ)
> xive_irq_free_data(virq);
> }
>
> @@ -1421,7 +1421,7 @@ static void xive_flush_cpu_queue(unsigned int cpu, struct xive_cpu *xc)
> * Ignore anything that isn't a XIVE irq and ignore
> * IPIs, so can just be dropped.
> */
> - if (d->domain != xive_irq_domain || hw_irq == 0)
> + if (d->domain != xive_irq_domain || hw_irq == XIVE_IPI_HW_IRQ)
> continue;
>
> /*
> @@ -1655,7 +1655,7 @@ static int xive_core_debug_show(struct seq_file *m, void *private)
> hw_irq = (unsigned int)irqd_to_hwirq(d);
>
> /* IPIs are special (HW number 0) */
> - if (hw_irq)
> + if (hw_irq != XIVE_IPI_HW_IRQ)
> xive_debug_show_irq(m, hw_irq, d);
> }
> return 0;
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: dt-bindings: imx-hdmi: Add binding doc for hdmi machine driver
From: Mark Brown @ 2020-12-08 17:09 UTC (permalink / raw)
To: Shengjiu Wang, timur, festevam, linux-kernel, alsa-devel,
linuxppc-dev, devicetree, perex, robh+dt, nicoleotsuka, Xiubo.Lee,
lgirdwood, tiwai
In-Reply-To: <1607251319-5821-1-git-send-email-shengjiu.wang@nxp.com>
On Sun, 6 Dec 2020 18:41:58 +0800, Shengjiu Wang wrote:
> Imx-hdmi is a new added machine driver for supporting hdmi devices
> on i.MX platforms. There is HDMI IP or external HDMI modules connect
> with SAI or AUD2HTX interface.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: dt-bindings: imx-hdmi: Add binding doc for hdmi machine driver
commit: e344cf5e4871f99495396f78d4401b8ac4c92465
[2/2] ASoC: fsl: Add imx-hdmi machine driver
commit: 6a5f850aa83a1d844d27e3e53ca2f247e55d438b
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH 08/13] powerpc: Increase NR_IRQS range to support more KVM guests
From: Greg Kurz @ 2020-12-08 17:23 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20201208151124.1329942-9-clg@kaod.org>
On Tue, 8 Dec 2020 16:11:19 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> PowerNV systems can handle up to 4K guests and 1M interrupt numbers
> per chip. Increase the range of allowed interrupts to support a larger
> number of guests.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 5181872f9452..c250fbd430d1 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -66,7 +66,7 @@ config NEED_PER_CPU_PAGE_FIRST_CHUNK
>
> config NR_IRQS
> int "Number of virtual interrupt numbers"
> - range 32 32768
> + range 32 1048576
> default "512"
> help
> This defines the number of virtual interrupt numbers the kernel
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/smp: Parse ibm,thread-groups with multiple properties
From: Gautham R Shenoy @ 2020-12-08 17:25 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R. Shenoy, Michael Neuling,
Vaidyanathan Srinivasan, Peter Zijlstra, linux-kernel,
Nicholas Piggin, linuxppc-dev, Valentin Schneider
In-Reply-To: <20201207121042.GH528281@linux.vnet.ibm.com>
Hello Srikar,
Thanks for taking a look at the patch.
On Mon, Dec 07, 2020 at 05:40:42PM +0530, Srikar Dronamraju wrote:
> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2020-12-04 10:18:45]:
>
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> <snipped>
>
> >
> > static int parse_thread_groups(struct device_node *dn,
> > - struct thread_groups *tg,
> > - unsigned int property)
> > + struct thread_groups_list *tglp)
> > {
> > - int i;
> > - u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
> > + int i = 0;
> > + u32 *thread_group_array;
> > u32 *thread_list;
> > size_t total_threads;
> > - int ret;
> > + int ret = 0, count;
> > + unsigned int property_idx = 0;
>
> NIT:
> tglx mentions in one of his recent comments to try keep a reverse fir tree
> ordering of variables where possible.
I suppose you mean moving the longer local variable declarations to to
the top and shorter ones to the bottom. Thanks. Will fix this.
>
> >
> > + count = of_property_count_u32_elems(dn, "ibm,thread-groups");
> > + thread_group_array = kcalloc(count, sizeof(u32), GFP_KERNEL);
> > ret = of_property_read_u32_array(dn, "ibm,thread-groups",
> > - thread_group_array, 3);
> > + thread_group_array, count);
> > if (ret)
> > - return ret;
> > -
> > - tg->property = thread_group_array[0];
> > - tg->nr_groups = thread_group_array[1];
> > - tg->threads_per_group = thread_group_array[2];
> > - if (tg->property != property ||
> > - tg->nr_groups < 1 ||
> > - tg->threads_per_group < 1)
> > - return -ENODATA;
> > + goto out_free;
> >
> > - total_threads = tg->nr_groups * tg->threads_per_group;
> > + while (i < count && property_idx < MAX_THREAD_GROUP_PROPERTIES) {
> > + int j;
> > + struct thread_groups *tg = &tglp->property_tgs[property_idx++];
>
> NIT: same as above.
Ok.
>
> >
> > - ret = of_property_read_u32_array(dn, "ibm,thread-groups",
> > - thread_group_array,
> > - 3 + total_threads);
> > - if (ret)
> > - return ret;
> > + tg->property = thread_group_array[i];
> > + tg->nr_groups = thread_group_array[i + 1];
> > + tg->threads_per_group = thread_group_array[i + 2];
> > + total_threads = tg->nr_groups * tg->threads_per_group;
> > +
> > + thread_list = &thread_group_array[i + 3];
> >
> > - thread_list = &thread_group_array[3];
> > + for (j = 0; j < total_threads; j++)
> > + tg->thread_list[j] = thread_list[j];
> > + i = i + 3 + total_threads;
>
> Can't we simply use memcpy instead?
We could. But this one makes it more explicit.
>
> > + }
> >
> > - for (i = 0 ; i < total_threads; i++)
> > - tg->thread_list[i] = thread_list[i];
> > + tglp->nr_properties = property_idx;
> >
> > - return 0;
> > +out_free:
> > + kfree(thread_group_array);
> > + return ret;
> > }
> >
> > /*
> > @@ -805,24 +827,39 @@ static int get_cpu_thread_group_start(int cpu, struct thread_groups *tg)
> > return -1;
> > }
> >
> > -static int init_cpu_l1_cache_map(int cpu)
> > +static int init_cpu_cache_map(int cpu, unsigned int cache_property)
> >
> > {
> > struct device_node *dn = of_get_cpu_node(cpu, NULL);
> > - struct thread_groups tg = {.property = 0,
> > - .nr_groups = 0,
> > - .threads_per_group = 0};
> > + struct thread_groups *tg = NULL;
> > int first_thread = cpu_first_thread_sibling(cpu);
> > int i, cpu_group_start = -1, err = 0;
> > + cpumask_var_t *mask;
> > + struct thread_groups_list *cpu_tgl = &tgl[cpu];
>
> NIT: same as 1st comment.
Sure, will fix this.
>
> >
> > if (!dn)
> > return -ENODATA;
> >
> > - err = parse_thread_groups(dn, &tg, THREAD_GROUP_SHARE_L1);
> > - if (err)
> > - goto out;
> > + if (!(cache_property == THREAD_GROUP_SHARE_L1))
> > + return -EINVAL;
> >
> > - cpu_group_start = get_cpu_thread_group_start(cpu, &tg);
> > + if (!cpu_tgl->nr_properties) {
> > + err = parse_thread_groups(dn, cpu_tgl);
> > + if (err)
> > + goto out;
> > + }
> > +
> > + for (i = 0; i < cpu_tgl->nr_properties; i++) {
> > + if (cpu_tgl->property_tgs[i].property == cache_property) {
> > + tg = &cpu_tgl->property_tgs[i];
> > + break;
> > + }
> > + }
> > +
> > + if (!tg)
> > + return -EINVAL;
> > +
> > + cpu_group_start = get_cpu_thread_group_start(cpu, tg);
>
> This whole hunk should be moved to a new function and called before
> init_cpu_cache_map. It will simplify the logic to great extent.
I suppose you are referring to the part where we select the correct
tg. Yeah, that can move to a different helper.
>
> >
> > if (unlikely(cpu_group_start == -1)) {
> > WARN_ON_ONCE(1);
> > @@ -830,11 +867,12 @@ static int init_cpu_l1_cache_map(int cpu)
> > goto out;
> > }
> >
> > - zalloc_cpumask_var_node(&per_cpu(cpu_l1_cache_map, cpu),
> > - GFP_KERNEL, cpu_to_node(cpu));
> > + mask = &per_cpu(cpu_l1_cache_map, cpu);
> > +
> > + zalloc_cpumask_var_node(mask, GFP_KERNEL, cpu_to_node(cpu));
> >
>
> This hunk (and the next hunk) should be moved to next patch.
>
The next patch is only about introducing THREAD_GROUP_SHARE_L2. Hence
I put in any other code in this patch, since it seems to be a logical
place to collate whatever we have in a generic form.
> > for (i = first_thread; i < first_thread + threads_per_core; i++) {
> > - int i_group_start = get_cpu_thread_group_start(i, &tg);
> > + int i_group_start = get_cpu_thread_group_start(i, tg);
> >
> > if (unlikely(i_group_start == -1)) {
> > WARN_ON_ONCE(1);
> > @@ -843,7 +881,7 @@ static int init_cpu_l1_cache_map(int cpu)
> > }
> >
> > if (i_group_start == cpu_group_start)
> > - cpumask_set_cpu(i, per_cpu(cpu_l1_cache_map, cpu));
> > + cpumask_set_cpu(i, *mask);
> > }
> >
> > out:
> > @@ -924,7 +962,7 @@ static int init_big_cores(void)
> > int cpu;
> >
> > for_each_possible_cpu(cpu) {
> > - int err = init_cpu_l1_cache_map(cpu);
> > + int err = init_cpu_cache_map(cpu, THREAD_GROUP_SHARE_L1);
> >
> > if (err)
> > return err;
> > --
> > 1.9.4
> >
>
> --
> Thanks and Regards
> Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH 09/13] powerpc/xive: Remove P9 DD1 flag XIVE_IRQ_FLAG_SHIFT_BUG
From: Greg Kurz @ 2020-12-08 17:39 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20201208151124.1329942-10-clg@kaod.org>
On Tue, 8 Dec 2020 16:11:20 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> This flag was used to support the PHB4 LSIs on P9 DD1 and we have
> stopped supporting this CPU when DD2 came out. See skiboot commit:
>
> https://github.com/open-power/skiboot/commit/0b0d15e3c170
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
Just a minor suggestion in case you need to post a v2. See below.
> arch/powerpc/include/asm/opal-api.h | 2 +-
> arch/powerpc/include/asm/xive.h | 2 +-
> arch/powerpc/kvm/book3s_xive_native.c | 3 ---
> arch/powerpc/kvm/book3s_xive_template.c | 3 ---
> arch/powerpc/sysdev/xive/common.c | 8 --------
> arch/powerpc/sysdev/xive/native.c | 2 --
> 6 files changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 1dffa3cb16ba..48ee604ca39a 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -1091,7 +1091,7 @@ enum {
> OPAL_XIVE_IRQ_TRIGGER_PAGE = 0x00000001,
> OPAL_XIVE_IRQ_STORE_EOI = 0x00000002,
> OPAL_XIVE_IRQ_LSI = 0x00000004,
> - OPAL_XIVE_IRQ_SHIFT_BUG = 0x00000008,
> + OPAL_XIVE_IRQ_SHIFT_BUG = 0x00000008, /* P9 DD1.0 workaround */
Maybe you can even comment the entire line so that any future
tentative to use that flag breaks build ?
> OPAL_XIVE_IRQ_MASK_VIA_FW = 0x00000010,
> OPAL_XIVE_IRQ_EOI_VIA_FW = 0x00000020,
> };
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index d332dd9a18de..ff805885a028 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -60,7 +60,7 @@ struct xive_irq_data {
> };
> #define XIVE_IRQ_FLAG_STORE_EOI 0x01
> #define XIVE_IRQ_FLAG_LSI 0x02
> -#define XIVE_IRQ_FLAG_SHIFT_BUG 0x04
> +#define XIVE_IRQ_FLAG_SHIFT_BUG 0x04 /* P9 DD1.0 workaround */
Same here, with an extra cleanup to stop using it when initializing
xive_irq_flags[] in common.c.
> #define XIVE_IRQ_FLAG_MASK_FW 0x08
> #define XIVE_IRQ_FLAG_EOI_FW 0x10
> #define XIVE_IRQ_FLAG_H_INT_ESB 0x20
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 9b395381179d..170d1d04e1d1 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -37,9 +37,6 @@ static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32 offset)
> * ordering.
> */
>
> - if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> - offset |= offset << 4;
> -
> val = in_be64(xd->eoi_mmio + offset);
> return (u8)val;
> }
> diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
> index 4ad3c0279458..ece36e024a8f 100644
> --- a/arch/powerpc/kvm/book3s_xive_template.c
> +++ b/arch/powerpc/kvm/book3s_xive_template.c
> @@ -61,9 +61,6 @@ static u8 GLUE(X_PFX,esb_load)(struct xive_irq_data *xd, u32 offset)
> if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
> offset |= XIVE_ESB_LD_ST_MO;
>
> - if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> - offset |= offset << 4;
> -
> val =__x_readq(__x_eoi_page(xd) + offset);
> #ifdef __LITTLE_ENDIAN__
> val >>= 64-8;
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 411cba12d73b..a9259470bf9f 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -200,10 +200,6 @@ static notrace u8 xive_esb_read(struct xive_irq_data *xd, u32 offset)
> if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
> offset |= XIVE_ESB_LD_ST_MO;
>
> - /* Handle HW errata */
> - if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> - offset |= offset << 4;
> -
> if ((xd->flags & XIVE_IRQ_FLAG_H_INT_ESB) && xive_ops->esb_rw)
> val = xive_ops->esb_rw(xd->hw_irq, offset, 0, 0);
> else
> @@ -214,10 +210,6 @@ static notrace u8 xive_esb_read(struct xive_irq_data *xd, u32 offset)
>
> static void xive_esb_write(struct xive_irq_data *xd, u32 offset, u64 data)
> {
> - /* Handle HW errata */
> - if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> - offset |= offset << 4;
> -
> if ((xd->flags & XIVE_IRQ_FLAG_H_INT_ESB) && xive_ops->esb_rw)
> xive_ops->esb_rw(xd->hw_irq, offset, data, 1);
> else
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 5f1e5aed8ab4..0310783241b5 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -64,8 +64,6 @@ int xive_native_populate_irq_data(u32 hw_irq, struct xive_irq_data *data)
> data->flags |= XIVE_IRQ_FLAG_STORE_EOI;
> if (opal_flags & OPAL_XIVE_IRQ_LSI)
> data->flags |= XIVE_IRQ_FLAG_LSI;
> - if (opal_flags & OPAL_XIVE_IRQ_SHIFT_BUG)
> - data->flags |= XIVE_IRQ_FLAG_SHIFT_BUG;
> if (opal_flags & OPAL_XIVE_IRQ_MASK_VIA_FW)
> data->flags |= XIVE_IRQ_FLAG_MASK_FW;
> if (opal_flags & OPAL_XIVE_IRQ_EOI_VIA_FW)
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/smp: Add support detecting thread-groups sharing L2 cache
From: Gautham R Shenoy @ 2020-12-08 17:42 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R. Shenoy, Michael Neuling,
Vaidyanathan Srinivasan, Peter Zijlstra, linux-kernel,
Nicholas Piggin, linuxppc-dev, Valentin Schneider
In-Reply-To: <20201207124039.GI528281@linux.vnet.ibm.com>
Hello Srikar,
On Mon, Dec 07, 2020 at 06:10:39PM +0530, Srikar Dronamraju wrote:
> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2020-12-04 10:18:46]:
>
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> >
> > On POWER systems, groups of threads within a core sharing the L2-cache
> > can be indicated by the "ibm,thread-groups" property array with the
> > identifier "2".
> >
> > This patch adds support for detecting this, and when present, populate
> > the populating the cpu_l2_cache_mask of every CPU to the core-siblings
> > which share L2 with the CPU as specified in the by the
> > "ibm,thread-groups" property array.
> >
> > On a platform with the following "ibm,thread-group" configuration
> > 00000001 00000002 00000004 00000000
> > 00000002 00000004 00000006 00000001
> > 00000003 00000005 00000007 00000002
> > 00000002 00000004 00000000 00000002
> > 00000004 00000006 00000001 00000003
> > 00000005 00000007
> >
> > Without this patch, the sched-domain hierarchy for CPUs 0,1 would be
> > CPU0 attaching sched-domain(s):
> > domain-0: span=0,2,4,6 level=SMT
> > domain-1: span=0-7 level=CACHE
> > domain-2: span=0-15,24-39,48-55 level=MC
> > domain-3: span=0-55 level=DIE
> >
> > CPU1 attaching sched-domain(s):
> > domain-0: span=1,3,5,7 level=SMT
> > domain-1: span=0-7 level=CACHE
> > domain-2: span=0-15,24-39,48-55 level=MC
> > domain-3: span=0-55 level=DIE
> >
> > The CACHE domain at 0-7 is incorrect since the ibm,thread-groups
> > sub-array
> > [00000002 00000002 00000004
> > 00000000 00000002 00000004 00000006
> > 00000001 00000003 00000005 00000007]
> > indicates that L2 (Property "2") is shared only between the threads of a single
> > group. There are "2" groups of threads where each group contains "4"
> > threads each. The groups being {0,2,4,6} and {1,3,5,7}.
> >
> > With this patch, the sched-domain hierarchy for CPUs 0,1 would be
> > CPU0 attaching sched-domain(s):
> > domain-0: span=0,2,4,6 level=SMT
> > domain-1: span=0-15,24-39,48-55 level=MC
> > domain-2: span=0-55 level=DIE
> >
> > CPU1 attaching sched-domain(s):
> > domain-0: span=1,3,5,7 level=SMT
> > domain-1: span=0-15,24-39,48-55 level=MC
> > domain-2: span=0-55 level=DIE
> >
> > The CACHE domain with span=0,2,4,6 for CPU 0 (span=1,3,5,7 for CPU 1
> > resp.) gets degenerated into the SMT domain. Furthermore, the
> > last-level-cache domain gets correctly set to the SMT sched-domain.
> >
> > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/smp.c | 66 +++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 58 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 6a242a3..a116d2d 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -76,6 +76,7 @@
> > struct task_struct *secondary_current;
> > bool has_big_cores;
> > bool coregroup_enabled;
> > +bool thread_group_shares_l2;
>
> Either keep this as static in this patch or add its declaration
>
This will be used in Patch 3 in kernel/cacheinfo.c, but not any other
place. Hence I am not making it static here.
> >
> > DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
> > DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
> > @@ -99,6 +100,7 @@ enum {
> >
> > #define MAX_THREAD_LIST_SIZE 8
> > #define THREAD_GROUP_SHARE_L1 1
> > +#define THREAD_GROUP_SHARE_L2 2
> > struct thread_groups {
> > unsigned int property;
> > unsigned int nr_groups;
> > @@ -107,7 +109,7 @@ struct thread_groups {
> > };
> >
> > /* Maximum number of properties that groups of threads within a core can share */
> > -#define MAX_THREAD_GROUP_PROPERTIES 1
> > +#define MAX_THREAD_GROUP_PROPERTIES 2
> >
> > struct thread_groups_list {
> > unsigned int nr_properties;
> > @@ -121,6 +123,13 @@ struct thread_groups_list {
> > */
> > DEFINE_PER_CPU(cpumask_var_t, cpu_l1_cache_map);
> >
> > +/*
> > + * On some big-cores system, thread_group_l2_cache_map for each CPU
> > + * corresponds to the set its siblings within the core that share the
> > + * L2-cache.
> > + */
> > +DEFINE_PER_CPU(cpumask_var_t, thread_group_l2_cache_map);
> > +
>
> NIT:
> We are trying to confuse ourselves with the names.
> For L1 we have cpu_l2_cache_map to store the tasks from the thread group.
> but cpu_smallcore_map for keeping track of tasks.
>
I suppose you mean cpu_l1_cache_map here. We are using
cpu_smallcore_map, because when the ibm,thread-groups-property=1, it
shares both L1 and the instruction data flow (SMT). We already have a
cpu_smt_map, hence, this was named cpu_smallcore_map a couple of years
ago when I wrote that patch.
> For L2 we have thread_group_l2_cache_map to store the tasks from the thread
> group. but cpu_l2_cache_map for keeping track of tasks.
>
> I think we should do some renaming to keep the names consistent.
> I would say probably say move the current cpu_l2_cache_map to
> cpu_llc_cache_map and move the new aka thread_group_l2_cache_map as
> cpu_l2_cache_map to be somewhat consistent.
Hmm.. cpu_llc_cache_map is still very generic. We want to have
something that defines l2 map.
I agree that we need to keep it consistent. How about renaming
cpu_l1_cache_map to thread_groups_l1_cache_map ?
That way thread_groups_l1_cache_map and thread_groups_l2_cache_map
refer to the corresponding L1 and L2 siblings as discovered from
ibm,thread-groups property.
We have the cpu_smallcore_mask and the cpu_l2_cache_map unchanged as
it was before.
>
> > /* SMP operations for this machine */
> > struct smp_ops_t *smp_ops;
> >
> > @@ -840,7 +851,8 @@ static int init_cpu_cache_map(int cpu, unsigned int cache_property)
> > if (!dn)
> > return -ENODATA;
> >
> > - if (!(cache_property == THREAD_GROUP_SHARE_L1))
> > + if (!(cache_property == THREAD_GROUP_SHARE_L1 ||
> > + cache_property == THREAD_GROUP_SHARE_L2))
> > return -EINVAL;
> >
> > if (!cpu_tgl->nr_properties) {
> > @@ -867,7 +879,10 @@ static int init_cpu_cache_map(int cpu, unsigned int cache_property)
> > goto out;
> > }
> >
> > - mask = &per_cpu(cpu_l1_cache_map, cpu);
> > + if (cache_property == THREAD_GROUP_SHARE_L1)
> > + mask = &per_cpu(cpu_l1_cache_map, cpu);
> > + else if (cache_property == THREAD_GROUP_SHARE_L2)
> > + mask = &per_cpu(thread_group_l2_cache_map, cpu);
> >
> > zalloc_cpumask_var_node(mask, GFP_KERNEL, cpu_to_node(cpu));
> >
> > @@ -973,6 +988,16 @@ static int init_big_cores(void)
> > }
> >
> > has_big_cores = true;
> > +
> > + for_each_possible_cpu(cpu) {
> > + int err = init_cpu_cache_map(cpu, THREAD_GROUP_SHARE_L2);
> > +
> > + if (err)
> > + return err;
> > + }
> > +
> > + thread_group_shares_l2 = true;
>
> Why do we need a separate loop. Why cant we merge this in the above loop
> itself?
No, there are platforms where one THREAD_GROUP_SHARE_L1 exists while
THREAD_GROUP_SHARE_L2 doesn't exist. It becomes easier if these are
separately tracked. Also, what do we gain if we put this in the same
loop? It will be (nr_possible_cpus * 2 * invocations of
init_cpu_cache_map()) as opposed to 2 * (nr_possible_cpus *
invocations of init_cpu_cache_map()). Isn't it ?
>
> > + pr_info("Thread-groups in a core share L2-cache\n");
>
> Can this be moved to a pr_debug? Does it help any regular user/admins to
> know if thread-groups shared l2 cache. Infact it may confuse users on what
> thread groups are and which thread groups dont share cache.
> I would prefer some other name than thread_group_shares_l2 but dont know any
> better alternatives and may be my choices are even worse.
Would you be ok with "L2 cache shared by threads of the small core" ?
>
> > return 0;
> > }
> >
> > @@ -1287,6 +1312,31 @@ static bool update_mask_by_l2(int cpu, cpumask_var_t *mask)
> > if (has_big_cores)
> > submask_fn = cpu_smallcore_mask;
> >
> > +
>
> NIT: extra blank line?
Will remove this.
>
> > + /*
> > + * If the threads in a thread-group share L2 cache, then then
> > + * the L2-mask can be obtained from thread_group_l2_cache_map.
> > + */
> > + if (thread_group_shares_l2) {
> > + /* Siblings that share L1 is a subset of siblings that share L2.*/
> > + or_cpumasks_related(cpu, cpu, submask_fn, cpu_l2_cache_mask);
> > + if (*mask) {
> > + cpumask_andnot(*mask,
> > + per_cpu(thread_group_l2_cache_map, cpu),
> > + cpu_l2_cache_mask(cpu));
> > + } else {
> > + mask = &per_cpu(thread_group_l2_cache_map, cpu);
> > + }
> > +
> > + for_each_cpu(i, *mask) {
> > + if (!cpu_online(i))
> > + continue;
> > + set_cpus_related(i, cpu, cpu_l2_cache_mask);
> > + }
> > +
> > + return true;
> > + }
> > +
>
> Ah this can be simplified to:
> if (thread_group_shares_l2) {
> cpumask_set_cpu(cpu, cpu_l2_cache_mask(cpu));
>
> for_each_cpu(i, per_cpu(thread_group_l2_cache_map, cpu)) {
> if (cpu_online(i))
> set_cpus_related(i, cpu, cpu_l2_cache_mask);
> }
Don't we want to enforce that the siblings sharing L1 be a subset of
the siblings sharing L2 ? Or do you recommend putting in a check for
that somewhere ?
> }
>
> No?
>
> > l2_cache = cpu_to_l2cache(cpu);
> > if (!l2_cache || !*mask) {
> > /* Assume only core siblings share cache with this CPU */
>
> --
> Thanks and Regards
> Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/cacheinfo: Print correct cache-sibling map/list for L2 cache
From: Gautham R Shenoy @ 2020-12-08 17:56 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R. Shenoy, Michael Neuling,
Vaidyanathan Srinivasan, Peter Zijlstra, linux-kernel,
Nicholas Piggin, linuxppc-dev, Valentin Schneider
In-Reply-To: <20201207131138.GJ528281@linux.vnet.ibm.com>
On Mon, Dec 07, 2020 at 06:41:38PM +0530, Srikar Dronamraju wrote:
> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2020-12-04 10:18:47]:
>
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> >
> >
> > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > ---
> >
> > +extern bool thread_group_shares_l2;
> > /*
> > * On big-core systems, each core has two groups of CPUs each of which
> > * has its own L1-cache. The thread-siblings which share l1-cache with
> > * @cpu can be obtained via cpu_smallcore_mask().
> > + *
> > + * On some big-core systems, the L2 cache is shared only between some
> > + * groups of siblings. This is already parsed and encoded in
> > + * cpu_l2_cache_mask().
> > */
> > static const struct cpumask *get_big_core_shared_cpu_map(int cpu, struct cache *cache)
> > {
> > if (cache->level == 1)
> > return cpu_smallcore_mask(cpu);
> > + if (cache->level == 2 && thread_group_shares_l2)
> > + return cpu_l2_cache_mask(cpu);
> >
> > return &cache->shared_cpu_map;
>
> As pointed with lkp@intel.org, we need to do this only with #CONFIG_SMP,
> even for cache->level = 1 too.
Yes, I have fixed that in the next version.
>
> I agree that we are displaying shared_cpu_map correctly. Should we have also
> update /clear shared_cpu_map in the first place. For example:- If for a P9
> core with CPUs 0-7, the cache->shared_cpu_map for L1 would have 0-7 but
> would display 0,2,4,6.
>
> The drawback of this is even if cpus 0,2,4,6 are released L1 cache will not
> be released. Is this as expected?
cacheinfo populates the cache->shared_cpu_map on the basis of which
CPUs share the common device-tree node for a particular cache. There
is one l1-cache object in the device-tree for a CPU node corresponding
to a big-core. That the L1 is further split between the threads of the
core is shown using ibm,thread-groups.
The ideal thing would be to add a "group_leader" field to "struct
cache" so that we can create separate cache objects , one per thread
group. I will take a stab at this in the v2.
Thanks for the review comments.
>
>
> --
> Thanks and Regards
> Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/rtas: Restrict RTAS requests from userspace
From: Tyrel Datwyler @ 2020-12-08 18:59 UTC (permalink / raw)
To: Andrew Donnellan, linuxppc-dev; +Cc: nathanl, leobras.c, stable, dja
In-Reply-To: <20200820044512.7543-1-ajd@linux.ibm.com>
On 8/19/20 9:45 PM, Andrew Donnellan wrote:
> A number of userspace utilities depend on making calls to RTAS to retrieve
> information and update various things.
>
> The existing API through which we expose RTAS to userspace exposes more
> RTAS functionality than we actually need, through the sys_rtas syscall,
> which allows root (or anyone with CAP_SYS_ADMIN) to make any RTAS call they
> want with arbitrary arguments.
>
> Many RTAS calls take the address of a buffer as an argument, and it's up to
> the caller to specify the physical address of the buffer as an argument. We
> allocate a buffer (the "RMO buffer") in the Real Memory Area that RTAS can
> access, and then expose the physical address and size of this buffer in
> /proc/powerpc/rtas/rmo_buffer. Userspace is expected to read this address,
> poke at the buffer using /dev/mem, and pass an address in the RMO buffer to
> the RTAS call.
>
> However, there's nothing stopping the caller from specifying whatever
> address they want in the RTAS call, and it's easy to construct a series of
> RTAS calls that can overwrite arbitrary bytes (even without /dev/mem
> access).
>
> Additionally, there are some RTAS calls that do potentially dangerous
> things and for which there are no legitimate userspace use cases.
>
> In the past, this would not have been a particularly big deal as it was
> assumed that root could modify all system state freely, but with Secure
> Boot and lockdown we need to care about this.
>
> We can't fundamentally change the ABI at this point, however we can address
> this by implementing a filter that checks RTAS calls against a list
> of permitted calls and forces the caller to use addresses within the RMO
> buffer.
>
> The list is based off the list of calls that are used by the librtas
> userspace library, and has been tested with a number of existing userspace
> RTAS utilities. For compatibility with any applications we are not aware of
> that require other calls, the filter can be turned off at build time.
>
> Reported-by: Daniel Axtens <dja@axtens.net>
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
>
> ---
> v1->v2:
> - address comments from mpe
> - shorten the names of some struct members
> - make the filter array static/ro_after_init, use const char *
> - genericise the fixed buffer size cases
> - simplify/get rid of some of the error printing
> - get rid of rtas_token_name()
> ---
> arch/powerpc/Kconfig | 13 ++++
> arch/powerpc/kernel/rtas.c | 153 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 166 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 1f48bbfb3ce9..8dd42b82379b 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -989,6 +989,19 @@ config PPC_SECVAR_SYSFS
> read/write operations on these variables. Say Y if you have
> secure boot enabled and want to expose variables to userspace.
>
> +config PPC_RTAS_FILTER
> + bool "Enable filtering of RTAS syscalls"
> + default y
> + depends on PPC_RTAS
> + help
> + The RTAS syscall API has security issues that could be used to
> + compromise system integrity. This option enforces restrictions on the
> + RTAS calls and arguments passed by userspace programs to mitigate
> + these issues.
> +
> + Say Y unless you know what you are doing and the filter is causing
> + problems for you.
> +
> endmenu
>
> config ISA_DMA_API
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 806d554ce357..954f41676f69 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -992,6 +992,147 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
> return NULL;
> }
>
> +#ifdef CONFIG_PPC_RTAS_FILTER
> +
> +/*
> + * The sys_rtas syscall, as originally designed, allows root to pass
> + * arbitrary physical addresses to RTAS calls. A number of RTAS calls
> + * can be abused to write to arbitrary memory and do other things that
> + * are potentially harmful to system integrity, and thus should only
> + * be used inside the kernel and not exposed to userspace.
> + *
> + * All known legitimate users of the sys_rtas syscall will only ever
> + * pass addresses that fall within the RMO buffer, and use a known
> + * subset of RTAS calls.
> + *
> + * Accordingly, we filter RTAS requests to check that the call is
> + * permitted, and that provided pointers fall within the RMO buffer.
> + * The rtas_filters list contains an entry for each permitted call,
> + * with the indexes of the parameters which are expected to contain
> + * addresses and sizes of buffers allocated inside the RMO buffer.
> + */
> +struct rtas_filter {
> + const char *name;
> + int token;
> + /* Indexes into the args buffer, -1 if not used */
> + int buf_idx1;
> + int size_idx1;
> + int buf_idx2;
> + int size_idx2;
> +
> + int fixed_size;
> +};
> +
> +static struct rtas_filter rtas_filters[] __ro_after_init = {
> + { "ibm,activate-firmware", -1, -1, -1, -1, -1 },
> + { "ibm,configure-connector", -1, 0, -1, 1, -1, 4096 }, /* Special cased */
> + { "display-character", -1, -1, -1, -1, -1 },
> + { "ibm,display-message", -1, 0, -1, -1, -1 },
> + { "ibm,errinjct", -1, 2, -1, -1, -1, 1024 },
> + { "ibm,close-errinjct", -1, -1, -1, -1, -1 },
> + { "ibm,open-errinct", -1, -1, -1, -1, -1 },
There is a typo here. Should be ibm,open-errinjct.
kernel: [ 1100.408626] sys_rtas: RTAS call blocked - exploit attempt?
kernel: [ 1100.408631] sys_rtas: token=0x26, nargs=0 (called by errinjct)
Which is producing this when trying to invoke the errinjct tool.
I'll send a fixes patch out shortly.
-Tyrel
> + { "ibm,get-config-addr-info2", -1, -1, -1, -1, -1 },
> + { "ibm,get-dynamic-sensor-state", -1, 1, -1, -1, -1 },
> + { "ibm,get-indices", -1, 2, 3, -1, -1 },
> + { "get-power-level", -1, -1, -1, -1, -1 },
> + { "get-sensor-state", -1, -1, -1, -1, -1 },
> + { "ibm,get-system-parameter", -1, 1, 2, -1, -1 },
> + { "get-time-of-day", -1, -1, -1, -1, -1 },
> + { "ibm,get-vpd", -1, 0, -1, 1, 2 },
> + { "ibm,lpar-perftools", -1, 2, 3, -1, -1 },
> + { "ibm,platform-dump", -1, 4, 5, -1, -1 },
> + { "ibm,read-slot-reset-state", -1, -1, -1, -1, -1 },
> + { "ibm,scan-log-dump", -1, 0, 1, -1, -1 },
> + { "ibm,set-dynamic-indicator", -1, 2, -1, -1, -1 },
> + { "ibm,set-eeh-option", -1, -1, -1, -1, -1 },
> + { "set-indicator", -1, -1, -1, -1, -1 },
> + { "set-power-level", -1, -1, -1, -1, -1 },
> + { "set-time-for-power-on", -1, -1, -1, -1, -1 },
> + { "ibm,set-system-parameter", -1, 1, -1, -1, -1 },
> + { "set-time-of-day", -1, -1, -1, -1, -1 },
> + { "ibm,suspend-me", -1, -1, -1, -1, -1 },
> + { "ibm,update-nodes", -1, 0, -1, -1, -1, 4096 },
> + { "ibm,update-properties", -1, 0, -1, -1, -1, 4096 },
> + { "ibm,physical-attestation", -1, 0, 1, -1, -1 },
> +};
> +
> +static bool in_rmo_buf(u32 base, u32 end)
> +{
> + return base >= rtas_rmo_buf &&
> + base < (rtas_rmo_buf + RTAS_RMOBUF_MAX) &&
> + base <= end &&
> + end >= rtas_rmo_buf &&
> + end < (rtas_rmo_buf + RTAS_RMOBUF_MAX);
> +}
> +
> +static bool block_rtas_call(int token, int nargs,
> + struct rtas_args *args)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(rtas_filters); i++) {
> + struct rtas_filter *f = &rtas_filters[i];
> + u32 base, size, end;
> +
> + if (token != f->token)
> + continue;
> +
> + if (f->buf_idx1 != -1) {
> + base = be32_to_cpu(args->args[f->buf_idx1]);
> + if (f->size_idx1 != -1)
> + size = be32_to_cpu(args->args[f->size_idx1]);
> + else if (f->fixed_size)
> + size = f->fixed_size;
> + else
> + size = 1;
> +
> + end = base + size - 1;
> + if (!in_rmo_buf(base, end))
> + goto err;
> + }
> +
> + if (f->buf_idx2 != -1) {
> + base = be32_to_cpu(args->args[f->buf_idx2]);
> + if (f->size_idx2 != -1)
> + size = be32_to_cpu(args->args[f->size_idx2]);
> + else if (f->fixed_size)
> + size = f->fixed_size;
> + else
> + size = 1;
> + end = base + size - 1;
> +
> + /*
> + * Special case for ibm,configure-connector where the
> + * address can be 0
> + */
> + if (!strcmp(f->name, "ibm,configure-connector") &&
> + base == 0)
> + return false;
> +
> + if (!in_rmo_buf(base, end))
> + goto err;
> + }
> +
> + return false;
> + }
> +
> +err:
> + pr_err_ratelimited("sys_rtas: RTAS call blocked - exploit attempt?\n");
> + pr_err_ratelimited("sys_rtas: token=0x%x, nargs=%d (called by %s)\n",
> + token, nargs, current->comm);
> + return true;
> +}
> +
> +#else
> +
> +static bool block_rtas_call(int token, int nargs,
> + struct rtas_args *args)
> +{
> + return false;
> +}
> +
> +#endif /* CONFIG_PPC_RTAS_FILTER */
> +
> /* We assume to be passed big endian arguments */
> SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
> {
> @@ -1029,6 +1170,9 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
> args.rets = &args.args[nargs];
> memset(args.rets, 0, nret * sizeof(rtas_arg_t));
>
> + if (block_rtas_call(token, nargs, &args))
> + return -EINVAL;
> +
> /* Need to handle ibm,suspend_me call specially */
> if (token == ibm_suspend_me_token) {
>
> @@ -1090,6 +1234,9 @@ void __init rtas_initialize(void)
> unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
> u32 base, size, entry;
> int no_base, no_size, no_entry;
> +#ifdef CONFIG_PPC_RTAS_FILTER
> + int i;
> +#endif
>
> /* Get RTAS dev node and fill up our "rtas" structure with infos
> * about it.
> @@ -1129,6 +1276,12 @@ void __init rtas_initialize(void)
> #ifdef CONFIG_RTAS_ERROR_LOGGING
> rtas_last_error_token = rtas_token("rtas-last-error");
> #endif
> +
> +#ifdef CONFIG_PPC_RTAS_FILTER
> + for (i = 0; i < ARRAY_SIZE(rtas_filters); i++) {
> + rtas_filters[i].token = rtas_token(rtas_filters[i].name);
> + }
> +#endif
> }
>
> int __init early_init_dt_scan_rtas(unsigned long node,
>
^ permalink raw reply
* Re: [PATCH 02/20] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram
From: Li Yang @ 2020-12-08 19:14 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Vladimir Oltean, lkml, Netdev, Jakub Kicinski, linuxppc-dev,
David S. Miller, Zhao Qiang
In-Reply-To: <20201205191744.7847-3-rasmus.villemoes@prevas.dk>
On Sat, Dec 5, 2020 at 1:21 PM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> Table 8-53 in the QUICC Engine Reference manual shows definitions of
> fields up to a size of 192 bytes, not just 128. But in table 8-111,
> one does find the text
>
> Base Address of the Global Transmitter Parameter RAM Page. [...]
> The user needs to allocate 128 bytes for this page. The address must
> be aligned to the page size.
>
> I've checked both rev. 7 (11/2015) and rev. 9 (05/2018) of the manual;
> they both have this inconsistency (and the table numbers are the
> same).
This does seem to be an inconsistency. I will try to see if I can
find someone who is familiar with this as this is really an old IP.
Figure 8-61 does mention that size = 128 byte + 64 byte if .... But
this part is not clear also. Not sure if the size of the parameter
RAM is really conditional.
>
> Adding a bit of debug printing, on my board the struct
> ucc_geth_tx_global_pram is allocated at offset 0x880, while
> the (opaque) ucc_geth_thread_data_tx gets allocated immediately
> afterwards, at 0x900. So whatever the engine writes into the thread
> data overlaps with the tail of the global tx pram (and devmem says
> that something does get written during a simple ping).
The overlapping does seem to be a problem. Maybe these global
parameters are not sampled at runtime or the parameter RAM is really
only using 128byte depending on the operation mode.
Are you getting useful information by reading from the additional 64
bytes, or getting changed behavior for setting these bytes after your
changes?
>
> I haven't observed any failure that could be attributed to this, but
> it seems to be the kind of thing that would be extremely hard to
> debug. So extend the struct definition so that we do allocate 192
> bytes.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> drivers/net/ethernet/freescale/ucc_geth.h | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
> index 3fe903972195..c80bed2c995c 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.h
> +++ b/drivers/net/ethernet/freescale/ucc_geth.h
> @@ -575,7 +575,14 @@ struct ucc_geth_tx_global_pram {
> u32 vtagtable[0x8]; /* 8 4-byte VLAN tags */
> u32 tqptr; /* a base pointer to the Tx Queues Memory
> Region */
> - u8 res2[0x80 - 0x74];
> + u8 res2[0x78 - 0x74];
> + u64 snums_en;
> + u32 l2l3baseptr; /* top byte consists of a few other bit fields */
> +
> + u16 mtu[8];
> + u8 res3[0xa8 - 0x94];
> + u32 wrrtablebase; /* top byte is reserved */
> + u8 res4[0xc0 - 0xac];
> } __packed;
>
> /* structure representing Extended Filtering Global Parameters in PRAM */
> --
> 2.23.0
>
^ permalink raw reply
* Re: [PATCH] drivers: usb: gadget: prefer pr_*() functions over raw printk()
From: Laurent Pinchart @ 2020-12-08 15:54 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: balbi, linux-usb, linuxppc-dev, linux-kernel, leoyang.li
In-Reply-To: <20201208144403.22097-1-info@metux.net>
Hi Enrico,
Thank you for the patch.
On Tue, Dec 08, 2020 at 03:44:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> Reduce a bit logging boilerplate by using the preferred pr_*()
> macros instead of raw printk().
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
> drivers/usb/gadget/function/uvc.h | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 2 +-
> drivers/usb/gadget/udc/fsl_udc_core.c | 4 +--
> drivers/usb/gadget/udc/fsl_usb2_udc.h | 4 +--
> drivers/usb/gadget/udc/fusb300_udc.c | 64 ++++++++++++++++-----------------
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.h | 2 +-
> 7 files changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> index 23ee25383c1f..d546eb7c348c 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -49,7 +49,7 @@ extern unsigned int uvc_gadget_trace_param;
> #define uvc_trace(flag, msg...) \
> do { \
> if (uvc_gadget_trace_param & flag) \
> - printk(KERN_DEBUG "uvcvideo: " msg); \
> + pr_debug("uvcvideo: " msg); \
> } while (0)
>
> #define uvcg_dbg(f, fmt, args...) \
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 2b893bceea45..4834fafb3f70 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1573,7 +1573,7 @@ static void usba_control_irq(struct usba_udc *udc, struct usba_ep *ep)
> * generate or receive a reply right away. */
> usba_ep_writel(ep, CLR_STA, USBA_RX_SETUP);
>
> - /* printk(KERN_DEBUG "setup: %d: %02x.%02x\n",
> + /* pr_debug("setup: %d: %02x.%02x\n",
> ep->state, crq.crq.bRequestType,
> crq.crq.bRequest); */
I wonder if this shouldn't be dropped instead, commented-out code isn't
very useful.
>
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
> index ad6ff9c4188e..cab4def04f9f 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -1474,7 +1474,7 @@ __acquires(udc->lock)
> mdelay(10);
> tmp = fsl_readl(&dr_regs->portsc1) | (ptc << 16);
> fsl_writel(tmp, &dr_regs->portsc1);
> - printk(KERN_INFO "udc: switch to test mode %d.\n", ptc);
> + pr_info("udc: switch to test mode %d.\n", ptc);
> }
>
> return;
> @@ -1952,7 +1952,7 @@ static int fsl_udc_start(struct usb_gadget *g,
> if (!IS_ERR_OR_NULL(udc_controller->transceiver)) {
> /* Suspend the controller until OTG enable it */
> udc_controller->stopped = 1;
> - printk(KERN_INFO "Suspend udc for OTG auto detect\n");
> + pr_info("Suspend udc for OTG auto detect\n");
>
> /* connect to bus through transceiver */
> if (!IS_ERR_OR_NULL(udc_controller->transceiver)) {
> diff --git a/drivers/usb/gadget/udc/fsl_usb2_udc.h b/drivers/usb/gadget/udc/fsl_usb2_udc.h
> index 4ba651ae9048..b180bf14dd0c 100644
> --- a/drivers/usb/gadget/udc/fsl_usb2_udc.h
> +++ b/drivers/usb/gadget/udc/fsl_usb2_udc.h
> @@ -509,7 +509,7 @@ struct fsl_udc {
> /*-------------------------------------------------------------------------*/
>
> #ifdef DEBUG
> -#define DBG(fmt, args...) printk(KERN_DEBUG "[%s] " fmt "\n", \
> +#define DBG(fmt, args...) pr_debug("[%s] " fmt "\n", \
> __func__, ## args)
> #else
> #define DBG(fmt, args...) do{}while(0)
> @@ -535,7 +535,7 @@ static void dump_msg(const char *label, const u8 * buf, unsigned int length)
> p += 3;
> }
> *p = 0;
> - printk(KERN_DEBUG "%6x: %s\n", start, line);
> + pr_debug("%6x: %s\n", start, line);
> buf += num;
> start += num;
> length -= num;
> diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
> index 9af8b415f303..c4e7e4b8e46f 100644
> --- a/drivers/usb/gadget/udc/fusb300_udc.c
> +++ b/drivers/usb/gadget/udc/fusb300_udc.c
> @@ -352,24 +352,24 @@ static void fusb300_wrcxf(struct fusb300_ep *ep,
> for (i = length >> 2; i > 0; i--) {
> data = *tmp | *(tmp + 1) << 8 | *(tmp + 2) << 16 |
> *(tmp + 3) << 24;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> tmp = tmp + 4;
> }
> switch (length % 4) {
> case 1:
> data = *tmp;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> break;
> case 2:
> data = *tmp | *(tmp + 1) << 8;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> break;
> case 3:
> data = *tmp | *(tmp + 1) << 8 | *(tmp + 2) << 16;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> break;
> default:
> @@ -390,7 +390,7 @@ static void fusb300_clear_epnstall(struct fusb300 *fusb300, u8 ep)
> u32 reg = ioread32(fusb300->reg + FUSB300_OFFSET_EPSET0(ep));
>
> if (reg & FUSB300_EPSET0_STL) {
> - printk(KERN_DEBUG "EP%d stall... Clear!!\n", ep);
> + pr_debug("EP%d stall... Clear!!\n", ep);
> reg |= FUSB300_EPSET0_STL_CLR;
> iowrite32(reg, fusb300->reg + FUSB300_OFFSET_EPSET0(ep));
> }
> @@ -402,7 +402,7 @@ static void ep0_queue(struct fusb300_ep *ep, struct fusb300_request *req)
> if (req->req.length) {
> fusb300_wrcxf(ep, req);
> } else
> - printk(KERN_DEBUG "%s : req->req.length = 0x%x\n",
> + pr_debug("%s : req->req.length = 0x%x\n",
> __func__, req->req.length);
> if ((req->req.length == req->req.actual) ||
> (req->req.actual < ep->ep.maxpacket))
> @@ -565,7 +565,7 @@ static void fusb300_rdcxf(struct fusb300 *fusb300,
>
> for (i = (length >> 2); i > 0; i--) {
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> *(tmp + 1) = (data >> 8) & 0xFF;
> *(tmp + 2) = (data >> 16) & 0xFF;
> @@ -576,18 +576,18 @@ static void fusb300_rdcxf(struct fusb300 *fusb300,
> switch (length % 4) {
> case 1:
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> break;
> case 2:
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> *(tmp + 1) = (data >> 8) & 0xFF;
> break;
> case 3:
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> *(tmp + 1) = (data >> 8) & 0xFF;
> *(tmp + 2) = (data >> 16) & 0xFF;
> @@ -610,7 +610,7 @@ static void fusb300_rdfifo(struct fusb300_ep *ep,
> req->req.actual += length;
>
> if (req->req.actual > req->req.length)
> - printk(KERN_DEBUG "req->req.actual > req->req.length\n");
> + pr_debug("req->req.actual > req->req.length\n");
>
> for (i = (length >> 2); i > 0; i--) {
> data = ioread32(fusb300->reg +
> @@ -649,7 +649,7 @@ static void fusb300_rdfifo(struct fusb300_ep *ep,
> reg = ioread32(fusb300->reg + FUSB300_OFFSET_IGR1);
> reg &= FUSB300_IGR1_SYNF0_EMPTY_INT;
> if (i)
> - printk(KERN_INFO "sync fifo is not empty!\n");
> + pr_info("sync fifo is not empty!\n");
> i++;
> } while (!reg);
> }
> @@ -677,7 +677,7 @@ static u8 fusb300_get_cxstall(struct fusb300 *fusb300)
> static void request_error(struct fusb300 *fusb300)
> {
> fusb300_set_cxstall(fusb300);
> - printk(KERN_DEBUG "request error!!\n");
> + pr_debug("request error!!\n");
> }
>
> static void get_status(struct fusb300 *fusb300, struct usb_ctrlrequest *ctrl)
> @@ -999,7 +999,7 @@ static void check_device_mode(struct fusb300 *fusb300)
> fusb300->gadget.speed = USB_SPEED_UNKNOWN;
> break;
> }
> - printk(KERN_INFO "dev_mode = %d\n", (reg & FUSB300_GCR_DEVEN_MSK));
> + pr_info("dev_mode = %d\n", (reg & FUSB300_GCR_DEVEN_MSK));
> }
>
>
> @@ -1076,14 +1076,14 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_WARM_RST_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_WARM_RST_INT);
> - printk(KERN_INFO"fusb300_warmreset\n");
> + pr_info("fusb300_warmreset\n");
> fusb300_reset();
> }
>
> if (int_grp1 & FUSB300_IGR1_HOT_RST_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_HOT_RST_INT);
> - printk(KERN_INFO"fusb300_hotreset\n");
> + pr_info("fusb300_hotreset\n");
> fusb300_reset();
> }
>
> @@ -1097,13 +1097,13 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_CX_COMABT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_CX_COMABT_INT);
> - printk(KERN_INFO"fusb300_ep0abt\n");
> + pr_info("fusb300_ep0abt\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_VBUS_CHG_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_VBUS_CHG_INT);
> - printk(KERN_INFO"fusb300_vbus_change\n");
> + pr_info("fusb300_vbus_change\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U3_EXIT_FAIL_INT) {
> @@ -1134,25 +1134,25 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_U3_EXIT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U3_EXIT_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U3_EXIT_INT\n");
> + pr_info("FUSB300_IGR1_U3_EXIT_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U2_EXIT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U2_EXIT_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U2_EXIT_INT\n");
> + pr_info("FUSB300_IGR1_U2_EXIT_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U1_EXIT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U1_EXIT_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U1_EXIT_INT\n");
> + pr_info("FUSB300_IGR1_U1_EXIT_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U3_ENTRY_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U3_ENTRY_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U3_ENTRY_INT\n");
> + pr_info("FUSB300_IGR1_U3_ENTRY_INT\n");
> fusb300_enable_bit(fusb300, FUSB300_OFFSET_SSCR1,
> FUSB300_SSCR1_GO_U3_DONE);
> }
> @@ -1160,31 +1160,31 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_U2_ENTRY_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U2_ENTRY_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U2_ENTRY_INT\n");
> + pr_info("FUSB300_IGR1_U2_ENTRY_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U1_ENTRY_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U1_ENTRY_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U1_ENTRY_INT\n");
> + pr_info("FUSB300_IGR1_U1_ENTRY_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_RESM_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_RESM_INT);
> - printk(KERN_INFO "fusb300_resume\n");
> + pr_info("fusb300_resume\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_SUSP_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_SUSP_INT);
> - printk(KERN_INFO "fusb300_suspend\n");
> + pr_info("fusb300_suspend\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_HS_LPM_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_HS_LPM_INT);
> - printk(KERN_INFO "fusb300_HS_LPM_INT\n");
> + pr_info("fusb300_HS_LPM_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_DEV_MODE_CHG_INT) {
> @@ -1195,11 +1195,11 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
>
> if (int_grp1 & FUSB300_IGR1_CX_COMFAIL_INT) {
> fusb300_set_cxstall(fusb300);
> - printk(KERN_INFO "fusb300_ep0fail\n");
> + pr_info("fusb300_ep0fail\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_CX_SETUP_INT) {
> - printk(KERN_INFO "fusb300_ep0setup\n");
> + pr_info("fusb300_ep0setup\n");
> if (setup_packet(fusb300, &ctrl)) {
> spin_unlock(&fusb300->lock);
> if (fusb300->driver->setup(&fusb300->gadget, &ctrl) < 0)
> @@ -1209,16 +1209,16 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> }
>
> if (int_grp1 & FUSB300_IGR1_CX_CMDEND_INT)
> - printk(KERN_INFO "fusb300_cmdend\n");
> + pr_info("fusb300_cmdend\n");
>
>
> if (int_grp1 & FUSB300_IGR1_CX_OUT_INT) {
> - printk(KERN_INFO "fusb300_cxout\n");
> + pr_info("fusb300_cxout\n");
> fusb300_ep0out(fusb300);
> }
>
> if (int_grp1 & FUSB300_IGR1_CX_IN_INT) {
> - printk(KERN_INFO "fusb300_cxin\n");
> + pr_info("fusb300_cxin\n");
> fusb300_ep0in(fusb300);
> }
>
> diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
> index 3e1267d38774..4f225552861a 100644
> --- a/drivers/usb/gadget/udc/goku_udc.c
> +++ b/drivers/usb/gadget/udc/goku_udc.c
> @@ -1748,7 +1748,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> int retval;
>
> if (!pdev->irq) {
> - printk(KERN_ERR "Check PCI %s IRQ setup!\n", pci_name(pdev));
> + pr_err("Check PCI %s IRQ setup!\n", pci_name(pdev));
When a pointer to a struct device is available, dev_err() would be much
better. That's however out of scope for this patch, but it would be nice
to address it. This would become
dev_err(&pdev->dev, "Check IRQ setup!\n");
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> retval = -ENODEV;
> goto err;
> }
> diff --git a/drivers/usb/gadget/udc/r8a66597-udc.h b/drivers/usb/gadget/udc/r8a66597-udc.h
> index 9a115caba661..fa4d62c32ea1 100644
> --- a/drivers/usb/gadget/udc/r8a66597-udc.h
> +++ b/drivers/usb/gadget/udc/r8a66597-udc.h
> @@ -247,7 +247,7 @@ static inline u16 get_xtal_from_pdata(struct r8a66597_platdata *pdata)
> clock = XTAL48;
> break;
> default:
> - printk(KERN_ERR "r8a66597: platdata clock is wrong.\n");
> + pr_err("r8a66597: platdata clock is wrong.\n");
> break;
> }
>
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] powerpc/rtas: fix typo of ibm,open-errinjct in rtas filter
From: Tyrel Datwyler @ 2020-12-08 19:54 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, linux-kernel, Tyrel Datwyler
Commit bd59380c5ba4 ("powerpc/rtas: Restrict RTAS requests from userspace")
introduced the following error when invoking the errinjct userspace
tool.
[root@ltcalpine2-lp5 librtas]# errinjct open
[327884.071171] sys_rtas: RTAS call blocked - exploit attempt?
[327884.071186] sys_rtas: token=0x26, nargs=0 (called by errinjct)
errinjct: Could not open RTAS error injection facility
errinjct: librtas: open: Unexpected I/O error
The entry for ibm,open-errinjct in rtas_filter array has a typo where
the "j" is omitted in the rtas call name. After fixing this typo the
errinjct tool functions again as expected.
[root@ltcalpine2-lp5 linux]# errinjct open
RTAS error injection facility open, token = 1
fixes: bd59380c5ba4 ("powerpc/rtas: Restrict RTAS requests from userspace")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
arch/powerpc/kernel/rtas.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 954f41676f69..cccb32cf0e08 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1030,7 +1030,7 @@ static struct rtas_filter rtas_filters[] __ro_after_init = {
{ "ibm,display-message", -1, 0, -1, -1, -1 },
{ "ibm,errinjct", -1, 2, -1, -1, -1, 1024 },
{ "ibm,close-errinjct", -1, -1, -1, -1, -1 },
- { "ibm,open-errinct", -1, -1, -1, -1, -1 },
+ { "ibm,open-errinjct", -1, -1, -1, -1, -1 },
{ "ibm,get-config-addr-info2", -1, -1, -1, -1, -1 },
{ "ibm,get-dynamic-sensor-state", -1, 1, -1, -1, -1 },
{ "ibm,get-indices", -1, 2, 3, -1, -1 },
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 02/20] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram
From: Rasmus Villemoes @ 2020-12-08 20:12 UTC (permalink / raw)
To: Li Yang
Cc: Vladimir Oltean, lkml, Netdev, Jakub Kicinski, linuxppc-dev,
David S. Miller, Zhao Qiang
In-Reply-To: <CADRPPNTgqwd37VSqiUcv2otGVr4mnQbuv6r887w_yCp=ha1dvA@mail.gmail.com>
On 08/12/2020 20.14, Li Yang wrote:
> On Sat, Dec 5, 2020 at 1:21 PM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> Table 8-53 in the QUICC Engine Reference manual shows definitions of
>> fields up to a size of 192 bytes, not just 128. But in table 8-111,
>> one does find the text
>>
>> Base Address of the Global Transmitter Parameter RAM Page. [...]
>> The user needs to allocate 128 bytes for this page. The address must
>> be aligned to the page size.
>>
>> I've checked both rev. 7 (11/2015) and rev. 9 (05/2018) of the manual;
>> they both have this inconsistency (and the table numbers are the
>> same).
>
> This does seem to be an inconsistency. I will try to see if I can
> find someone who is familiar with this as this is really an old IP.
>
> Figure 8-61 does mention that size = 128 byte + 64 byte if .... But
> this part is not clear also.
Hm, indeed, that sentence is simply cut short, it literally says
"Additional 64 bytes are needed if". The next line contains
"Hierarchical Scheduler, or IP" in a smaller font, but that seems to be
a label for the arrow.
>
> The overlapping does seem to be a problem. Maybe these global
> parameters are not sampled at runtime or the parameter RAM is really
> only using 128byte depending on the operation mode.
Yes, I'm thinking something like that is likely to be the case, since
this hasn't seemed to ever cause any problems. But who knows, maybe a
few frames just get fragmented very occasionally becauces the MTU0 field
spuriously has some random small value.
>
> Are you getting useful information by reading from the additional 64
> bytes,
AFAICT, after the additional allocation, the extra 64 bytes stay at 0,
but that's to be expected; they are supposed to be written by the CPU
and read by the engine AFAIU.
or getting changed behavior for setting these bytes after your
> changes?
No, as I said:
>> I haven't observed any failure that could be attributed to this,
I haven't played around with explicitly writing to those 64 bytes after
initialization. This whole series started because I searched for the
string "MTU" in the manual, but at the end, it didn't seem that I
actually needed to modify those MTU fields.
Rasmus
^ permalink raw reply
* Re: [PATCH 18/20] ethernet: ucc_geth: add helper to replace repeated switch statements
From: Rasmus Villemoes @ 2020-12-08 20:55 UTC (permalink / raw)
To: Christophe Leroy, Li Yang, David S. Miller, Jakub Kicinski
Cc: Vladimir Oltean, Zhao Qiang, linuxppc-dev, linux-kernel, netdev
In-Reply-To: <ed16ea1d-5017-96bd-c1a9-5201f51231fd@csgroup.eu>
On 08/12/2020 16.21, Christophe Leroy wrote:
>
>
> Le 05/12/2020 à 20:17, Rasmus Villemoes a écrit :
>> The translation from the ucc_geth_num_of_threads enum value to the
>> actual count can be written somewhat more compactly with a small
>> lookup table, allowing us to replace the four switch statements.
>>
> I think you would allow GCC to provide a much better optimisation with
> something like:
>
Your version compiles to 120 bytes of object code, mine around 49
(including the 5 byte lookup table). They're about the same in line count.
Rasmus
^ permalink raw reply
* Re: [PATCH v2 0/5] drop unused BACKLIGHT_GENERIC option
From: Arnd Bergmann @ 2020-12-08 21:04 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Alexandre Belloni, Sam Ravnborg, Tony Lindgren,
linux-kernel@vger.kernel.org, James Bottomley, Chen-Yu Tsai,
Thierry Reding, Paul Mackerras, Will Deacon, Daniel Thompson,
Helge Deller, Russell King - ARM Linux, Krzysztof Kozlowski,
Jonathan Hunter, Ludovic Desroches, Catalin Marinas,
open list:BROADCOM NVRAM DRIVER, Arnd Bergmann, Maxime Ripard,
Andrey Zhizhikin, SoC Team, open list:TEGRA ARCHITECTURE SUPPORT,
Lee Jones, linux-omap, Linux ARM, Jernej Skrabec, Parisc List,
Emil Velikov, Nicolas Ferre, linuxppc-dev
In-Reply-To: <20201208170021.GA6168@alpha.franken.de>
On Tue, Dec 8, 2020 at 6:00 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
> On Tue, Dec 08, 2020 at 05:34:46PM +0100, Arnd Bergmann wrote:
> > [3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
> > commit: 2257682282531de45929c6006152f6e2ee881b42
>
> this one is already in mips-next.
Ok, dropped from my tree,
Arnd
^ permalink raw reply
* Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Bjorn Helgaas @ 2020-12-08 21:06 UTC (permalink / raw)
To: Michael Walle
Cc: kw, heiko, shawn.lin, paulus, thomas.petazzoni, jonnyc, toan,
will, robh, lorenzo.pieralisi, michal.simek, linux-rockchip,
bcm-kernel-feedback-list, linux-arm-kernel, linux-pci, Qian Cai,
rjui, f.fainelli, linux-rpi-kernel, Jonathan.Cameron, bhelgaas,
jonathan.derrick, sbranden, wangzhou1, rrichter, linuxppc-dev,
nsaenzjulienne
In-Reply-To: <20201208154150.20978-1-michael@walle.cc>
[+cc Qian]
On Tue, Dec 08, 2020 at 04:41:50PM +0100, Michael Walle wrote:
> Hi Lorenzo, Krzysztof,
>
> >On Sun, 29 Nov 2020 23:07:38 +0000, Krzysztof Wilczyński wrote:
> >> Unify ECAM-related constants into a single set of standard constants
> >> defining memory address shift values for the byte-level address that can
> >> be used when accessing the PCI Express Configuration Space, and then
> >> move native PCI Express controller drivers to use newly introduced
> >> definitions retiring any driver-specific ones.
> >>
> >> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
> >> PCI Express specification (see PCI Express Base Specification, Revision
> >> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
> >> implement it the same way.
> >>
> >> [...]
> >
> >Applied to pci/ecam, thanks!
> >
> >[1/5] PCI: Unify ECAM constants in native PCI Express drivers
> > https://git.kernel.org/lpieralisi/pci/c/f3c07cf692
> >[2/5] PCI: thunder-pem: Add constant for custom ".bus_shift" initialiser
> > https://git.kernel.org/lpieralisi/pci/c/3c38579263
> >[3/5] PCI: iproc: Convert to use the new ECAM constants
> > https://git.kernel.org/lpieralisi/pci/c/333ec9d3cc
> >[4/5] PCI: vmd: Update type of the __iomem pointers
> > https://git.kernel.org/lpieralisi/pci/c/89094c12ea
> >[5/5] PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
> > https://git.kernel.org/lpieralisi/pci/c/3dc62532a5
>
> Patch 1/5 breaks LS1028A boards:
I temporarily dropped this series while we figure out what went wrong
here.
Bjorn
^ 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