Re: [RFC PATCH v2 01/31] Documentation: kvx: Add basic documentation

2023-01-23 Thread Bagas Sanjaya
On Fri, Jan 20, 2023 at 03:09:32PM +0100, Yann Sionneau wrote:
> Add some documentation for the kvx architecture and its Linux port.

"Document the kvx Linux port. The documentation covers design decision,
memory management, exception handling, and SMP."

>  Documentation/arch.rst   |   1 +
>  Documentation/kvx/index.rst  |  17 ++
>  Documentation/kvx/kvx-exceptions.rst | 256 
>  Documentation/kvx/kvx-iommu.rst  | 191 ++
>  Documentation/kvx/kvx-mmu.rst| 287 +++
>  Documentation/kvx/kvx-smp.rst|  39 
>  Documentation/kvx/kvx.rst| 273 +
>  7 files changed, 1064 insertions(+)
>  create mode 100644 Documentation/kvx/index.rst
>  create mode 100644 Documentation/kvx/kvx-exceptions.rst
>  create mode 100644 Documentation/kvx/kvx-iommu.rst
>  create mode 100644 Documentation/kvx/kvx-mmu.rst
>  create mode 100644 Documentation/kvx/kvx-smp.rst
>  create mode 100644 Documentation/kvx/kvx.rst
> 

The documentation reads a rather odd and unclear to me, so I have to
write the improv:

 >8 
diff --git a/Documentation/kvx/kvx-exceptions.rst 
b/Documentation/kvx/kvx-exceptions.rst
index 5e01e934192f13..efb162edadb6a0 100644
--- a/Documentation/kvx/kvx-exceptions.rst
+++ b/Documentation/kvx/kvx-exceptions.rst
@@ -1,9 +1,9 @@
-==
-Exceptions
-==
+=
+Exception handling in kvx
+=
 
 On kvx, handlers are set using ``$ev`` (exception vector) register which
-specifies a base address. An offset is added to ``$ev`` upon exception
+specifies the base address. An offset is added to ``$ev`` upon exception
 and the result is used as the new ``$pc``.
 The offset depends on which exception vector the cpu wants to jump to:
 
@@ -35,12 +35,13 @@ Interrupts and traps are serviced similarly, ie:
 
  - Jump to handler
  - Save all registers
- - Prepare the call (do_IRQ or trap_handler)
+ - Prepare the call (``do_IRQ`` or ``trap_handler``)
  - restore all registers
  - return from exception
 
-entry.S file is (as for other architectures) the entry point into the kernel.
-It contains all assembly routines related to interrupts/traps/syscall.
+As in other architectures, ``entry.S`` file is the entry point into the
+kernel. It contains all assembly routines related to
+interrupts/traps/syscall.
 
 Syscall handling
 
@@ -51,7 +52,7 @@ a syscall from the kernel.
 
 Syscalls are handled differently than interrupts/exceptions. From an ABI
 point of view, syscalls are like function calls: any caller-saved register
-can be clobbered by the syscall. However, syscall parameters are passed
+can be clobberred by the syscall. However, syscall parameters are passed
 using registers r0 through r7. These registers must be preserved to avoid
 clobberring them before the actual syscall function.
 
@@ -59,23 +60,23 @@ On syscall from userspace (``scall`` instruction), the 
processor will put
 the syscall number in $es.sn and switch from user to kernel privilege
 mode. ``kvx_syscall_handler`` will be called in kernel mode.
 
-The following steps are then taken:
+Below is the path when executing syscall:
 
- 1. Switch to kernel stack
- 2. Extract syscall number
- 3. If the syscall number is bogus, set the syscall function to 
``sys_ni_syscall``
- 4. If tracing is enabled
+ 1. Switch to kernel stack.
+ 2. Extract syscall number. If it is bogus, set the syscall function to
+``sys_ni_syscall``.
+ 3. If tracing is enabled:
 
