Re: [RFC PATCH 2/3] crypto: nx - don't abuse shash MAY_SLEEP flag

2019-04-14 Thread Michael Ellerman
Eric Biggers  writes:
> From: Eric Biggers 
>
> The nx driver uses the MAY_SLEEP flag in shash_desc::flags as an
> indicator to not retry sending the operation to the hardware as many
> times before returning -EBUSY.  This is bogus because (1) that's not
> what the MAY_SLEEP flag is for, and (2) the shash API doesn't allow
> failing if the hardware is busy anyway.

Can you elaborate a bit on that 2nd point? What is the driver meant to
do if the hardware is busy or out to lunch? Retry forever?

> For now, just make it always retry the larger number of times.  This
> doesn't actually fix this driver, but it at least makes it not use the
> shash_desc::flags field anymore.  Then this field can be removed, as no
> other drivers use it.

This looks fine to me, thanks for fixing it up.

cheers


[PATCH] powerpc/powernv: Remove ifdef

2019-04-14 Thread Oliver O'Halloran
The PowerNV platform depends on CONFIG_PPC64 so this #ifdef in the middle
of platforms/powernv/pci.c really doesn't need to be there.

Signed-off-by: Oliver O'Halloran 
---
 arch/powerpc/platforms/powernv/pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c 
b/arch/powerpc/platforms/powernv/pci.c
index d235c2c..a5b2c7b 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -1039,7 +1039,6 @@ int pnv_pci_set_tunnel_bar(struct pci_dev *dev, u64 addr, 
int enable)
 }
 EXPORT_SYMBOL_GPL(pnv_pci_set_tunnel_bar);
 
