date:20201130

On Sun, Nov 29, 2020 at 12:16:26PM -0800, Andy Lutomirski wrote:
> On Sat, Nov 28, 2020 at 7:54 PM Andy Lutomirski  wrote:
> >
> > On Sat, Nov 28, 2020 at 8:02 AM Nicholas Piggin  wrote:
> > >
> > > On big systems, the mm refcount can become highly contented when doing
> > > a lot of context switching with threaded applications (particularly
> > > switching between the idle thread and an application thread).
> > >
> > > Abandoning lazy tlb slows switching down quite a bit in the important
> > > user->idle->user cases, so so instead implement a non-refcounted scheme
> > > that causes __mmdrop() to IPI all CPUs in the mm_cpumask and shoot down
> > > any remaining lazy ones.
> > >
> > > Shootdown IPIs are some concern, but they have not been observed to be
> > > a big problem with this scheme (the powerpc implementation generated
> > > 314 additional interrupts on a 144 CPU system during a kernel compile).
> > > There are a number of strategies that could be employed to reduce IPIs
> > > if they turn out to be a problem for some workload.
> >
> > I'm still wondering whether we can do even better.
> >
> 
> Hold on a sec.. __mmput() unmaps VMAs, frees pagetables, and flushes
> the TLB.  On x86, this will shoot down all lazies as long as even a
> single pagetable was freed.  (Or at least it will if we don't have a
> serious bug, but the code seems okay.  We'll hit pmd_free_tlb, which
> sets tlb->freed_tables, which will trigger the IPI.)  So, on
> architectures like x86, the shootdown approach should be free.  The
> only way it ought to have any excess IPIs is if we have CPUs in
> mm_cpumask() that don't need IPI to free pagetables, which could
> happen on paravirt.
> 
> Can you try to figure out why you saw any increase in IPIs?  It would
> be nice if we can make the new code unconditional.

Power doesn't do IPI based TLBI.

Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option

On Sat, Nov 28, 2020 at 07:54:57PM -0800, Andy Lutomirski wrote:
> Version (b) seems fairly straightforward to implement -- add RCU
> protection and a atomic_t special_ref_cleared (initially 0) to struct
> mm_struct itself.  After anyone clears a bit to mm_cpumask (which is
> already a barrier),

No it isn't. clear_bit() implies no barrier what so ever. That's x86
you're thinking about.

> they read mm_users.  If it's zero, then they scan
> mm_cpumask and see if it's empty.  If it is, they atomically swap
> special_ref_cleared to 1.  If it was zero before the swap, they do
> mmdrop().  I can imagine some tweaks that could make this a big
> faster, at least in the limit of a huge number of CPUs.

Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option

On Sat, Nov 28, 2020 at 07:54:57PM -0800, Andy Lutomirski wrote:
> This means that mm_cpumask operations won't need to be full barriers
> forever, and we might not want to take the implied full barriers in
> set_bit() and clear_bit() for granted.

There is no implied full barrier for those ops.

Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option

On Mon, Nov 30, 2020 at 10:30:00AM +0100, Peter Zijlstra wrote:
> On Sat, Nov 28, 2020 at 07:54:57PM -0800, Andy Lutomirski wrote:
> > This means that mm_cpumask operations won't need to be full barriers
> > forever, and we might not want to take the implied full barriers in
> > set_bit() and clear_bit() for granted.
> 
> There is no implied full barrier for those ops.

Neither ARM nor Power implies any ordering on those ops. But Power has
some of the worst atomic performance in the world despite of that.

IIRC they don't do their LL/SC in L1.

Re: [PATCH] powerpc: fix the allyesconfig build

2020-11-30 Thread Stephen Rothwell

Hi Geert,

On Mon, 30 Nov 2020 09:58:23 +0100 Geert Uytterhoeven  
wrote:
>
> Thanks for your patch!

No worries, it has been a small irritant to me for quite a while.

> I prefer to fix this in the driver instead.  The space saving by packing the
> structure is minimal.
> I've sent a patch
> https://lore.kernel.org/r/20201130085743.1656317-1-geert+rene...@glider.be
> (when lore is back)

Absolutely, thanks.

-- 
Cheers,
Stephen Rothwell


pgpaTi1a3U9Kc.pgp
Description: OpenPGP digital signature

Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers

2020-11-30 Thread Lorenzo Pieralisi

On Sun, Nov 29, 2020 at 11:07:39PM +, Krzysztof Wilczyński wrote:
> Add ECAM-related constants to provide a set of standard constants
> defining memory address shift values to the byte-level address that can
> be used to access the PCI Express Configuration Space, and then move
> native PCI Express controller drivers to use the newly introduced
> definitions retiring driver-specific ones.
> 
> Refactor pci_ecam_map_bus() function to use newly added constants so
> that limits to the bus, device function and offset (now limited to 4K as
> per the specification) are in place to prevent the defective or
> malicious caller from supplying incorrect configuration offset and thus
> targeting the wrong device when accessing extended configuration space.
> This refactor also allows for the ".bus_shit" initialisers to be dropped
  

Nice typo, I'd fix it while applying it though if you don't mind ;-),
no need to resend it.

Jokes aside, nice piece of work, thanks for that.

> when the user is not using a custom value as a default value will be
> used as per the PCI Express Specification.
> 
> Suggested-by: Bjorn Helgaas 
> Signed-off-by: Krzysztof Wilczyński 

I think Bjorn's reviewed-by still stands so I will apply it.

Thanks !
Lorenzo