-- Jump to ``trace_syscall_enter``
+- Jump to ``trace_syscall_enter``.
 - Save syscall arguments (``r0`` -> ``r7``) on stack in ``pt_regs``.
 - Call ``do_trace_syscall_enter`` function.
 
- 5. Restore syscall arguments since they have been modified by C call
- 6. Call the syscall function
- 7. Save ``$r0`` in ``pt_regs`` since it can be clobberred afterward
- 8. If tracing is enabled, call ``trace_syscall_exit``.
- 9. Call ``work_pending``
- 10. Return to user !
+ 4. Restore syscall arguments since they have been modified by C call.
+ 5. Call the syscall function.
+ 6. Save ``$r0`` in ``pt_regs`` since it can be clobberred afterward.
+ 7. If tracing is enabled, call ``trace_syscall_exit``.
+ 8. Call ``work_pending``.
+ 9. Return to user.
 
 The trace call is handled out of the fast path. All slow path handling
 is done in another part of code to avoid messing with the cache.
@@ -84,25 +85,25 @@ Signals
 ---
 
 Signals are handled when exiting kernel before returning to user.
-When handling a signal, the path is the following:
+When handling a signal, the execution path is:
 
- 1. User application is executing normally
-Then any exception happens (syscall, interrupt, trap)
- 2. The exception handling path is taken
-and before returning to user, pending signals are checked
- 3. Signal are handled by ``do_signal``.
-Registers are saved and a special part of the stack is modified
+ 1. Us

Re: [RFC PATCH v2 31/31] kvx: Add IPI driver

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:10, Yann Sionneau wrote:
> +
> +int __init kvx_ipi_ctrl_probe(irqreturn_t (*ipi_irq_handler)(int, void *))
> +{
> + struct device_node *np;
> + int ret;
> + unsigned int ipi_irq;
> + void __iomem *ipi_base;
> +
> + np = of_find_compatible_node(NULL, NULL, "kalray,kvx-ipi-ctrl");

Nope, big no.

Drivers go to drivers, not to arch code. Use proper driver infrastructure.

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 27/31] kvx: Add kvx default config file

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:09, Yann Sionneau wrote:
> Add a default config file for kvx based Coolidge SoC.
> 
> Co-developed-by: Ashley Lesdalons 
> Signed-off-by: Ashley Lesdalons 
> Co-developed-by: Benjamin Mugnier 
> Signed-off-by: Benjamin Mugnier 
> Co-developed-by: Clement Leger 
> Signed-off-by: Clement Leger 
> Co-developed-by: Guillaume Thouvenin 
> Signed-off-by: Guillaume Thouvenin 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Co-developed-by: Julian Vetter 
> Signed-off-by: Julian Vetter 
> Co-developed-by: Samuel Jones 
> Signed-off-by: Samuel Jones 
> Co-developed-by: Thomas Costis 
> Signed-off-by: Thomas Costis 
> Co-developed-by: Vincent Chardon 
> Signed-off-by: Vincent Chardon 
> Co-developed-by: Yann Sionneau 
> Signed-off-by: Yann Sionneau 
> ---
> 
> Notes:
> V1 -> V2: default_defconfig renamed to defconfig
> 
>  arch/kvx/configs/defconfig | 127 +
>  1 file changed, 127 insertions(+)
>  create mode 100644 arch/kvx/configs/defconfig
> 
> diff --git a/arch/kvx/configs/defconfig b/arch/kvx/configs/defconfig
> new file mode 100644
> index ..960784da0b1b
> --- /dev/null
> +++ b/arch/kvx/configs/defconfig
> @@ -0,0 +1,127 @@
> +CONFIG_DEFAULT_HOSTNAME="KVXlinux"
> +CONFIG_SERIAL_KVX_SCALL_COMM=y
> +CONFIG_CONFIGFS_FS=y
> +CONFIG_DEBUG_KERNEL=y
> +CONFIG_DEBUG_INFO=y
> +CONFIG_DEBUG_INFO_DWARF4=y
> +CONFIG_PRINTK_TIME=y
> +CONFIG_CONSOLE_LOGLEVEL_DEFAULT=15
> +CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> +CONFIG_PANIC_TIMEOUT=-1
> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_GDB_SCRIPTS=y
> +CONFIG_FRAME_POINTER=y
> +CONFIG_HZ_100=y
> +CONFIG_SERIAL_EARLYCON=y
> +CONFIG_HOTPLUG_PCI_PCIE=y
> +CONFIG_PCIEAER=y
> +CONFIG_PCIE_DPC=y
> +CONFIG_HOTPLUG_PCI=y
> +CONFIG_SERIAL_8250=y