-#ifdef CONFIG_PPC64/* for thread.tidr */
 int pnv_pci_get_as_notify_info(struct task_struct *task, u32 *lpid, u32 *pid,
   u32 *tid)
 {
@@ -1060,7 +1059,6 @@ int pnv_pci_get_as_notify_info(struct task_struct *task, 
u32 *lpid, u32 *pid,
return 0;
 }
 EXPORT_SYMBOL_GPL(pnv_pci_get_as_notify_info);
-#endif
 
 void pnv_pci_shutdown(void)
 {
-- 
2.9.4



Re: [PATCH 0/8]

2019-04-14 Thread Sam Bobroff
On Thu, Apr 11, 2019 at 05:55:33PM -0700, Tyrel Datwyler wrote:
> On 03/19/2019 07:58 PM, Sam Bobroff wrote:
> > Hi all,
> > 
> > This patch set adds support for EEH recovery of hot plugged devices on 
> > pSeries
> > machines. Specifically, devices discovered by PCI rescanning using
> > /sys/bus/pci/rescan, which includes devices hotplugged by QEMU's device_add
> > command. (pSeries doesn't currently use slot power control for hotplugging.)
> 
> Slight nit that its not that pSeries doesn't support slot power control
> hotplugging, its that QEMU pSeries guests don't support it. We most definitely
> use the slot power control for hotplugging in PowerVM pSeries Linux guests. 
> More

Ah, I think I see what you mean: pSeries can (and does!) use slot power
control for hotplugging, it's just that Linux doesn't. Right :-) I'll
change it to "Linux on pSeries doesn't" for the next version.

> specifically we had to work around short comings in the rpaphp driver when
> dealing with QEMU. This being that while at initial glance the design implies
> that it had multiple devices per PHB in mind, it didn't, and only actually
> supported a single slot per PHB. Further, when we developed the QEMU pci 
> hotplug
> feature we had to deal with only having a single PHB per QEMU guest and as a
> result needed a way to plug multiple pci devices into a single PHB. Hence, 
> came
> the pci rescan work around in drmgr.
> 
> Mike Roth and I have had discussions over the years to get the slot power
> control hotplugging working properly with QEMU, and while I did get the RPA
> hotplug driver fixed to register all available slots associated with a PHB, 
> EEH
> remained an issue. So, I'm very happy to see this patchset get that working 
> with
> the rescan work around.
> 
> > 
> > As a side effect this also provides EEH support for devices removed by
> > /sys/bus/pci/devices/*/remove and re-discovered by writing to 
> > /sys/bus/pci/rescan,
> > on all platforms.
> 
> +1, this seems like icing on the cake. ;)

Yes :-)

Although maybe I should mention that we can't really benefit from this
on PowerNV *yet* because there seem to be some other problems with
removing and re-scanning devices: in my tests devices are often unusable
after being rediscovered.

(I'm hoping to take a look at that soon.)

> > 
> > The approach I've taken is to use the fact that the existing
> > pcibios_bus_add_device() platform hooks (which are used to set up EEH on
> > Virtual Function devices (VFs)) are actually called for all devices, so I've
> > widened their scope and made other adjustments necessary to allow them to 
> > work
> > for hotplugged and boot-time devices as well.
> > 
> > Because some of the changes are in generic PowerPC code, it's
> > possible that I've disturbed something for another PowerPC platform. I've 
> > tried
> > to minimize this by leaving that code alone as much as possible and so there
> > are a few cases where eeh_add_device_{early,late}() or 
> > eeh_add_sysfs_files() is
> > called more than once. I think these can be looked at later, as duplicate 
> > calls
> > are not harmful.
> > 
> > The patch "Convert PNV_PHB_FLAG_EEH" isn't strictly necessary and I'm not 
> > sure
> > if it's better to keep it, because it simplifies the code or drop it, 
> > because
> > we may need a separate flag per PHB later on. Thoughts anyone?
> > 
> > The first patch is a rework of the pcibios_init reordering patch I posted
> > earlier, which I've included here because it's necessary for this set.
> > 
> > I have done some testing for PowerNV on Power9 using a modified pnv_php 
> > module
> > and some testing on pSeries with slot power control using a modified rpaphp
> > module, and the EEH-related parts seem to work.
> 
> I'm interested in what modifications with rpaphp. Its unclear if you are 
> saying
> rpaphp modified so that slot power hotplug works with a QEMU pSeries guest? If
> thats the case it would be optimal to get that upstream and remove the work
> rescan workaround for guests that don't need it.

Unfortunately no, I didn't do enough work to really get it working.  I
just wanted to get an idea of how that code path interacted with the EEH
code I was changing, so that hopefully when we get to fixing it, the EEH
part will be easier to do.

The hack I tested with was:

- rtas_errd changed so that it doesn't pass -V to drmgr (-V seems to
  trigger drmgr to use the PCI rescan system rather that slot power
  control).
- of_pci_parse_addrs() changed so that if the assigned-addresses node
  is missing (which it is when the guest is running under QEMU/KVM) we
  call pci_setup_device() to configure the BARs.

It did look pretty good -- the EEH part may actually work fine once we get
the rest sorted out.

> 
> -Tyrel
> 
> > 
> > Cheers,
> > Sam.
> > 
> > Sam Bobroff (8):
> >   powerpc/64: Adjust order in pcibios_init()
> >   powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
> >   powerpc/eeh: Convert PNV_PHB_FLAG_EEH to global flag
> >   power

Re: [PATCH v3] powerpc/xmon: add read-only mode

2019-04-14 Thread Oliver
On Mon, Apr 15, 2019 at 10:17 AM Christopher M. Riedl
 wrote:
>
> Operations which write to memory and special purpose registers should be
> restricted on systems with integrity guarantees (such as Secure Boot)
> and, optionally, to avoid self-destructive behaviors.
>
> Add a config option, XMON_DEFAULT_RO_MODE, to set default xmon behavior.
> The kernel cmdline options xmon=ro and xmon=rw override this default.
>
> The following xmon operations are affected:
> memops:
> disable memmove
> disable memset
> disable memzcan
> memex:
> no-op'd mwrite
> super_regs:
> no-op'd write_spr
> bpt_cmds:
> disable
> proc_call:
> disable
>
> Signed-off-by: Christopher M. Riedl 

Reviewed-by: Oliver O'Halloran 

> ---
>
> v2->v3:
> Use XMON_DEFAULT_RO_MODE to set xmon read-only mode
> Untangle read-only mode from STRICT_KERNEL_RWX and PAGE_KERNEL_ROX
> Update printed msg string for write ops in read-only mode

Making PAGE_KERNEL_ROX the default in more cases is still a good idea.
Feel free to send a follow up patch if you want to keep pulling on
that thread.

>  arch/powerpc/Kconfig.debug |  8 
>  arch/powerpc/xmon/xmon.c   | 42 ++
>  2 files changed, 50 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index 4e00cb0a5464..8de4823dfb86 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -117,6 +117,14 @@ config XMON_DISASSEMBLY
>   to say Y here, unless you're building for a memory-constrained
>   system.
>
> +config XMON_DEFAULT_RO_MODE
> +   bool "Restrict xmon to read-only operations"
> +   depends on XMON
> +   default y
> +   help
> +  Operate xmon in read-only mode. The cmdline options 'xmon=rw' and
> +  'xmon=ro' override this default.
> +
>  config DEBUGGER
> bool
> depends on KGDB || XMON
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index a0f44f992360..ce98c8049eb6 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -80,6 +80,7 @@ static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
>  #endif
>  static unsigned long in_xmon __read_mostly = 0;
>  static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
> +static bool xmon_is_ro = IS_ENABLED(CONFIG_XMON_DEFAULT_RO_MODE);
>
>  static unsigned long adrs;
>  static int size = 1;
> @@ -202,6 +203,8 @@ static void dump_tlb_book3e(void);
>  #define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + 
> (v)[3])
>  #endif
>
> +static const char *xmon_ro_msg = "Operation disabled: xmon in read-only 
> mode\n";
> +
>  static char *help_string = "\
>  Commands:\n\
>bshow breakpoints\n\
> @@ -989,6 +992,10 @@ cmds(struct pt_regs *excp)
> memlocate();
> break;
> case 'z':
> +   if (xmon_is_ro) {
> +   printf(xmon_ro_msg);
> +   break;
> +   }
> memzcan();
> break;
> case 'i':
> @@ -1042,6 +1049,10 @@ cmds(struct pt_regs *excp)
> set_lpp_cmd();
> break;
> case 'b':
> +   if (xmon_is_ro) {
> +   printf(xmon_ro_msg);
> +   break;
> +   }
> bpt_cmds();
> break;
> case 'C':
> @@ -1055,6 +1066,10 @@ cmds(struct pt_regs *excp)
> bootcmds();
> break;
> case 'p':
> +   if (xmon_is_ro) {
> +   printf(xmon_ro_msg);
> +   break;
> +   }
> proccall();
> break;
> case 'P':
> @@ -1777,6 +1792,11 @@ read_spr(int n, unsigned long *vp)
>  static void
>  write_spr(int n, unsigned long val)
>  {
> +   if (xmon_is_ro) {
> +   printf(xmon_ro_msg);
> +   return;
> +   }
> +
> if (setjmp(bus_error_jmp) == 0) {
> catch_spr_faults = 1;
> sync();
> @@ -2016,6 +2036,12 @@ mwrite(unsigned long adrs, void *buf, int size)
> char *p, *q;
>
> n = 0;
> +
> +   if (xmon_is_ro) {
> +   printf(xmon_ro_msg);
> +   return n;
> +   }
> +
> if (setjmp(bus_error_jmp) == 0) {
> catch_memory_errors = 1;
> sync();
> @@ -2884,9 +2910,17 @@ memops(int cmd)
> scanhex((void *)&mcount);
> switch( cmd ){
> case 'm':
> +   if (xmon_is_ro) {
> +   printf(xmo

Re: [PATCH 3/3] mm: introduce ARCH_HAS_PTE_DEVMAP

2019-04-14 Thread Oliver
On Sat, Apr 13, 2019 at 4:57 AM Robin Murphy  wrote:
>
> ARCH_HAS_ZONE_DEVICE is somewhat meaningless in itself, and combined
> with the long-out-of-date comment can lead to the impression than an
> architecture may just enable it (since __add_pages() now "comprehends
> device memory" for itself) and expect things to work.

Good cleanup. ARCH_HAS_ZONE_DEVICE made sense at the time, but it
probably should have been renamed after the memory hotplug rework.

+cc mpe since he does the merging, and

Acked-by: Oliver O'Halloran 

> In practice, however, ZONE_DEVICE users have little chance of
> functioning correctly without __HAVE_ARCH_PTE_DEVMAP, so let's clean
> that up the same way as ARCH_HAS_PTE_SPECIAL and make it the proper
> dependency so the real situation is clearer.
>
> Signed-off-by: Robin Murphy 
> ---
>  arch/powerpc/Kconfig | 2 +-
>  arch/powerpc/include/asm/book3s/64/pgtable.h | 1 -
>  arch/x86/Kconfig | 2 +-
>  arch/x86/include/asm/pgtable.h   | 4 ++--
>  arch/x86/include/asm/pgtable_types.h | 1 -
>  include/linux/mm.h   | 4 ++--
>  include/linux/pfn_t.h| 4 ++--
>  mm/Kconfig   | 5 ++---
>  mm/gup.c | 2 +-
>  9 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 5e3d0853c31d..77e1993bba80 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -135,6 +135,7 @@ config PPC
> select ARCH_HAS_MMIOWB  if PPC64
> select ARCH_HAS_PHYS_TO_DMA
> select ARCH_HAS_PMEM_APIif PPC64
> +   select ARCH_HAS_PTE_DEVMAP  if PPC_BOOK3S_64
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_MEMBARRIER_CALLBACKS
> select ARCH_HAS_SCALED_CPUTIME  if VIRT_CPU_ACCOUNTING_NATIVE 
> && PPC64
> @@ -142,7 +143,6 @@ config PPC
> select ARCH_HAS_TICK_BROADCAST  if 
> GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE  if PPC64
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> -   select ARCH_HAS_ZONE_DEVICE if PPC_BOOK3S_64
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
> b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 581f91be9dd4..02c22ac8f387 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -90,7 +90,6 @@
>  #define _PAGE_SOFT_DIRTY   _RPAGE_SW3 /* software: software dirty 
> tracking */
>  #define _PAGE_SPECIAL  _RPAGE_SW2 /* software: special page */
>  #define _PAGE_DEVMAP   _RPAGE_SW1 /* software: ZONE_DEVICE page */
> -#define __HAVE_ARCH_PTE_DEVMAP
>
>  /*
>   * Drivers request for cache inhibited pte mapping using _PAGE_NO_CACHE
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5ad92419be19..ffd50f27f395 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -60,6 +60,7 @@ config X86
> select ARCH_HAS_KCOVif X86_64
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_PMEM_APIif X86_64
> +   select ARCH_HAS_PTE_DEVMAP  if X86_64
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_REFCOUNT
> select ARCH_HAS_UACCESS_FLUSHCACHE  if X86_64
> @@ -69,7 +70,6 @@ config X86
> select ARCH_HAS_STRICT_MODULE_RWX
> select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> -   select ARCH_HAS_ZONE_DEVICE if X86_64
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 2779ace16d23..89a1f6fd48bf 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -254,7 +254,7 @@ static inline int has_transparent_hugepage(void)
> return boot_cpu_has(X86_FEATURE_PSE);
>  }
>
> -#ifdef __HAVE_ARCH_PTE_DEVMAP
> +#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
>  static inline int pmd_devmap(pmd_t pmd)
>  {
> return !!(pmd_val(pmd) & _PAGE_DEVMAP);
> @@ -715,7 +715,7 @@ static inline int pte_present(pte_t a)
> return pte_flags(a) & (_PAGE_PRESENT | _PAGE_PROTNONE);
>  }
>
> -#ifdef __HAVE_ARCH_PTE_DEVMAP
> +#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
>  static inline int pte_devmap(pte_t a)
>  {
> return (pte_flags(a) & _PAGE_DEVMAP) == _PAGE_DEVMAP;
> diff --git a/arch/x86/include/asm/pgtable_types.h 
> b/arch/x86/include/asm/pgtable_types.h
> index d6ff0bbdb394..b5e49e6bac63 100644
> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -103,7 +103,6 @@
>  #if

[RFC PATCH 2/3] crypto: nx - don't abuse shash MAY_SLEEP flag

2019-04-14 Thread Eric Biggers
From: Eric Biggers 

The nx driver uses the MAY_SLEEP flag in shash_desc::flags as an
indicator to not retry sending the operation to the hardware as many
times before returning -EBUSY.  This is bogus because (1) that's not
what the MAY_SLEEP flag is for, and (2) the shash API doesn't allow
failing if the hardware is busy anyway.

For now, just make it always retry the larger number of times.  This
doesn't actually fix this driver, but it at least makes it not use the
shash_desc::flags field anymore.  Then this field can be removed, as no
other drivers use it.

Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers 
---
 drivers/crypto/nx/nx-aes-xcbc.c | 12 
 drivers/crypto/nx/nx-sha256.c   |  6 ++
 drivers/crypto/nx/nx-sha512.c   |  6 ++
 3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index ad3358e74f5c4..8f5820b78a83b 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -105,8 +105,7 @@ static int nx_xcbc_empty(struct shash_desc *desc, u8 *out)
nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg);
nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg);
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
atomic_inc(&(nx_ctx->stats->aes_ops));
@@ -134,8 +133,7 @@ static int nx_xcbc_empty(struct shash_desc *desc, u8 *out)
nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg);
nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg);
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
atomic_inc(&(nx_ctx->stats->aes_ops));
@@ -279,8 +277,7 @@ static int nx_xcbc_update(struct shash_desc *desc,
goto out;
}
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
 
@@ -361,8 +358,7 @@ static int nx_xcbc_final(struct shash_desc *desc, u8 *out)
goto out;
}
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
 
diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index a6764af83c6dc..e06f0431dee5f 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -162,8 +162,7 @@ static int nx_sha256_update(struct shash_desc *desc, const 
u8 *data,
goto out;
}
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
 
@@ -243,8 +242,7 @@ static int nx_sha256_final(struct shash_desc *desc, u8 *out)
goto out;
}
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
 
diff --git a/drivers/crypto/nx/nx-sha512.c b/drivers/crypto/nx/nx-sha512.c
index 92956bc6e45ee..0293b17903d0c 100644
--- a/drivers/crypto/nx/nx-sha512.c
+++ b/drivers/crypto/nx/nx-sha512.c
@@ -166,8 +166,7 @@ static int nx_sha512_update(struct shash_desc *desc, const 
u8 *data,
goto out;
}
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
 
@@ -249,8 +248,7 @@ static int nx_sha512_final(struct shash_desc *desc, u8 *out)
goto out;
}
 
-   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-  desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+   rc = nx_hcall_sync(nx_ctx, &nx_ctx->op, 0);
if (rc)
goto out;
 
-- 
2.21.0



[PATCH v3] powerpc/xmon: add read-only mode

2019-04-14 Thread Christopher M. Riedl
Operations which write to memory and special purpose registers should be
restricted on systems with integrity guarantees (such as Secure Boot)
and, optionally, to avoid self-destructive behaviors.

Add a config option, XMON_DEFAULT_RO_MODE, to set default xmon behavior.
The kernel cmdline options xmon=ro and xmon=rw override this default.

The following xmon operations are affected:
memops:
disable memmove
disable memset
disable memzcan
memex:
no-op'd mwrite
super_regs:
no-op'd write_spr
bpt_cmds:
disable
proc_call:
disable

Signed-off-by: Christopher M. Riedl 
---

v2->v3:
Use XMON_DEFAULT_RO_MODE to set xmon read-only mode
Untangle read-only mode from STRICT_KERNEL_RWX and PAGE_KERNEL_ROX
Update printed msg string for write ops in read-only mode

 arch/powerpc/Kconfig.debug |  8 
 arch/powerpc/xmon/xmon.c   | 42 ++
 2 files changed, 50 insertions(+)

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 4e00cb0a5464..8de4823dfb86 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -117,6 +117,14 @@ config XMON_DISASSEMBLY
  to say Y here, unless you're building for a memory-constrained
  system.
 
+config XMON_DEFAULT_RO_MODE
+   bool "Restrict xmon to read-only operations"
+   depends on XMON
+   default y
+   help
+  Operate xmon in read-only mode. The cmdline options 'xmon=rw' and
+  'xmon=ro' override this default.
+
 config DEBUGGER
bool
depends on KGDB || XMON
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0f44f992360..ce98c8049eb6 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -80,6 +80,7 @@ static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
 #endif
 static unsigned long in_xmon __read_mostly = 0;
 static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
+static bool xmon_is_ro = IS_ENABLED(CONFIG_XMON_DEFAULT_RO_MODE);
 
 static unsigned long adrs;
 static int size = 1;
@@ -202,6 +203,8 @@ static void dump_tlb_book3e(void);
 #define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + 
(v)[3])
 #endif
 
+static const char *xmon_ro_msg = "Operation disabled: xmon in read-only 
mode\n";
+
 static char *help_string = "\
 Commands:\n\
   bshow breakpoints\n\
@@ -989,6 +992,10 @@ cmds(struct pt_regs *excp)
memlocate();
break;
case 'z':
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   break;
+   }
memzcan();
break;
case 'i':
@@ -1042,6 +1049,10 @@ cmds(struct pt_regs *excp)
set_lpp_cmd();
break;
case 'b':
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   break;
+   }
bpt_cmds();
break;
case 'C':
@@ -1055,6 +1066,10 @@ cmds(struct pt_regs *excp)
bootcmds();
break;
case 'p':
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   break;
+   }
proccall();
break;
case 'P':
@@ -1777,6 +1792,11 @@ read_spr(int n, unsigned long *vp)
 static void
 write_spr(int n, unsigned long val)
 {
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   return;
+   }
+
if (setjmp(bus_error_jmp) == 0) {
catch_spr_faults = 1;
sync();
@@ -2016,6 +2036,12 @@ mwrite(unsigned long adrs, void *buf, int size)
char *p, *q;
 
n = 0;
+
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   return n;
+   }
+
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
@@ -2884,9 +2910,17 @@ memops(int cmd)
scanhex((void *)&mcount);
switch( cmd ){
case 'm':
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   break;
+   }
memmove((void *)mdest, (void *)msrc, mcount);
break;
case 's':
+   if (xmon_is_ro) {
+   printf(xmon_ro_msg);
+   break;
+   }
memset((void *)mdest, mval, mcount);
break;
case 'd':
@@ -3796,6 +3830,14 @@ static int __init early_parse_xmon(char *p)
} else if (strncmp(p, "on", 2) == 0) {
  

Re: [PATCH 1/2] ASoC: mpc5200_dma: Fix invalid license ID

2019-04-14 Thread Fabio Estevam
On Sun, Apr 14, 2019 at 4:15 PM Andra Danciu  wrote:
>
> As the file had no other license notice/reference, it falls under the
> project license and therefore the proper SPDX id is: GPL-2.0-only
>
> Cc: Daniel Baluta 
> Fixes: 1edfc2485d8dc ("ASoC: mpc5200_dma: Switch to SPDX identifier")
> Reported-by: Thomas Gleixner 
> Signed-off-by: Andra Danciu 
> ---
>  sound/soc/fsl/mpc5200_dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
> index 4396442c2fdd..ccf9301889fe 100644
> --- a/sound/soc/fsl/mpc5200_dma.c
> +++ b/sound/soc/fsl/mpc5200_dma.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL
> +// SPDX-License-Identifier: GPL-2.0-only

According to Documentation/process/license-rules.rst this should be:

// SPDX-License-Identifier: GPL-2.0


Re: [PATCH v2 1/2] cpuidle : auto-promotion for cpuidle states

2019-04-14 Thread Abhishek

Hi Rafael,

Thanks for the Review. Few inline replies below.


On 04/09/2019 03:31 PM, Rafael J. Wysocki wrote:

On Fri, Apr 5, 2019 at 11:17 AM Abhishek Goel
 wrote:

Currently, the cpuidle governors (menu /ladder) determine what idle state

There are three governors in 5.1-rc.


an idling CPU should enter into based on heuristics that depend on the
idle history on that CPU. Given that no predictive heuristic is perfect,
there are cases where the governor predicts a shallow idle state, hoping
that the CPU will be busy soon. However, if no new workload is scheduled
on that CPU in the near future, the CPU will end up in the shallow state.

In case of POWER, this is problematic, when the predicted state in the
aforementioned scenario is a lite stop state, as such lite states will
inhibit SMT folding, thereby depriving the other threads in the core from
using the core resources.

To address this, such lite states need to be autopromoted.

I don't quite agree with this statement and it doesn't even match what
the patch does AFAICS.  "Autopromotion" would be going from the given
state to a deeper one without running state selection in between, but
that's not what's going on here.

Thinking to call it "timed-exit". Is that good?

The cpuidle-core can queue timer to correspond with the residency value of the 
next
available state. Thus leading to auto-promotion to a deeper idle state as
soon as possible.

No, it doesn't automatically cause a deeper state to be used next
time.  It simply kicks the CPU out of the idle state and one more
iteration of the idle loop runs on it.  Whether or not a deeper state
will be selected in that iteration depends on the governor
computations carried out in it.

I did not mean that next state is chosen automatically. I should have been
more descriptive here instead of just using "as soon as possible"

Now, this appears to be almost analogous to the "polling" state used
on x86 which uses the next idle state's target residency as a timeout.

While generally I'm not a big fan of setting up timers in the idle
loop (it sort of feels like pulling your own hair in order to get
yourself out of a swamp), if idle states like these are there in your
platform, setting up a timer to get out of them in the driver's
->enter() routine might not be particularly objectionable.  Doing that
in the core is a whole different story, though.

Generally, this adds quite a bit of complexity (on the "ugly" side of
things IMO) to the core to cover a corner case present in one
platform, while IMO it can be covered in the driver for that platform
directly.

As of now, since this code doesn't add any benefit to the other platform,
I will post a patch with this implementation covered in platform-specific
driver code.
You are right that all the information needed for this implementation 
are also

available there in platform driver code, so we should be good to go.



[PATCH 2/2] ASoC: mpc5200_psc_i2s: Fix invalid license ID

2019-04-14 Thread Andra Danciu
As the file had no other license notice/reference, it falls under the
project license and therefore the proper SPDX id is: GPL-2.0-only

Cc: Daniel Baluta 
Fixes: 864a8472c4412 ("ASoC: mpc5200_psc_i2s: Switch to SPDX identifier")
Reported-by: Thomas Gleixner 
Signed-off-by: Andra Danciu 
---
 sound/soc/fsl/mpc5200_psc_i2s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/mpc5200_psc_i2s.c b/sound/soc/fsl/mpc5200_psc_i2s.c
index 6de97461ba25..9bc01f374b39 100644
--- a/sound/soc/fsl/mpc5200_psc_i2s.c
+++ b/sound/soc/fsl/mpc5200_psc_i2s.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL
+// SPDX-License-Identifier: GPL-2.0-only
 //
 // Freescale MPC5200 PSC in I2S mode
 // ALSA SoC Digital Audio Interface (DAI) driver
-- 
2.11.0



[PATCH 1/2] ASoC: mpc5200_dma: Fix invalid license ID

2019-04-14 Thread Andra Danciu
As the file had no other license notice/reference, it falls under the
project license and therefore the proper SPDX id is: GPL-2.0-only

Cc: Daniel Baluta 
Fixes: 1edfc2485d8dc ("ASoC: mpc5200_dma: Switch to SPDX identifier")
Reported-by: Thomas Gleixner 
Signed-off-by: Andra Danciu 
---
 sound/soc/fsl/mpc5200_dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 4396442c2fdd..ccf9301889fe 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL
+// SPDX-License-Identifier: GPL-2.0-only
 //
 // Freescale MPC5200 PSC DMA
 // ALSA SoC Platform driver
-- 
2.11.0



[PATCH 0/2] Fix invalid license IDS

2019-04-14 Thread Andra Danciu
This series of patches solves two issues reported by Thomas Gleixner

Andra Danciu (2):
  ASoC: mpc5200_dma: Fix invalid license ID
  ASoC: mpc5200_psc_i2s: Fix invalid license ID

 sound/soc/fsl/mpc5200_dma.c | 2 +-
 sound/soc/fsl/mpc5200_psc_i2s.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.11.0



Re: [PATCH 03/12] PowerPC-83xx: add missing of_node_put after of_device_is_available

2019-04-14 Thread Markus Elfring
> @@ -221,8 +221,10 @@ int mpc837x_usb_cfg(void)
>   int ret = 0;
>
>   np = of_find_compatible_node(NULL, NULL, "fsl-usb2-dr");
> - if (!np || !of_device_is_available(np))
> + if (!np || !of_device_is_available(np)) {
> + of_node_put(np);
>   return -ENODEV;
> + }
>   prop = of_get_property(np, "phy_type", NULL);
>
>   if (!prop || (strcmp(prop, "ulpi") && strcmp(prop, "serial"))) {

How do you think about to adjust the exception handling in this function
implementation a bit more according to the Linux coding style?

Regards,
Markus