Re: [RFC PATCH 03/25] kvx: Add build infrastructure

Jules Maselbas
2023-01-05T13:13:09Z

Hi,

On Tue, Jan 03, 2023 at 09:29:21AM -0800, Randy Dunlap wrote:
> Hi,
>
-- snip snip
> > +config ARCH_MMAP_RND_BITS_MAX
> > + default 24
> > +
>
> Do the above and below configs need an "int" line for their type?
> I notice that the patch does that below for PGTABLE_LEVELS.
>
I don't think it needs an "int" line, it is added in arch/Kconfig
(which includes arch/kvx/Kconfig)

> > +config ARCH_MMAP_RND_BITS_MIN
> > + default 18
> > +
> > +config STACKTRACE_SUPPORT
> > + def_bool y
> > +
> > +config LOCKDEP_SUPPORT
> > + def_bool y
> > +
> > +config GENERIC_BUG
> > + def_bool y
> > + depends on BUG
> > +
> > +config KVX_4K_PAGES
> > + def_bool y
> > +
> > +config KVX
> > + def_bool y
> [deletes]
> > +
> > +config PGTABLE_LEVELS
> > + int
> > + default 3
> ^^^^^^^^^^^^^^^^^^^^^^^^
I think the "int" here can be removed, as done for
ARCH_MMAP_RND_BITS_MAX and ARCH_MMAP_RND_BITS_MIN

> > +
> > +config HAVE_KPROBES
> > + def_bool n
> > +
> > +menu "System setup"
> > +
> > +config POISON_INITMEM
> > + bool "Enable to poison freed initmem"
> > + default y
> > + help
> > + In order to debug initmem, using poison allows to verify if
> > + some data/code is stille using them. Enable this for debug
>
> still
>
> > + purposes
>
> purposes.
>
> > +
> > +config KVX_PHYS_OFFSET
> > + hex "RAM address of memory base"
> > + default 0x100000000
> > +
> > +config KVX_PAGE_OFFSET
> > + hex "kernel virtual address of memory base"
> > + default 0xFFFFFF8000000000
> > +
> > +config ARCH_FLATMEM_ENABLE
> > + def_bool y
> > +
> > +config ARCH_SPARSEMEM_ENABLE
> > + def_bool y
> > +
> > +config ARCH_SPARSEMEM_DEFAULT
> > + def_bool ARCH_SPARSEMEM_ENABLE
> > +
> > +config ARCH_SELECT_MEMORY_MODEL
> > + def_bool ARCH_SPARSEMEM_ENABLE
> > +
> > +config STACK_MAX_DEPTH_TO_PRINT
> > + int "Maximum depth of stack to print"
> > + range 1 128
> > + default "24"
> > +
> > +config L2_CACHE
> > + bool "Enable l2 cache driver"
>
> L2
>
> > + default y
> > + help
> > + Enable L2 cache driver in order to handle it. This driver can be
> > + disabled to eliminate any overhead related to L2 cache
> > + inval/writeback. Note that L2 cache is mandatory for SMP in order
> > + to maintain coherency on DDR between all PEs.
> > +
> > +config SECURE_DAME_HANDLING
> > + bool "Secure DAME handling"
> > + default y
> > + help
> > + In order to securely handle Data Asynchronous Memory Errors, we need
> > + to do a barrier upon kernel entry when coming from userspace. This
> > + barrier guarantee us that any pending DAME will be serviced right
>
> guarantees
>
> > + away. We also need to do a barrier when returning from kernel to user.
> > + This way, if the kernel or the user triggered a DAME, it will be
> > + serviced by knowing we are coming from kernel or user and avoid
> > + pulling the wrong lever (panic for kernel or sigfault for user).
> > + This can be costly but ensure that user cannot interfere with kernel.
>
> ensures
>
> > + /!\ Do not disable unless you want to open a giant breach between
> > + user and kernel /!\
> > +
> > +config CACHECTL_UNSAFE_PHYS_OPERATIONS
> > + bool "Enable cachectl syscall unsafe physical operations"
> > + default n
> > + help
> > + Enable cachectl syscall to allow writebacking/invalidating ranges
> > + based on physical addresses. These operations requires the
> > + CAP_SYS_ADMIN capability
>
> capability.
>
> > +
> > +config ENABLE_TCA
> > + bool "Enable TCA coprocessor support"
> > + default y
> > + help
> > + This option enables TCA coprocessor support. It will allow the user to
> > + use the coprocessor and save registers on context switch if used.
> > + Registers content will also be cleared when switching.
> > +
> > +config SMP
> > + bool "Symmetric multi-processing support"
> > + default n
> > + select GENERIC_SMP_IDLE_THREAD
> > + select GENERIC_IRQ_IPI
> > + select IRQ_DOMAIN_HIERARCHY
> > + select IRQ_DOMAIN
> > + select L2_CACHE
> > + help
> > + This enables support for systems with more than one CPU. If you have
> > + a system with only one CPU, say N. If you have a system with more
> > + than one CPU, say Y.
> > +
> > + If you say N here, the kernel will run on uni- and multiprocessor
> > + machines, but will use only one CPU of a multiprocessor machine. If
> > + you say Y here, the kernel will run on many, but not all,
> > + uniprocessor machines. On a uniprocessor machine, the kernel
> > + will run faster if you say N here.
> > +
> > +config NR_CPUS
> > + int "Maximum number of CPUs"
> > + range 1 16
> > + default "16"
> > + depends on SMP
> > + help
> > + Kalray support can handle a maximum of 16 CPUs.
> > +
> > +config KVX_PAGE_SHIFT
> > + int
> > + default 12
> > +
> > +config CMDLINE
> > + string "Default kernel command string"
> > + default ""
> > + help
> > + On some architectures there is currently no way for the boot loader
> > + to pass arguments to the kernel. For these architectures, you should
> > + supply some command-line options at build time by entering them
> > + here.
> > +
> > +endmenu
> > +
> > +menu "Kernel Features"
> > +source "kernel/Kconfig.hz"
> > +endmenu
> > diff --git a/arch/kvx/Kconfig.debug b/arch/kvx/Kconfig.debug
> > new file mode 100644
> > index 000000000000..027e919a1e14
> > --- /dev/null
> > +++ b/arch/kvx/Kconfig.debug
> > @@ -0,0 +1,70 @@
> > +menu "KVX debugging"
> > +
> > +config KVX_DEBUG_ASN
> > + bool "Check ASN before writing TLB entry"
> > + default n
> > + help
> > + This option allows to check if the ASN of the current
> > + process is matching the ASN found in MMC. If it is not the
>
> s/is matching/matches/
>
> > + case an error will be printed.
> > +
> > +config KVX_DEBUG_TLB_WRITE
> > + bool "Enable TLBs write checks"
> > + default n
> > + help
> > + Enabling this option will enable TLB access checks. This is
> > + particularly helpful when modifying the assembly code responsible
> > + of TLB refill. If set, mmc.e will be checked each time the tlb are
>
> for TLB refill. TLB
>
> > + written and a panic will be thrown on error.
> > +
> > +config KVX_DEBUG_TLB_ACCESS
> > + bool "Enable TLBs accesses logging"
> > + default n
> > + help
> > + Enabling this option will enable TLB entry manipulation logging.
> > + Each time an entry will be added to the TLBs, it will be logged in
>
> s/will be/is/ s/will be/is/
>
> > + an array readable via gdb scripts. This can be useful to understand
> > + strange crashes related to suspicious virtual/physical addresses.
> > +
> > +config KVX_DEBUG_TLB_ACCESS_BITS
> > + int "Number of bits used as index of entries in log table"
> > + default 12
> > + depends on KVX_DEBUG_TLB_ACCESS
> > + help
> > + Set the number of bits used as index of entries that will be logged
> > + in a ring buffer called kvx_tlb_access. One entry in the table
> > + contains registers TEL, TEH and MMC. It also logs the type of the
> > + operations (0:read, 1:write, 2:probe). Buffer is per CPU. For one
> > + entry 24 bytes are used. So by default it uses 96Ko of memory per
>
> What does "Ko" mean?