> ---
>  drivers/pci/controller/dwc/pcie-al.c| 12 ++---
>  drivers/pci/controller/dwc/pcie-hisi.c  |  2 --
>  drivers/pci/controller/pci-aardvark.c   | 13 +++---
>  drivers/pci/controller/pci-host-generic.c   |  1 -
>  drivers/pci/controller/pci-thunder-ecam.c   |  1 -
>  drivers/pci/controller/pcie-brcmstb.c   | 16 ++--
>  drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
>  drivers/pci/controller/pcie-rockchip.h  |  8 +-
>  drivers/pci/controller/pcie-tango.c |  1 -
>  drivers/pci/controller/pcie-xilinx-nwl.c|  9 ++-
>  drivers/pci/controller/pcie-xilinx.c| 11 ++---
>  drivers/pci/controller/vmd.c| 11 -
>  drivers/pci/ecam.c  | 23 --
>  include/linux/pci-ecam.h| 27 +
>  14 files changed, 73 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-al.c 
> b/drivers/pci/controller/dwc/pcie-al.c
> index f973fbca90cf..af9e51ab1af8 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops al_pcie_ops = {
> - .bus_shift= 20,
>   .init =  al_pcie_init,
>   .pci_ops  = {
>   .map_bus= al_pcie_map_bus,
> @@ -138,8 +137,6 @@ struct al_pcie {
>   struct al_pcie_target_bus_cfg target_bus_cfg;
>  };
>  
> -#define PCIE_ECAM_DEVFN(x)   (((x) & 0xff) << 12)
> -
>  #define to_al_pcie(x)dev_get_drvdata((x)->dev)
>  
>  static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct 
> pci_bus *bus,
>   struct al_pcie_target_bus_cfg *target_bus_cfg = &pcie->target_bus_cfg;
>   unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
>   unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
> - void __iomem *pci_base_addr;
> -
> - pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> -  (busnr_ecam << 20) +
> -  PCIE_ECAM_DEVFN(devfn));
>  
>   if (busnr_reg != target_bus_cfg->reg_val) {
>   dev_dbg(pcie->pci->dev, "Changing target bus busnum val from 
> 0x%x to 0x%x\n",
> @@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct 
> pci_bus *bus,
>  target_bus_cfg->reg_mask);
>   }
>  
> - return pci_base_addr + where;
> + return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
>  }
>  
>  static struct pci_ops al_child_pci_ops = {
> @@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>  
>   target_bus_cfg = &pcie->target_bus_cfg;
>  
> - ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> + ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
>   if (ecam_bus_mask > 255) {
>   dev_warn(pcie->dev, "ECAM window size is larger than 256MB. 
> Cutting off at 256\n");
>   ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c 
> b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..8fc5960faf28 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops hisi_pcie_ops = {
> - .bus_shift= 20,
>   .init =  hisi_pcie_init,
>   .pci_ops  = {
>

[PATCH] KVM: PPC: Book3S HV: XIVE: Fix vCPU id sanity check

2020-11-30 Thread Greg Kurz

Commit 062cfab7069f ("KVM: PPC: Book3S HV: XIVE: Make VP block size
configurable") updated kvmppc_xive_vcpu_id_valid() in a way that
allows userspace to trigger an assertion in skiboot and crash the host:

[  696.186248988,3] XIVE[ IC 08  ] eq_blk != vp_blk (0 vs. 1) for target 
0x438c/0
[  696.186314757,0] Assert fail: hw/xive.c:2370:0
[  696.186342458,0] Aborting!
xive-kvCPU 0043 Backtrace:
 S: 31e2b8f0 R: 30013840   .backtrace+0x48
 S: 31e2b990 R: 3001b2d0   ._abort+0x4c
 S: 31e2ba10 R: 3001b34c   .assert_fail+0x34
 S: 31e2ba90 R: 30058984   .xive_eq_for_target.part.20+0xb0
 S: 31e2bb40 R: 30059fdc   .xive_setup_silent_gather+0x2c
 S: 31e2bc20 R: 3005a334   .opal_xive_set_vp_info+0x124
 S: 31e2bd20 R: 300051a4   opal_entry+0x134
 --- OPAL call token: 0x8a caller R1: 0xc01f28563850 ---

XIVE maintains the interrupt context state of non-dispatched vCPUs in
an internal VP structure. We allocate a bunch of those on startup to
accommodate all possible vCPUs. Each VP has an id, that we derive from
the vCPU id for efficiency:

static inline u32 kvmppc_xive_vp(struct kvmppc_xive *xive, u32 server)
{
return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
}

The KVM XIVE device used to allocate KVM_MAX_VCPUS VPs. This was
limitting the number of concurrent VMs because the VP space is
limited on the HW. Since most of the time, VMs run with a lot less
vCPUs, commit 062cfab7069f ("KVM: PPC: Book3S HV: XIVE: Make VP
block size configurable") gave the possibility for userspace to
tune the size of the VP block through the KVM_DEV_XIVE_NR_SERVERS
attribute.

The check in kvmppc_pack_vcpu_id() was changed from

cpu < KVM_MAX_VCPUS * xive->kvm->arch.emul_smt_mode

to

cpu < xive->nr_servers * xive->kvm->arch.emul_smt_mode

The previous check was based on the fact that the VP block had
KVM_MAX_VCPUS entries and that kvmppc_pack_vcpu_id() guarantees
that packed vCPU ids are below KVM_MAX_VCPUS. We've changed the
size of the VP block, but kvmppc_pack_vcpu_id() has nothing to
do with it and it certainly doesn't ensure that the packed vCPU
ids are below xive->nr_servers. kvmppc_xive_vcpu_id_valid() might
thus return true when the VM was configured with a non-standard
VSMT mode, even if the packed vCPU id is higher than what we
expect. We end up using an unallocated VP id, which confuses
OPAL. The assert in OPAL is probably abusive and should be
converted to a regular error that the kernel can handle, but
we shouldn't really use broken VP ids in the first place.

Fix kvmppc_xive_vcpu_id_valid() so that it checks the packed
vCPU id is below xive->nr_servers, which is explicitly what we
want.

Fixes: 062cfab7069f ("KVM: PPC: Book3S HV: XIVE: Make VP block size 
configurable")
Cc: sta...@vger.kernel.org # v5.5+
Signed-off-by: Greg Kurz 
---
 arch/powerpc/kvm/book3s_xive.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 85215e79db42..a0ebc29f30b2 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1214,12 +1214,9 @@ void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)
 static bool kvmppc_xive_vcpu_id_valid(struct kvmppc_xive *xive, u32 cpu)
 {
/* We have a block of xive->nr_servers VPs. We just need to check
-* raw vCPU ids are below the expected limit for this guest's
-* core stride ; kvmppc_pack_vcpu_id() will pack them down to an
-* index that can be safely used to compute a VP id that belongs
-* to the VP block.
+* packed vCPU ids are below that.
 */
-   return cpu < xive->nr_servers * xive->kvm->arch.emul_smt_mode;
+   return kvmppc_pack_vcpu_id(xive->kvm, cpu) < xive->nr_servers;
 }
 
 int kvmppc_xive_compute_vp_id(struct kvmppc_xive *xive, u32 cpu, u32 *vp)

Re: [PATCH] KVM: PPC: Book3S HV: XIVE: Fix vCPU id sanity check

2020-11-30 Thread Cédric Le Goater

On 11/30/20 1:19 PM, Greg Kurz wrote:
> Commit 062cfab7069f ("KVM: PPC: Book3S HV: XIVE: Make VP block size
> configurable") updated kvmppc_xive_vcpu_id_valid() in a way that
> allows userspace to trigger an assertion in skiboot and crash the host:
> 
> [  696.186248988,3] XIVE[ IC 08  ] eq_blk != vp_blk (0 vs. 1) for target 
> 0x438c/0
> [  696.186314757,0] Assert fail: hw/xive.c:2370:0
> [  696.186342458,0] Aborting!
> xive-kvCPU 0043 Backtrace:
>  S: 31e2b8f0 R: 30013840   .backtrace+0x48
>  S: 31e2b990 R: 3001b2d0   ._abort+0x4c
>  S: 31e2ba10 R: 3001b34c   .assert_fail+0x34
>  S: 31e2ba90 R: 30058984   .xive_eq_for_target.part.20+0xb0
>  S: 31e2bb40 R: 30059fdc   .xive_setup_silent_gather+0x2c
>  S: 31e2bc20 R: 3005a334   .opal_xive_set_vp_info+0x124
>  S: 31e2bd20 R: 300051a4   opal_entry+0x134
>  --- OPAL call token: 0x8a caller R1: 0xc01f28563850 ---
> 
> XIVE maintains the interrupt context state of non-dispatched vCPUs in
> an internal VP structure. We allocate a bunch of those on startup to
> accommodate all possible vCPUs. Each VP has an id, that we derive from
> the vCPU id for efficiency:
> 
> static inline u32 kvmppc_xive_vp(struct kvmppc_xive *xive, u32 server)
> {
>   return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
> }
> 
> The KVM XIVE device used to allocate KVM_MAX_VCPUS VPs. This was
> limitting the number of concurrent VMs because the VP space is
> limited on the HW. Since most of the time, VMs run with a lot less
> vCPUs, commit 062cfab7069f ("KVM: PPC: Book3S HV: XIVE: Make VP
> block size configurable") gave the possibility for userspace to
> tune the size of the VP block through the KVM_DEV_XIVE_NR_SERVERS
> attribute.
> 
> The check in kvmppc_pack_vcpu_id() was changed from
> 
>   cpu < KVM_MAX_VCPUS * xive->kvm->arch.emul_smt_mode
> 
> to
> 
>   cpu < xive->nr_servers * xive->kvm->arch.emul_smt_mode
> 
> The previous check was based on the fact that the VP block had
> KVM_MAX_VCPUS entries and that kvmppc_pack_vcpu_id() guarantees
> that packed vCPU ids are below KVM_MAX_VCPUS. We've changed the
> size of the VP block, but kvmppc_pack_vcpu_id() has nothing to
> do with it and it certainly doesn't ensure that the packed vCPU
> ids are below xive->nr_servers. kvmppc_xive_vcpu_id_valid() might
> thus return true when the VM was configured with a non-standard
> VSMT mode, even if the packed vCPU id is higher than what we
> expect. We end up using an unallocated VP id, which confuses
> OPAL. The assert in OPAL is probably abusive and should be
> converted to a regular error that the kernel can handle, but
> we shouldn't really use broken VP ids in the first place.
> 
> Fix kvmppc_xive_vcpu_id_valid() so that it checks the packed
> vCPU id is below xive->nr_servers, which is explicitly what we
> want.
> 
> Fixes: 062cfab7069f ("KVM: PPC: Book3S HV: XIVE: Make VP block size 
> configurable")
> Cc: sta...@vger.kernel.org # v5.5+
> Signed-off-by: Greg Kurz 

Reviewed-by: Cédric Le Goater 

Thanks,

C.

> ---
>  arch/powerpc/kvm/book3s_xive.c |7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index 85215e79db42..a0ebc29f30b2 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -1214,12 +1214,9 @@ void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)
>  static bool kvmppc_xive_vcpu_id_valid(struct kvmppc_xive *xive, u32 cpu)
>  {
>   /* We have a block of xive->nr_servers VPs. We just need to check
> -  * raw vCPU ids are below the expected limit for this guest's
> -  * core stride ; kvmppc_pack_vcpu_id() will pack them down to an
> -  * index that can be safely used to compute a VP id that belongs
> -  * to the VP block.
> +  * packed vCPU ids are below that.
>*/
> - return cpu < xive->nr_servers * xive->kvm->arch.emul_smt_mode;
> + return kvmppc_pack_vcpu_id(xive->kvm, cpu) < xive->nr_servers;
>  }
>  
>  int kvmppc_xive_compute_vp_id(struct kvmppc_xive *xive, u32 cpu, u32 *vp)
> 
>

Re: [PATCH 2/8] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode

2020-11-30 Thread Mathieu Desnoyers

- On Nov 28, 2020, at 11:01 AM, Nicholas Piggin npig...@gmail.com wrote:

> And get rid of the generic sync_core_before_usermode facility. This is
> functionally a no-op in the core scheduler code, but it also catches

This sentence is incomplete.

> 
> This helper is the wrong way around I think. The idea that membarrier
> state requires a core sync before returning to user is the easy one
> that does not need hiding behind membarrier calls. The gap in core
> synchronization due to x86's sysret/sysexit and lazy tlb mode, is the
> tricky detail that is better put in x86 lazy tlb code.

Ideally yes this complexity should sit within the x86 architecture code
if only that architecture requires it.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

[PATCH] powerpc/pseries: Define PCI bus speed for Gen4 and Gen5

2020-11-30 Thread Frederic Barrat

Update bus speed definition for PCI Gen4 and 5.

Signed-off-by: Frederic Barrat 
---
 arch/powerpc/platforms/pseries/pci.c | 51 
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pci.c 
b/arch/powerpc/platforms/pseries/pci.c
index 911534b89c85..72a4d4167849 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -290,6 +290,25 @@ static void fixup_winbond_82c105(struct pci_dev* dev)
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105,
 fixup_winbond_82c105);
 
+static enum pci_bus_speed prop_to_pci_speed(u32 prop)
+{
+   switch (prop) {
+   case 0x01:
+   return PCIE_SPEED_2_5GT;
+   case 0x02:
+   return PCIE_SPEED_5_0GT;
+   case 0x04:
+   return PCIE_SPEED_8_0GT;
+   case 0x08:
+   return PCIE_SPEED_16_0GT;
+   case 0x10:
+   return PCIE_SPEED_32_0GT;
+   default:
+   pr_debug("Unexpected PCI link speed property value\n");
+   return PCI_SPEED_UNKNOWN;
+   }
+}
+
 int pseries_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
struct device_node *dn, *pdn;
@@ -322,35 +341,7 @@ int pseries_root_bridge_prepare(struct pci_host_bridge 
*bridge)
return 0;
}
 
-   switch (pcie_link_speed_stats[0]) {
-   case 0x01:
-   bus->max_bus_speed = PCIE_SPEED_2_5GT;
-   break;
-   case 0x02:
-   bus->max_bus_speed = PCIE_SPEED_5_0GT;
-   break;
-   case 0x04:
-   bus->max_bus_speed = PCIE_SPEED_8_0GT;
-   break;
-   default:
-   bus->max_bus_speed = PCI_SPEED_UNKNOWN;
-   break;
-   }
-
-   switch (pcie_link_speed_stats[1]) {
-   case 0x01:
-   bus->cur_bus_speed = PCIE_SPEED_2_5GT;
-   break;
-   case 0x02:
-   bus->cur_bus_speed = PCIE_SPEED_5_0GT;
-   break;
-   case 0x04:
-   bus->cur_bus_speed = PCIE_SPEED_8_0GT;
-   break;
-   default:
-   bus->cur_bus_speed = PCI_SPEED_UNKNOWN;
-   break;
-   }
-
+   bus->max_bus_speed = prop_to_pci_speed(pcie_link_speed_stats[0]);
+   bus->cur_bus_speed = prop_to_pci_speed(pcie_link_speed_stats[1]);
return 0;
 }
-- 
2.26.2

Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers

2020-11-30 Thread Krzysztof Wilczyński

Hi Lorenzo!

On 20-11-30 11:08:58, Lorenzo Pieralisi wrote:
[...]
> > Refactor pci_ecam_map_bus() function to use newly added constants so
> > that limits to the bus, device function and offset (now limited to 4K as
> > per the specification) are in place to prevent the defective or
> > malicious caller from supplying incorrect configuration offset and thus
> > targeting the wrong device when accessing extended configuration space.
> > This refactor also allows for the ".bus_shit" initialisers to be dropped
>   
> 
> Nice typo, I'd fix it while applying it though if you don't mind ;-),
> no need to resend it.

Oh doh!  Apologies. :)

> Jokes aside, nice piece of work, thanks for that.
> 
> > when the user is not using a custom value as a default value will be
> > used as per the PCI Express Specification.
> > 
> > Suggested-by: Bjorn Helgaas 
> > Signed-off-by: Krzysztof Wilczyński 
> 
> I think Bjorn's reviewed-by still stands so I will apply it.
[...]

Thank you!

Krzysztof

[PATCH 0/6] Add documentation for Documentation/features at the built docs

2020-11-30 Thread Mauro Carvalho Chehab

Hi Jon,

This series got already submitted last year:

   https://lore.kernel.org/lkml/cover.1561222784.git.mchehab+sams...@kernel.org/

Yet, on that time, there were too many other patches related to ReST
conversion floating around. So, at the end, I guess this one got missed.

So, I did a rebase on the top of upstream, and added a few new changes.

Patch 1 contains the original implementation back then. It adds a
get_feat.pl script that parses the contents of Documentation/features.

Patch 2 is new: it re-implements the output of the full contents of the
features table as a set of per-subsystem tables. 

Patch 3 replaces the existing Documentation/features/list-arch.sh
by a call to the new script, in order to avoid having two scripts
doing the same thing.

Patch 4 is a sphinx extension to allow generating features output
via a meta-tag.

Patch 5 adds a complete feature list covering all archs at the
admin guide.

Patch 6 adds a per-arch feature list on each architecture book.

-

The scripts/get_feat.pl supports several types of output:

- $ scripts/get_feat.pl current

  Outputs the supported feadures by the architecture of the
  running Kernel, as an ASCII table;

- $  scripts/get_feat.pl list

  Outputs the supported features on an easy to be parsed
  format. By default, it uses the current architecture as well;

- $  scripts/get_feat.pl rest --feature jump-labels

  Output what architecture supports a given feature
  (on the above example, "jump-labels" feature)

- $ scripts/get_feat.pl rest --arch um

  Outputs the features support for an specific architecture
  (on the above example, for "um" architecture.

- $ scripts/get_feat.pl rest

  Outputs a text file with ASCII tables (ReST compatible)
  with all features, grouped per subsystem.

  E. g. something like:

===
Feature status on all architectures
===

Subsystem: core
===


+-+-+-++--+
|Feature  |Kconfig  |Description
  |Architecture|Status|

+=+=+=++==+
|cBPF-JIT |HAVE_CBPF_JIT|arch supports 
cBPF JIT optimizations |alpha   |TODO  |
| | |   
  ++--+
| | |   
  |arc |TODO  |
| | |   
  ++--+
| | |   
  |arm |TODO  |
| | |   
  ++--+
| | |   
  |arm64   |TODO  |
| | |   
  ++--+
| | |   
  |c6x |TODO  |
| | |   
  ++--+
| | |   
  |csky|TODO  |
| | |   
  ++--+
| | |   
  |h8300   |TODO  |
| | |   
  ++--+
| | |   
  |hexagon |TODO  |
...

Adding those patchsets will basically place the contents of all
files under Documentation

[PATCH 6/6] docs: archis: add a per-architecture features list

2020-11-30 Thread Mauro Carvalho Chehab

Add a feature list matrix for each architecture to their
respective Kernel books.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/arm/features.rst  |  3 +++
 Documentation/arm/index.rst |  2 ++
 Documentation/arm64/features.rst|  3 +++
 Documentation/arm64/index.rst   |  2 ++
 Documentation/ia64/features.rst |  3 +++
 Documentation/ia64/index.rst|  2 ++
 Documentation/index.rst |  2 +-
 Documentation/m68k/features.rst |  3 +++
 Documentation/m68k/index.rst|  2 ++
 Documentation/mips/features.rst |  3 +++
 Documentation/mips/index.rst|  2 ++
 Documentation/nios2/index.rst   | 12 
 Documentation/openrisc/features.rst |  3 +++
 Documentation/openrisc/index.rst|  2 ++
 Documentation/parisc/features.rst   |  3 +++
 Documentation/parisc/index.rst  |  2 ++
 Documentation/powerpc/features.rst  |  3 +++
 Documentation/powerpc/index.rst |  2 ++
 Documentation/riscv/features.rst|  3 +++
 Documentation/riscv/index.rst   |  2 ++
 Documentation/s390/features.rst |  3 +++
 Documentation/s390/index.rst|  2 ++
 Documentation/sh/features.rst   |  3 +++
 Documentation/sh/index.rst  |  2 ++
 Documentation/sparc/features.rst|  3 +++
 Documentation/sparc/index.rst   |  2 ++
 Documentation/x86/features.rst  |  3 +++
 Documentation/x86/index.rst |  1 +
 Documentation/xtensa/features.rst   |  3 +++
 Documentation/xtensa/index.rst  |  2 ++
 30 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/arm/features.rst
 create mode 100644 Documentation/arm64/features.rst
 create mode 100644 Documentation/ia64/features.rst
 create mode 100644 Documentation/m68k/features.rst
 create mode 100644 Documentation/mips/features.rst
 create mode 100644 Documentation/nios2/index.rst
 create mode 100644 Documentation/openrisc/features.rst
 create mode 100644 Documentation/parisc/features.rst
 create mode 100644 Documentation/powerpc/features.rst
 create mode 100644 Documentation/riscv/features.rst
 create mode 100644 Documentation/s390/features.rst
 create mode 100644 Documentation/sh/features.rst
 create mode 100644 Documentation/sparc/features.rst
 create mode 100644 Documentation/x86/features.rst
 create mode 100644 Documentation/xtensa/features.rst

diff --git a/Documentation/arm/features.rst b/Documentation/arm/features.rst
new file mode 100644
index ..7414ec03dd15
--- /dev/null
+++ b/Documentation/arm/features.rst
@@ -0,0 +1,3 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. kernel-feat:: $srctree/Documentation/features arm
diff --git a/Documentation/arm/index.rst b/Documentation/arm/index.rst
index 5fc072dd0c5e..a2e9e1bba7b9 100644
--- a/Documentation/arm/index.rst
+++ b/Documentation/arm/index.rst
@@ -23,6 +23,8 @@ ARM Architecture
vlocks
porting
 
+   features
+
 SoC-specific documents
 ==
 
diff --git a/Documentation/arm64/features.rst b/Documentation/arm64/features.rst
new file mode 100644
index ..dfa4cb3cd3ef
--- /dev/null
+++ b/Documentation/arm64/features.rst
@@ -0,0 +1,3 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. kernel-feat:: $srctree/Documentation/features arm64
diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
index 937634c49979..97d65ba12a35 100644
--- a/Documentation/arm64/index.rst
+++ b/Documentation/arm64/index.rst
@@ -24,6 +24,8 @@ ARM64 Architecture
 tagged-address-abi
 tagged-pointers
 
+features
+
 .. only::  subproject and html
 
Indices
diff --git a/Documentation/ia64/features.rst b/Documentation/ia64/features.rst
new file mode 100644
index ..d7226fdcf5f8
--- /dev/null
+++ b/Documentation/ia64/features.rst
@@ -0,0 +1,3 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. kernel-feat:: $srctree/Documentation/features ia64
diff --git a/Documentation/ia64/index.rst b/Documentation/ia64/index.rst
index 4bdfe28067ee..761f2154dfa2 100644
--- a/Documentation/ia64/index.rst
+++ b/Documentation/ia64/index.rst
@@ -15,3 +15,5 @@ IA-64 Architecture
irq-redir
mca
serial
+
+   features
diff --git a/Documentation/index.rst b/Documentation/index.rst
index 57719744774c..5888e8a7272f 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -160,7 +160,7 @@ implementation.
ia64/index
m68k/index
mips/index
-   nios2/nios2
+   nios2/index
openrisc/index
parisc/index
powerpc/index
diff --git a/Documentation/m68k/features.rst b/Documentation/m68k/features.rst
new file mode 100644
index ..5107a2119472
--- /dev/null
+++ b/Documentation/m68k/features.rst
@@ -0,0 +1,3 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. kernel-feat:: $srctree/Documentation/features m68k
diff --git a/Documentation/m68k/index.rst b/Documentation/m68k/index.rst
index b89cb6a86d9b..0f890dbb5fe2 100644
--- a/Documentation/m68k/index.rst
+++ b/Documentation/m68k/index.rst
@@ -10,6 +10,8 @@ m68k Architecture
kernel-options
budd

Re: [PATCH v4 10/18] dt-bindings: usb: Convert DWC USB3 bindings to DT schema

2020-11-30 Thread Rob Herring

On Wed, Nov 25, 2020 at 1:32 AM Serge Semin
 wrote:
>
> On Sat, Nov 21, 2020 at 06:42:28AM -0600, Rob Herring wrote:
> > On Thu, Nov 12, 2020 at 01:29:46PM +0300, Serge Semin wrote:
> > > On Wed, Nov 11, 2020 at 02:14:23PM -0600, Rob Herring wrote:
> > > > On Wed, Nov 11, 2020 at 12:08:45PM +0300, Serge Semin wrote:
> > > > > DWC USB3 DT node is supposed to be compliant with the Generic xHCI
> > > > > Controller schema, but with additional vendor-specific properties, the
> > > > > controller-specific reference clocks and PHYs. So let's convert the
> > > > > currently available legacy text-based DWC USB3 bindings to the DT 
> > > > > schema
> > > > > and make sure the DWC USB3 nodes are also validated against the
> > > > > usb-xhci.yaml schema.
> > > > >
> > > > > Note we have to discard the nodename restriction of being prefixed 
> > > > > with
> > > > > "dwc3@" string, since in accordance with the usb-hcd.yaml schema USB 
> > > > > nodes
> > > > > are supposed to be named as "^usb(@.*)".
> > > > >
> > > > > Signed-off-by: Serge Semin 
> > > > >
> > > > > ---
> > > > >
> > > > > Changelog v2:
> > > > > - Discard '|' from the descriptions, since we don't need to preserve
> > > > >   the text formatting in any of them.
> > > > > - Drop quotes from around the string constants.
> > > > > - Fix the "clock-names" prop description to be referring the 
> > > > > enumerated
> > > > >   clock-names instead of the ones from the Databook.
> > > > >
> > > > > Changelog v3:
> > > > > - Apply usb-xhci.yaml# schema only if the controller is supposed to 
> > > > > work
> > > > >   as either host or otg.
> > > > >
> > > > > Changelog v4:
> > > > > - Apply usb-drd.yaml schema first. If the controller is configured
> > > > >   to work in a gadget mode only, then apply the usb.yaml schema too,
> > > > >   otherwise apply the usb-xhci.yaml schema.
> > > > > - Discard the Rob'es Reviewed-by tag. Please review the patch one more
> > > > >   time.
> > > > > ---
> > > > >  .../devicetree/bindings/usb/dwc3.txt  | 125 
> > > > >  .../devicetree/bindings/usb/snps,dwc3.yaml| 303 
> > > > > ++
> > > > >  2 files changed, 303 insertions(+), 125 deletions(-)
> > > > >  delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt
> > > > >  create mode 100644 
> > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> >
> >
> > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml 
> > > > > b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > new file mode 100644
> > > > > index ..079617891da6
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> > > > > @@ -0,0 +1,303 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/usb/snps,dwc3.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Synopsys DesignWare USB3 Controller
> > > > > +
> > > > > +maintainers:
> > > > > +  - Felipe Balbi 
> > > > > +
> > > > > +description:
> > > > > +  This is usually a subnode to DWC3 glue to which it is connected, 
> > > > > but can also
> > > > > +  be presented as a standalone DT node with an optional 
> > > > > vendor-specific
> > > > > +  compatible string.
> > > > > +
> > >
> > > > > +allOf:
> > > > > +  - $ref: usb-drd.yaml#
> > > > > +  - if:
> > > > > +  properties:
> > > > > +dr_mode:
> > > > > +  const: peripheral
> >
>
> > Another thing, this evaluates to true if dr_mode is not present. You
> > need to add 'required'?
>
> Right. Will something like this do that?

Yes.

>
> + allOf:
> +  - $ref: usb-drd.yaml#
> +  - if:
> +  properties:
> +dr_mode:
> +  const: peripheral
> +
> +  required:
> +- dr_mode
> +then:
> +  $ref: usb.yaml#
> +else
> +  $ref: usb-xhci.yaml#
>
> > If dr_mode is otg, then don't you need to apply
> > both usb.yaml and usb-xhci.yaml?
>
> No I don't. Since there is no peripheral-specific DT schema, then the
> only schema any USB-gadget node needs to pass is usb.yaml, which
> is already included into the usb-xhci.yaml schema. So for pure OTG devices
> with xHCI host and gadget capabilities it's enough to evaluate: allOf:
> [$ref: usb-drd.yaml#, $ref: usb-xhci.yaml#].  Please see the
> sketch/ASCII-figure below and the following text for details.

Okay.

Rob

Re: [PATCH v5] PCI: Unify ECAM constants in native PCI Express drivers

2020-11-30 Thread Krzysztof Wilczyński

[+CC David for visibility]

Hi Bjorn,

Thank you for the review!

On 20-11-28 12:35:16, Bjorn Helgaas wrote:
[...]
> It's ironic that we don't use PCIE_ECAM_OFFSET in drivers/pci/ecam.c.
> We could do something like this, which would also let us drop
> .bus_shift completely in all the conforming implementations.  It also
> closes the hole that we didn't limit "where" to 4K for
> pci_ecam_map_bus() users.
> 
>   if (per_bus_mapping) {
> base = cfg->winp[busn];
> busn = 0;
>   } else {
> base = cfg->win;
>   }
> 
>   if (cfg->ops->bus_shift) {
> u32 bus_offset = (busn & 0xff) << cfg->ops->bus_shift;
> u32 devfn_offset = (devfn & 0xff) << (cfg->ops->bus_shift - 8);
> 
> where &= 0xfff;
> 
> return base + (bus_offset | devfn_offset | where);
>   }
> 
>   return base + PCIE_ECAM_OFFSET(busn, devfn, where);
[...]

Thank you for suggesting this!  I sent v6 recently that includes this.

> >  static void __iomem *ppc4xx_pciex_get_config_base(struct ppc4xx_pciex_port 
> > *port,
> >   struct pci_bus *bus,
> > - unsigned int devfn)
> > + unsigned int devfn,
> > + int offset)
> 
> The interface change (to add "offset") could be a preparatory patch by
> itself.
> 
> But I'm actually not sure it's worth even touching this file.  This is
> the only place outside drivers/pci that includes linux/pci-ecam.h.  I
> think I might rather put PCIE_ECAM_OFFSET() and related things in
> drivers/pci/pci.h and keep it all inside drivers/pci.

Makes sense to drop it.  We can always introduce chances on PPC 4xx
platform in the future if we ever want it to leverage all the new macros
and constants.

These changes are not included in v6.

> >  static const struct pci_ecam_ops pci_thunder_pem_ops = {
> > -   .bus_shift  = 24,
> > +   .bus_shift  = THUNDER_PCIE_ECAM_BUS_SHIFT,
> > .init   = thunder_pem_platform_init,
> > .pci_ops= {
> > .map_bus= pci_ecam_map_bus,
> 
> This could be split to its own patch, no big deal either way.

Done.  v6 is now a series that includes this as a separate patch.

> >  const struct pci_ecam_ops xgene_v2_pcie_ecam_ops = {
> > -   .bus_shift  = 16,
> > .init   = xgene_v2_pcie_ecam_init,
> > .pci_ops= {
> > .map_bus= xgene_pcie_map_bus,
> 
> Thanks for mentioning this change in the cover letter.  It could also
> be split off to a preparatory patch, since it's not related to
> PCIE_ECAM_OFFSET(), which is the main point of this patch.

Done.
 
> >  static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
> >unsigned int busno,
> > -  unsigned int slot,
> > -  unsigned int fn,
> > +  unsigned int devfn,
> 
> This interface change *could* be a separate preparatory patch, too,
> but I'm starting to feel even more OCD than usual :)

Done.  It's a separate patch in v6, although I kept it together with the
change to introduce the PCIE_ECAM_OFFSET() macro since I was retiring the
use of PCI_SLOT() and PCI_FUNC() macros.

> > @@ -94,7 +95,7 @@ struct vmd_dev {
> > struct pci_dev  *dev;
> >  
> > spinlock_t  cfg_lock;
> > -   char __iomem*cfgbar;
> > +   void __iomem*cfgbar;
> 
> This type change might be worth pushing to a separate patch since the
> casting issues are not completely trivial.

Done.

The patch included in the series as part of v6 already got a review from
David Laight (thank you!) who suggests that this might not be a good
idea to do, and keeping existing type would be better.

Krzysztof

Re: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option

2020-11-30 Thread Alexandre Belloni

On 30/11/2020 15:21:33+, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from all ARM configurations.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
> unused")
> Cc: Sam Ravnborg 
> Signed-off-by: Andrey Zhizhikin 
Acked-by: Alexandre Belloni 

> ---
>  arch/arm/configs/at91_dt_defconfig| 1 -
>  arch/arm/configs/cm_x300_defconfig| 1 -
>  arch/arm/configs/colibri_pxa300_defconfig | 1 -
>  arch/arm/configs/jornada720_defconfig | 1 -
>  arch/arm/configs/magician_defconfig   | 1 -
>  arch/arm/configs/mini2440_defconfig   | 1 -
>  arch/arm/configs/omap2plus_defconfig  | 1 -
>  arch/arm/configs/pxa3xx_defconfig | 1 -
>  arch/arm/configs/qcom_defconfig   | 1 -
>  arch/arm/configs/sama5_defconfig  | 1 -
>  arch/arm/configs/sunxi_defconfig  | 1 -
>  arch/arm/configs/tegra_defconfig  | 1 -
>  arch/arm/configs/u8500_defconfig  | 1 -
>  13 files changed, 13 deletions(-)
> 
> diff --git a/arch/arm/configs/at91_dt_defconfig 
> b/arch/arm/configs/at91_dt_defconfig
> index 4a0ba2ae1a25..6e52c9c965e6 100644
> --- a/arch/arm/configs/at91_dt_defconfig
> +++ b/arch/arm/configs/at91_dt_defconfig
> @@ -132,7 +132,6 @@ CONFIG_DRM_ATMEL_HLCDC=y
>  CONFIG_DRM_PANEL_SIMPLE=y
>  CONFIG_FB_ATMEL=y
>  CONFIG_BACKLIGHT_ATMEL_LCDC=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  CONFIG_BACKLIGHT_PWM=y
>  CONFIG_FRAMEBUFFER_CONSOLE=y
>  CONFIG_LOGO=y
> diff --git a/arch/arm/configs/cm_x300_defconfig 
> b/arch/arm/configs/cm_x300_defconfig
> index 2f7acde2d921..502a9d870ca4 100644
> --- a/arch/arm/configs/cm_x300_defconfig
> +++ b/arch/arm/configs/cm_x300_defconfig
> @@ -87,7 +87,6 @@ CONFIG_FB=y
>  CONFIG_FB_PXA=y
>  CONFIG_LCD_CLASS_DEVICE=y
>  CONFIG_LCD_TDO24M=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  CONFIG_BACKLIGHT_DA903X=m
>  CONFIG_FRAMEBUFFER_CONSOLE=y
>  CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
> diff --git a/arch/arm/configs/colibri_pxa300_defconfig 
> b/arch/arm/configs/colibri_pxa300_defconfig
> index 0dae3b185284..26e5a67f8e2d 100644
> --- a/arch/arm/configs/colibri_pxa300_defconfig
> +++ b/arch/arm/configs/colibri_pxa300_defconfig
> @@ -34,7 +34,6 @@ CONFIG_FB=y
>  CONFIG_FB_PXA=y
>  # CONFIG_LCD_CLASS_DEVICE is not set
>  CONFIG_BACKLIGHT_CLASS_DEVICE=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  # CONFIG_VGA_CONSOLE is not set
>  CONFIG_FRAMEBUFFER_CONSOLE=y
>  CONFIG_LOGO=y
> diff --git a/arch/arm/configs/jornada720_defconfig 
> b/arch/arm/configs/jornada720_defconfig
> index 9f079be2b84b..069f60ffdcd8 100644
> --- a/arch/arm/configs/jornada720_defconfig
> +++ b/arch/arm/configs/jornada720_defconfig
> @@ -48,7 +48,6 @@ CONFIG_FB=y
>  CONFIG_FB_S1D13XXX=y
>  CONFIG_LCD_CLASS_DEVICE=y
>  CONFIG_BACKLIGHT_CLASS_DEVICE=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  # CONFIG_VGA_CONSOLE is not set
>  CONFIG_FRAMEBUFFER_CONSOLE=y
>  CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
> diff --git a/arch/arm/configs/magician_defconfig 
> b/arch/arm/configs/magician_defconfig
> index d2e684f6565a..b4670d42f378 100644
> --- a/arch/arm/configs/magician_defconfig
> +++ b/arch/arm/configs/magician_defconfig
> @@ -95,7 +95,6 @@ CONFIG_FB_PXA_OVERLAY=y
>  CONFIG_FB_W100=y
>  CONFIG_LCD_CLASS_DEVICE=y
>  CONFIG_BACKLIGHT_CLASS_DEVICE=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  CONFIG_BACKLIGHT_PWM=y
>  # CONFIG_VGA_CONSOLE is not set
>  CONFIG_FRAMEBUFFER_CONSOLE=y
> diff --git a/arch/arm/configs/mini2440_defconfig 
> b/arch/arm/configs/mini2440_defconfig
> index 301f29a1fcc3..898490aaa39e 100644
> --- a/arch/arm/configs/mini2440_defconfig
> +++ b/arch/arm/configs/mini2440_defconfig
> @@ -158,7 +158,6 @@ CONFIG_FB_S3C2410=y
>  CONFIG_LCD_CLASS_DEVICE=y
>  CONFIG_LCD_PLATFORM=y
>  CONFIG_BACKLIGHT_CLASS_DEVICE=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  CONFIG_BACKLIGHT_PWM=y
>  CONFIG_FRAMEBUFFER_CONSOLE=y
>  CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
> diff --git a/arch/arm/configs/omap2plus_defconfig 
> b/arch/arm/configs/omap2plus_defconfig
> index de3b7813a1ce..7eae097a75d2 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -388,7 +388,6 @@ CONFIG_FB_TILEBLITTING=y
>  CONFIG_LCD_CLASS_DEVICE=y
>  CONFIG_LCD_PLATFORM=y
>  CONFIG_BACKLIGHT_CLASS_DEVICE=y
> -CONFIG_BACKLIGHT_GENERIC=m
>  CONFIG_BACKLIGHT_PWM=m
>  CONFIG_BACKLIGHT_PANDORA=m
>  CONFIG_BACKLIGHT_GPIO=m
> diff --git a/arch/arm/configs/pxa3xx_defconfig 
> b/arch/arm/configs/pxa3xx_defconfig
> index 06bbc7a59b60..f0c34017f2aa 100644
> --- a/arch/arm/configs/pxa3xx_defconfig
> +++ b/arch/arm/configs/pxa3xx_defconfig
> @@ -74,7 +74,6 @@ CONFIG_FB_PXA=y
>  CONFIG_LCD_CLASS_DEVICE=y
>  CONFIG_LCD_TDO24M=y
>  CONFIG_BACKLIGHT_CLASS_DEVICE=y
> -# CONFIG_BACKLIGHT_GENERIC is not set
>  CONFIG_BACKLIGHT_DA903X=y
>  # CONFIG_VGA_CO

Re: [PATCH v4] dt-bindings: misc: convert fsl,qoriq-mc from txt to YAML

2020-11-30 Thread Rob Herring

On Mon, 23 Nov 2020 11:00:35 +0200, Laurentiu Tudor wrote:
> From: Ionut-robert Aron 
> 
> Convert fsl,qoriq-mc to YAML in order to automate the verification
> process of dts files. In addition, update MAINTAINERS accordingly
> and, while at it, add some missing files.
> 
> Signed-off-by: Ionut-robert Aron 
> [laurentiu.tu...@nxp.com: update MINTAINERS, updates & fixes in schema]
> Signed-off-by: Laurentiu Tudor 
> ---
> Changes in v4:
>  - use $ref to point to fsl,qoriq-mc-dpmac binding
> 
> Changes in v3:
>  - dropped duplicated "fsl,qoriq-mc-dpmac" schema and replaced with
>reference to it
>  - fixed a dt_binding_check warning
> 
> Changes in v2:
>  - fixed errors reported by yamllint
>  - dropped multiple unnecessary quotes
>  - used schema instead of text in description
>  - added constraints on dpmac reg property
> 
>  .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 196 --
>  .../bindings/misc/fsl,qoriq-mc.yaml   | 186 +
>  .../ethernet/freescale/dpaa2/overview.rst |   5 +-
>  MAINTAINERS   |   4 +-
>  4 files changed, 193 insertions(+), 198 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>  create mode 100644 Documentation/devicetree/bindings/misc/fsl,qoriq-mc.yaml
> 

As there's a dependency on fsl,qoriq-mc-dpmac, this needs to go via 
netdev tree.

Reviewed-by: Rob Herring

Re: [PATCH v6 4/5] PCI: vmd: Update type of the __iomem pointers

2020-11-30 Thread Bjorn Helgaas

On Mon, Nov 30, 2020 at 09:06:56AM +, David Laight wrote:
> From: Krzysztof Wilczynski
> > Sent: 29 November 2020 23:08
> > 
> > Use "void __iomem" instead "char __iomem" pointer type when working with
> > the accessor functions (with names like readb() or writel(), etc.) to
> > better match a given accessor function signature where commonly the
> > address pointing to an I/O memory region would be a "void __iomem"
> > pointer.
> 
> ISTM that is heading in the wrong direction.
> 
> I think (form the variable names etc) that these are pointers
> to specific registers.
> 
> So what you ought to have is a type for that register block.
> Typically this is actually a structure - to give some type
> checking that the offsets are being used with the correct
> base address.

In this case, "cfgbar" is not really a pointer to a register; it's the
address of memory-mapped config space.  The VMD hardware turns
accesses to that space into PCI config transactions on its secondary
side.  xgene_pcie_get_cfg_base() and brcm_pcie_map_conf() are similar
situations and use "void *".

Bjorn

Re: [PATCH 01/13] ibmvfc: add vhost fields and defaults for MQ enablement

On 11/27/20 9:45 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
>> index 9d58cfd774d3..8225bdbb127e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.h
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.h
>> @@ -41,6 +41,11 @@
>>  #define IBMVFC_DEFAULT_LOG_LEVEL2
>>  #define IBMVFC_MAX_CDB_LEN  16
>>  #define IBMVFC_CLS3_ERROR   0
>> +#define IBMVFC_MQ   0
> 
> Given that IBMVFC_MQ is getting set to 0 here, that means mq_enabled is also
> always zero, so am I correct that a lot of this code being added is not
> yet capable of being executed?

Not with out a direct intervention from a hard coding a different value when
building the code. See comment below.

> 
>> +#define IBMVFC_SCSI_CHANNELS0
> 
> Similar comment here...
> 
>> +#define IBMVFC_SCSI_HW_QUEUES   1
> 
> I don't see any subsequent patches in this series that would ever result
> in nr_hw_queues getting set to anything other than 1. Is that future work
> planned or am I missing something?

Yes, there is still some changes to EH that need to be included before those
values are safe to be set to anything else by the average user.

-Tyrel

> 
>> +#define IBMVFC_MIG_NO_SUB_TO_CRQ0
>> +#define IBMVFC_MIG_NO_N_TO_M0
>>  
>>  /*
>>   * Ensure we have resources for ERP and initialization:
>> @@ -826,6 +831,10 @@ struct ibmvfc_host {
>>  int delay_init;
>>  int scan_complete;
>>  int logged_in;
>> +int mq_enabled;
>> +int using_channels;
>> +int do_enquiry;
>> +int client_scsi_channels;
>>  int aborting_passthru;
>>  int events_to_log;
>>  #define IBMVFC_AE_LINKUP0x0001
>>
> 
>

Re: [PATCH 04/13] ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels

On 11/27/20 9:46 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> Allocate a set of Sub-CRQs in advance. During channel setup the client
>> and VIOS negotiate the number of queues the VIOS supports and the number
>> that the client desires to request. Its possible that the final channel
>> resources allocated is less than requested, but the client is still
>> responsible for sending handles for every queue it is hoping for.
>>
>> Also, provide deallocation cleanup routines.
>>
>> Signed-off-by: Tyrel Datwyler 
>> ---
>>  drivers/scsi/ibmvscsi/ibmvfc.c | 115 +
>>  drivers/scsi/ibmvscsi/ibmvfc.h |   1 +
>>  2 files changed, 116 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index 260b82e3cc01..571abdb48384 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -4983,6 +4983,114 @@ static int ibmvfc_init_crq(struct ibmvfc_host *vhost)
>>  return retrc;
>>  }
>>  
>> +static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
>> +  int index)
>> +{
>> +struct device *dev = vhost->dev;
>> +struct vio_dev *vdev = to_vio_dev(dev);
>> +struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];
>> +int rc = -ENOMEM;
>> +
>> +ENTER;
>> +
>> +scrq->msgs = (struct ibmvfc_sub_crq *)get_zeroed_page(GFP_KERNEL);
>> +if (!scrq->msgs)
>> +return rc;
>> +
>> +scrq->size = PAGE_SIZE / sizeof(*scrq->msgs);
>> +scrq->msg_token = dma_map_single(dev, scrq->msgs, PAGE_SIZE,
>> + DMA_BIDIRECTIONAL);
>> +
>> +if (dma_mapping_error(dev, scrq->msg_token))
>> +goto dma_map_failed;
>> +
>> +rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
>> +   &scrq->cookie, &scrq->hw_irq);
>> +
>> +if (rc) {
>> +dev_warn(dev, "Error registering sub-crq: %d\n", rc);
>> +dev_warn(dev, "Firmware may not support MQ\n");
>> +goto reg_failed;
>> +}
>> +
>> +scrq->hwq_id = index;
>> +scrq->vhost = vhost;
>> +
>> +LEAVE;
>> +return 0;
>> +
>> +reg_failed:
>> +dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
>> +dma_map_failed:
>> +free_page((unsigned long)scrq->msgs);
>> +LEAVE;
>> +return rc;
>> +}
>> +
>> +static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int 
>> index)
>> +{
>> +struct device *dev = vhost->dev;
>> +struct vio_dev *vdev = to_vio_dev(dev);
>> +struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];
>> +long rc;
>> +
>> +ENTER;
>> +
>> +do {
>> +rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
>> +scrq->cookie);
>> +} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
>> +
>> +if (rc)
>> +dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);
>> +
>> +dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
>> +free_page((unsigned long)scrq->msgs);
>> +LEAVE;
>> +}
>> +
>> +static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
>> +{
>> +int i, j;
>> +
>> +ENTER;
>> +
>> +vhost->scsi_scrqs.scrqs = kcalloc(vhost->client_scsi_channels,
>> +  sizeof(*vhost->scsi_scrqs.scrqs),
>> +  GFP_KERNEL);
>> +if (!vhost->scsi_scrqs.scrqs)
>> +return -1;
>> +
>> +for (i = 0; i < vhost->client_scsi_channels; i++) {
>> +if (ibmvfc_register_scsi_channel(vhost, i)) {
>> +for (j = i; j > 0; j--)
>> +ibmvfc_deregister_scsi_channel(vhost, j - 1);
>> +kfree(vhost->scsi_scrqs.scrqs);
>> +LEAVE;
>> +return -1;
>> +}
>> +}
>> +
>> +LEAVE;
>> +return 0;
>> +}
>> +
>> +static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
>> +{
>> +int i;
>> +
>> +ENTER;
>> +if (!vhost->scsi_scrqs.scrqs)
>> +return;
>> +
>> +for (i = 0; i < vhost->client_scsi_channels; i++)
>> +ibmvfc_deregister_scsi_channel(vhost, i);
>> +
>> +vhost->scsi_scrqs.active_queues = 0;
>> +kfree(vhost->scsi_scrqs.scrqs);
> 
> Do you want to NULL this out after you free it do you don't keep
> a reference to a freed page around?

This isn't actually a page, but a dynamically allocated array of
ibmvfc_sub_queues, but it should be NULL'ed regardless.

-Tyrel

> 
>> +LEAVE;
>> +}
>> +
>>  /**
>>   * ibmvfc_free_mem - Free memory for vhost
>>   * @vhost:  ibmvfc host struct
> 
> 
>

Re: [PATCH 06/13] ibmvfc: add handlers to drain and complete Sub-CRQ responses

On 11/27/20 9:47 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> The logic for iterating over the Sub-CRQ responses is similiar to that
>> of the primary CRQ. Add the necessary handlers for processing those
>> responses.
>>
>> Signed-off-by: Tyrel Datwyler 
>> ---
>>  drivers/scsi/ibmvscsi/ibmvfc.c | 72 ++
>>  1 file changed, 72 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index 6eaedda4917a..a8730522920e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -3371,6 +3371,78 @@ static int ibmvfc_toggle_scrq_irq(struct 
>> ibmvfc_sub_queue *scrq, int enable)
>>  return rc;
>>  }
>>  
>> +static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host 
>> *vhost)
>> +{
>> +struct ibmvfc_event *evt = (struct ibmvfc_event 
>> *)be64_to_cpu(crq->ioba);
>> +
>> +switch (crq->valid) {
>> +case IBMVFC_CRQ_CMD_RSP:
>> +break;
>> +default:
>> +dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", 
>> crq->valid);
> 
> Is this correct? Can't we get transport events here as well?

Yes we can. We still handle them in the primary CRQ so at least for the time
being we can ignore them, but yeah we shouldn't log scary messages about them.

-Tyrel

> 
>> +return;
>> +}
>> +
>> +/* The only kind of payload CRQs we should get are responses to
>> + * things we send. Make sure this response is to something we
>> + * actually sent
>> + */
>> +if (unlikely(!ibmvfc_valid_event(&vhost->pool, evt))) {
>> +dev_err(vhost->dev, "Returned correlation_token 0x%08llx is 
>> invalid!\n",
>> +crq->ioba);
>> +return;
>> +}
>> +
>> +if (unlikely(atomic_read(&evt->free))) {
>> +dev_err(vhost->dev, "Received duplicate correlation_token 
>> 0x%08llx!\n",
>> +crq->ioba);
>> +return;
>> +}
>> +
>> +del_timer(&evt->timer);
>> +list_del(&evt->queue);
>> +ibmvfc_trc_end(evt);
>> +evt->done(evt);
>> +}
>> +
> 
> 
>

Re: [PATCH 09/13] ibmvfc: implement channel enquiry and setup commands

On 11/27/20 9:49 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> 
>> @@ -4462,6 +4464,118 @@ static void ibmvfc_discover_targets(struct 
>> ibmvfc_host *vhost)
>>  ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
>>  }
>>  
>> +static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
>> +{
>> +struct ibmvfc_host *vhost = evt->vhost;
>> +u32 mad_status = be16_to_cpu(evt->xfer_iu->channel_setup.common.status);
>> +int level = IBMVFC_DEFAULT_LOG_LEVEL;
>> +
>> +ibmvfc_free_event(evt);
>> +
>> +switch (mad_status) {
>> +case IBMVFC_MAD_SUCCESS:
>> +ibmvfc_dbg(vhost, "Channel Setup succeded\n");
>> +vhost->do_enquiry = 0;
>> +break;
>> +case IBMVFC_MAD_FAILED:
>> +level += ibmvfc_retry_host_init(vhost);
>> +ibmvfc_log(vhost, level, "Channel Setup failed\n");
>> +fallthrough;
>> +case IBMVFC_MAD_DRIVER_FAILED:
>> +return;
>> +default:
>> +dev_err(vhost->dev, "Invalid Channel Setup response: 0x%x\n",
>> +mad_status);
>> +ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
>> +return;
>> +}
>> +
>> +ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
>> +wake_up(&vhost->work_wait_q);
>> +}
>> +
>> +static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
>> +{
>> +struct ibmvfc_channel_setup_mad *mad;
>> +struct ibmvfc_channel_setup *setup_buf = vhost->channel_setup_buf;
>> +struct ibmvfc_event *evt = ibmvfc_get_event(vhost);
>> +
>> +memset(setup_buf, 0, sizeof(*setup_buf));
>> +setup_buf->flags = cpu_to_be32(IBMVFC_CANCEL_CHANNELS);
>> +
>> +ibmvfc_init_event(evt, ibmvfc_channel_setup_done, IBMVFC_MAD_FORMAT);
>> +mad = &evt->iu.channel_setup;
>> +memset(mad, 0, sizeof(*mad));
>> +mad->common.version = cpu_to_be32(1);
>> +mad->common.opcode = cpu_to_be32(IBMVFC_CHANNEL_SETUP);
>> +mad->common.length = cpu_to_be16(sizeof(*mad));
>> +mad->buffer.va = cpu_to_be64(vhost->channel_setup_dma);
>> +mad->buffer.len = cpu_to_be32(sizeof(*vhost->channel_setup_buf));
>> +
>> +ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
>> +
>> +if (!ibmvfc_send_event(evt, vhost, default_timeout))
>> +ibmvfc_dbg(vhost, "Sent channel setup\n");
>> +else
>> +ibmvfc_link_down(vhost, IBMVFC_LINK_DOWN);
>> +}
>> +
>> +static void ibmvfc_channel_enquiry_done(struct ibmvfc_event *evt)
>> +{
>> +struct ibmvfc_host *vhost = evt->vhost;
>> +struct ibmvfc_channel_enquiry *rsp = &evt->xfer_iu->channel_enquiry;
>> +u32 mad_status = be16_to_cpu(rsp->common.status);
>> +int level = IBMVFC_DEFAULT_LOG_LEVEL;
>> +
>> +switch (mad_status) {
>> +case IBMVFC_MAD_SUCCESS:
>> +ibmvfc_dbg(vhost, "Channel Enquiry succeeded\n");
>> +vhost->max_vios_scsi_channels = 
>> be32_to_cpu(rsp->num_scsi_subq_channels);
> 
> You need a ibmvfc_free_event(evt) here so you don't leak events.
> 

Indeed

>> +break;
>> +case IBMVFC_MAD_FAILED:
>> +level += ibmvfc_retry_host_init(vhost);
>> +ibmvfc_log(vhost, level, "Channel Enquiry failed\n");
>> +ibmvfc_free_event(evt);
> 
> Looks like you are freeing this event twice due to the fallthrough...

Good catch

> 
>> +fallthrough;
>> +case IBMVFC_MAD_DRIVER_FAILED:
>> +ibmvfc_free_event(evt);
>> +return;
>> +default:
>> +dev_err(vhost->dev, "Invalid Channel Enquiry response: 0x%x\n",
>> +mad_status);
>> +ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
>> +ibmvfc_free_event(evt);
>> +return;
>> +}
>> +
>> +ibmvfc_channel_setup(vhost);
>> +}
>> +
> 
> 
>

Re: [PATCH v6 4/5] PCI: vmd: Update type of the __iomem pointers

2020-11-30 Thread Derrick, Jonathan

On Mon, 2020-11-30 at 11:20 -0600, Bjorn Helgaas wrote:
> On Mon, Nov 30, 2020 at 09:06:56AM +, David Laight wrote:
> > From: Krzysztof Wilczynski
> > > Sent: 29 November 2020 23:08
> > > 
> > > Use "void __iomem" instead "char __iomem" pointer type when working with
> > > the accessor functions (with names like readb() or writel(), etc.) to
> > > better match a given accessor function signature where commonly the
> > > address pointing to an I/O memory region would be a "void __iomem"
> > > pointer.
> > 
> > ISTM that is heading in the wrong direction.
> > 
> > I think (form the variable names etc) that these are pointers
> > to specific registers.
> > 
> > So what you ought to have is a type for that register block.
> > Typically this is actually a structure - to give some type
> > checking that the offsets are being used with the correct
> > base address.
> 
> In this case, "cfgbar" is not really a pointer to a register; it's the
> address of memory-mapped config space.  The VMD hardware turns
> accesses to that space into PCI config transactions on its secondary
> side.  xgene_pcie_get_cfg_base() and brcm_pcie_map_conf() are similar
> situations and use "void *".
> 
> Bjorn

Yes it's just the passthrough window for PCI config bus ops.

Reviewed-by: Jon Derrick

Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers

2020-11-30 Thread Derrick, Jonathan

On Sun, 2020-11-29 at 23:07 +, Krzysztof Wilczyński wrote:
> Add ECAM-related constants to provide a set of standard constants
> defining memory address shift values to the byte-level address that can
> be used to access the PCI Express Configuration Space, and then move
> native PCI Express controller drivers to use the newly introduced
> definitions retiring driver-specific ones.
> 
> Refactor pci_ecam_map_bus() function to use newly added constants so
> that limits to the bus, device function and offset (now limited to 4K as
> per the specification) are in place to prevent the defective or
> malicious caller from supplying incorrect configuration offset and thus
> targeting the wrong device when accessing extended configuration space.
> This refactor also allows for the ".bus_shit" initialisers to be dropped
> when the user is not using a custom value as a default value will be
> used as per the PCI Express Specification.
> 
> Suggested-by: Bjorn Helgaas 
> Signed-off-by: Krzysztof Wilczyński 
> ---
>  drivers/pci/controller/dwc/pcie-al.c| 12 ++---
>  drivers/pci/controller/dwc/pcie-hisi.c  |  2 --
>  drivers/pci/controller/pci-aardvark.c   | 13 +++---
>  drivers/pci/controller/pci-host-generic.c   |  1 -
>  drivers/pci/controller/pci-thunder-ecam.c   |  1 -
>  drivers/pci/controller/pcie-brcmstb.c   | 16 ++--
>  drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
>  drivers/pci/controller/pcie-rockchip.h  |  8 +-
>  drivers/pci/controller/pcie-tango.c |  1 -
>  drivers/pci/controller/pcie-xilinx-nwl.c|  9 ++-
>  drivers/pci/controller/pcie-xilinx.c| 11 ++---
>  drivers/pci/controller/vmd.c| 11 -
>  drivers/pci/ecam.c  | 23 --
>  include/linux/pci-ecam.h| 27 +
>  14 files changed, 73 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-al.c 
> b/drivers/pci/controller/dwc/pcie-al.c
> index f973fbca90cf..af9e51ab1af8 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops al_pcie_ops = {
> - .bus_shift= 20,
>   .init =  al_pcie_init,
>   .pci_ops  = {
>   .map_bus= al_pcie_map_bus,
> @@ -138,8 +137,6 @@ struct al_pcie {
>   struct al_pcie_target_bus_cfg target_bus_cfg;
>  };
>  
> -#define PCIE_ECAM_DEVFN(x)   (((x) & 0xff) << 12)
> -
>  #define to_al_pcie(x)dev_get_drvdata((x)->dev)
>  
>  static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct 
> pci_bus *bus,
>   struct al_pcie_target_bus_cfg *target_bus_cfg = &pcie->target_bus_cfg;
>   unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
>   unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
> - void __iomem *pci_base_addr;
> -
> - pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> -  (busnr_ecam << 20) +
> -  PCIE_ECAM_DEVFN(devfn));
>  
>   if (busnr_reg != target_bus_cfg->reg_val) {
>   dev_dbg(pcie->pci->dev, "Changing target bus busnum val from 
> 0x%x to 0x%x\n",
> @@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct 
> pci_bus *bus,
>  target_bus_cfg->reg_mask);
>   }
>  
> - return pci_base_addr + where;
> + return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
>  }
>  
>  static struct pci_ops al_child_pci_ops = {
> @@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>  
>   target_bus_cfg = &pcie->target_bus_cfg;
>  
> - ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> + ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
>   if (ecam_bus_mask > 255) {
>   dev_warn(pcie->dev, "ECAM window size is larger than 256MB. 
> Cutting off at 256\n");
>   ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c 
> b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..8fc5960faf28 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops hisi_pcie_ops = {
> - .bus_shift= 20,
>   .init =  hisi_pcie_init,
>   .pci_ops  = {
>   .map_bus= hisi_pcie_map_bus,
> @@ -135,7 +134,6 @@ static int hisi_pcie_platform_init(struct 
> pci_config_window *cfg)
>  }
>  
>  static const struct pci_ecam_ops hisi_pcie_platform_ops = {
> - .bus_shift= 20,
>   .init =  hisi_pcie_platform_init,
>

Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option

2020-11-30 Thread Andy Lutomirski

other arch folk: there's some background here:

https://lkml.kernel.org/r/calcetrvxube8lfnn-qs+dzroqaiw+sfug1j047ybyv31sat...@mail.gmail.com

On Sun, Nov 29, 2020 at 12:16 PM Andy Lutomirski  wrote:
>
> On Sat, Nov 28, 2020 at 7:54 PM Andy Lutomirski  wrote:
> >
> > On Sat, Nov 28, 2020 at 8:02 AM Nicholas Piggin  wrote:
> > >
> > > On big systems, the mm refcount can become highly contented when doing
> > > a lot of context switching with threaded applications (particularly
> > > switching between the idle thread and an application thread).
> > >
> > > Abandoning lazy tlb slows switching down quite a bit in the important
> > > user->idle->user cases, so so instead implement a non-refcounted scheme
> > > that causes __mmdrop() to IPI all CPUs in the mm_cpumask and shoot down
> > > any remaining lazy ones.
> > >
> > > Shootdown IPIs are some concern, but they have not been observed to be
> > > a big problem with this scheme (the powerpc implementation generated
> > > 314 additional interrupts on a 144 CPU system during a kernel compile).
> > > There are a number of strategies that could be employed to reduce IPIs
> > > if they turn out to be a problem for some workload.
> >
> > I'm still wondering whether we can do even better.
> >
>
> Hold on a sec.. __mmput() unmaps VMAs, frees pagetables, and flushes
> the TLB.  On x86, this will shoot down all lazies as long as even a
> single pagetable was freed.  (Or at least it will if we don't have a
> serious bug, but the code seems okay.  We'll hit pmd_free_tlb, which
> sets tlb->freed_tables, which will trigger the IPI.)  So, on
> architectures like x86, the shootdown approach should be free.  The
> only way it ought to have any excess IPIs is if we have CPUs in
> mm_cpumask() that don't need IPI to free pagetables, which could
> happen on paravirt.

Indeed, on x86, we do this:

[   11.558844]  flush_tlb_mm_range.cold+0x18/0x1d
[   11.559905]  tlb_finish_mmu+0x10e/0x1a0
[   11.561068]  exit_mmap+0xc8/0x1a0
[   11.561932]  mmput+0x29/0xd0
[   11.562688]  do_exit+0x316/0xa90
[   11.563588]  do_group_exit+0x34/0xb0
[   11.564476]  __x64_sys_exit_group+0xf/0x10
[   11.565512]  do_syscall_64+0x34/0x50

and we have info->freed_tables set.

What are the architectures that have large systems like?

x86: we already zap lazies, so it should cost basically nothing to do
a little loop at the end of __mmput() to make sure that no lazies are
left.  If we care about paravirt performance, we could implement one
of the optimizations I mentioned above to fix up the refcounts instead
of sending an IPI to any remaining lazies.

arm64: AFAICT arm64's flush uses magic arm64 hardware support for
remote flushes, so any lazy mm references will still exist after
exit_mmap().  (arm64 uses lazy TLB, right?)  So this is kind of like
the x86 paravirt case.  Are there large enough arm64 systems that any
of this matters?

s390x: The code has too many acronyms for me to understand it fully,
but I think it's more or less the same situation as arm64.  How big do
s390x systems come?

power: Ridiculously complicated, seems to vary by system and kernel config.

So, Nick, your unconditional IPI scheme is apparently a big
improvement for power, and it should be an improvement and have low
cost for x86.  On arm64 and s390x it will add more IPIs on process
exit but reduce contention on context switching depending on how lazy
TLB works.  I suppose we could try it for all architectures without
any further optimizations.  Or we could try one of the perhaps
excessively clever improvements I linked above.  arm64, s390x people,
what do you think?

Re: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option

On Mon, Nov 30, 2020 at 03:21:33PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from all ARM configurations.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
> unused")
> Cc: Sam Ravnborg 
> Signed-off-by: Andrey Zhizhikin 
> ---
>  arch/arm/configs/at91_dt_defconfig| 1 -
>  arch/arm/configs/cm_x300_defconfig| 1 -
>  arch/arm/configs/colibri_pxa300_defconfig | 1 -
>  arch/arm/configs/jornada720_defconfig | 1 -
>  arch/arm/configs/magician_defconfig   | 1 -
>  arch/arm/configs/mini2440_defconfig   | 1 -
>  arch/arm/configs/omap2plus_defconfig  | 1 -
>  arch/arm/configs/pxa3xx_defconfig | 1 -
>  arch/arm/configs/qcom_defconfig   | 1 -
>  arch/arm/configs/sama5_defconfig  | 1 -
>  arch/arm/configs/sunxi_defconfig  | 1 -
>  arch/arm/configs/tegra_defconfig  | 1 -
>  arch/arm/configs/u8500_defconfig  | 1 -
>  13 files changed, 13 deletions(-)

You need to send it to arm-soc maintainers, otherwise no one might feel
responsible enough to pick it up.
Reviewed-by: Krzysztof Kozlowski 

+CC Arnd and Olof,

Dear Arnd and Olof,

Maybe it is worth to add arm-soc entry to the MAINTAINERS file?
Otherwise how one could get your email address? Not mentioning the
secret-soc address. :)

Best regards,
Krzysztof

Re: [PATCH 2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option

On Mon, Nov 30, 2020 at 03:21:34PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from arm64 configuration.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
> unused")
> Cc: Sam Ravnborg 
> Signed-off-by: Andrey Zhizhikin 
> ---
>  arch/arm64/configs/defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski 

The same trouble as with ARM patch - this should go directly via
arm-soc.

Best regards,
Krzysztof

Re: [PATCH 3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option

On Mon, Nov 30, 2020 at 03:21:35PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from all MIPS configurations.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
> unused")
> Cc: Sam Ravnborg 
> Signed-off-by: Andrey Zhizhikin 
> ---
>  arch/mips/configs/gcw0_defconfig  | 1 -
>  arch/mips/configs/gpr_defconfig   | 1 -
>  arch/mips/configs/lemote2f_defconfig  | 1 -
>  arch/mips/configs/loongson3_defconfig | 1 -
>  arch/mips/configs/mtx1_defconfig  | 1 -
>  arch/mips/configs/rs90_defconfig  | 1 -
>  6 files changed, 6 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH 4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option

On Mon, Nov 30, 2020 at 03:21:36PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
> unused")
> Cc: Sam Ravnborg 
> Signed-off-by: Andrey Zhizhikin 
> ---
>  arch/parisc/configs/generic-64bit_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

Re: [PATCH 5/5] powerpc/configs: drop unused BACKLIGHT_GENERIC option

On Mon, Nov 30, 2020 at 03:21:37PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
> unused")
> Cc: Sam Ravnborg 
> Signed-off-by: Andrey Zhizhikin 
> ---
>  arch/powerpc/configs/powernv_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

[PATCH net v2 1/2] ibmvnic: Ensure that SCRQ entry reads are correctly ordered

2020-11-30 Thread Thomas Falcon

Ensure that received Subordinate Command-Response Queue (SCRQ)
entries are properly read in order by the driver. These queues
are used in the ibmvnic device to process RX buffer and TX completion
descriptors. dma_rmb barriers have been added after checking for a
pending descriptor to ensure the correct descriptor entry is checked
and after reading the SCRQ descriptor to ensure the entire
descriptor is read before processing.

Fixes: 032c5e828 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2aa40b2..5ea9f5c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2403,6 +2403,12 @@ static int ibmvnic_poll(struct napi_struct *napi, int 
budget)
 
if (!pending_scrq(adapter, adapter->rx_scrq[scrq_num]))
break;
+   /* The queue entry at the current index is peeked at above
+* to determine that there is a valid descriptor awaiting
+* processing. We want to be sure that the current slot
+* holds a valid descriptor before reading its contents.
+*/
+   dma_rmb();
next = ibmvnic_next_scrq(adapter, adapter->rx_scrq[scrq_num]);
rx_buff =
(struct ibmvnic_rx_buff *)be64_to_cpu(next->
@@ -3098,6 +3104,13 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter 
*adapter,
unsigned int pool = scrq->pool_index;
int num_entries = 0;
 
+   /* The queue entry at the current index is peeked at above
+* to determine that there is a valid descriptor awaiting
+* processing. We want to be sure that the current slot
+* holds a valid descriptor before reading its contents.
+*/
+   dma_rmb();
+
next = ibmvnic_next_scrq(adapter, scrq);
for (i = 0; i < next->tx_comp.num_comps; i++) {
if (next->tx_comp.rcs[i]) {
@@ -3498,6 +3511,11 @@ static union sub_crq *ibmvnic_next_scrq(struct 
ibmvnic_adapter *adapter,
}
spin_unlock_irqrestore(&scrq->lock, flags);
 
+   /* Ensure that the entire buffer descriptor has been
+* loaded before reading its contents
+*/
+   dma_rmb();
+
return entry;
 }
 
-- 
1.8.3.1

[PATCH net v2 0/2] ibmvnic: Bug fixes for queue descriptor processing

2020-11-30 Thread Thomas Falcon

This series resolves a few issues in the ibmvnic driver's
RX buffer and TX completion processing. The first patch
includes memory barriers to synchronize queue descriptor
reads. The second patch fixes a memory leak that could
occur if the device returns a TX completion with an error
code in the descriptor, in which case the respective socket
buffer and other relevant data structures may not be freed
or updated properly.

v2: Provide more detailed comments explaining specifically what
reads are being ordered, suggested by Michael Ellerman

Thomas Falcon (2):
  ibmvnic: Ensure that SCRQ entry reads are correctly ordered
  ibmvnic: Fix TX completion error handling

 drivers/net/ethernet/ibm/ibmvnic.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[PATCH net v2 2/2] ibmvnic: Fix TX completion error handling

2020-11-30 Thread Thomas Falcon

TX completions received with an error return code are not
being processed properly. When an error code is seen, do not
proceed to the next completion before cleaning up the existing
entry's data structures.

Fixes: 032c5e828 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5ea9f5c..10878f8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3113,11 +3113,9 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter 
*adapter,
 
next = ibmvnic_next_scrq(adapter, scrq);
for (i = 0; i < next->tx_comp.num_comps; i++) {
-   if (next->tx_comp.rcs[i]) {
+   if (next->tx_comp.rcs[i])
dev_err(dev, "tx error %x\n",
next->tx_comp.rcs[i]);
-   continue;
-   }
index = be32_to_cpu(next->tx_comp.correlators[i]);
if (index & IBMVNIC_TSO_POOL_MASK) {
tx_pool = &adapter->tso_pool[pool];
-- 
1.8.3.1

Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option

2020-11-30 Thread Sam Ravnborg

On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> Since the removal of generic_bl driver from the source tree in commit
> 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") BACKLIGHT_GENERIC config option became obsolete as well and
> therefore subject to clean-up from all configuration files.
> 
> This series introduces patches to address this removal, separated by
> architectures in the kernel tree.
> 
> Andrey Zhizhikin (5):
>   ARM: configs: drop unused BACKLIGHT_GENERIC option
>   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
>   MIPS: configs: drop unused BACKLIGHT_GENERIC option
>   parisc: configs: drop unused BACKLIGHT_GENERIC option
>   powerpc/configs: drop unused BACKLIGHT_GENERIC option

For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
savedefconfig / cp defconfig ... run now and then - this will remove
all such symbols.

If the patches goes in like they are submitted then:
Acked-by: Sam Ravnborg 

> 
>  arch/arm/configs/at91_dt_defconfig  | 1 -
>  arch/arm/configs/cm_x300_defconfig  | 1 -
>  arch/arm/configs/colibri_pxa300_defconfig   | 1 -
>  arch/arm/configs/jornada720_defconfig   | 1 -
>  arch/arm/configs/magician_defconfig | 1 -
>  arch/arm/configs/mini2440_defconfig | 1 -
>  arch/arm/configs/omap2plus_defconfig| 1 -
>  arch/arm/configs/pxa3xx_defconfig   | 1 -
>  arch/arm/configs/qcom_defconfig | 1 -
>  arch/arm/configs/sama5_defconfig| 1 -
>  arch/arm/configs/sunxi_defconfig| 1 -
>  arch/arm/configs/tegra_defconfig| 1 -
>  arch/arm/configs/u8500_defconfig| 1 -
>  arch/arm64/configs/defconfig| 1 -
>  arch/mips/configs/gcw0_defconfig| 1 -
>  arch/mips/configs/gpr_defconfig | 1 -
>  arch/mips/configs/lemote2f_defconfig| 1 -
>  arch/mips/configs/loongson3_defconfig   | 1 -
>  arch/mips/configs/mtx1_defconfig| 1 -
>  arch/mips/configs/rs90_defconfig| 1 -
>  arch/parisc/configs/generic-64bit_defconfig | 1 -
>  arch/powerpc/configs/powernv_defconfig  | 1 -
>  22 files changed, 22 deletions(-)
> 
> 
> base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
> prerequisite-patch-id: bfd382cf1dc021d20204f10ea9403319c1c32b12
> prerequisite-patch-id: 5397c0c8648bb3e0b830207ea867138c11c6e644
> prerequisite-patch-id: a3c284dff5fe6d02828918a886db6a8ed3197e20
> -- 
> 2.17.1

Re: [PATCH v8 11/12] mm/vmalloc: Hugepage vmalloc mappings

2020-11-30 Thread Edgecombe, Rick P

On Sun, 2020-11-29 at 01:25 +1000, Nicholas Piggin wrote:
> Support huge page vmalloc mappings. Config option
> HAVE_ARCH_HUGE_VMALLOC
> enables support on architectures that define HAVE_ARCH_HUGE_VMAP and
> supports PMD sized vmap mappings.
> 
> vmalloc will attempt to allocate PMD-sized pages if allocating PMD
> size
> or larger, and fall back to small pages if that was unsuccessful.
> 
> Allocations that do not use PAGE_KERNEL prot are not permitted to use
> huge pages, because not all callers expect this (e.g., module
> allocations vs strict module rwx).

Several architectures (x86, arm64, others?) allocate modules initially
with PAGE_KERNEL and so I think this test will not exclude module
allocations in those cases.

[snip]

> @@ -2400,6 +2453,7 @@ static inline void set_area_direct_map(const
> struct vm_struct *area,
>  {
>   int i;
>  
> + /* HUGE_VMALLOC passes small pages to set_direct_map */
>   for (i = 0; i < area->nr_pages; i++)
>   if (page_address(area->pages[i]))
>   set_direct_map(area->pages[i]);
> @@ -2433,11 +2487,12 @@ static void vm_remove_mappings(struct
> vm_struct *area, int deallocate_pages)
>* map. Find the start and end range of the direct mappings to
> make sure
>* the vm_unmap_aliases() flush includes the direct map.
>*/
> - for (i = 0; i < area->nr_pages; i++) {
> + for (i = 0; i < area->nr_pages; i += 1U << area->page_order) {
>   unsigned long addr = (unsigned long)page_address(area-
> >pages[i]);
>   if (addr) {
> + unsigned long page_size = PAGE_SIZE << area-
> >page_order;
>   start = min(addr, start);
> - end = max(addr + PAGE_SIZE, end);
> + end = max(addr + page_size, end);
>   flush_dmap = 1;
>   }
>   }

The logic around this is a bit tangled. The reset of the direct map has
to succeed, but if the set_direct_map_() functions require a split they
could fail. For x86, set_memory_ro() calls on a vmalloc alias will
mirror the page size and permission on the direct map and so the direct
map will be broken to 4k pages if it's a RO vmalloc allocation.

But after this, module vmalloc()'s could have large pages which would
result in large RO pages on the direct map. Then it could possibly fail
when trying to reset a 4k page out of a large RO direct map mapping. 

I think either module allocations need to be actually excluded from
having large pages (seems like you might have seen other issues as
well?), or another option could be to use the changes here:
https://lore.kernel.org/lkml/20201125092208.12544-4-r...@kernel.org/
to reset the direct map for a large page range at a time for large 
vmalloc pages.

Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option

On Mon, Nov 30, 2020 at 08:11:33PM +0100, Sam Ravnborg wrote:
> On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> > Since the removal of generic_bl driver from the source tree in commit
> > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > therefore subject to clean-up from all configuration files.
> > 
> > This series introduces patches to address this removal, separated by
> > architectures in the kernel tree.
> > 
> > Andrey Zhizhikin (5):
> >   ARM: configs: drop unused BACKLIGHT_GENERIC option
> >   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> >   MIPS: configs: drop unused BACKLIGHT_GENERIC option
> >   parisc: configs: drop unused BACKLIGHT_GENERIC option
> >   powerpc/configs: drop unused BACKLIGHT_GENERIC option
> 
> For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
> savedefconfig / cp defconfig ... run now and then - this will remove
> all such symbols.

savedefconfig can be tricky because of risk of loosing options:
1. it will remove options which became the default or became selected,
2. later when the default is changed or selecting option is removed, the
   first option from #1 will not be brought back.

This was already for example with DEBUG_FS and the conclusion that time
was - do not run savedefconfig automatically.

Therefore if some symbol(s) can be safely removed, patch is welcomed.

Best regards,
Krzysztof

> 
> If the patches goes in like they are submitted then:
> Acked-by: Sam Ravnborg

[RFC Qemu PATCH v2 0/2] spapr: nvdimm: Asynchronus flush hcall support

2020-11-30 Thread Shivaprasad G Bhat

The nvdimm devices are expected to ensure write persistent during power
failure kind of scenarios.

The libpmem has architecture specific instructions like dcbf on power
to flush the cache data to backend nvdimm device during normal writes.

Qemu - virtual nvdimm devices are memory mapped. The dcbf in the guest
doesn't traslate to actual flush to the backend file on the host in case
of file backed vnvdimms. This is addressed by virtio-pmem in case of x86_64
by making asynchronous flushes.

On PAPR, issue is addressed by adding a new hcall to
request for an explicit asynchronous flush requests from the guest ndctl
driver when the backend nvdimm cannot ensure write persistence with dcbf
alone. So, the approach here is to convey when the asynchronous flush is
required in a device tree property. The guest makes the hcall when the
property is found, instead of relying on dcbf.

The first patch adds the necessary asynchronous hcall support infrastructure
code at the DRC level. Second patch implements the hcall using the
infrastructure.

Hcall semantics are in review and not final.

A new device property sync-dax is added to the nvdimm device. When the
sync-dax is off(default), the asynchronous hcalls will be called.

With respect to save from new qemu to restore on old qemu, having the
sync-dax by default off(when not specified) causes IO errors in guests as
the async-hcall would not be supported on old qemu. The new hcall
implementation being supported only on the new pseries machine version,
the current machine version checks may be sufficient to prevent
such migration. Please suggest what should be done.

The below demonstration shows the map_sync behavior with sync-dax on & off.
(https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c)

The pmem0 is from nvdimm with With sync-dax=on, and pmem1 is from nvdimm with
syn-dax=off, mounted as
/dev/pmem0 on /mnt1 type xfs
(rw,relatime,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota)
/dev/pmem1 on /mnt2 type xfs
(rw,relatime,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota)

[root@atest-guest ~]# ./mapsync /mnt1/newfile> When sync-dax=off
[root@atest-guest ~]# ./mapsync /mnt2/newfile> when sync-dax=on
Failed to mmap with Operation not supported

---
v1 - https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg06330.html
Changes from v1
- Fixed a missed-out unlock
- using QLIST_FOREACH instead of QLIST_FOREACH_SAFE while generating token

Shivaprasad G Bhat (2):
spapr: drc: Add support for async hcalls at the drc level
spapr: nvdimm: Implement async flush hcalls

--
Signature

[RFC Qemu PATCH v2 1/2] spapr: drc: Add support for async hcalls at the drc level

2020-11-30 Thread Shivaprasad G Bhat

The patch adds support for async hcalls at the DRC level for the
spapr devices. To be used by spapr-scm devices in the patch/es to follow.

Signed-off-by: Shivaprasad G Bhat 
---
 hw/ppc/spapr_drc.c |  149 
 include/hw/ppc/spapr_drc.h |   25 +++
 2 files changed, 174 insertions(+)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 77718cde1f..4ecd04f686 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -15,6 +15,7 @@
 #include "qapi/qmp/qnull.h"
 #include "cpu.h"
 #include "qemu/cutils.h"
+#include "qemu/guest-random.h"
 #include "hw/ppc/spapr_drc.h"
 #include "qom/object.h"
 #include "migration/vmstate.h"
@@ -421,6 +422,148 @@ void spapr_drc_detach(SpaprDrc *drc)
 spapr_drc_release(drc);
 }
 
+
+/*
+ * @drc : device DRC targetting which the async hcalls to be made.
+ *
+ * All subsequent requests to run/query the status should use the
+ * unique token returned here.
+ */
+uint64_t spapr_drc_get_new_async_hcall_token(SpaprDrc *drc)
+{
+Error *err = NULL;
+uint64_t token;
+SpaprDrcDeviceAsyncHCallState *tmp, *next, *state;
+
+state = g_malloc0(sizeof(*state));
+state->pending = true;
+
+qemu_mutex_lock(&drc->async_hcall_states_lock);
+retry:
+if (qemu_guest_getrandom(&token, sizeof(token), &err) < 0) {
+error_report_err(err);
+g_free(state);
+qemu_mutex_unlock(&drc->async_hcall_states_lock);
+return 0;
+}
+
+if (!token) /* Token should be non-zero */
+goto retry;
+
+if (!QLIST_EMPTY(&drc->async_hcall_states)) {
+QLIST_FOREACH_SAFE(tmp, &drc->async_hcall_states, node, next) {
+if (tmp->continue_token == token) {
+/* If the token already in use, get a new one */
+goto retry;
+}
+}
+}
+
+state->continue_token = token;
+QLIST_INSERT_HEAD(&drc->async_hcall_states, state, node);
+
+qemu_mutex_unlock(&drc->async_hcall_states_lock);
+
+return state->continue_token;
+}
+
+static void *spapr_drc_async_hcall_runner(void *opaque)
+{
+int response = -1;
+SpaprDrcDeviceAsyncHCallState *state = opaque;
+
+/*
+ * state is freed only after this thread finishes(after pthread_join()),
+ * don't worry about it becoming NULL.
+ */
+
+response = state->func(state->data);
+
+state->hcall_ret = response;
+state->pending = 0;
+
+return NULL;
+}
+
+/*
+ * @drc  : device DRC targetting which the async hcalls to be made.
+ * token : The continue token to be used for tracking as recived from
+ * spapr_drc_get_new_async_hcall_token
+ * @func() : the worker function which needs to be executed asynchronously
+ * @data : data to be passed to the asynchronous function. Worker is supposed
+ * to free/cleanup the data that is passed here
+ */
+void spapr_drc_run_async_hcall(SpaprDrc *drc, uint64_t token,
+   SpaprDrcAsyncHcallWorkerFunc *func, void *data)
+{
+SpaprDrcDeviceAsyncHCallState *state;
+
+qemu_mutex_lock(&drc->async_hcall_states_lock);
+QLIST_FOREACH(state, &drc->async_hcall_states, node) {
+if (state->continue_token == token) {
+state->func = func;
+state->data = data;
+qemu_thread_create(&state->thread, "sPAPR Async HCALL",
+   spapr_drc_async_hcall_runner, state,
+   QEMU_THREAD_JOINABLE);
+break;
+}
+}
+qemu_mutex_unlock(&drc->async_hcall_states_lock);
+}
+
+/*
+ * spapr_drc_finish_async_hcalls
+ *  Waits for all pending async requests to complete
+ *  thier execution and free the states
+ */
+static void spapr_drc_finish_async_hcalls(SpaprDrc *drc)
+{
+SpaprDrcDeviceAsyncHCallState *state, *next;
+
+if (QLIST_EMPTY(&drc->async_hcall_states)) {
+return;
+}
+
+qemu_mutex_lock(&drc->async_hcall_states_lock);
+QLIST_FOREACH_SAFE(state, &drc->async_hcall_states, node, next) {
+qemu_thread_join(&state->thread);
+QLIST_REMOVE(state, node);
+g_free(state);
+}
+qemu_mutex_unlock(&drc->async_hcall_states_lock);
+}
+
+/*
+ * spapr_drc_get_async_hcall_status
+ *  Fetches the status of the hcall worker and returns H_BUSY
+ *  if the worker is still running.
+ */
+int spapr_drc_get_async_hcall_status(SpaprDrc *drc, uint64_t token)
+{
+int ret = H_PARAMETER;
+SpaprDrcDeviceAsyncHCallState *state, *node;
+
+qemu_mutex_lock(&drc->async_hcall_states_lock);
+QLIST_FOREACH_SAFE(state, &drc->async_hcall_states, node, node) {
+if (state->continue_token == token) {
+if (state->pending) {
+ret = H_BUSY;
+break;
+} else {
+ret = state->hcall_ret;
+qemu_thread_join(&state->thread);
+QLIST_REMOVE(state, node);
+g_free(state);
+break;
+}
+

[RFC Qemu PATCH v2 2/2] spapr: nvdimm: Implement async flush hcalls

2020-11-30 Thread Shivaprasad G Bhat

When the persistent memory beacked by a file, a cpu cache flush instruction
is not sufficient to ensure the stores are correctly flushed to the media.

The patch implements the async hcalls for flush operation on demand from the
guest kernel.

The device option sync-dax is by default off and enables explicit asynchronous
flush requests from guest. It can be disabled by setting syn-dax=on.

Signed-off-by: Shivaprasad G Bhat 
---
 hw/mem/nvdimm.c |1 +
 hw/ppc/spapr_nvdimm.c   |   79 +++
 include/hw/mem/nvdimm.h |   10 ++
 include/hw/ppc/spapr.h  |3 +-
 4 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 03c2201b56..37a4db0135 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -220,6 +220,7 @@ static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, 
const void *buf,
 
 static Property nvdimm_properties[] = {
 DEFINE_PROP_BOOL(NVDIMM_UNARMED_PROP, NVDIMMDevice, unarmed, false),
+DEFINE_PROP_BOOL(NVDIMM_SYNC_DAX_PROP, NVDIMMDevice, sync_dax, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index a833a63b5e..557e36aa98 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "hw/ppc/spapr_drc.h"
 #include "hw/ppc/spapr_nvdimm.h"
@@ -155,6 +156,11 @@ static int spapr_dt_nvdimm(SpaprMachineState *spapr, void 
*fdt,
  "operating-system")));
 _FDT(fdt_setprop(fdt, child_offset, "ibm,cache-flush-required", NULL, 0));
 
+if (!nvdimm->sync_dax) {
+_FDT(fdt_setprop(fdt, child_offset, "ibm,async-flush-required",
+ NULL, 0));
+}
+
 return child_offset;
 }
 
@@ -370,6 +376,78 @@ static target_ulong h_scm_bind_mem(PowerPCCPU *cpu, 
SpaprMachineState *spapr,
 return H_SUCCESS;
 }
 
+typedef struct SCMAsyncFlushData {
+int fd;
+uint64_t token;
+} SCMAsyncFlushData;
+
+static int flush_worker_cb(void *opaque)
+{
+int ret = H_SUCCESS;
+SCMAsyncFlushData *req_data = opaque;
+
+/* flush raw backing image */
+if (qemu_fdatasync(req_data->fd) < 0) {
+error_report("papr_scm: Could not sync nvdimm to backend file: %s",
+ strerror(errno));
+ret = H_HARDWARE;
+}
+
+g_free(req_data);
+
+return ret;
+}
+
+static target_ulong h_scm_async_flush(PowerPCCPU *cpu, SpaprMachineState 
*spapr,
+  target_ulong opcode, target_ulong *args)
+{
+int ret;
+uint32_t drc_index = args[0];
+uint64_t continue_token = args[1];
+SpaprDrc *drc = spapr_drc_by_index(drc_index);
+PCDIMMDevice *dimm;
+HostMemoryBackend *backend = NULL;
+SCMAsyncFlushData *req_data = NULL;
+
+if (!drc || !drc->dev ||
+spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PMEM) {
+return H_PARAMETER;
+}
+
+if (continue_token != 0) {
+ret = spapr_drc_get_async_hcall_status(drc, continue_token);
+if (ret == H_BUSY) {
+args[0] = continue_token;
+return H_LONG_BUSY_ORDER_1_SEC;
+}
+
+return ret;
+}
+
+dimm = PC_DIMM(drc->dev);
+backend = MEMORY_BACKEND(dimm->hostmem);
+
+req_data = g_malloc0(sizeof(SCMAsyncFlushData));
+req_data->fd = memory_region_get_fd(&backend->mr);
+
+continue_token = spapr_drc_get_new_async_hcall_token(drc);
+if (!continue_token) {
+g_free(req_data);
+return H_P2;
+}
+req_data->token = continue_token;
+
+spapr_drc_run_async_hcall(drc, continue_token, &flush_worker_cb, req_data);
+
+ret = spapr_drc_get_async_hcall_status(drc, continue_token);
+if (ret == H_BUSY) {
+args[0] = req_data->token;
+return ret;
+}
+
+return ret;
+}
+
 static target_ulong h_scm_unbind_mem(PowerPCCPU *cpu, SpaprMachineState *spapr,
  target_ulong opcode, target_ulong *args)
 {
@@ -486,6 +564,7 @@ static void spapr_scm_register_types(void)
 spapr_register_hypercall(H_SCM_BIND_MEM, h_scm_bind_mem);
 spapr_register_hypercall(H_SCM_UNBIND_MEM, h_scm_unbind_mem);
 spapr_register_hypercall(H_SCM_UNBIND_ALL, h_scm_unbind_all);
+spapr_register_hypercall(H_SCM_ASYNC_FLUSH, h_scm_async_flush);
 }
 
 type_init(spapr_scm_register_types)
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index c699842dd0..9e8795766e 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -51,6 +51,7 @@ OBJECT_DECLARE_TYPE(NVDIMMDevice, NVDIMMClass, NVDIMM)
 #define NVDIMM_LABEL_SIZE_PROP "label-size"
 #define NVDIMM_UUID_PROP   "uuid"
 #define NVDIMM_UNARMED_PROP"unarmed"
+#define NVDIMM_SYNC_DAX_PROP"sync-dax"
 
 struct NVDIMMDevice {
 /* private */
@@ -85,6 +86,15 @@ struct NVDIMMDevice {
  */
 bool unarmed;
 
+/*
+ * On PPC

[PATCH 0/5] drop unused BACKLIGHT_GENERIC option

Since the removal of generic_bl driver from the source tree in commit
7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") BACKLIGHT_GENERIC config option became obsolete as well and
therefore subject to clean-up from all configuration files.

This series introduces patches to address this removal, separated by
architectures in the kernel tree.

Andrey Zhizhikin (5):
  ARM: configs: drop unused BACKLIGHT_GENERIC option
  arm64: defconfig: drop unused BACKLIGHT_GENERIC option
  MIPS: configs: drop unused BACKLIGHT_GENERIC option
  parisc: configs: drop unused BACKLIGHT_GENERIC option
  powerpc/configs: drop unused BACKLIGHT_GENERIC option

 arch/arm/configs/at91_dt_defconfig  | 1 -
 arch/arm/configs/cm_x300_defconfig  | 1 -
 arch/arm/configs/colibri_pxa300_defconfig   | 1 -
 arch/arm/configs/jornada720_defconfig   | 1 -
 arch/arm/configs/magician_defconfig | 1 -
 arch/arm/configs/mini2440_defconfig | 1 -
 arch/arm/configs/omap2plus_defconfig| 1 -
 arch/arm/configs/pxa3xx_defconfig   | 1 -
 arch/arm/configs/qcom_defconfig | 1 -
 arch/arm/configs/sama5_defconfig| 1 -
 arch/arm/configs/sunxi_defconfig| 1 -
 arch/arm/configs/tegra_defconfig| 1 -
 arch/arm/configs/u8500_defconfig| 1 -
 arch/arm64/configs/defconfig| 1 -
 arch/mips/configs/gcw0_defconfig| 1 -
 arch/mips/configs/gpr_defconfig | 1 -
 arch/mips/configs/lemote2f_defconfig| 1 -
 arch/mips/configs/loongson3_defconfig   | 1 -
 arch/mips/configs/mtx1_defconfig| 1 -
 arch/mips/configs/rs90_defconfig| 1 -
 arch/parisc/configs/generic-64bit_defconfig | 1 -
 arch/powerpc/configs/powernv_defconfig  | 1 -
 22 files changed, 22 deletions(-)


base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
prerequisite-patch-id: bfd382cf1dc021d20204f10ea9403319c1c32b12
prerequisite-patch-id: 5397c0c8648bb3e0b830207ea867138c11c6e644
prerequisite-patch-id: a3c284dff5fe6d02828918a886db6a8ed3197e20
-- 
2.17.1

[PATCH 2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from arm64 configuration.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
unused")
Cc: Sam Ravnborg 
Signed-off-by: Andrey Zhizhikin 
---
 arch/arm64/configs/defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8e3f7ae71de5..280ed7404a1d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -681,7 +681,6 @@ CONFIG_DRM_PANFROST=m
 CONFIG_FB=y
 CONFIG_FB_MODE_HELPERS=y
 CONFIG_FB_EFI=y
-CONFIG_BACKLIGHT_GENERIC=m
 CONFIG_BACKLIGHT_PWM=m
 CONFIG_BACKLIGHT_LP855X=m
 CONFIG_LOGO=y
-- 
2.17.1

[PATCH 3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from all MIPS configurations.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
unused")
Cc: Sam Ravnborg 
Signed-off-by: Andrey Zhizhikin 
---
 arch/mips/configs/gcw0_defconfig  | 1 -
 arch/mips/configs/gpr_defconfig   | 1 -
 arch/mips/configs/lemote2f_defconfig  | 1 -
 arch/mips/configs/loongson3_defconfig | 1 -
 arch/mips/configs/mtx1_defconfig  | 1 -
 arch/mips/configs/rs90_defconfig  | 1 -
 6 files changed, 6 deletions(-)

diff --git a/arch/mips/configs/gcw0_defconfig b/arch/mips/configs/gcw0_defconfig
index 7e28a4fe9d84..460683b52285 100644
--- a/arch/mips/configs/gcw0_defconfig
+++ b/arch/mips/configs/gcw0_defconfig
@@ -73,7 +73,6 @@ CONFIG_DRM_PANEL_NOVATEK_NT39016=y
 CONFIG_DRM_INGENIC=y
 CONFIG_DRM_ETNAVIV=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
diff --git a/arch/mips/configs/gpr_defconfig b/arch/mips/configs/gpr_defconfig
index 9085f4d6c698..87e20f3391ed 100644
--- a/arch/mips/configs/gpr_defconfig
+++ b/arch/mips/configs/gpr_defconfig
@@ -251,7 +251,6 @@ CONFIG_SSB_DRIVER_PCICORE=y
 # CONFIG_VGA_ARB is not set
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_USB_HID=m
 CONFIG_USB_HIDDEV=y
diff --git a/arch/mips/configs/lemote2f_defconfig 
b/arch/mips/configs/lemote2f_defconfig
index 3a9a453b1264..688c91918db2 100644
--- a/arch/mips/configs/lemote2f_defconfig
+++ b/arch/mips/configs/lemote2f_defconfig
@@ -145,7 +145,6 @@ CONFIG_FB_SIS_300=y
 CONFIG_FB_SIS_315=y
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-CONFIG_BACKLIGHT_GENERIC=m
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
diff --git a/arch/mips/configs/loongson3_defconfig 
b/arch/mips/configs/loongson3_defconfig
index 38a817ead8e7..9c5fadef38cb 100644
--- a/arch/mips/configs/loongson3_defconfig
+++ b/arch/mips/configs/loongson3_defconfig
@@ -286,7 +286,6 @@ CONFIG_DRM_VIRTIO_GPU=y
 CONFIG_FB_RADEON=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_PLATFORM=m
-CONFIG_BACKLIGHT_GENERIC=m
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
diff --git a/arch/mips/configs/mtx1_defconfig b/arch/mips/configs/mtx1_defconfig
index 914af125a7fa..0ef2373404e5 100644
--- a/arch/mips/configs/mtx1_defconfig
+++ b/arch/mips/configs/mtx1_defconfig
@@ -450,7 +450,6 @@ CONFIG_WDT_MTX1=y
 # CONFIG_VGA_ARB is not set
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_SOUND=m
 CONFIG_SND=m
diff --git a/arch/mips/configs/rs90_defconfig b/arch/mips/configs/rs90_defconfig
index dfbb9fed9a42..4f540bb94628 100644
--- a/arch/mips/configs/rs90_defconfig
+++ b/arch/mips/configs/rs90_defconfig
@@ -97,7 +97,6 @@ CONFIG_DRM_FBDEV_OVERALLOC=300
 CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_DRM_INGENIC=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
-- 
2.17.1

[PATCH 4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
unused")
Cc: Sam Ravnborg 
Signed-off-by: Andrey Zhizhikin 
---
 arch/parisc/configs/generic-64bit_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/parisc/configs/generic-64bit_defconfig 
b/arch/parisc/configs/generic-64bit_defconfig
index 7e2d7026285e..8f81fcbf04c4 100644
--- a/arch/parisc/configs/generic-64bit_defconfig
+++ b/arch/parisc/configs/generic-64bit_defconfig
@@ -191,7 +191,6 @@ CONFIG_DRM=y
 CONFIG_DRM_RADEON=y
 CONFIG_FIRMWARE_EDID=y
 CONFIG_FB_MODE_HELPERS=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
 CONFIG_HIDRAW=y
 CONFIG_HID_PID=y
-- 
2.17.1

[PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from all ARM configurations.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
unused")
Cc: Sam Ravnborg 
Signed-off-by: Andrey Zhizhikin 
---
 arch/arm/configs/at91_dt_defconfig| 1 -
 arch/arm/configs/cm_x300_defconfig| 1 -
 arch/arm/configs/colibri_pxa300_defconfig | 1 -
 arch/arm/configs/jornada720_defconfig | 1 -
 arch/arm/configs/magician_defconfig   | 1 -
 arch/arm/configs/mini2440_defconfig   | 1 -
 arch/arm/configs/omap2plus_defconfig  | 1 -
 arch/arm/configs/pxa3xx_defconfig | 1 -
 arch/arm/configs/qcom_defconfig   | 1 -
 arch/arm/configs/sama5_defconfig  | 1 -
 arch/arm/configs/sunxi_defconfig  | 1 -
 arch/arm/configs/tegra_defconfig  | 1 -
 arch/arm/configs/u8500_defconfig  | 1 -
 13 files changed, 13 deletions(-)

diff --git a/arch/arm/configs/at91_dt_defconfig 
b/arch/arm/configs/at91_dt_defconfig
index 4a0ba2ae1a25..6e52c9c965e6 100644
--- a/arch/arm/configs/at91_dt_defconfig
+++ b/arch/arm/configs/at91_dt_defconfig
@@ -132,7 +132,6 @@ CONFIG_DRM_ATMEL_HLCDC=y
 CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_FB_ATMEL=y
 CONFIG_BACKLIGHT_ATMEL_LCDC=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_LOGO=y
diff --git a/arch/arm/configs/cm_x300_defconfig 
b/arch/arm/configs/cm_x300_defconfig
index 2f7acde2d921..502a9d870ca4 100644
--- a/arch/arm/configs/cm_x300_defconfig
+++ b/arch/arm/configs/cm_x300_defconfig
@@ -87,7 +87,6 @@ CONFIG_FB=y
 CONFIG_FB_PXA=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_TDO24M=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_DA903X=m
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
diff --git a/arch/arm/configs/colibri_pxa300_defconfig 
b/arch/arm/configs/colibri_pxa300_defconfig
index 0dae3b185284..26e5a67f8e2d 100644
--- a/arch/arm/configs/colibri_pxa300_defconfig
+++ b/arch/arm/configs/colibri_pxa300_defconfig
@@ -34,7 +34,6 @@ CONFIG_FB=y
 CONFIG_FB_PXA=y
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_LOGO=y
diff --git a/arch/arm/configs/jornada720_defconfig 
b/arch/arm/configs/jornada720_defconfig
index 9f079be2b84b..069f60ffdcd8 100644
--- a/arch/arm/configs/jornada720_defconfig
+++ b/arch/arm/configs/jornada720_defconfig
@@ -48,7 +48,6 @@ CONFIG_FB=y
 CONFIG_FB_S1D13XXX=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
diff --git a/arch/arm/configs/magician_defconfig 
b/arch/arm/configs/magician_defconfig
index d2e684f6565a..b4670d42f378 100644
--- a/arch/arm/configs/magician_defconfig
+++ b/arch/arm/configs/magician_defconfig
@@ -95,7 +95,6 @@ CONFIG_FB_PXA_OVERLAY=y
 CONFIG_FB_W100=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
diff --git a/arch/arm/configs/mini2440_defconfig 
b/arch/arm/configs/mini2440_defconfig
index 301f29a1fcc3..898490aaa39e 100644
--- a/arch/arm/configs/mini2440_defconfig
+++ b/arch/arm/configs/mini2440_defconfig
@@ -158,7 +158,6 @@ CONFIG_FB_S3C2410=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_PLATFORM=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
diff --git a/arch/arm/configs/omap2plus_defconfig 
b/arch/arm/configs/omap2plus_defconfig
index de3b7813a1ce..7eae097a75d2 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -388,7 +388,6 @@ CONFIG_FB_TILEBLITTING=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_PLATFORM=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-CONFIG_BACKLIGHT_GENERIC=m
 CONFIG_BACKLIGHT_PWM=m
 CONFIG_BACKLIGHT_PANDORA=m
 CONFIG_BACKLIGHT_GPIO=m
diff --git a/arch/arm/configs/pxa3xx_defconfig 
b/arch/arm/configs/pxa3xx_defconfig
index 06bbc7a59b60..f0c34017f2aa 100644
--- a/arch/arm/configs/pxa3xx_defconfig
+++ b/arch/arm/configs/pxa3xx_defconfig
@@ -74,7 +74,6 @@ CONFIG_FB_PXA=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_TDO24M=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_DA903X=y
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index c882167e1496..d6733e745b80 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -159,7 +159,6 @@ CONFIG_FB=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 # CONFIG_LCD_CLASS_DEVICE is not set

[PATCH 5/5] powerpc/configs: drop unused BACKLIGHT_GENERIC option

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is 
unused")
Cc: Sam Ravnborg 
Signed-off-by: Andrey Zhizhikin 
---
 arch/powerpc/configs/powernv_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/configs/powernv_defconfig 
b/arch/powerpc/configs/powernv_defconfig
index cf30fc24413b..60a30fffeda0 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -208,7 +208,6 @@ CONFIG_FB_MATROX_G=y
 CONFIG_FB_RADEON=m
 CONFIG_FB_IBM_GXT4500=m
 CONFIG_LCD_PLATFORM=m
-CONFIG_BACKLIGHT_GENERIC=m
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_LOGO=y
 CONFIG_HID_A4TECH=m
-- 
2.17.1

RE: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option

2020-11-30 Thread ZHIZHIKIN Andrey

Hello Krzysztof,

> -Original Message-
> From: Krzysztof Kozlowski 
> Sent: Monday, November 30, 2020 7:52 PM
> To: ZHIZHIKIN Andrey 
> Cc: li...@armlinux.org.uk; nicolas.fe...@microchip.com;
> alexandre.bell...@bootlin.com; ludovic.desroc...@microchip.com;
> t...@atomide.com; mrip...@kernel.org; w...@csie.org;
> jernej.skra...@siol.net; thierry.red...@gmail.com; jonath...@nvidia.com;
> catalin.mari...@arm.com; w...@kernel.org; tsbog...@alpha.franken.de;
> james.bottom...@hansenpartnership.com; del...@gmx.de;
> m...@ellerman.id.au; b...@kernel.crashing.org; pau...@samba.org;
> lee.jo...@linaro.org; s...@ravnborg.org; emil.l.veli...@gmail.com;
> daniel.thomp...@linaro.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-o...@vger.kernel.org; linux-
> te...@vger.kernel.org; linux-m...@vger.kernel.org; linux-
> par...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Arnd Bergmann
> ; Olof Johansson 
> Subject: Re: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC
> option
> 
> This email is not from Hexagon’s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> On Mon, Nov 30, 2020 at 03:21:33PM +, Andrey Zhizhikin wrote:
> > Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it
> > is
> > unused") removed geenric_bl driver from the tree, together with
> > corresponding config option.
> >
> > Remove BACKLIGHT_GENERIC config item from all ARM configurations.
> >
> > Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it
> > is unused")
> > Cc: Sam Ravnborg 
> > Signed-off-by: Andrey Zhizhikin
> > 
> > ---
> >  arch/arm/configs/at91_dt_defconfig| 1 -
> >  arch/arm/configs/cm_x300_defconfig| 1 -
> >  arch/arm/configs/colibri_pxa300_defconfig | 1 -
> >  arch/arm/configs/jornada720_defconfig | 1 -
> >  arch/arm/configs/magician_defconfig   | 1 -
> >  arch/arm/configs/mini2440_defconfig   | 1 -
> >  arch/arm/configs/omap2plus_defconfig  | 1 -
> >  arch/arm/configs/pxa3xx_defconfig | 1 -
> >  arch/arm/configs/qcom_defconfig   | 1 -
> >  arch/arm/configs/sama5_defconfig  | 1 -
> >  arch/arm/configs/sunxi_defconfig  | 1 -
> >  arch/arm/configs/tegra_defconfig  | 1 -
> >  arch/arm/configs/u8500_defconfig  | 1 -
> >  13 files changed, 13 deletions(-)
> 
> You need to send it to arm-soc maintainers, otherwise no one might feel
> responsible enough to pick it up.

Good point, thanks a lot!

I was not aware of the fact that there is a separate ML that should receive 
patches targeted ARM SOCs. Can you (or anyone else) please share it, so I can 
re-send it there as well?

> Reviewed-by: Krzysztof Kozlowski 
> 
> +CC Arnd and Olof,
> 
> Dear Arnd and Olof,
> 
> Maybe it is worth to add arm-soc entry to the MAINTAINERS file?
> Otherwise how one could get your email address? Not mentioning the
> secret-soc address. :)

"scripts/get_maintainer.pl --no-git-fallback --no-multiline --no-rolestats 
--no-n --separator , " across entire series gave only the list of addresses 
that I've included in the series itself. If I would be given a arm-soc mailing 
list as output of the script - I would've included it as well.

> 
> Best regards,
> Krzysztof

Regards,
Andrey

RE: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option

2020-11-30 Thread ZHIZHIKIN Andrey

Hello Sam,

> -Original Message-
> From: Sam Ravnborg 
> Sent: Monday, November 30, 2020 8:12 PM
> To: ZHIZHIKIN Andrey 
> Cc: li...@armlinux.org.uk; nicolas.fe...@microchip.com;
> alexandre.bell...@bootlin.com; ludovic.desroc...@microchip.com;
> t...@atomide.com; mrip...@kernel.org; w...@csie.org;
> jernej.skra...@siol.net; thierry.red...@gmail.com; jonath...@nvidia.com;
> catalin.mari...@arm.com; w...@kernel.org; tsbog...@alpha.franken.de;
> james.bottom...@hansenpartnership.com; del...@gmx.de;
> m...@ellerman.id.au; b...@kernel.crashing.org; pau...@samba.org;
> lee.jo...@linaro.org; emil.l.veli...@gmail.com;
> daniel.thomp...@linaro.org; k...@kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux-
> o...@vger.kernel.org; linux-te...@vger.kernel.org; linux-
> m...@vger.kernel.org; linux-par...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; Arnd Bergmann ; Olof Johansson
> 
> Subject: Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option
> 
> 
> On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> > Since the removal of generic_bl driver from the source tree in commit
> > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > therefore subject to clean-up from all configuration files.
> >
> > This series introduces patches to address this removal, separated by
> > architectures in the kernel tree.
> >
> > Andrey Zhizhikin (5):
> >   ARM: configs: drop unused BACKLIGHT_GENERIC option
> >   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> >   MIPS: configs: drop unused BACKLIGHT_GENERIC option
> >   parisc: configs: drop unused BACKLIGHT_GENERIC option
> >   powerpc/configs: drop unused BACKLIGHT_GENERIC option
> 
> For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
> savedefconfig / cp defconfig ... run now and then - this will remove all such
> symbols.

This series stretches across several archs, so I would expect that it could've 
been addressed for some of them but would take more time than submitting this 
"point fix" now, targeting one obsolete config option.

I've personally hit it for arm64 on the first place, and looking further into 
the option itself I realized that it is missing from the kernel tree completely 
and decided to fix all archs affected at once.

> 
> If the patches goes in like they are submitted then:
> Acked-by: Sam Ravnborg 
> 
> >
> >  arch/arm/configs/at91_dt_defconfig  | 1 -
> >  arch/arm/configs/cm_x300_defconfig  | 1 -
> >  arch/arm/configs/colibri_pxa300_defconfig   | 1 -
> >  arch/arm/configs/jornada720_defconfig   | 1 -
> >  arch/arm/configs/magician_defconfig | 1 -
> >  arch/arm/configs/mini2440_defconfig | 1 -
> >  arch/arm/configs/omap2plus_defconfig| 1 -
> >  arch/arm/configs/pxa3xx_defconfig   | 1 -
> >  arch/arm/configs/qcom_defconfig | 1 -
> >  arch/arm/configs/sama5_defconfig| 1 -
> >  arch/arm/configs/sunxi_defconfig| 1 -
> >  arch/arm/configs/tegra_defconfig| 1 -
> >  arch/arm/configs/u8500_defconfig| 1 -
> >  arch/arm64/configs/defconfig| 1 -
> >  arch/mips/configs/gcw0_defconfig| 1 -
> >  arch/mips/configs/gpr_defconfig | 1 -
> >  arch/mips/configs/lemote2f_defconfig| 1 -
> >  arch/mips/configs/loongson3_defconfig   | 1 -
> >  arch/mips/configs/mtx1_defconfig| 1 -
> >  arch/mips/configs/rs90_defconfig| 1 -
> >  arch/parisc/configs/generic-64bit_defconfig | 1 -
> >  arch/powerpc/configs/powernv_defconfig  | 1 -
> >  22 files changed, 22 deletions(-)
> >
> >
> > base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
> > prerequisite-patch-id: bfd382cf1dc021d20204f10ea9403319c1c32b12
> > prerequisite-patch-id: 5397c0c8648bb3e0b830207ea867138c11c6e644
> > prerequisite-patch-id: a3c284dff5fe6d02828918a886db6a8ed3197e20
> > --
> > 2.17.1

Regards,
Andrey

Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option

2020-11-30 Thread Sam Ravnborg

Hi Krzysztof,
On Mon, Nov 30, 2020 at 10:25:01PM +0200, Krzysztof Kozlowski wrote:
> On Mon, Nov 30, 2020 at 08:11:33PM +0100, Sam Ravnborg wrote:
> > On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> > > Since the removal of generic_bl driver from the source tree in commit
> > > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > > therefore subject to clean-up from all configuration files.
> > > 
> > > This series introduces patches to address this removal, separated by
> > > architectures in the kernel tree.
> > > 
> > > Andrey Zhizhikin (5):
> > >   ARM: configs: drop unused BACKLIGHT_GENERIC option
> > >   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> > >   MIPS: configs: drop unused BACKLIGHT_GENERIC option
> > >   parisc: configs: drop unused BACKLIGHT_GENERIC option
> > >   powerpc/configs: drop unused BACKLIGHT_GENERIC option
> > 
> > For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
> > savedefconfig / cp defconfig ... run now and then - this will remove
> > all such symbols.
> 
> savedefconfig can be tricky because of risk of loosing options:
> 1. it will remove options which became the default or became selected,
> 2. later when the default is changed or selecting option is removed, the
>first option from #1 will not be brought back.
> 
> This was already for example with DEBUG_FS and the conclusion that time
> was - do not run savedefconfig automatically.
> 
> Therefore if some symbol(s) can be safely removed, patch is welcomed.

Thanks for letting me know, I have missed that discussion and
was obviously not aware.
I already acked'ed the patches and hope the soc people will pick them
up.

Sam

Re: [PATCH kernel v4 2/8] genirq/irqdomain: Clean legacy IRQ allocation

Alexey,

On Tue, Nov 24 2020 at 17:17, Alexey Kardashevskiy wrote:
> There are 10 users of __irq_domain_alloc_irqs() and only one - IOAPIC -
> passes realloc==true. There is no obvious reason for handling this
> specific case in the generic code.

There is also no obvious reason for _NOT_ handling it at the core code.

> This splits out __irq_domain_alloc_irqs_data() to make it clear what
> IOAPIC does and makes __irq_domain_alloc_irqs() cleaner.

That's your interpretation of cleaner.

You need to expose __irq_domain_alloc_irqs_data() for that which is a
core only functionality, so it's not cleaner. It's exposing internals
which are not to be exposed.

The right thing to do is to get rid of the legacy allocation of x86
during early_irq_init() which is possible with the recent restructuring
of the interrupt initialization code in x86. That's a cleanup which will
actually remove code and not expose internals just because.

> This should cause no behavioral change.

Should not cause is a pretty weak statement.

You're missing a nasty detail here. Contrary to the normal irqdomain
rules virq 0 _IS_ valid on x86 for historical reasons and that's not
trivial to change.

Thanks,

tglx

Re: [PATCH v8 11/12] mm/vmalloc: Hugepage vmalloc mappings

2020-11-30 Thread Edgecombe, Rick P

On Mon, 2020-11-30 at 12:21 -0800, Rick Edgecombe wrote:
> another option could be to use the changes here:
> https://lore.kernel.org/lkml/20201125092208.12544-4-r...@kernel.org/
> to reset the direct map for a large page range at a time for large 
> vmalloc pages.

Oops, sorry. This wouldn't be so simple because hibernate currently
expects NP direct map pages to be 4k.

Re: [PATCH v2] clk: renesas: r9a06g032: Drop __packed for portability

2020-11-30 Thread Stephen Rothwell

Hi Geert,

On Mon, 30 Nov 2020 09:57:43 +0100 Geert Uytterhoeven  
wrote:
>
> The R9A06G032 clock driver uses an array of packed structures to reduce
> kernel size.  However, this array contains pointers, which are no longer
> aligned naturally, and cannot be relocated on PPC64.  Hence when
> compile-testing this driver on PPC64 with CONFIG_RELOCATABLE=y (e.g.
> PowerPC allyesconfig), the following warnings are produced:
> 
> WARNING: 136 bad relocations
> c0616be3 R_PPC64_UADDR64   .rodata+0x000cf338
> c0616bfe R_PPC64_UADDR64   .rodata+0x000cf370
> ...
> 
> Fix this by dropping the __packed attribute from the r9a06g032_clkdesc
> definition, trading a small size increase for portability.
> 
> This increases the 156-entry clock table by 1 byte per entry, but due to
> the compiler generating more efficient code for unpacked accesses, the
> net size increase is only 76 bytes (gcc 9.3.0 on arm32).
> 
> Reported-by: Stephen Rothwell 
> Fixes: 4c3d88526eba2143 ("clk: renesas: Renesas R9A06G032 clock driver")
> Signed-off-by: Geert Uytterhoeven 
> ---
> v2:
>   - Fix authorship.
> ---
>  drivers/clk/renesas/r9a06g032-clocks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks.

I have added that to my fixes tree until it gets picked up elsewhere.

-- 
Cheers,
Stephen Rothwell


pgpO2hQKtMGxP.pgp
Description: OpenPGP digital signature

Re: [PATCH kernel v4 5/8] genirq: Add free_irq hook for IRQ descriptor and use for mapping disposal

Alexey,

On Tue, Nov 24 2020 at 17:17, Alexey Kardashevskiy wrote:
> We want to make the irq_desc.kobj's release hook free associated resources
> but we do not want to pollute the irqdesc code with domains.

Can you please describe your changelog in factual ways without 'we and I
and want'? See Documentation/process/

> This adds a free_irq hook which is called when the last reference to
> the descriptor is dropped.
>
> The first user is mapped irqs. This potentially can break the existing
> users; however they seem to do the right thing and call dispose once
> per mapping.

Q: How is this supposed to work with CONFIG_SPARSE_IRQ=n?
A: Not at all.

Also 'seem to do the right thing' is from the same quality as 'should
not break stuff'. Either you have done a proper analysis or you did
not. Aside of that how is anyone supposed to decode the subtle wreckage
which is this going to cause if 'seem to do the right thing' turns out
to be wrong?

> -void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
> +static void irq_domain_disassociate(struct irq_domain *domain, unsigned int 
> irq)
>  {
>   struct irq_data *irq_data = irq_get_irq_data(irq);
>   irq_hw_number_t hwirq;
> @@ -582,6 +582,13 @@ void irq_domain_associate_many(struct irq_domain 
> *domain, unsigned int irq_base,
>  }
>  EXPORT_SYMBOL_GPL(irq_domain_associate_many);
>  
> +static void irq_mapped_free_desc(struct irq_desc *desc)

That function name is really misleading and badly chosen. The function
is not about freeing the irq descriptor as the name suggests. It's
called from that code in order to clean up the mapping.

> +{
> + unsigned int virq = desc->irq_data.irq;
> +
> + irq_domain_disassociate(desc->irq_data.domain, virq);
> +}
> +
>  /**
>   * irq_create_direct_mapping() - Allocate an irq for direct mapping
>   * @domain: domain to allocate the irq for or NULL for default domain
> @@ -638,6 +645,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>  {
>   struct device_node *of_node;
>   int virq;
> + struct irq_desc *desc;

This code uses reverse fir tree ordering of variables

struct device_node *of_node;
struct irq_desc *desc;
int virq;

Why? Because it's simpler to read than the vertical camel case above.

Thanks,

tglx

Re: [PATCH kernel v4 5/8] genirq: Add free_irq hook for IRQ descriptor and use for mapping disposal

On Mon, Nov 30 2020 at 23:18, Thomas Gleixner wrote:
> On Tue, Nov 24 2020 at 17:17, Alexey Kardashevskiy wrote:
>> We want to make the irq_desc.kobj's release hook free associated resources
>> but we do not want to pollute the irqdesc code with domains.
>
> Can you please describe your changelog in factual ways without 'we and I
> and want'? See Documentation/process/

And while we are at process. MAINTAINERS clearly says:

IRQ DOMAINS (IRQ NUMBER MAPPING LIBRARY)
T:  git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

IRQ SUBSYSTEM
T:  git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

So why are these patches not applying against that git branch?

Thanks,

tglx

Re: [PATCH kernel v4 6/8] genirq/irqdomain: Move hierarchical IRQ cleanup to kobject_release