Are you sure this is the result of savedefconfig? Order looks a bit odd
in several places, so I want to double check.

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 02/31] Documentation: Add binding for kalray,kv3-1-core-intc

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas 

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).


> 
> Add documentation for `kalray,kv3-1-core-intc` binding.
> 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Signed-off-by: Yann Sionneau 
> ---
> 
> Notes:
> V1 -> V2: new patch
> 
>  .../kalray,kv3-1-core-intc.yaml   | 46 +++
>  1 file changed, 46 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
>  
> b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
> new file mode 100644
> index ..1e3d0593173a
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-core-intc.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-core-intc#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray kv3-1 Core Interrupt Controller
> +
> +description: |
> +  The Kalray Core Interrupt Controller is tightly integrated in each kv3 core
> +  present in the Coolidge SoC.
> +
> +  It provides the following features:
> +  - 32 independent interrupt sources
> +  - 2-bit configurable priority level
> +  - 2-bit configurable ownership level
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +const: kalray,kv3-1-core-intc

Blank line between each of these,

> +  "#interrupt-cells":
> +const: 1
> +description:
> +  The IRQ number.
> +  reg:
> +maxItems: 0

??? No way... What's this?

> +  "kalray,intc-nr-irqs":

Drop quotes.

> +description: Number of irqs handled by the controller.

Why this is variable per board? Why do you need it ?

> +
> +required:
> +  - compatible
> +  - "#interrupt-cells"
> +  - interrupt-controller

missing additionalProperties: false

This binding looks poor, like you started from something odd. Please
don't. Take the newest reviewed binding or better example-schema and use
it to build yours. This would solve several trivial mistakes and style
issues.

> +
> +examples:
> +  - |
> +intc: interrupt-controller {

What's the IO address space?


Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 03/31] Documentation: Add binding for kalray,kv3-1-apic-gic

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas 
> 
> Add documentation for `kalray,kv3-1-apic-gic` binding.
> 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Signed-off-by: Yann Sionneau 
> ---

All the comments apply here and to all your other patches - wrong
subject, missing blank lines, missing additionalProperties, missing
tests (patches were for sure not tested as you can see from bot's
answers) etc. Really, please start from scratch on example-schema.

> 
> Notes:
> V1 -> V2: new patch
> 
>  .../kalray,kv3-1-apic-gic.yaml| 66 +++
>  1 file changed, 66 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-apic-gic.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-apic-gic.yaml
>  
> b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-apic-gic.yaml
> new file mode 100644
> index ..7a37f19db2fb
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/kalray,kv3-1-apic-gic.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/interrupt-controller/kalray,kv3-1-apic-gic#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray kv3-1 APIC-GIC
> +
> +description: |
> +  Each cluster in the Coolidge SoC includes an Advanced Programmable 
> Interrupt
> +  Controller (APIC) which is split in two part:
> +- a Generic Interrupt Controller (referred as APIC-GIC)
> +- a Mailbox Controller   (referred as APIC-Mailbox)
> +  The APIC-GIC acts as an intermediary interrupt controller, muxing/routing
> +  incoming interrupts to output interrupts connected to kvx cores interrupts 
> lines.
> +  The 139 possible input interrupt lines are organized as follow:
> + - 128 from the mailbox controller (one it per mailboxes)
> + - 1   from the NoC router
> + - 5   from IOMMUs
> + - 1   from L2 cache DMA job FIFO
> + - 1   from cluster watchdog
> + - 2   for SECC, DECC
> + - 1   from Data NoC
> +  The 72 possible output interrupt lines:
> + -  68 : 4 interrupts per cores (17 cores)
> + -  1 for L2 cache controller
> + -  3 extra that are for padding
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +const: kalray,kv3-1-apic-gic

