* [PATCH v8 14/14] powerpc/vas: Free send window in VAS instance after credits returned
From: Haren Myneni @ 2020-03-19 6:20 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev, ajd
In-Reply-To: <1584598120.9256.15237.camel@hbabu-laptop>
NX may be processing requests while trying to close window. Wait until
all credits are returned and then free send window from VAS instance.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas-window.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index d5754a8..a1d3fb1 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1317,14 +1317,14 @@ int vas_win_close(struct vas_window *window)
unmap_paste_region(window);
- clear_vinst_win(window);
-
poll_window_busy_state(window);
unpin_close_window(window);
poll_window_credits(window);
+ clear_vinst_win(window);
+
poll_window_castout(window);
/* if send window, drop reference to matching receive window */
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Christophe Leroy @ 2020-03-19 7:00 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Vasily Gorbik, Jonathan Corbet, Catalin Marinas,
Dave Hansen, Heiko Carstens, David S . Miller,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Andrew Morton, Longpeng, Will Deacon,
Thomas Gleixner
In-Reply-To: <20200318220634.32100-2-mike.kravetz@oracle.com>
Le 18/03/2020 à 23:06, Mike Kravetz a écrit :
> The architecture independent routine hugetlb_default_setup sets up
> the default huge pages size. It has no way to verify if the passed
> value is valid, so it accepts it and attempts to validate at a later
> time. This requires undocumented cooperation between the arch specific
> and arch independent code.
>
> For architectures that support more than one huge page size, provide
> a routine arch_hugetlb_valid_size to validate a huge page size.
> hugetlb_default_setup can use this to validate passed values.
>
> arch_hugetlb_valid_size will also be used in a subsequent patch to
> move processing of the "hugepagesz=" in arch specific code to a common
> routine in arch independent code.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> arch/arm64/include/asm/hugetlb.h | 2 ++
> arch/arm64/mm/hugetlbpage.c | 19 ++++++++++++++-----
> arch/powerpc/include/asm/hugetlb.h | 3 +++
> arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++-------
> arch/riscv/include/asm/hugetlb.h | 3 +++
> arch/riscv/mm/hugetlbpage.c | 28 ++++++++++++++++++----------
> arch/s390/include/asm/hugetlb.h | 3 +++
> arch/s390/mm/hugetlbpage.c | 18 +++++++++++++-----
> arch/sparc/include/asm/hugetlb.h | 3 +++
> arch/sparc/mm/init_64.c | 23 ++++++++++++++++-------
> arch/x86/include/asm/hugetlb.h | 3 +++
> arch/x86/mm/hugetlbpage.c | 21 +++++++++++++++------
> include/linux/hugetlb.h | 7 +++++++
> mm/hugetlb.c | 16 +++++++++++++---
> 14 files changed, 126 insertions(+), 43 deletions(-)
>
[snip]
> diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
> index bd6504c28c2f..3b5939016955 100644
> --- a/arch/powerpc/include/asm/hugetlb.h
> +++ b/arch/powerpc/include/asm/hugetlb.h
> @@ -64,6 +64,9 @@ static inline void arch_clear_hugepage_flags(struct page *page)
> {
> }
>
> +#define arch_hugetlb_valid_size arch_hugetlb_valid_size
> +extern bool __init arch_hugetlb_valid_size(unsigned long long size);
Don't add 'extern' keyword, it is irrelevant for a function declaration.
checkpatch --strict doesn't like it either
(https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12318//artifact/linux/checkpatch.log)
> +
> #include <asm-generic/hugetlb.h>
>
> #else /* ! CONFIG_HUGETLB_PAGE */
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 33b3461d91e8..b78f660252f3 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -558,7 +558,7 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
> return vma_kernel_pagesize(vma);
> }
>
> -static int __init add_huge_page_size(unsigned long long size)
> +bool __init arch_hugetlb_valid_size(unsigned long long size)
> {
> int shift = __ffs(size);
> int mmu_psize;
> @@ -566,20 +566,26 @@ static int __init add_huge_page_size(unsigned long long size)
> /* Check that it is a page size supported by the hardware and
> * that it fits within pagetable and slice limits. */
> if (size <= PAGE_SIZE || !is_power_of_2(size))
> - return -EINVAL;
> + return false;
>
> mmu_psize = check_and_get_huge_psize(shift);
> if (mmu_psize < 0)
> - return -EINVAL;
> + return false;
>
> BUG_ON(mmu_psize_defs[mmu_psize].shift != shift);
>
> - /* Return if huge page size has already been setup */
> - if (size_to_hstate(size))
> - return 0;
> + return true;
> +}
>
> - hugetlb_add_hstate(shift - PAGE_SHIFT);
> +static int __init add_huge_page_size(unsigned long long size)
> +{
> + int shift = __ffs(size);
> +
> + if (!arch_hugetlb_valid_size(size))
> + return -EINVAL;
>
> + if (!size_to_hstate(size))
> + hugetlb_add_hstate(shift - PAGE_SHIFT);
> return 0;
> }
>
[snip]
> diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
> index 5bfd5aef5378..51e6208fdeec 100644
> --- a/arch/x86/mm/hugetlbpage.c
> +++ b/arch/x86/mm/hugetlbpage.c
> @@ -181,16 +181,25 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> #endif /* CONFIG_HUGETLB_PAGE */
>
> #ifdef CONFIG_X86_64
> +bool __init arch_hugetlb_valid_size(unsigned long long size)
> +{
> + if (size == PMD_SIZE)
> + return true;
> + else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
> + return true;
> + else
> + return false;
> +}
> +
> static __init int setup_hugepagesz(char *opt)
> {
> - unsigned long ps = memparse(opt, &opt);
> - if (ps == PMD_SIZE) {
> - hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
> - } else if (ps == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) {
> - hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> + unsigned long long ps = memparse(opt, &opt);
> +
> + if (arch_hugetlb_valid_size(ps)) {
> + hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
> } else {
> hugetlb_bad_size();
> - printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
> + printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n",
> ps >> 20);
Nowadays we use pr_err() instead of printk.
It would also likely allow you to have everything fit on a single line.
> return 0;
> }
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index b831e9fa1a26..33343eb980d0 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -678,6 +678,13 @@ static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
> return &mm->page_table_lock;
> }
>
> +#ifndef arch_hugetlb_valid_size
> +static inline bool arch_hugetlb_valid_size(unsigned long long size)
> +{
> + return (size == HPAGE_SIZE);
Not sure the ( ) are necessary.
> +}
> +#endif
> +
> #ifndef hugepages_supported
> /*
> * Some platform decide whether they support huge pages at boot
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index d8ebd876871d..2f99359b93af 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3224,12 +3224,22 @@ static int __init hugetlb_nrpages_setup(char *s)
> }
> __setup("hugepages=", hugetlb_nrpages_setup);
>
> -static int __init hugetlb_default_setup(char *s)
> +static int __init default_hugepagesz_setup(char *s)
> {
> - default_hstate_size = memparse(s, &s);
> + unsigned long long size;
Why unsigned long long ?
default_hstate_size is long.
I can't imagine 32 bits platforms having a hugepage with a 64 bits size.
> + char *saved_s = s;
> +
> + size = memparse(s, &s);
The updated s is not reused after that so you can pass NULL instead of
&s and then you don't need the saved_s.
> +
> + if (!arch_hugetlb_valid_size(size)) {
> + pr_err("HugeTLB: unsupported default_hugepagesz %s\n", saved_s);
> + return 0;
> + }
> +
> + default_hstate_size = size;
> return 1;
> }
> -__setup("default_hugepagesz=", hugetlb_default_setup);
> +__setup("default_hugepagesz=", default_hugepagesz_setup);
>
> static unsigned int cpuset_mems_nr(unsigned int *array)
> {
>
Christophe
^ permalink raw reply
* [PATCH] Fix "[v3, 28/32] powerpc/64s: interrupt implement exit logic in C"
From: Nicholas Piggin @ 2020-03-19 6:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Accessing thread_info flags can cause an SLB fault, so it must not
be done with MSR[RI]=0, which leads to SLB unrecoverable fault error.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/syscall_64.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index 87d95b455b83..a56ae78f2d1d 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -313,12 +313,18 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
{
unsigned long *ti_flagsp = ¤t_thread_info()->flags;
unsigned long flags;
+ unsigned long ret = 0;
if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
unrecoverable_exception(regs);
BUG_ON(regs->msr & MSR_PR);
BUG_ON(!FULL_REGS(regs));
+ if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
+ clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
+ ret = 1;
+ }
+
local_irq_save(flags);
if (regs->softe == IRQS_ENABLED) {
@@ -370,10 +376,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
*/
kuap_restore_amr(regs);
- if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
- clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
- return 1;
- }
- return 0;
+ return ret;
}
#endif
--
2.23.0
^ permalink raw reply related
* Re: [PATCH 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Christophe Leroy @ 2020-03-19 7:04 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Vasily Gorbik, Jonathan Corbet, Catalin Marinas,
Dave Hansen, Heiko Carstens, David S . Miller,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Andrew Morton, Longpeng, Will Deacon,
Thomas Gleixner
In-Reply-To: <20200318220634.32100-3-mike.kravetz@oracle.com>
Le 18/03/2020 à 23:06, Mike Kravetz a écrit :
> Now that architectures provide arch_hugetlb_valid_size(), parsing
> of "hugepagesz=" can be done in architecture independent code.
> Create a single routine to handle hugepagesz= parsing and remove
> all arch specific routines. We can also remove the interface
> hugetlb_bad_size() as this is no longer used outside arch independent
> code.
>
> This also provides consistent behavior of hugetlbfs command line
> options. The hugepagesz= option should only be specified once for
> a specific size, but some architectures allow multiple instances.
> This appears to be more of an oversight when code was added by some
> architectures to set up ALL huge pages sizes.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> arch/arm64/mm/hugetlbpage.c | 15 ---------------
> arch/powerpc/mm/hugetlbpage.c | 15 ---------------
> arch/riscv/mm/hugetlbpage.c | 16 ----------------
> arch/s390/mm/hugetlbpage.c | 18 ------------------
> arch/sparc/mm/init_64.c | 22 ----------------------
> arch/x86/mm/hugetlbpage.c | 16 ----------------
> include/linux/hugetlb.h | 1 -
> mm/hugetlb.c | 24 ++++++++++++++++++------
> 8 files changed, 18 insertions(+), 109 deletions(-)
>
[snip]
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 2f99359b93af..cd4ec07080fb 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void)
> }
> subsys_initcall(hugetlb_init);
>
> -/* Should be called on processing a hugepagesz=... option */
> -void __init hugetlb_bad_size(void)
> -{
> - parsed_valid_hugepagesz = false;
> -}
> -
> void __init hugetlb_add_hstate(unsigned int order)
> {
> struct hstate *h;
> @@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s)
> }
> __setup("hugepages=", hugetlb_nrpages_setup);
>
> +static int __init hugepagesz_setup(char *s)
> +{
> + unsigned long long size;
> + char *saved_s = s;
> +
> + size = memparse(s, &s);
You don't use s after that, so you can pass NULL instead of &s and avoid
the saved_s
> +
> + if (!arch_hugetlb_valid_size(size)) {
> + parsed_valid_hugepagesz = false;
> + pr_err("HugeTLB: unsupported hugepagesz %s\n", saved_s);
> + return 0;
> + }
> +
> + hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
> + return 1;
> +}
> +__setup("hugepagesz=", hugepagesz_setup);
> +
> static int __init default_hugepagesz_setup(char *s)
> {
> unsigned long long size;
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 3/4] mm: Implement reset_numa_mem
From: Michal Hocko @ 2020-03-19 7:44 UTC (permalink / raw)
To: Christopher Lameter
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju, Bharata B Rao,
linux-mm, Kirill Tkhai, Mel Gorman, Andrew Morton, linuxppc-dev,
Joonsoo Kim, Vlastimil Babka
In-Reply-To: <alpine.DEB.2.21.2003181919450.18605@www.lameter.com>
On Wed 18-03-20 19:20:41, Cristopher Lameter wrote:
> On Wed, 18 Mar 2020, Srikar Dronamraju wrote:
>
> > For a memoryless or offline nodes, node_numa_mem refers to a N_MEMORY
> > fallback node. Currently kernel has an API set_numa_mem that sets
> > node_numa_mem for memoryless node. However this API cannot be used for
> > offline nodes. Hence all offline nodes will have their node_numa_mem set
> > to 0. However systems can themselves have node 0 as offline i.e
>
> That is a significant change to the basic assumptions for memory less
> nodes. Node 0 needed to have memory and processors. Not sure what else
> may break.
This assumption is simply incorrect. There many examples but just
one from top of my head 3e8589963773 ("memcg: make it work on sparse
non-0-node systems"). We simply have to forget that some nodes are
special.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3] ocxl: control via sysfs whether the FPGA is reloaded on a link reset
From: Frederic Barrat @ 2020-03-19 8:34 UTC (permalink / raw)
To: linuxppc-dev, clombard, felix, ajd, alastair
In-Reply-To: <20200318100210.80035-1-fbarrat@linux.ibm.com>
Le 18/03/2020 à 11:02, Frederic Barrat a écrit :
> From: Philippe Bergheaud <felix@linux.ibm.com>
>
> Some opencapi FPGA images allow to control if the FPGA should be reloaded
> on the next adapter reset. If it is supported, the image specifies it
> through a Vendor Specific DVSEC in the config space of function 0.
>
> Signed-off-by: Philippe Bergheaud <felix@linux.ibm.com>
> ---
Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
> Changelog:
> v2:
> - refine ResetReload debug message
> - do not call get_function_0() if pci_dev is for function 0
> v3:
> - avoid get_function_0() in ocxl_config_set_reset_reload also
>
> Documentation/ABI/testing/sysfs-class-ocxl | 10 ++++
> drivers/misc/ocxl/config.c | 68 +++++++++++++++++++++-
> drivers/misc/ocxl/ocxl_internal.h | 6 ++
> drivers/misc/ocxl/sysfs.c | 35 +++++++++++
> include/misc/ocxl-config.h | 1 +
> 5 files changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-ocxl b/Documentation/ABI/testing/sysfs-class-ocxl
> index b5b1fa197592..b9ea671d5805 100644
> --- a/Documentation/ABI/testing/sysfs-class-ocxl
> +++ b/Documentation/ABI/testing/sysfs-class-ocxl
> @@ -33,3 +33,13 @@ Date: January 2018
> Contact: linuxppc-dev@lists.ozlabs.org
> Description: read/write
> Give access the global mmio area for the AFU
> +
> +What: /sys/class/ocxl/<afu name>/reload_on_reset
> +Date: February 2020
> +Contact: linuxppc-dev@lists.ozlabs.org
> +Description: read/write
> + Control whether the FPGA is reloaded on a link reset
> + 0 Do not reload FPGA image from flash
> + 1 Reload FPGA image from flash
> + unavailable
> + The device does not support this capability
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index c8e19bfb5ef9..b364b6ceb996 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -71,6 +71,20 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
> return 0;
> }
>
> +/**
> + * get_function_0() - Find a related PCI device (function 0)
> + * @device: PCI device to match
> + *
> + * Returns a pointer to the related device, or null if not found
> + */
> +static struct pci_dev *get_function_0(struct pci_dev *dev)
> +{
> + unsigned int devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0);
> +
> + return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
> + dev->bus->number, devfn);
> +}
> +
> static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
> {
> u16 val;
> @@ -159,7 +173,7 @@ static int read_dvsec_afu_info(struct pci_dev *dev, struct ocxl_fn_config *fn)
> static int read_dvsec_vendor(struct pci_dev *dev)
> {
> int pos;
> - u32 cfg, tlx, dlx;
> + u32 cfg, tlx, dlx, reset_reload;
>
> /*
> * vendor specific DVSEC is optional
> @@ -183,6 +197,58 @@ static int read_dvsec_vendor(struct pci_dev *dev)
> dev_dbg(&dev->dev, " CFG version = 0x%x\n", cfg);
> dev_dbg(&dev->dev, " TLX version = 0x%x\n", tlx);
> dev_dbg(&dev->dev, " DLX version = 0x%x\n", dlx);
> +
> + if (ocxl_config_get_reset_reload(dev, &reset_reload) != 0)
> + dev_dbg(&dev->dev, " ResetReload is not available\n");
> + else
> + dev_dbg(&dev->dev, " ResetReload = 0x%x\n", reset_reload);
> + return 0;
> +}
> +
> +int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val)
> +{
> + int reset_reload = -1;
> + int pos = 0;
> + struct pci_dev *dev0 = dev;
> +
> + if (PCI_FUNC(dev->devfn) != 0)
> + dev0 = get_function_0(dev);
> +
> + if (dev0)
> + pos = find_dvsec(dev0, OCXL_DVSEC_VENDOR_ID);
> +
> + if (pos)
> + pci_read_config_dword(dev0,
> + pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
> + &reset_reload);
> + if (reset_reload == -1)
> + return reset_reload;
> +
> + *val = reset_reload & BIT(0);
> + return 0;
> +}
> +
> +int ocxl_config_set_reset_reload(struct pci_dev *dev, int val)
> +{
> + int reset_reload = -1;
> + int pos = 0;
> + struct pci_dev *dev0 = dev;
> +
> + if (PCI_FUNC(dev->devfn) != 0)
> + dev0 = get_function_0(dev);
> +
> + if (dev0)
> + pos = find_dvsec(dev0, OCXL_DVSEC_VENDOR_ID);
> +
> + if (pos)
> + pci_read_config_dword(dev0,
> + pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
> + &reset_reload);
> + if (reset_reload == -1)
> + return reset_reload;
> +
> + val &= BIT(0);
> + pci_write_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, val);
> return 0;
> }
>
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 345bf843a38e..af9a84aeee6f 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -112,6 +112,12 @@ void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
> */
> int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
>
> +/*
> + * Control whether the FPGA is reloaded on a link reset
> + */
> +int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val);
> +int ocxl_config_set_reset_reload(struct pci_dev *dev, int val);
> +
> /*
> * Check if an AFU index is valid for the given function.
> *
> diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
> index 58f1ba264206..8f69f7311343 100644
> --- a/drivers/misc/ocxl/sysfs.c
> +++ b/drivers/misc/ocxl/sysfs.c
> @@ -51,11 +51,46 @@ static ssize_t contexts_show(struct device *device,
> afu->pasid_count, afu->pasid_max);
> }
>
> +static ssize_t reload_on_reset_show(struct device *device,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ocxl_afu *afu = to_afu(device);
> + struct ocxl_fn *fn = afu->fn;
> + struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
> + int val;
> +
> + if (ocxl_config_get_reset_reload(pci_dev, &val))
> + return scnprintf(buf, PAGE_SIZE, "unavailable\n");
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> +}
> +
> +static ssize_t reload_on_reset_store(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ocxl_afu *afu = to_afu(device);
> + struct ocxl_fn *fn = afu->fn;
> + struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
> + int rc, val;
> +
> + rc = sscanf(buf, "%i", &val);
> + if ((rc != 1) || !(val == 1 || val == 0))
> + return -EINVAL;
> +
> + if (ocxl_config_set_reset_reload(pci_dev, val))
> + return -ENODEV;
> +
> + return count;
> +}
> +
> static struct device_attribute afu_attrs[] = {
> __ATTR_RO(global_mmio_size),
> __ATTR_RO(pp_mmio_size),
> __ATTR_RO(afu_version),
> __ATTR_RO(contexts),
> + __ATTR_RW(reload_on_reset),
> };
>
> static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
> diff --git a/include/misc/ocxl-config.h b/include/misc/ocxl-config.h
> index 3526fa996a22..ccfd3b463517 100644
> --- a/include/misc/ocxl-config.h
> +++ b/include/misc/ocxl-config.h
> @@ -41,5 +41,6 @@
> #define OCXL_DVSEC_VENDOR_CFG_VERS 0x0C
> #define OCXL_DVSEC_VENDOR_TLX_VERS 0x10
> #define OCXL_DVSEC_VENDOR_DLX_VERS 0x20
> +#define OCXL_DVSEC_VENDOR_RESET_RELOAD 0x38
>
> #endif /* _OCXL_CONFIG_H_ */
>
^ permalink raw reply
* Re: [patch V2 04/15] orinoco_usb: Use the regular completion interfaces
From: Greg Kroah-Hartman @ 2020-03-19 8:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Arnd Bergmann,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Logan Gunthorpe,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204407.793899611@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:06PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> The completion usage in this driver is interesting:
>
> - it uses a magic complete function which according to the comment was
> implemented by invoking complete() four times in a row because
> complete_all() was not exported at that time.
>
> - it uses an open coded wait/poll which checks completion:done. Only one wait
> side (device removal) uses the regular wait_for_completion() interface.
>
> The rationale behind this is to prevent that wait_for_completion() consumes
> completion::done which would prevent that all waiters are woken. This is not
> necessary with complete_all() as that sets completion::done to UINT_MAX which
> is left unmodified by the woken waiters.
>
> Replace the magic complete function with complete_all() and convert the
> open coded wait/poll to regular completion interfaces.
>
> This changes the wait to exclusive wait mode. But that does not make any
> difference because the wakers use complete_all() which ignores the
> exclusive mode.
>
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
> V2: New patch to avoid conversion to swait functions later.
> ---
> drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 21 ++++----------------
> 1 file changed, 5 insertions(+), 16 deletions(-)
>
> --- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
> +++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
> @@ -365,17 +365,6 @@ static struct request_context *ezusb_all
> return ctx;
> }
>
> -
> -/* Hopefully the real complete_all will soon be exported, in the mean
> - * while this should work. */
> -static inline void ezusb_complete_all(struct completion *comp)
> -{
> - complete(comp);
> - complete(comp);
> - complete(comp);
> - complete(comp);
> -}
That's so funny... :(
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [patch V2 03/15] usb: gadget: Use completion interface instead of open coding it
From: Greg Kroah-Hartman @ 2020-03-19 8:41 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Arnd Bergmann,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Logan Gunthorpe,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204407.700914073@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:05PM +0100, Thomas Gleixner wrote:
> ep_io() uses a completion on stack and open codes the waiting with:
>
> wait_event_interruptible (done.wait, done.done);
> and
> wait_event (done.wait, done.done);
>
> This waits in non-exclusive mode for complete(), but there is no reason to
> do so because the completion can only be waited for by the task itself and
> complete() wakes exactly one exlusive waiter.
>
> Replace the open coded implementation with the corresponding
> wait_for_completion*() functions.
>
> No functional change.
>
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> ---
> V2: New patch to avoid the conversion to swait interfaces later
> ---
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [patch V2 11/15] completion: Use simple wait queues
From: Greg Kroah-Hartman @ 2020-03-19 8:42 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Paul E . McKenney,
Arnd Bergmann, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Logan Gunthorpe,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204408.521507446@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:13PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> completion uses a wait_queue_head_t to enqueue waiters.
>
> wait_queue_head_t contains a spinlock_t to protect the list of waiters
> which excludes it from being used in truly atomic context on a PREEMPT_RT
> enabled kernel.
>
> The spinlock in the wait queue head cannot be replaced by a raw_spinlock
> because:
>
> - wait queues can have custom wakeup callbacks, which acquire other
> spinlock_t locks and have potentially long execution times
>
> - wake_up() walks an unbounded number of list entries during the wake up
> and may wake an unbounded number of waiters.
>
> For simplicity and performance reasons complete() should be usable on
> PREEMPT_RT enabled kernels.
>
> completions do not use custom wakeup callbacks and are usually single
> waiter, except for a few corner cases.
>
> Replace the wait queue in the completion with a simple wait queue (swait),
> which uses a raw_spinlock_t for protecting the waiter list and therefore is
> safe to use inside truly atomic regions on PREEMPT_RT.
>
> There is no semantical or functional change:
>
> - completions use the exclusive wait mode which is what swait provides
>
> - complete() wakes one exclusive waiter
>
> - complete_all() wakes all waiters while holding the lock which protects
> the wait queue against newly incoming waiters. The conversion to swait
> preserves this behaviour.
>
> complete_all() might cause unbound latencies with a large number of waiters
> being woken at once, but most complete_all() usage sites are either in
> testing or initialization code or have only a really small number of
> concurrent waiters which for now does not cause a latency problem. Keep it
> simple for now.
>
> The fixup of the warning check in the USB gadget driver is just a straight
> forward conversion of the lockless waiter check from one waitqueue type to
> the other.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
> V2: Split out the orinoco and usb gadget parts and amended change log
> ---
> drivers/usb/gadget/function/f_fs.c | 2 +-
> include/linux/completion.h | 8 ++++----
> kernel/sched/completion.c | 36 +++++++++++++++++++-----------------
> 3 files changed, 24 insertions(+), 22 deletions(-)
For USB portion:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [patch V2 08/15] Documentation: Add lock ordering and nesting documentation
From: Davidlohr Bueso @ 2020-03-19 8:51 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204408.211530902@linutronix.de>
On Wed, 18 Mar 2020, Thomas Gleixner wrote:
>+Owner semantics
>+===============
>+
>+Most lock types in the Linux kernel have strict owner semantics, i.e. the
>+context (task) which acquires a lock has to release it.
>+
>+There are two exceptions:
>+
>+ - semaphores
>+ - rwsems
>+
>+semaphores have no strict owner semantics for historical reasons. They are
I would rephrase this to:
semaphores have no owner semantics for historical reason, and as such
trylock and release operations can be called from interrupt context. They
are ...
>+often used for both serialization and waiting purposes. That's generally
>+discouraged and should be replaced by separate serialization and wait
>+mechanisms.
^ , such as mutexes or completions.
>+
>+rwsems have grown interfaces which allow non owner release for special
>+purposes. This usage is problematic on PREEMPT_RT because PREEMPT_RT
>+substitutes all locking primitives except semaphores with RT-mutex based
>+implementations to provide priority inheritance for all lock types except
>+the truly spinning ones. Priority inheritance on ownerless locks is
>+obviously impossible.
>+
>+For now the rwsem non-owner release excludes code which utilizes it from
>+being used on PREEMPT_RT enabled kernels. In same cases this can be
>+mitigated by disabling portions of the code, in other cases the complete
>+functionality has to be disabled until a workable solution has been found.
Thanks,
Davidlohr
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Sachin Sant @ 2020-03-19 8:52 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Nathan Lynch, Srikar Dronamraju, Mel Gorman, Michal Hocko,
Pekka Enberg, linux-mm, Kirill Tkhai, David Rientjes,
Christopher Lameter, bharata, linuxppc-dev, Joonsoo Kim
In-Reply-To: <e060ad43-ff4e-0e59-2e64-ce8a4916ec70@suse.cz>
> OK how about this version? It's somewhat ugly, but important is that the fast
> path case (c->page exists) is unaffected and another common case (c->page is
> NULL, but node is NUMA_NO_NODE) is just one extra check - impossible to avoid at
> some point anyway.
>
I attempted the suggested tests.
Test 1: March 18 linux-next + Patch 1 [1] + Patch 2 [2]
Machine boots fine. numactl o/p after boot:
# numactl -H
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35631 MB
node 1 free: 32724 MB
node distances:
node 0 1
0: 10 40
1: 40 10
#
Test 2: Code base as used in Test 1 + 3 patch series from Srikar [3]
Machine boots fine. numactl o/p after boot:
# numactl -H
available: 1 nodes (1)
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35631 MB
node 1 free: 32711 MB
node distances:
node 1
1: 10
#
Thanks!
-Sachin
[1] https://lore.kernel.org/linux-mm/e060ad43-ff4e-0e59-2e64-ce8a4916ec70@suse.cz/T/#mb8d0a358dc5c5fb9661fa45047072a92c510faf2
[2] https://lore.kernel.org/linux-mm/e060ad43-ff4e-0e59-2e64-ce8a4916ec70@suse.cz/T/#mce342e54a95ce2ee3043901e52d70ee2fd94c516
[3] https://lore.kernel.org/linux-mm/20200311110237.5731-1-srikar@linux.vnet.ibm.com/
^ permalink raw reply
* Re: [PATCH v3 25/32] powerpc/64: system call implement entry/exit logic in C
From: Christophe Leroy @ 2020-03-19 9:18 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Michal Suchanek
In-Reply-To: <20200225173541.1549955-26-npiggin@gmail.com>
Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
> System call entry and particularly exit code is beyond the limit of what
> is reasonable to implement in asm.
>
> This conversion moves all conditional branches out of the asm code,
> except for the case that all GPRs should be restored at exit.
>
> Null syscall test is about 5% faster after this patch, because the exit
> work is handled under local_irq_disable, and the hard mask and pending
> interrupt replay is handled after that, which avoids games with MSR.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>
> v2,rebase (from Michal):
> - Add endian conversion for dtl_idx (ms)
> - Fix sparse warning about missing declaration (ms)
> - Add unistd.h to fix some defconfigs, add SPDX, minor formatting (mpe)
>
> v3: Fixes thanks to reports from mpe and selftests errors:
> - Several soft-mask debug and unsafe smp_processor_id() warnings due to
> tracing and other false positives due to checks in "unreconciled" code.
> - Fix a bug with syscall tracing functions that set registers (e.g.,
> PTRACE_SETREG) not setting GPRs properly.
> - Fix silly tabort_syscall bug that causes kernel crashes when making system
> calls in transactional state.
>
> arch/powerpc/include/asm/asm-prototypes.h | 17 +-
> .../powerpc/include/asm/book3s/64/kup-radix.h | 14 +-
> arch/powerpc/include/asm/cputime.h | 29 ++
> arch/powerpc/include/asm/hw_irq.h | 4 +
> arch/powerpc/include/asm/ptrace.h | 3 +
> arch/powerpc/include/asm/signal.h | 3 +
> arch/powerpc/include/asm/switch_to.h | 5 +
> arch/powerpc/include/asm/time.h | 3 +
> arch/powerpc/kernel/Makefile | 3 +-
> arch/powerpc/kernel/entry_64.S | 338 +++---------------
> arch/powerpc/kernel/signal.h | 2 -
> arch/powerpc/kernel/syscall_64.c | 213 +++++++++++
> arch/powerpc/kernel/systbl.S | 9 +-
> 13 files changed, 328 insertions(+), 315 deletions(-)
> create mode 100644 arch/powerpc/kernel/syscall_64.c
>
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index 983c0084fb3f..4b3609554e76 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -97,6 +97,12 @@ ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp,
> unsigned long __init early_init(unsigned long dt_ptr);
> void __init machine_init(u64 dt_ptr);
> #endif
> +#ifdef CONFIG_PPC64
This ifdef is not necessary as it has no pending #else.
Having function declaration without definition is not an issue.
Keeping in mind that we are aiming at generalising this to PPC32.
> +long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, unsigned long r0, struct pt_regs *regs);
> +notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs);
> +notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr);
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr);
> +#endif
>
> long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
> u32 len_high, u32 len_low);
> @@ -104,14 +110,6 @@ long sys_switch_endian(void);
> notrace unsigned int __check_irq_replay(void);
> void notrace restore_interrupts(void);
>
> -/* ptrace */
> -long do_syscall_trace_enter(struct pt_regs *regs);
> -void do_syscall_trace_leave(struct pt_regs *regs);
> -
> -/* process */
> -void restore_math(struct pt_regs *regs);
> -void restore_tm_state(struct pt_regs *regs);
> -
> /* prom_init (OpenFirmware) */
> unsigned long __init prom_init(unsigned long r3, unsigned long r4,
> unsigned long pp,
> @@ -122,9 +120,6 @@ unsigned long __init prom_init(unsigned long r3, unsigned long r4,
> void __init early_setup(unsigned long dt_ptr);
> void early_setup_secondary(void);
>
> -/* time */
> -void accumulate_stolen_time(void);
> -
> /* misc runtime */
> extern u64 __bswapdi2(u64);
> extern s64 __lshrdi3(s64, int);
> diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> index 90dd3a3fc8c7..71081d90f999 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> @@ -3,6 +3,7 @@
> #define _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
>
> #include <linux/const.h>
> +#include <asm/reg.h>
>
> #define AMR_KUAP_BLOCK_READ UL(0x4000000000000000)
> #define AMR_KUAP_BLOCK_WRITE UL(0x8000000000000000)
> @@ -56,7 +57,14 @@
>
> #ifdef CONFIG_PPC_KUAP
>
> -#include <asm/reg.h>
> +#include <asm/mmu.h>
> +#include <asm/ptrace.h>
> +
> +static inline void kuap_check_amr(void)
> +{
> + if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) && mmu_has_feature(MMU_FTR_RADIX_KUAP))
> + WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
> +}
>
> /*
> * We support individually allowing read or write, but we don't support nesting
> @@ -127,6 +135,10 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
> (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
> "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
> }
> +#else /* CONFIG_PPC_KUAP */
> +static inline void kuap_check_amr(void)
> +{
> +}
> #endif /* CONFIG_PPC_KUAP */
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
> index 2431b4ada2fa..6639a6847cc0 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -44,6 +44,28 @@ static inline unsigned long cputime_to_usecs(const cputime_t ct)
> #ifdef CONFIG_PPC64
> #define get_accounting(tsk) (&get_paca()->accounting)
> static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
Could we have the below additions sit outside of this PPC64 ifdef, to be
reused on PPC32 ?
> +
> +/*
> + * account_cpu_user_entry/exit runs "unreconciled", so can't trace,
> + * can't use use get_paca()
> + */
> +static notrace inline void account_cpu_user_entry(void)
> +{
> + unsigned long tb = mftb();
> + struct cpu_accounting_data *acct = &local_paca->accounting;
In the spirit of reusing that code on PPC32, can we use get_accounting()
? Or an alternate version of get_accounting(), eg
get_accounting_notrace() to be defined ?
> +
> + acct->utime += (tb - acct->starttime_user);
> + acct->starttime = tb;
> +}
> +static notrace inline void account_cpu_user_exit(void)
> +{
> + unsigned long tb = mftb();
> + struct cpu_accounting_data *acct = &local_paca->accounting;
> +
> + acct->stime += (tb - acct->starttime);
> + acct->starttime_user = tb;
> +}
> +
> #else
> #define get_accounting(tsk) (&task_thread_info(tsk)->accounting)
> /*
> @@ -61,5 +83,12 @@ static inline void arch_vtime_task_switch(struct task_struct *prev)
> #endif
>
> #endif /* __KERNEL__ */
> +#else /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> +static inline void account_cpu_user_entry(void)
> +{
> +}
> +static inline void account_cpu_user_exit(void)
> +{
> +}
> #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> #endif /* __POWERPC_CPUTIME_H */
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index e3a905e3d573..310583e62bd9 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -228,9 +228,13 @@ static inline bool arch_irqs_disabled(void)
> #ifdef CONFIG_PPC_BOOK3E
> #define __hard_irq_enable() wrtee(MSR_EE)
> #define __hard_irq_disable() wrtee(0)
> +#define __hard_EE_RI_disable() wrtee(0)
> +#define __hard_RI_enable() do { } while (0)
> #else
> #define __hard_irq_enable() __mtmsrd(MSR_EE|MSR_RI, 1)
> #define __hard_irq_disable() __mtmsrd(MSR_RI, 1)
> +#define __hard_EE_RI_disable() __mtmsrd(0, 1)
> +#define __hard_RI_enable() __mtmsrd(MSR_RI, 1)
> #endif
>
> #define hard_irq_disable() do { \
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index ee3ada66deb5..082a40153b94 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -138,6 +138,9 @@ extern unsigned long profile_pc(struct pt_regs *regs);
> #define profile_pc(regs) instruction_pointer(regs)
> #endif
>
> +long do_syscall_trace_enter(struct pt_regs *regs);
> +void do_syscall_trace_leave(struct pt_regs *regs);
> +
> #define kernel_stack_pointer(regs) ((regs)->gpr[1])
> static inline int is_syscall_success(struct pt_regs *regs)
> {
> diff --git a/arch/powerpc/include/asm/signal.h b/arch/powerpc/include/asm/signal.h
> index 0803ca8b9149..99e1c6de27bc 100644
> --- a/arch/powerpc/include/asm/signal.h
> +++ b/arch/powerpc/include/asm/signal.h
> @@ -6,4 +6,7 @@
> #include <uapi/asm/signal.h>
> #include <uapi/asm/ptrace.h>
>
> +struct pt_regs;
> +void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags);
> +
> #endif /* _ASM_POWERPC_SIGNAL_H */
> diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
> index 5b03d8a82409..476008bc3d08 100644
> --- a/arch/powerpc/include/asm/switch_to.h
> +++ b/arch/powerpc/include/asm/switch_to.h
> @@ -5,6 +5,7 @@
> #ifndef _ASM_POWERPC_SWITCH_TO_H
> #define _ASM_POWERPC_SWITCH_TO_H
>
> +#include <linux/sched.h>
> #include <asm/reg.h>
>
> struct thread_struct;
> @@ -22,6 +23,10 @@ extern void switch_booke_debug_regs(struct debug_reg *new_debug);
>
> extern int emulate_altivec(struct pt_regs *);
>
> +void restore_math(struct pt_regs *regs);
> +
> +void restore_tm_state(struct pt_regs *regs);
> +
> extern void flush_all_to_thread(struct task_struct *);
> extern void giveup_all(struct task_struct *);
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index e0107495c4de..39ce95016a3a 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -194,5 +194,8 @@ DECLARE_PER_CPU(u64, decrementers_next_tb);
> /* Convert timebase ticks to nanoseconds */
> unsigned long long tb_to_ns(unsigned long long tb_ticks);
>
> +/* SPLPAR */
> +void accumulate_stolen_time(void);
> +
> #endif /* __KERNEL__ */
> #endif /* __POWERPC_TIME_H */
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 78a1b22d4fd8..5700231a8988 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -50,7 +50,8 @@ obj-y := cputable.o ptrace.o syscalls.o \
> of_platform.o prom_parse.o
> obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \
> signal_64.o ptrace32.o \
> - paca.o nvram_64.o firmware.o note.o
> + paca.o nvram_64.o firmware.o note.o \
> + syscall_64.o
> obj-$(CONFIG_VDSO32) += vdso32/
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
[snip]
> diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
> index 800433685888..d396efca4068 100644
> --- a/arch/powerpc/kernel/signal.h
> +++ b/arch/powerpc/kernel/signal.h
> @@ -10,8 +10,6 @@
> #ifndef _POWERPC_ARCH_SIGNAL_H
> #define _POWERPC_ARCH_SIGNAL_H
>
> -extern void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags);
> -
> extern void __user *get_sigframe(struct ksignal *ksig, unsigned long sp,
> size_t frame_size, int is_32);
>
> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
Could some part of it go in a syscall.c to be reused on PPC32 ?
> new file mode 100644
> index 000000000000..20f77cc19df8
> --- /dev/null
> +++ b/arch/powerpc/kernel/syscall_64.c
> @@ -0,0 +1,213 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/err.h>
> +#include <asm/asm-prototypes.h>
> +#include <asm/book3s/64/kup-radix.h>
> +#include <asm/cputime.h>
> +#include <asm/hw_irq.h>
> +#include <asm/kprobes.h>
> +#include <asm/paca.h>
> +#include <asm/ptrace.h>
> +#include <asm/reg.h>
> +#include <asm/signal.h>
> +#include <asm/switch_to.h>
> +#include <asm/syscall.h>
> +#include <asm/time.h>
> +#include <asm/unistd.h>
> +
> +extern void __noreturn tabort_syscall(unsigned long nip, unsigned long msr);
> +
> +typedef long (*syscall_fn)(long, long, long, long, long, long);
> +
> +/* Has to run notrace because it is entered "unreconciled" */
> +notrace long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8,
> + unsigned long r0, struct pt_regs *regs)
> +{
> + unsigned long ti_flags;
> + syscall_fn f;
> +
> + BUG_ON(!(regs->msr & MSR_PR));
> +
> + if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
> + unlikely(regs->msr & MSR_TS_T))
> + tabort_syscall(regs->nip, regs->msr);
> +
> + account_cpu_user_entry();
> +
> +#ifdef CONFIG_PPC_SPLPAR
> + if (IS_ENABLED(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) &&
> + firmware_has_feature(FW_FEATURE_SPLPAR)) {
> + struct lppaca *lp = local_paca->lppaca_ptr;
> +
> + if (unlikely(local_paca->dtl_ridx != be64_to_cpu(lp->dtl_idx)))
> + accumulate_stolen_time();
> + }
> +#endif
> +
> + kuap_check_amr();
> +
> + /*
> + * A syscall should always be called with interrupts enabled
> + * so we just unconditionally hard-enable here. When some kind
> + * of irq tracing is used, we additionally check that condition
> + * is correct
> + */
> + if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG)) {
> + WARN_ON(irq_soft_mask_return() != IRQS_ENABLED);
> + WARN_ON(local_paca->irq_happened);
> + }
> + /*
> + * This is not required for the syscall exit path, but makes the
> + * stack frame look nicer. If this was initialised in the first stack
> + * frame, or if the unwinder was taught the first stack frame always
> + * returns to user with IRQS_ENABLED, this store could be avoided!
> + */
> + regs->softe = IRQS_ENABLED;
softe doesn't exist on PPC32. Can we do that through a helper ?
> +
> + __hard_irq_enable();
This doesn't exist on PPC32. Should we define __hard_irq_enable() as
arch_local_irq_enable() on PPC32 ?
> +
> + ti_flags = current_thread_info()->flags;
> + if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> + /*
> + * We use the return value of do_syscall_trace_enter() as the
> + * syscall number. If the syscall was rejected for any reason
> + * do_syscall_trace_enter() returns an invalid syscall number
> + * and the test against NR_syscalls will fail and the return
> + * value to be used is in regs->gpr[3].
> + */
> + r0 = do_syscall_trace_enter(regs);
> + if (unlikely(r0 >= NR_syscalls))
> + return regs->gpr[3];
> + r3 = regs->gpr[3];
> + r4 = regs->gpr[4];
> + r5 = regs->gpr[5];
> + r6 = regs->gpr[6];
> + r7 = regs->gpr[7];
> + r8 = regs->gpr[8];
> +
> + } else if (unlikely(r0 >= NR_syscalls)) {
> + return -ENOSYS;
> + }
> +
> + /* May be faster to do array_index_nospec? */
> + barrier_nospec();
> +
> + if (unlikely(ti_flags & _TIF_32BIT)) {
Use is_compat_task() instead ?
> + f = (void *)compat_sys_call_table[r0];
> +
> + r3 &= 0x00000000ffffffffULL;
> + r4 &= 0x00000000ffffffffULL;
> + r5 &= 0x00000000ffffffffULL;
> + r6 &= 0x00000000ffffffffULL;
> + r7 &= 0x00000000ffffffffULL;
> + r8 &= 0x00000000ffffffffULL;
> +
> + } else {
> + f = (void *)sys_call_table[r0];
> + }
> +
> + return f(r3, r4, r5, r6, r7, r8);
> +}
> +
> +/*
> + * This should be called after a syscall returns, with r3 the return value
> + * from the syscall. If this function returns non-zero, the system call
> + * exit assembly should additionally load all GPR registers and CTR and XER
> + * from the interrupt frame.
> + *
> + * The function graph tracer can not trace the return side of this function,
> + * because RI=0 and soft mask state is "unreconciled", so it is marked notrace.
> + */
> +notrace unsigned long syscall_exit_prepare(unsigned long r3,
> + struct pt_regs *regs)
> +{
> + unsigned long *ti_flagsp = ¤t_thread_info()->flags;
> + unsigned long ti_flags;
> + unsigned long ret = 0;
> +
> + regs->result = r3;
> +
> + /* Check whether the syscall is issued inside a restartable sequence */
> + rseq_syscall(regs);
> +
> + ti_flags = *ti_flagsp;
> +
> + if (unlikely(r3 >= (unsigned long)-MAX_ERRNO)) {
> + if (likely(!(ti_flags & (_TIF_NOERROR | _TIF_RESTOREALL)))) {
> + r3 = -r3;
> + regs->ccr |= 0x10000000; /* Set SO bit in CR */
> + }
> + }
> +
> + if (unlikely(ti_flags & _TIF_PERSYSCALL_MASK)) {
> + if (ti_flags & _TIF_RESTOREALL)
> + ret = _TIF_RESTOREALL;
> + else
> + regs->gpr[3] = r3;
> + clear_bits(_TIF_PERSYSCALL_MASK, ti_flagsp);
> + } else {
> + regs->gpr[3] = r3;
> + }
> +
> + if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) {
> + do_syscall_trace_leave(regs);
> + ret |= _TIF_RESTOREALL;
> + }
> +
> +again:
> + local_irq_disable();
> + ti_flags = READ_ONCE(*ti_flagsp);
> + while (unlikely(ti_flags & _TIF_USER_WORK_MASK)) {
> + local_irq_enable();
> + if (ti_flags & _TIF_NEED_RESCHED) {
> + schedule();
> + } else {
> + /*
> + * SIGPENDING must restore signal handler function
> + * argument GPRs, and some non-volatiles (e.g., r1).
> + * Restore all for now. This could be made lighter.
> + */
> + if (ti_flags & _TIF_SIGPENDING)
> + ret |= _TIF_RESTOREALL;
> + do_notify_resume(regs, ti_flags);
> + }
> + local_irq_disable();
> + ti_flags = READ_ONCE(*ti_flagsp);
> + }
> +
> + if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
> + unsigned long mathflags = MSR_FP;
> +
> + if (IS_ENABLED(CONFIG_ALTIVEC))
> + mathflags |= MSR_VEC;
> +
> + if ((regs->msr & mathflags) != mathflags)
> + restore_math(regs);
> + }
> +
> + /* This must be done with RI=1 because tracing may touch vmaps */
> + trace_hardirqs_on();
> +
> + /* This pattern matches prep_irq_for_idle */
> + __hard_EE_RI_disable();
> + if (unlikely(lazy_irq_pending())) {
> + __hard_RI_enable();
> + trace_hardirqs_off();
> + local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
> + local_irq_enable();
> + /* Took an interrupt which may have more exit work to do. */
> + goto again;
> + }
> + local_paca->irq_happened = 0;
> + irq_soft_mask_set(IRQS_ENABLED);
> +
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> + local_paca->tm_scratch = regs->msr;
> +#endif
> +
> + kuap_check_amr();
> +
> + account_cpu_user_exit();
> +
> + return ret;
> +}
> diff --git a/arch/powerpc/kernel/systbl.S b/arch/powerpc/kernel/systbl.S
> index 5b905a2f4e4d..d34276f3c495 100644
> --- a/arch/powerpc/kernel/systbl.S
> +++ b/arch/powerpc/kernel/systbl.S
> @@ -16,25 +16,22 @@
>
> #ifdef CONFIG_PPC64
> .p2align 3
> +#define __SYSCALL(nr, entry) .8byte entry
> +#else
> +#define __SYSCALL(nr, entry) .long entry
> #endif
>
> .globl sys_call_table
> sys_call_table:
> #ifdef CONFIG_PPC64
> -#define __SYSCALL(nr, entry) .8byte DOTSYM(entry)
> #include <asm/syscall_table_64.h>
> -#undef __SYSCALL
> #else
> -#define __SYSCALL(nr, entry) .long entry
> #include <asm/syscall_table_32.h>
> -#undef __SYSCALL
> #endif
>
> #ifdef CONFIG_COMPAT
> .globl compat_sys_call_table
> compat_sys_call_table:
> #define compat_sys_sigsuspend sys_sigsuspend
> -#define __SYSCALL(nr, entry) .8byte DOTSYM(entry)
> #include <asm/syscall_table_c32.h>
> -#undef __SYSCALL
> #endif
>
Christophe
^ permalink raw reply
* Re: [patch V2 07/15] powerpc/ps3: Convert half completion to rcuwait
From: Peter Zijlstra @ 2020-03-19 9:18 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-usb, linux-pci, Oleg Nesterov, netdev, Joel Fernandes,
Will Deacon, Thomas Gleixner, Davidlohr Bueso, Paul E . McKenney,
Logan Gunthorpe, Ingo Molnar, Linus Torvalds, Arnd Bergmann,
Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer, Kalle Valo,
Felipe Balbi, Greg Kroah-Hartman, Randy Dunlap, linux-wireless,
LKML, linuxppc-dev, David S. Miller
In-Reply-To: <20200319090024.wbrywc77tff3ro7i@linutronix.de>
On Thu, Mar 19, 2020 at 10:00:24AM +0100, Sebastian Andrzej Siewior wrote:
> On 2020-03-18 21:43:09 [+0100], Thomas Gleixner wrote:
> > --- a/arch/powerpc/platforms/ps3/device-init.c
> > +++ b/arch/powerpc/platforms/ps3/device-init.c
> > @@ -725,12 +728,12 @@ static int ps3_notification_read_write(s
> > unsigned long flags;
> > int res;
> >
> > - init_completion(&dev->done);
> > spin_lock_irqsave(&dev->lock, flags);
> > res = write ? lv1_storage_write(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
> > &dev->tag)
> > : lv1_storage_read(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
> > &dev->tag);
> > + dev->done = false;
> > spin_unlock_irqrestore(&dev->lock, flags);
> > if (res) {
> > pr_err("%s:%u: %s failed %d\n", __func__, __LINE__, op, res);
> > @@ -738,14 +741,10 @@ static int ps3_notification_read_write(s
> > }
> > pr_debug("%s:%u: notification %s issued\n", __func__, __LINE__, op);
> >
> > - res = wait_event_interruptible(dev->done.wait,
> > - dev->done.done || kthread_should_stop());
> > + rcuwait_wait_event(&dev->wait, dev->done || kthread_should_stop(), TASK_IDLE);
> > +
> …
>
> Not sure it matters but this struct `dev' is allocated on stack. Should
> the interrupt fire *before* rcuwait_wait_event() set wait.task to NULL
> then it is of random value on the first invocation of rcuwait_wake_up().
> ->
>
> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
> index 197347c3c0b24..e87360a0fb40d 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -809,6 +809,7 @@ static int ps3_probe_thread(void *data)
> }
>
> spin_lock_init(&dev.lock);
> + rcuwait_init(&dev.wait);
>
> res = request_irq(irq, ps3_notification_interrupt, 0,
> "ps3_notification", &dev);
>
Very good, sorry for that.
^ permalink raw reply
* Re: [patch V2 07/15] powerpc/ps3: Convert half completion to rcuwait
From: Davidlohr Bueso @ 2020-03-19 9:21 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-usb, Peter Zijlstra, linux-pci, Sebastian Andrzej Siewior,
Oleg Nesterov, netdev, Joel Fernandes, Will Deacon, Ingo Molnar,
Paul E . McKenney, Logan Gunthorpe, Linus Torvalds, Arnd Bergmann,
Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer, Kalle Valo,
Felipe Balbi, Greg Kroah-Hartman, Randy Dunlap, linux-wireless,
LKML, linuxppc-dev, David S. Miller
In-Reply-To: <20200318204408.102694393@linutronix.de>
On Wed, 18 Mar 2020, Thomas Gleixner wrote:
>AFAICT the kthread uses TASK_INTERRUPTIBLE to not increase loadavg, kthreads
>cannot receive signals by default and this one doesn't look different. Use
>TASK_IDLE instead.
Hmm it seems in general this needs to be done kernel-wide. This kthread abuse of
TASK_INTERRUPTIBLE seems to be a common thing. There's also the users doing
schedule_timeout_interruptible()...
Thanks,
Davidlohr
^ permalink raw reply
* Re: [PATCH v2 1/4] mm: Check for node_online in node_present_pages
From: Vlastimil Babka @ 2020-03-19 9:38 UTC (permalink / raw)
To: Michael Ellerman, Michal Hocko, Srikar Dronamraju
Cc: Sachin Sant, Nathan Lynch, Bharata B Rao, linux-mm, Kirill Tkhai,
Mel Gorman, Joonsoo Kim, Andrew Morton, linuxppc-dev,
Christopher Lameter
In-Reply-To: <87tv2ldw1k.fsf@mpe.ellerman.id.au>
On 3/19/20 1:32 AM, Michael Ellerman wrote:
> Seems like a nice solution to me
Thanks :)
>> ----8<----
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 17dc00e33115..1d4f2d7a0080 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
>> struct page *page;
>> unsigned int order = oo_order(oo);
>>
>> - if (node == NUMA_NO_NODE)
>> + if (node == NUMA_NO_NODE || !node_online(node))
>
> Why don't we need the node_present_pages() check here?
Page allocator is fine with a node without present pages, as long as there's a
zonelist, which online nodes must have (ideally all possible nodes should have,
and then we can remove this).
SLUB on the other hand doesn't allocate cache per-cpu structures for nodes
without present pages (understandably) that's why the other place includes the
node_present_pages() check.
Thanks
^ permalink raw reply
* Re: [patch V2 07/15] powerpc/ps3: Convert half completion to rcuwait
From: Sebastian Andrzej Siewior @ 2020-03-19 9:00 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-usb, Peter Zijlstra, linux-pci, Oleg Nesterov, netdev,
Joel Fernandes, Will Deacon, Ingo Molnar, Davidlohr Bueso,
Paul E . McKenney, Logan Gunthorpe, Linus Torvalds, Arnd Bergmann,
Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer, Kalle Valo,
Felipe Balbi, Greg Kroah-Hartman, Randy Dunlap, linux-wireless,
LKML, linuxppc-dev, David S. Miller
In-Reply-To: <20200318204408.102694393@linutronix.de>
On 2020-03-18 21:43:09 [+0100], Thomas Gleixner wrote:
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -725,12 +728,12 @@ static int ps3_notification_read_write(s
> unsigned long flags;
> int res;
>
> - init_completion(&dev->done);
> spin_lock_irqsave(&dev->lock, flags);
> res = write ? lv1_storage_write(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
> &dev->tag)
> : lv1_storage_read(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
> &dev->tag);
> + dev->done = false;
> spin_unlock_irqrestore(&dev->lock, flags);
> if (res) {
> pr_err("%s:%u: %s failed %d\n", __func__, __LINE__, op, res);
> @@ -738,14 +741,10 @@ static int ps3_notification_read_write(s
> }
> pr_debug("%s:%u: notification %s issued\n", __func__, __LINE__, op);
>
> - res = wait_event_interruptible(dev->done.wait,
> - dev->done.done || kthread_should_stop());
> + rcuwait_wait_event(&dev->wait, dev->done || kthread_should_stop(), TASK_IDLE);
> +
…
Not sure it matters but this struct `dev' is allocated on stack. Should
the interrupt fire *before* rcuwait_wait_event() set wait.task to NULL
then it is of random value on the first invocation of rcuwait_wake_up().
->
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 197347c3c0b24..e87360a0fb40d 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -809,6 +809,7 @@ static int ps3_probe_thread(void *data)
}
spin_lock_init(&dev.lock);
+ rcuwait_init(&dev.wait);
res = request_irq(irq, ps3_notification_interrupt, 0,
"ps3_notification", &dev);
Sebastian
^ permalink raw reply related
* Re: [PATCH v2 4/8] powernv/memtrace: always online added memory blocks
From: Michael Ellerman @ 2020-03-19 9:49 UTC (permalink / raw)
To: David Hildenbrand, linux-kernel
Cc: linux-hyperv, Baoquan He, David Hildenbrand, Rafael J. Wysocki,
Michal Hocko, linux-mm, Paul Mackerras, Greg Kroah-Hartman,
Andrew Morton, Wei Yang, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200317104942.11178-5-david@redhat.com>
David Hildenbrand <david@redhat.com> writes:
> Let's always try to online the re-added memory blocks. In case add_memory()
> already onlined the added memory blocks, the first device_online() call
> will fail and stop processing the remaining memory blocks.
>
> This avoids manually having to check memhp_auto_online.
>
> Note: PPC always onlines all hotplugged memory directly from the kernel
> as well - something that is handled by user space on other
> architectures.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
Fine by me.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
> index d6d64f8718e6..13b369d2cc45 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -231,16 +231,10 @@ static int memtrace_online(void)
> continue;
> }
>
> - /*
> - * If kernel isn't compiled with the auto online option
> - * we need to online the memory ourselves.
> - */
> - if (!memhp_auto_online) {
> - lock_device_hotplug();
> - walk_memory_blocks(ent->start, ent->size, NULL,
> - online_mem_block);
> - unlock_device_hotplug();
> - }
> + lock_device_hotplug();
> + walk_memory_blocks(ent->start, ent->size, NULL,
> + online_mem_block);
> + unlock_device_hotplug();
>
> /*
> * Memory was added successfully so clean up references to it
> --
> 2.24.1
^ permalink raw reply
* Re: [patch V2 07/15] powerpc/ps3: Convert half completion to rcuwait
From: Christoph Hellwig @ 2020-03-19 10:04 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-usb, Peter Zijlstra, linux-pci, Sebastian Andrzej Siewior,
Oleg Nesterov, Joel Fernandes, Will Deacon, Ingo Molnar,
Davidlohr Bueso, Paul E . McKenney, Logan Gunthorpe,
Linus Torvalds, Arnd Bergmann, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
Randy Dunlap, linux-wireless, LKML, netdev, linuxppc-dev,
David S. Miller
In-Reply-To: <20200318204408.102694393@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:09PM +0100, Thomas Gleixner wrote:
> The PS3 notification interrupt and kthread use a hacked up completion to
> communicate. Since we're wanting to change the completion implementation and
> this is abuse anyway, replace it with a simple rcuwait since there is only ever
> the one waiter.
>
> AFAICT the kthread uses TASK_INTERRUPTIBLE to not increase loadavg, kthreads
> cannot receive signals by default and this one doesn't look different. Use
> TASK_IDLE instead.
I think the right fix here is to jut convert the thing to a threaded
interrupt handler and kill off the stupid kthread.
But I wonder how alive the whole PS3 support is to start with..
^ permalink raw reply
* [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
From: Borislav Petkov @ 2020-03-19 10:16 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
Peter Zijlstra, x86, Heiko Carstens, lkml, Christian Borntraeger,
iommu, Ingo Molnar, Paul Mackerras, Tom Lendacky, Andy Lutomirski,
Thomas Gleixner, Robin Murphy, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <20200317111822.GA15609@zn.tnic>
Hi,
here's v2 with build breakage fixed on ppc and s390 (obviously I can't
grep :-\) and commit message extended to explain more why.
Thx.
---
From: Borislav Petkov <bp@suse.de>
Date: Tue, 17 Mar 2020 12:03:05 +0100
Back then when the whole SME machinery started getting mainlined, it
was agreed that for simplicity, clarity and sanity's sake, the terms
denoting encrypted and not-encrypted memory should be "encrypted" and
"decrypted". And the majority of the code sticks to that convention
except those two. So rename them.
The intent of "encrypted" and "decrypted" is to represent the binary
concept of memory which is encrypted and of memory which is not.
The further differentiation between decrypted and unencrypted memory is
not needed in the code (for now) and therefore keep things simple by
using solely the two terms.
No functional changes.
[ Convert forgotten s390 and ppc function variants. ]
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/powerpc/include/asm/mem_encrypt.h | 2 +-
arch/powerpc/platforms/pseries/Kconfig | 2 +-
arch/s390/Kconfig | 2 +-
arch/s390/mm/init.c | 2 +-
arch/x86/Kconfig | 2 +-
arch/x86/mm/mem_encrypt.c | 4 ++--
include/linux/dma-direct.h | 8 ++++----
kernel/dma/Kconfig | 2 +-
kernel/dma/direct.c | 14 +++++++-------
kernel/dma/mapping.c | 2 +-
10 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/mem_encrypt.h b/arch/powerpc/include/asm/mem_encrypt.h
index ba9dab07c1be..f0705cd3b079 100644
--- a/arch/powerpc/include/asm/mem_encrypt.h
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -15,7 +15,7 @@ static inline bool mem_encrypt_active(void)
return is_secure_guest();
}
-static inline bool force_dma_unencrypted(struct device *dev)
+static inline bool force_dma_decrypted(struct device *dev)
{
return is_secure_guest();
}
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 24c18362e5ea..a78e2c3e1d92 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -151,7 +151,7 @@ config PPC_SVM
depends on PPC_PSERIES
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8abe77536d9d..ab1dbb7415b4 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -192,7 +192,7 @@ config S390
select VIRT_CPU_ACCOUNTING
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
select SWIOTLB
select GENERIC_ALLOCATOR
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index ac44bd76db4b..5fed85f9fea6 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -157,7 +157,7 @@ int set_memory_decrypted(unsigned long addr, int numpages)
}
/* are we a protected virtualization guest? */
-bool force_dma_unencrypted(struct device *dev)
+bool force_dma_decrypted(struct device *dev)
{
return is_prot_virt_guest();
}
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..2ae904f505e1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1525,7 +1525,7 @@ config AMD_MEM_ENCRYPT
depends on X86_64 && CPU_SUP_AMD
select DYNAMIC_PHYSICAL_MASK
select ARCH_USE_MEMREMAP_PROT
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
---help---
Say yes to enable support for the encryption of system memory.
This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index a03614bd3e1a..66d09f269e6d 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -350,8 +350,8 @@ bool sev_active(void)
return sme_me_mask && sev_enabled;
}
-/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
-bool force_dma_unencrypted(struct device *dev)
+/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_DECRYPTED */
+bool force_dma_decrypted(struct device *dev)
{
/*
* For SEV, all DMA must be to unencrypted addresses.
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 24b8684aa21d..9f955844e9c7 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -26,14 +26,14 @@ static inline phys_addr_t __dma_to_phys(struct device *dev, dma_addr_t dev_addr)
}
#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
-#ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
-bool force_dma_unencrypted(struct device *dev);
+#ifdef CONFIG_ARCH_HAS_FORCE_DMA_DECRYPTED
+bool force_dma_decrypted(struct device *dev);
#else
-static inline bool force_dma_unencrypted(struct device *dev)
+static inline bool force_dma_decrypted(struct device *dev)
{
return false;
}
-#endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
+#endif /* CONFIG_ARCH_HAS_FORCE_DMA_DECRYPTED */
/*
* If memory encryption is supported, phys_to_dma will set the memory encryption
diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
index 4c103a24e380..55c4147bb2b1 100644
--- a/kernel/dma/Kconfig
+++ b/kernel/dma/Kconfig
@@ -51,7 +51,7 @@ config ARCH_HAS_SYNC_DMA_FOR_CPU_ALL
config ARCH_HAS_DMA_PREP_COHERENT
bool
-config ARCH_HAS_FORCE_DMA_UNENCRYPTED
+config ARCH_HAS_FORCE_DMA_DECRYPTED
bool
config DMA_NONCOHERENT_CACHE_SYNC
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index ac7956c38f69..a0576c0ccacd 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -26,7 +26,7 @@ unsigned int zone_dma_bits __ro_after_init = 24;
static inline dma_addr_t phys_to_dma_direct(struct device *dev,
phys_addr_t phys)
{
- if (force_dma_unencrypted(dev))
+ if (force_dma_decrypted(dev))
return __phys_to_dma(dev, phys);
return phys_to_dma(dev, phys);
}
@@ -49,7 +49,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
{
u64 dma_limit = min_not_zero(dma_mask, dev->bus_dma_limit);
- if (force_dma_unencrypted(dev))
+ if (force_dma_decrypted(dev))
*phys_limit = __dma_to_phys(dev, dma_limit);
else
*phys_limit = dma_to_phys(dev, dma_limit);
@@ -138,7 +138,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
return NULL;
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
- !force_dma_unencrypted(dev)) {
+ !force_dma_decrypted(dev)) {
/* remove any dirty cache lines on the kernel alias */
if (!PageHighMem(page))
arch_dma_prep_coherent(page, size);
@@ -179,7 +179,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
}
ret = page_address(page);
- if (force_dma_unencrypted(dev))
+ if (force_dma_decrypted(dev))
set_memory_decrypted((unsigned long)ret, 1 << get_order(size));
memset(ret, 0, size);
@@ -190,7 +190,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
ret = uncached_kernel_address(ret);
}
done:
- if (force_dma_unencrypted(dev))
+ if (force_dma_decrypted(dev))
*dma_handle = __phys_to_dma(dev, page_to_phys(page));
else
*dma_handle = phys_to_dma(dev, page_to_phys(page));
@@ -203,7 +203,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
unsigned int page_order = get_order(size);
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
- !force_dma_unencrypted(dev)) {
+ !force_dma_decrypted(dev)) {
/* cpu_addr is a struct page cookie, not a kernel address */
dma_free_contiguous(dev, cpu_addr, size);
return;
@@ -213,7 +213,7 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
dma_free_from_pool(cpu_addr, PAGE_ALIGN(size)))
return;
- if (force_dma_unencrypted(dev))
+ if (force_dma_decrypted(dev))
set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr))
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 98e3d873792e..dbd0605a39c5 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -154,7 +154,7 @@ EXPORT_SYMBOL(dma_get_sgtable_attrs);
*/
pgprot_t dma_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
{
- if (force_dma_unencrypted(dev))
+ if (force_dma_decrypted(dev))
prot = pgprot_decrypted(prot);
if (dev_is_dma_coherent(dev) ||
(IS_ENABLED(CONFIG_DMA_NONCOHERENT_CACHE_SYNC) &&
--
2.21.0
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related
* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
From: Christoph Hellwig @ 2020-03-19 10:20 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
Peter Zijlstra, x86, Heiko Carstens, lkml, Christian Borntraeger,
iommu, Ingo Molnar, Paul Mackerras, Tom Lendacky, Andy Lutomirski,
Thomas Gleixner, Borislav Petkov, Robin Murphy, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <20200319101657.GB13073@zn.tnic>
On Thu, Mar 19, 2020 at 11:16:57AM +0100, Borislav Petkov wrote:
> Hi,
>
> here's v2 with build breakage fixed on ppc and s390 (obviously I can't
> grep :-\) and commit message extended to explain more why.
I thought we agreed that decrypted is absolutely the wrong term.
So NAK - if you want to change things it needs to go the other way.
^ permalink raw reply
* Re: [PATCH] cpufreq: powernv: Fix frame-size-overflow in powernv_cpufreq_work_fn
From: Rafael J. Wysocki @ 2020-03-19 10:18 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, pratik.r.sampat, linux-pm, linux-kernel, linuxppc-dev, dja
In-Reply-To: <20200316135743.57735-1-psampat@linux.ibm.com>
On Monday, March 16, 2020 2:57:43 PM CET Pratik Rajesh Sampat wrote:
> The patch avoids allocating cpufreq_policy on stack hence fixing frame
> size overflow in 'powernv_cpufreq_work_fn'
>
> Fixes: 227942809b52 ("cpufreq: powernv: Restore cpu frequency to policy->cur on unthrottling")
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Any objections or concerns here?
If not, I'll queue it up.
> ---
> drivers/cpufreq/powernv-cpufreq.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 56f4bc0d209e..20ee0661555a 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -902,6 +902,7 @@ static struct notifier_block powernv_cpufreq_reboot_nb = {
> void powernv_cpufreq_work_fn(struct work_struct *work)
> {
> struct chip *chip = container_of(work, struct chip, throttle);
> + struct cpufreq_policy *policy;
> unsigned int cpu;
> cpumask_t mask;
>
> @@ -916,12 +917,14 @@ void powernv_cpufreq_work_fn(struct work_struct *work)
> chip->restore = false;
> for_each_cpu(cpu, &mask) {
> int index;
> - struct cpufreq_policy policy;
>
> - cpufreq_get_policy(&policy, cpu);
> - index = cpufreq_table_find_index_c(&policy, policy.cur);
> - powernv_cpufreq_target_index(&policy, index);
> - cpumask_andnot(&mask, &mask, policy.cpus);
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy)
> + continue;
> + index = cpufreq_table_find_index_c(policy, policy->cur);
> + powernv_cpufreq_target_index(policy, index);
> + cpumask_andnot(&mask, &mask, policy->cpus);
> + cpufreq_cpu_put(policy);
> }
> out:
> put_online_cpus();
>
^ permalink raw reply
* Re: [patch V2 07/15] powerpc/ps3: Convert half completion to rcuwait
From: Sebastian Andrzej Siewior @ 2020-03-19 10:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-usb, Peter Zijlstra, linux-pci, Oleg Nesterov,
Joel Fernandes, Will Deacon, Thomas Gleixner, Davidlohr Bueso,
Paul E . McKenney, Logan Gunthorpe, Ingo Molnar, Linus Torvalds,
Arnd Bergmann, Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer,
Kalle Valo, Felipe Balbi, Greg Kroah-Hartman, Randy Dunlap,
linux-wireless, LKML, netdev, linuxppc-dev, David S. Miller
In-Reply-To: <20200319100459.GA18506@infradead.org>
On 2020-03-19 03:04:59 [-0700], Christoph Hellwig wrote:
> But I wonder how alive the whole PS3 support is to start with..
OtherOS can only be used on "old" PS3 which do not have have their
firmware upgraded past version 3.21, released April 1, 2010 [0].
It was not possible to install OtherOS on PS3-slim and I don't remember
if it was a successor or a budget version (but it had lower power
consumption as per my memory).
*I* remember from back then that a few universities bought quite a few
of them and used them as a computation cluster. However, whatever broke
over the last 10 years is broken.
[0] https://en.wikipedia.org/wiki/OtherOS
Sebastian
^ permalink raw reply
* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
From: Borislav Petkov @ 2020-03-19 10:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, Dave Hansen, Vasily Gorbik, linuxppc-dev,
Peter Zijlstra, x86, Heiko Carstens, lkml, Christian Borntraeger,
iommu, Ingo Molnar, Paul Mackerras, Tom Lendacky, Andy Lutomirski,
Thomas Gleixner, Robin Murphy, Marek Szyprowski
In-Reply-To: <20200319102011.GA3617@lst.de>
On Thu, Mar 19, 2020 at 11:20:11AM +0100, Christoph Hellwig wrote:
> I thought we agreed that decrypted is absolutely the wrong term.
I don't think we did. At least I don't know where we did that.
> So NAK - if you want to change things it needs to go the other way.
We are already using "decrypted" everywhere in arch/x86/. Changing that
would be a *lot* more churn.
And it is just a term, for chrissakes, to denote memory which is not
encrypted. And it would make our lifes easier if we had only *two* terms
instead of three or more. Especially if the concept we denote with this
is a binary one: encrypted memory and *not* encrypted memory.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v3] powerpc/perf: Use SIER_USER_MASK while updating SPRN_SIER for EBB events
From: Michael Ellerman @ 2020-03-19 10:52 UTC (permalink / raw)
To: Athira Rajeev; +Cc: maddy, linuxppc-dev
In-Reply-To: <1584533181-4331-1-git-send-email-atrajeev@linux.vnet.ibm.com>
Hi Athira,
Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:
> Sampled Instruction Event Register (SIER), is a PMU register,
^
that
> captures architecture state for a given sample. And sier_user_mask
^ ^
don't think we need "architecture" SIER_USER_MASK
> defined in commit 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit
> book3s") defines the architected bits that needs to be saved from the SPR.
Not quite, it defines the bits that are visible to userspace.
And I think it's true that for EBB events the bits we need/want to save
are only the user visible bits.
> Currently all of the bits from SIER are saved for EBB events. Patch fixes
> this by ANDing the "sier_user_mask" to data from SIER in ebb_switch_out().
> This will force save only architected bits from the SIER.
s/architected/user visible/
But, why does it matter? The kernel saves the user visible bits, as well
as the kernel-only bits into the thread struct. And then later the
kernel restores that value into the hardware before returning to
userspace.
But the hardware enforces the visibility of the bits, so userspace can't
observe any bits that it shouldn't.
Or is there some other mechanism whereby userspace can see those bits? ;)
If there was, what would the security implications of that be?
cheers
> Fixes: 330a1eb7775b ("powerpc/perf: Core EBB support for 64-bit book3s")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
> v2 -> v3:
> - Corrected name of SIER register in commit message
> as pointed by Segher Boessenkool
> v1 -> v2:
> - Make the commit message more clearer.
>
> arch/powerpc/perf/core-book3s.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3086055..48b61cc 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -579,7 +579,7 @@ static void ebb_switch_out(unsigned long mmcr0)
> return;
>
> current->thread.siar = mfspr(SPRN_SIAR);
> - current->thread.sier = mfspr(SPRN_SIER);
> + current->thread.sier = mfspr(SPRN_SIER) & SIER_USER_MASK;
> current->thread.sdar = mfspr(SPRN_SDAR);
> current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
> current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
From: Robin Murphy @ 2020-03-19 11:06 UTC (permalink / raw)
To: Borislav Petkov, Christoph Hellwig
Cc: linux-s390, Dave Hansen, Vasily Gorbik, Tom Lendacky,
Peter Zijlstra, x86, Heiko Carstens, lkml, Christian Borntraeger,
iommu, Ingo Molnar, Paul Mackerras, Andy Lutomirski,
Thomas Gleixner, linuxppc-dev, Marek Szyprowski
In-Reply-To: <20200319102834.GC13073@zn.tnic>
[since this is in my inbox...]
On 2020-03-19 10:28 am, Borislav Petkov wrote:
> On Thu, Mar 19, 2020 at 11:20:11AM +0100, Christoph Hellwig wrote:
>> I thought we agreed that decrypted is absolutely the wrong term.
>
> I don't think we did. At least I don't know where we did that.
>
>> So NAK - if you want to change things it needs to go the other way.
>
> We are already using "decrypted" everywhere in arch/x86/. Changing that
> would be a *lot* more churn.
>
> And it is just a term, for chrissakes, to denote memory which is not
> encrypted. And it would make our lifes easier if we had only *two* terms
> instead of three or more. Especially if the concept we denote with this
> is a binary one: encrypted memory and *not* encrypted memory.
Let me add another vote from a native English speaker that "unencrypted"
is the appropriate term to imply the *absence* of encryption, whereas
"decrypted" implies the *reversal* of applied encryption.
Naming things is famously hard, for good reason - names are *important*
for understanding. Just because a decision was already made one way
doesn't mean that that decision was necessarily right. Churning one area
to be consistently inaccurate just because it's less work than churning
another area to be consistently accurate isn't really the best excuse.
Robin.
^ 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