Ko stands for kilo-octet which is the french equivalent for kilo byte.
I've found more typos like this in the documentation and they will be
corrected as well.

> > + CPU to store 2^12 (4096) entries.
> > +
> > +config KVX_MMU_STATS
> > + bool "Register mmu stats debugfs entries"
>
> Preferably MMU
>
> > + default n
> > + select DEBUG_FS
>
> Preferably:
> depends on DEBUG_FS
ack

> > + help
> > + Enable debugfs attribute which will allow inspecting various metric
>
> metrics
>
> > + regarding MMU:
> > + - Number of nomapping traps handled
> > + - avg/min/max time for nomapping refill (user/kernel)
> > +
> > +config DEBUG_EXCEPTION_STACK
> > + bool "Enable exception stack debugging"
> > + default n
> > + help
> > + Enable stack check debugging when entering/exiting
> > + exception handlers.
> > + This can be particularly helpful after modifying stack
> > + handling to see if stack when exiting is the same as the one
> > + when entering exception handler.
> > +
> > +config DEBUG_SFR_SET_MASK
> > + bool "Enable sfr set_mask debugging"
> > + default n
> > + help
> > + Verify that values written using kvx_sfr_set_mask are matching the
>
> s/are matching/match/
>
> > + mask. This ensure that no extra bits of sfr will be overridden by some
>
> ensures
>
> and preferably s/sfr/SFR/ in 2 places (not in kvx_sfr_set_mask).
>
> > + incorrectly truncated values. This can lead to huge problems by
> > + modifying important bits in system registers.
> > +
> > +endmenu

Thanks, all the typos will be corrected :)

Jules