Missing reg

> +  "#interrupt-cells":
> +const: 1
> +description:
> +  The IRQ number.
> +  interrupt-controller: true
> +  interrupt-parent: true

Drop, should not be needed.

> +  interrupts:
> +maxItems: 4
> +description: |
> + Specifies the interrupt line(s) in the interrupt-parent controller node;
> + valid values depend on the type of parent interrupt controller
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#interrupt-cells"
> +  - interrupt-controller
> +  - interrupt-parent
> +  - interrupts
> +
> +examples:
> +  - |
> +apic_gic: interrupt-controller@a2 {
> +compatible = "kalray,kv3-1-apic-gic";
> +reg = <0 0xa2 0 0x12000>;
> +#interrupt-cells = <1>;
> +interrupt-controller;
> +interrupt-parent = <&intc>;
> +interrups = <4 5 6 7>;
> +};
> +
> +...

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 06/31] Documentation: Add binding for kalray,kv3-1-ipi-ctrl

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas 
> 
> Add documentation for `kalray,kv3-1-ipi-ctrl` binding.
> 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Signed-off-by: Yann Sionneau 
> ---
> 
> Notes:
> V1 -> V2: new patch
> 
>  .../kalray/kalray,kv3-1-ipi-ctrl.yaml | 44 +++
>  1 file changed, 44 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/kalray/kalray,kv3-1-ipi-ctrl.yaml

Wrong directory. Interrupt controllers go to respective subsystem directory.

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 29/31] kvx: Add support for cpuinfo

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:10, Yann Sionneau wrote:
> +static int __init setup_cpuinfo(void)
> +{
> + int cpu;
> + struct clk *clk;
> + unsigned long cpu_freq = 10;
> + struct device_node *node = of_get_cpu_node(0, NULL);
> +
> + clk = of_clk_get(node, 0);
> + if (IS_ERR(clk)) {
> + printk(KERN_WARNING
> +"Device tree missing CPU 'clock' parameter. Assuming 
> frequency is 1GHZ");
> + goto setup_cpu_freq;
> + }
> +
> + cpu_freq = clk_get_rate(clk);

What about cpufreq? I don't think this is useful.

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 07/31] Documentation: Add binding for kalray,kv3-1-pwr-ctrl

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas 
> 
> Add documentation for `kalray,kv3-1-pwr-ctrl` binding.
> 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Signed-off-by: Yann Sionneau 
> ---
> 
> Notes:
> V1 -> V2: new patch
> 
>  .../kalray/kalray,kv3-1-pwr-ctrl.yaml | 29 +++
>  1 file changed, 29 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/kalray/kalray,kv3-1-pwr-ctrl.yaml

All the usual comments plus - wrong directory. Power controllers go to
respective power directory.

> 
> diff --git 
> a/Documentation/devicetree/bindings/kalray/kalray,kv3-1-pwr-ctrl.yaml 
> b/Documentation/devicetree/bindings/kalray/kalray,kv3-1-pwr-ctrl.yaml
> new file mode 100644
> index ..968674bb0c63
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/kalray/kalray,kv3-1-pwr-ctrl.yaml
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/kalray/kalray,kv3-1-pwr-ctrl#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray cluster Power Controller (pwr-ctrl)
> +
> +description: |
> +  The Power Controller (pwr-ctrl) control cores reset and wake-up procedure.
> +
> +properties:
> +  compatible:
> +const: kalray,kv3-1-pwr-ctrl
> +  reg:
> +maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +pwr_ctrl: power-controller@a4 {
> +compatible = "kalray,kv3-1-pwr-ctrl";
> +reg = <0x00 0xa4 0x00 0x4158>;

I really doubt that you tested it... Examples are not run with address
cells 2.


Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 20/31] kvx: Add memory management

2023-01-23 Thread Mike Rapoport
On Fri, Jan 20, 2023 at 03:09:51PM +0100, Yann Sionneau wrote:
> Add memory management support for kvx, including: cache and tlb
> management, page fault handling, ioremap/mmap and streaming dma support.
> 
> Co-developed-by: Clement Leger 
> Signed-off-by: Clement Leger 
> Co-developed-by: Guillaume Thouvenin 
> Signed-off-by: Guillaume Thouvenin 
> Co-developed-by: Jean-Christophe Pince 
> Signed-off-by: Jean-Christophe Pince 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Co-developed-by: Julian Vetter 
> Signed-off-by: Julian Vetter 
> Co-developed-by: Julien Hascoet 
> Signed-off-by: Julien Hascoet 
> Co-developed-by: Louis Morhet 
> Signed-off-by: Louis Morhet 
> Co-developed-by: Marc Poulhiès 
> Signed-off-by: Marc Poulhiès 
> Co-developed-by: Marius Gligor 
> Signed-off-by: Marius Gligor 
> Co-developed-by: Vincent Chardon 
> Signed-off-by: Vincent Chardon 
> Co-developed-by: Yann Sionneau 
> Signed-off-by: Yann Sionneau 
> ---
> 
> Notes:
> V1 -> V2: removed L2 cache management
> 
>  arch/kvx/include/asm/cache.h|  43 +++
>  arch/kvx/include/asm/cacheflush.h   | 158 ++
>  arch/kvx/include/asm/fixmap.h   |  47 +++
>  arch/kvx/include/asm/hugetlb.h  |  36 +++
>  arch/kvx/include/asm/mem_map.h  |  44 +++
>  arch/kvx/include/asm/mmu.h  | 289 ++
>  arch/kvx/include/asm/mmu_context.h  | 156 ++
>  arch/kvx/include/asm/mmu_stats.h|  38 +++
>  arch/kvx/include/asm/page.h | 187 
>  arch/kvx/include/asm/page_size.h|  29 ++
>  arch/kvx/include/asm/pgalloc.h  | 101 +++
>  arch/kvx/include/asm/pgtable-bits.h | 102 +++
>  arch/kvx/include/asm/pgtable.h  | 451 
>  arch/kvx/include/asm/rm_fw.h|  16 +
>  arch/kvx/include/asm/sparsemem.h|  15 +
>  arch/kvx/include/asm/symbols.h  |  16 +
>  arch/kvx/include/asm/tlb.h  |  24 ++
>  arch/kvx/include/asm/tlb_defs.h | 131 
>  arch/kvx/include/asm/tlbflush.h |  58 
>  arch/kvx/include/asm/vmalloc.h  |  10 +
>  arch/kvx/mm/cacheflush.c| 154 ++
>  arch/kvx/mm/dma-mapping.c   |  85 ++
>  arch/kvx/mm/extable.c   |  24 ++
>  arch/kvx/mm/fault.c | 264 
>  arch/kvx/mm/init.c  | 277 +
>  arch/kvx/mm/mmap.c  |  31 ++
>  arch/kvx/mm/mmu.c   | 202 +
>  arch/kvx/mm/mmu_stats.c |  94 ++
>  arch/kvx/mm/tlb.c   | 433 ++
>  29 files changed, 3515 insertions(+)
>  create mode 100644 arch/kvx/include/asm/cache.h
>  create mode 100644 arch/kvx/include/asm/cacheflush.h
>  create mode 100644 arch/kvx/include/asm/fixmap.h
>  create mode 100644 arch/kvx/include/asm/hugetlb.h
>  create mode 100644 arch/kvx/include/asm/mem_map.h
>  create mode 100644 arch/kvx/include/asm/mmu.h
>  create mode 100644 arch/kvx/include/asm/mmu_context.h
>  create mode 100644 arch/kvx/include/asm/mmu_stats.h
>  create mode 100644 arch/kvx/include/asm/page.h
>  create mode 100644 arch/kvx/include/asm/page_size.h
>  create mode 100644 arch/kvx/include/asm/pgalloc.h
>  create mode 100644 arch/kvx/include/asm/pgtable-bits.h
>  create mode 100644 arch/kvx/include/asm/pgtable.h
>  create mode 100644 arch/kvx/include/asm/rm_fw.h
>  create mode 100644 arch/kvx/include/asm/sparsemem.h
>  create mode 100644 arch/kvx/include/asm/symbols.h
>  create mode 100644 arch/kvx/include/asm/tlb.h
>  create mode 100644 arch/kvx/include/asm/tlb_defs.h
>  create mode 100644 arch/kvx/include/asm/tlbflush.h
>  create mode 100644 arch/kvx/include/asm/vmalloc.h
>  create mode 100644 arch/kvx/mm/cacheflush.c
>  create mode 100644 arch/kvx/mm/dma-mapping.c
>  create mode 100644 arch/kvx/mm/extable.c
>  create mode 100644 arch/kvx/mm/fault.c
>  create mode 100644 arch/kvx/mm/init.c
>  create mode 100644 arch/kvx/mm/mmap.c
>  create mode 100644 arch/kvx/mm/mmu.c
>  create mode 100644 arch/kvx/mm/mmu_stats.c
>  create mode 100644 arch/kvx/mm/tlb.c

... 

> diff --git a/arch/kvx/include/asm/mmu.h b/arch/kvx/include/asm/mmu.h
> new file mode 100644
> index ..09f3fdd66a34
> --- /dev/null
> +++ b/arch/kvx/include/asm/mmu.h
> @@ -0,0 +1,289 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2017-2023 Kalray Inc.
> + * Author(s): Guillaume Thouvenin
> + *Clement Leger
> + *Marc Poulhiès
> + */
> +
> +#ifndef _ASM_KVX_MMU_H
> +#define _ASM_KVX_MMU_H
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Virtual addresses can use at most 41 bits */
> +#define MMU_VIRT_BITS41
> +
> +/*
> + * See Documentation/kvx/kvx-mmu.txt for details about the division of the
> + * virtual memory space.
> + */
> +#if defined(CONFIG_KVX_4K_PAGES)
> +#define MMU_USR_ADDR_BITS39
> +#else
> +#error "Only 4Ko page size is suppo

Re: [RFC PATCH v2 30/31] kvx: Add power controller driver

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:10, Yann Sionneau wrote:
> From: Jules Maselbas 
> 
> The Power Controller (pwr-ctrl) control cores reset and wake-up
> procedure.


> +
> +static struct device_node * __init get_pwr_ctrl_node(void)
> +{
> + const phandle *ph;
> + struct device_node *cpu;
> + struct device_node *node;
> +
> + cpu = of_get_cpu_node(raw_smp_processor_id(), NULL);
> + if (!cpu) {
> + pr_err("Failed to get CPU node\n");
> + return NULL;
> + }
> +
> + ph = of_get_property(cpu, "power-controller", NULL);
> + if (!ph) {
> + pr_err("Failed to get power-controller phandle\n");
> + return NULL;
> + }
> +
> + node = of_find_node_by_phandle(be32_to_cpup(ph));
> + if (!node) {
> + pr_err("Failed to get power-controller node\n");
> + return NULL;
> + }
> +
> + return node;
> +}
> +
> +int __init kvx_pwr_ctrl_probe(void)
> +{
> + struct device_node *ctrl;
> +
> + ctrl = get_pwr_ctrl_node();
> + if (!ctrl) {
> + pr_err("Failed to get power controller node\n");
> + return -EINVAL;
> + }
> +
> + if (!of_device_is_compatible(ctrl, "kalray,kvx-pwr-ctrl")) {
> + pr_err("Failed to get power controller node\n");

No. Drivers go to drivers, not to arch directory. This should be a
proper driver instead of some fake stub doing its own driver matching.
You need to rework this.

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 05/31] Documentation: Add binding for kalray,coolidge-itgen

2023-01-23 Thread Krzysztof Kozlowski
On 20/01/2023 15:09, Yann Sionneau wrote:
> From: Jules Maselbas 
> 
> Add documentation for `kalray,coolidge-itgen` binding.
> 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Signed-off-by: Yann Sionneau 

The same comments apply plus more...

> ---
> 
> Notes:
> V1 -> V2: new patch
> 
>  .../kalray,coolidge-itgen.yaml| 48 +++
>  1 file changed, 48 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-itgen.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-itgen.yaml
>  
> b/Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-itgen.yaml
> new file mode 100644
> index ..47b503bff1d9
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/kalray,coolidge-itgen.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license. Checkpatch should complain about this - did you run it?

This applies to all your other patches (both, run checkpatch and use
proper license).


> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/interrupt-controller/kalray,coolidge-itgen#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kalray Coolidge SoC Interrupt Generator (ITGEN)
> +
> +description: |
> +  The Interrupt Generator (ITGEN) is an interrupt controller block.
> +  It's purpose is to convert IRQ lines coming from SoC peripherals into 
> writes
> +  on the AXI bus. The ITGEN intended purpose is to write into the APIC 
> mailboxes.
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +const: kalray,coolidge-itgen
> +

So why suddenly this patch has proper blank lines...

Missing reg.

> +  "#interrupt-cells":
> +const: 2
> +description: |
> +  - 1st cell is for the IRQ number
> +  - 2nd cell is for the trigger type as defined 
> dt-bindings/interrupt-controller/irq.h
> +
> +  interrupt-controller: true
> +
> +  msi-parent: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#interrupt-cells"
> +  - interrupt-controller
> +  - msi-parent
> +
> +examples:
> +  - |
> +itgen: interrupt-controller@2700 {
> +compatible = "kalray,coolidge-itgen";
> +reg = <0 0x2700 0 0x1104>;
> +#interrupt-cells = <2>;
> +interrupt-controller;
> +msi-parent = <&apic_mailbox>;
> +};
> +
> +...

Best regards,
Krzysztof

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit



Re: [RFC PATCH v2 01/31] Documentation: kvx: Add basic documentation

2023-01-23 Thread Mike Rapoport
Hi,

While reviewing kvx-mmu.rst I've spotted several places where the
documentation does not match the code, please make sure that the
documentation in this patch actually reflects what the code is doing.

Also, sectioning looked a bit strange to me, make sure to take a look at
the generated html and see if it is rendered as intended.

On Fri, Jan 20, 2023 at 03:09:32PM +0100, Yann Sionneau wrote:
> Add some documentation for the kvx architecture and its Linux port.
> 
> Co-developed-by: Clement Leger 
> Signed-off-by: Clement Leger 
> Co-developed-by: Jules Maselbas 
> Signed-off-by: Jules Maselbas 
> Co-developed-by: Guillaume Thouvenin 
> Signed-off-by: Guillaume Thouvenin 
> Signed-off-by: Yann Sionneau 
> ---
> 
> Notes:
> V1 -> V2:
>  - converted to .rst, typos and formatting fixes
>  - reword few paragraphs
> 
>  Documentation/arch.rst   |   1 +
>  Documentation/kvx/index.rst  |  17 ++
>  Documentation/kvx/kvx-exceptions.rst | 256 
>  Documentation/kvx/kvx-iommu.rst  | 191 ++
>  Documentation/kvx/kvx-mmu.rst| 287 +++
>  Documentation/kvx/kvx-smp.rst|  39 
>  Documentation/kvx/kvx.rst| 273 +
>  7 files changed, 1064 insertions(+)
>  create mode 100644 Documentation/kvx/index.rst
>  create mode 100644 Documentation/kvx/kvx-exceptions.rst
>  create mode 100644 Documentation/kvx/kvx-iommu.rst
>  create mode 100644 Documentation/kvx/kvx-mmu.rst
>  create mode 100644 Documentation/kvx/kvx-smp.rst
>  create mode 100644 Documentation/kvx/kvx.rst
> 
> diff --git a/Documentation/arch.rst b/Documentation/arch.rst
> index 41a66a8b38e4..1ccda8ef6eef 100644
> --- a/Documentation/arch.rst
> +++ b/Documentation/arch.rst
> @@ -13,6 +13,7 @@ implementation.
> arm/index
> arm64/index
> ia64/index
> +   kvx/index
> loongarch/index
> m68k/index
> mips/index

...

> diff --git a/Documentation/kvx/kvx-mmu.rst b/Documentation/kvx/kvx-mmu.rst
> new file mode 100644
> index ..b7186331396c
> --- /dev/null
> +++ b/Documentation/kvx/kvx-mmu.rst
> @@ -0,0 +1,287 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===
> +MMU
> +===
> +
> +Virtual addresses are on 41 bits for kvx when using 64-bit mode.

There is only 64-bit support at the moment, so no need to mention it.

> +To differentiate kernel from user space, we use the high order bit
> +(bit 40). When bit 40 is set, then the higher remaining bits must also be
> +set to 1. The virtual address must be extended with 1 when the bit 40 is set,
> +if not the address must be zero extended. Bit 40 is set for kernel space
> +mappings and not set for user space mappings.

I'd reorder this paragraph to first mention that bit 40 is set for kernel
space and then write about sign extension.

> +
> +Memory Map
> +--
> +
> +In Linux physical memories are arranged into banks according to the cost of 
> an
> +access in term of distance to a memory. As we are UMA architecture we only 
> have
> +one bank and thus one node.
> +
> +A node is divided into several kind of zone. For example if DMA can only 
> access
> +a specific area in the physical memory we will define a ZONE_DMA for this 
> purpose.
> +In our case we are considering that DMA can access all DDR so we don't have 
> a specific
> +zone for this. On 64 bit architecture all DDR can be mapped in virtual 
> kernel space
> +so there is no need for a ZONE_HIGHMEM. That means that in our case there is
> +only one ZONE_NORMAL. This will be updated if DMA cannot access all memory.

These two paragraphs have nothing to do with the virtual memory map. Please
drop them.

> +
> +Currently, the memory mapping is the following for 4KB page:
> +
> +   === == === 
> ==
> +  StartEnd Attr   SizeName
> +   === == === 
> ==
> +        003F   ---256GBUser
> +   0040     007F   ---256GB MMAP
> +   0080     FF7F   ------  Gap
> +   FF80        ---512GBKernel
> + FF80    FF8F  RWX64GB  Direct 
> Map
> + FF90    FF90 3FFF RWX1GB   Vmalloc
> + FF90 4000     RW 447GB Free area
> +   === == === 
> ==
> +
> +Enable the MMU
> +--
> +
> +All kernel functions and symbols are in virtual memory except for kvx_start()
> +function which is loaded at 0x0 in physical memory.
> +To be able to switch from physical addresses to virtual addresses we choose 
> to
> +setup the TLB at the very beginning of the boot process to be able to map 
> bo