All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] gpu: host1x: Store device address to all bufs
From: Mikko Perttunen @ 2016-11-08 17:51 UTC (permalink / raw)
  To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Arto Merilainen,
	Andrew Chew, Mikko Perttunen

From: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Currently job pinning is optimized to handle only the first buffer
using a certain host1x_bo object and all subsequent buffers using
the same host1x_bo are considered done.

In most cases this is correct, however, in case the same host1x_bo
is used in multiple gathers inside the same job, we skip also
storing the device address (physical or iova) to this buffer.

This patch reworks the host1x_job_pin() to store the device address
to all gathers.

Signed-off-by: Andrew Chew <achew-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/host1x/job.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index a91b7c4..92c3df9 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -1,7 +1,7 @@
 /*
  * Tegra host1x Job
  *
- * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2010-2015, NVIDIA Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -539,9 +539,12 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev)
 
 		g->base = job->gather_addr_phys[i];
 
-		for (j = i + 1; j < job->num_gathers; j++)
-			if (job->gathers[j].bo == g->bo)
+		for (j = i + 1; j < job->num_gathers; j++) {
+			if (job->gathers[j].bo == g->bo) {
 				job->gathers[j].handled = true;
+				job->gathers[j].base = g->base;
+			}
+		}
 
 		err = do_relocs(job, g->bo);
 		if (err)
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/4] gpu: host1x: Add locking to syncpt
From: Mikko Perttunen @ 2016-11-08 17:51 UTC (permalink / raw)
  To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Arto Merilainen,
	Mikko Perttunen
In-Reply-To: <20161108175135.32004-1-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

From: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Currently syncpoints are not locked by mutex and this causes races
if we are aggressively freeing and allocating syncpoints.

This patch adds missing mutex protection to syncpoint structures.

Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Shridhar Rasal <srasal-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/host1x/dev.h    |  3 ++-
 drivers/gpu/host1x/syncpt.c | 25 +++++++++++++++++++++----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index 5220510..06dd4f8 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013, NVIDIA Corporation.
+ * Copyright (c) 2012-2015, NVIDIA Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -120,6 +120,7 @@ struct host1x {
 
 	struct host1x_syncpt *nop_sp;
 
+	struct mutex syncpt_mutex;
 	struct mutex chlist_mutex;
 	struct host1x_channel chlist;
 	unsigned long allocated_channels;
diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
index 9558932..f3b04ed 100644
--- a/drivers/gpu/host1x/syncpt.c
+++ b/drivers/gpu/host1x/syncpt.c
@@ -1,7 +1,7 @@
 /*
  * Tegra host1x Syncpoints
  *
- * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2010-2015, NVIDIA Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -61,22 +61,24 @@ static struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
 	struct host1x_syncpt *sp = host->syncpt;
 	char *name;
 
+	mutex_lock(&host->syncpt_mutex);
+
 	for (i = 0; i < host->info->nb_pts && sp->name; i++, sp++)
 		;
 
 	if (i >= host->info->nb_pts)
-		return NULL;
+		goto err_alloc_syncpt;
 
 	if (flags & HOST1X_SYNCPT_HAS_BASE) {
 		sp->base = host1x_syncpt_base_request(host);
 		if (!sp->base)
-			return NULL;
+			goto err_alloc_base;
 	}
 
 	name = kasprintf(GFP_KERNEL, "%02u-%s", sp->id,
 			dev ? dev_name(dev) : NULL);
 	if (!name)
-		return NULL;
+		goto err_alloc_name;
 
 	sp->dev = dev;
 	sp->name = name;
@@ -86,7 +88,17 @@ static struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
 	else
 		sp->client_managed = false;
 
+	mutex_unlock(&host->syncpt_mutex);
 	return sp;
+
+err_alloc_name:
+	host1x_syncpt_base_free(sp->base);
+	sp->base = NULL;
+err_alloc_base:
+	sp = NULL;
+err_alloc_syncpt:
+	mutex_unlock(&host->syncpt_mutex);
+	return NULL;
 }
 
 u32 host1x_syncpt_id(struct host1x_syncpt *sp)
@@ -378,6 +390,7 @@ int host1x_syncpt_init(struct host1x *host)
 	for (i = 0; i < host->info->nb_bases; i++)
 		bases[i].id = i;
 
+	mutex_init(&host->syncpt_mutex);
 	host->syncpt = syncpt;
 	host->bases = bases;
 
@@ -405,12 +418,16 @@ void host1x_syncpt_free(struct host1x_syncpt *sp)
 	if (!sp)
 		return;
 
+	mutex_lock(&sp->host->syncpt_mutex);
+
 	host1x_syncpt_base_free(sp->base);
 	kfree(sp->name);
 	sp->base = NULL;
 	sp->dev = NULL;
 	sp->name = NULL;
 	sp->client_managed = false;
+
+	mutex_unlock(&sp->host->syncpt_mutex);
 }
 EXPORT_SYMBOL(host1x_syncpt_free);
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 3/4] drm/tegra: Support kernel mappings with IOMMU
From: Mikko Perttunen @ 2016-11-08 17:51 UTC (permalink / raw)
  To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Arto Merilainen,
	Mikko Perttunen
In-Reply-To: <20161108175135.32004-1-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

From: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Host1x command buffer patching requires that the buffer object can be
mapped into kernel address space, however, the recent addition of
IOMMU did not account to this requirement. Therefore Host1x engines
cannot be used if IOMMU is enabled.

This patch implements kmap, kunmap, mmap and munmap functions to
host1x bo objects.

Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/gem.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 19bf9cd..2508372 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -2,7 +2,7 @@
  * NVIDIA Tegra DRM GEM helper functions
  *
  * Copyright (C) 2012 Sascha Hauer, Pengutronix
- * Copyright (C) 2013 NVIDIA CORPORATION, All rights reserved.
+ * Copyright (C) 2013-2015 NVIDIA CORPORATION, All rights reserved.
  *
  * Based on the GEM/CMA helpers
  *
@@ -47,23 +47,51 @@ static void *tegra_bo_mmap(struct host1x_bo *bo)
 {
 	struct tegra_bo *obj = host1x_to_tegra_bo(bo);
 
-	return obj->vaddr;
+	if (obj->vaddr)
+		return obj->vaddr;
+	else if (obj->gem.import_attach)
+		return dma_buf_vmap(obj->gem.import_attach->dmabuf);
+	else
+		return vmap(obj->pages, obj->num_pages, VM_MAP,
+			    pgprot_writecombine(PAGE_KERNEL));
 }
 
 static void tegra_bo_munmap(struct host1x_bo *bo, void *addr)
 {
+	struct tegra_bo *obj = host1x_to_tegra_bo(bo);
+
+	if (obj->vaddr)
+		return;
+	else if (obj->gem.import_attach)
+		dma_buf_vunmap(obj->gem.import_attach->dmabuf, addr);
+	else
+		vunmap(addr);
 }
 
 static void *tegra_bo_kmap(struct host1x_bo *bo, unsigned int page)
 {
 	struct tegra_bo *obj = host1x_to_tegra_bo(bo);
 
-	return obj->vaddr + page * PAGE_SIZE;
+	if (obj->vaddr)
+		return obj->vaddr + page * PAGE_SIZE;
+	else if (obj->gem.import_attach)
+		return dma_buf_kmap(obj->gem.import_attach->dmabuf, page);
+	else
+		return vmap(obj->pages + page, 1, VM_MAP,
+			    pgprot_writecombine(PAGE_KERNEL));
 }
 
 static void tegra_bo_kunmap(struct host1x_bo *bo, unsigned int page,
 			    void *addr)
 {
+	struct tegra_bo *obj = host1x_to_tegra_bo(bo);
+
+	if (obj->vaddr)
+		return;
+	else if (obj->gem.import_attach)
+		dma_buf_kunmap(obj->gem.import_attach->dmabuf, page, addr);
+	else
+		vunmap(addr);
 }
 
 static struct host1x_bo *tegra_bo_get(struct host1x_bo *bo)
-- 
2.9.3

^ permalink raw reply related

* [PATCH 4/4] drm/tegra: Set sgt pointer in BO pin
From: Mikko Perttunen @ 2016-11-08 17:51 UTC (permalink / raw)
  To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Mikko Perttunen
In-Reply-To: <20161108175135.32004-1-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Fix tegra_bo_pin to set the parameter sgt pointer.
Host1x job pinning requires the sgt to determine
physical memory addresses of gathers.

Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 2508372..c08e527 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -36,6 +36,8 @@ static dma_addr_t tegra_bo_pin(struct host1x_bo *bo, struct sg_table **sgt)
 {
 	struct tegra_bo *obj = host1x_to_tegra_bo(bo);
 
+	*sgt = obj->sgt;
+
 	return obj->paddr;
 }
 
-- 
2.9.3

^ permalink raw reply related

* [U-Boot] U-Boot overlaps BSS and initrd on arm64
From: Tom Rini @ 2016-11-08 17:51 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <CACT4Y+atGZwcUxmpfg3q6SMuQaHx5hYQ3d45bDsHxJ1ps-20Ew@mail.gmail.com>

On Tue, Nov 08, 2016 at 09:41:13AM -0800, Dmitry Vyukov wrote:
> Hello,
> 
> We've got a boot problem on arm64 devices. Here is boot log:
> https://storage.kernelci.org/mainline/v4.9-rc4/arm64-defconfig+CONFIG_KASAN=y/lab-baylibre-seattle/boot-juno-r2.txt
> https://kernelci.org/boot/id/581ece5a59b514e448f03bd7/
> 
> Here is some debugging that Andrey and Mark did:
> 
> On Tue, Nov 8, 2016 at 2:00 AM, Andrey Ryabinin wrote:
> > I've looked at juno-r2: https://kernelci.org/boot/id/581ece5a59b514e448f03bd7/
> > So we have
> > Dtb address  0x81f00000
> > Load address 0x80000000
> > Which gives us 31Mb for kernel.
> >
> > It says that Kernel image is 24.62 MiB, but that's without BSS.
> > If bss is big enough it might overwrite dtb.
> > And indeed, build details -
> > https://kernelci.org/build/id/581e850959b514e564f03bdc/
> > shows that bss is 8.5 Mb which is enough to overlap with dtb.
> 
> On Tue, Nov 8, 2016 at 3:21 AM, Mark Rutland wrote:
> > FWIW, since v3.17 we've had an image_size field in the arm64 Image
> > header which describes the "real" size of the Image, BSS included. See
> > [1,2].
> > It should be possible to modify U-Boot to use that to automatically
> > place the DTB and initrd at non-clashing locations (or at least to
> > expose the value somehow).
> > I had assumed U-Boot already did that, but it doesn't seem to be the
> > case.

Yes, we've supported the image_size field since v2016.07 and that board
is running v2016.01.  Unfortunately the booting.txt changes that added
the image_size field weren't publicized widely so we didn't see it until
someone else ran into the problem you're describing.

FWIW, it sounds like the Juno board would be a good candidate to be a
board to add testing modern U-Boot on, in kernelci.org.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161108/edc91979/attachment.sig>

^ permalink raw reply

* Re: [PATCH v9 7/7] KVM: x86: virtualize cpuid faulting
From: Thomas Gleixner @ 2016-11-08 17:53 UTC (permalink / raw)
  To: Kyle Huey
  Cc: David Matlack, Robert O'Callahan, Andy Lutomirski,
	Ingo Molnar, H. Peter Anvin, X86 ML, Paolo Bonzini,
	Radim Krčmář, Jeff Dike, Richard Weinberger,
	Alexander Viro, Shuah Khan, Dave Hansen, Borislav Petkov,
	Peter Zijlstra, Boris Ostrovsky, Len Brown, Rafael J. Wysocki,
	Dmitry Safonov, linux-kernel@vger.kernel.org,
	open list:USER-MODE LINUX (UML), open list:USER-MODE LINUX (UML),
	open list:FILESYSTEMS (VFS and infrastructure),
	open list:KERNEL SELFTEST FRAMEWORK, kvm list
In-Reply-To: <CAP045ApKtyuWUtOpiLPnvKqwBBugm6tN+CzPLgEWgaqSx0+BeQ@mail.gmail.com>

On Tue, 8 Nov 2016, Kyle Huey wrote:
> > It will simplify the MSR get/set code, and make it easier to plumb
> > support for new bits in these MSRs.
> 
> I'm inclined to do this for MSR_PLATFORM_INFO but not
> MSR_MISC_FEATURES_ENABLES.  The former actually has other bits, and
> isn't used outside the msr handling code (yet, anyways).
> MSR_MISC_FEATURES_ENABLES doesn't have any other bits (it's actually
> not documented by Intel at all outside of that virtualization paper)
> and after masking bits in cpuid.c or adding a helper function the
> complexity would be a wash at best.

The feature MSR is also used for enabling ring3 MWAIT, which is obviously
not documented either. So there is more stuff coming along....

Thanks,

	tglx



^ permalink raw reply

* [SPDK] Virtual Subsystem Support
From: Kumaraparameshwaran Rathnavel @ 2016-11-08 17:53 UTC (permalink / raw)
  To: spdk

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

Hi All,

I find that 16.08 release does not support virtual subsystem. But the latest code has it and I find that code is getting updated frequently. So do we have any stable code commit that supports Virtual Subsystem so that I don’t need to change my code base frequently. I need the virtual subsystem to be implemented.

Thanking You,
Param

^ permalink raw reply

* [Intel-wired-lan] I am looking for a specific driver - 3.4.8-k
From: Skidmore, Donald C @ 2016-11-08 17:54 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <1478627090.2341.8.camel@intel.com>



> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Tuesday, November 08, 2016 9:45 AM
> To: SKahn at graco.com; intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] I am looking for a specific driver - 3.4.8-k
> 
> On Tue, 2016-11-08 at 08:46 -0600, SKahn at graco.com wrote:
> > I am trying to locate a driver for X520 Dual Port 10 G Ethernet PICe
> > Adapter, this version specifically - Manufacturer = Intel Adapter
> > Driver = 3.4.8-k .
> >
> > This is for a Cisco UCSC system with RedHat Linux 5.8 64 bit
> 
> You should be able to find the version of ixgbe driver you are looking for on
> e1000.sf.net.

Also the "-k" at the end of the version means the driver is an in-kernel driver.  This is further complicated by how RedHat can backport patches into its driver without it reflecting in the driver version number.  So why you can go to source forge to get a similarly versioned driver and it should be close it however will not be exactly the same as was in RedHat Linux 5.8.  It will contain kcompat code and not contain patches than may have be back ported by RedHat, along with other possible differences.  These might not be that important by I thought I would mention it.

Thanks,
-Don <donald.c.skidmore@intel.com>


^ permalink raw reply

* Re: Summary of LPC guest MSI discussion in Santa Fe
From: Will Deacon @ 2016-11-08 17:54 UTC (permalink / raw)
  To: Auger Eric
  Cc: drjones-H+wXaHxf7aLQT0dZR+AlfA,
	christoffer.dall-QSEj5FYQhm4dnm+yROfE0A,
	jason-NLaQJdtUoK4Be96aLqz0jA, kvm-u79uwXL29TY76Z2rM5mHXA,
	marc.zyngier-5wv7dgnIgG8, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
	punit.agrawal-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	pranav.sawargaonkar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
	dwmw-vV1OtcyAfmbQXOPxS62xeg, jcm-H+wXaHxf7aLQT0dZR+AlfA,
	tglx-hfZtesqFncYOwBW4kG4KsQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	eric.auger.pro-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <dae12190-1eb6-20a9-5740-9e5be8bb65fc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
> On 08/11/2016 03:45, Will Deacon wrote:
> > Rather than treat these as separate problems, a better interface is to
> > tell userspace about a set of reserved regions, and have this include
> > the MSI doorbell, irrespective of whether or not it can be remapped.
> > Don suggested that we statically pick an address for the doorbell in a
> > similar way to x86, and have the kernel map it there. We could even pick
> > 0xfee00000. If it conflicts with a reserved region on the platform (due
> > to (4)), then we'd obviously have to (deterministically?) allocate it
> > somewhere else, but probably within the bottom 4G.
> 
> This is tentatively achieved now with
> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
> (http://www.mail-archive.com/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg1264506.html)

Yup, I saw that fly by. Hopefully some of the internals can be reused
with the current thinking on user ABI.

> > The next question is how to tell userspace about all of the reserved
> > regions. Initially, the idea was to extend VFIO, however Alex pointed
> > out a horrible scenario:
> > 
> >   1. QEMU spawns a VM on system 0
> >   2. VM is migrated to system 1
> >   3. QEMU attempts to passthrough a device using PCI hotplug
> > 
> > In this scenario, the guest memory map is chosen at step (1), yet there
> > is no VFIO fd available to determine the reserved regions. Furthermore,
> > the reserved regions may vary between system 0 and system 1. This pretty
> > much rules out using VFIO to determine the reserved regions.Alex suggested
> > that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> > would solve part of the problem, but migration between systems with
> > different memory maps can still cause problems if the reserved regions
> > of the new system conflict with the guest memory map chosen by QEMU.
> 
> 
> OK so I understand we do not want anymore the VFIO chain capability API
> (patch 5 of above series) but we prefer a sysfs approach instead.

Right.

> I understand the sysfs approach which allows the userspace to get the
> info earlier and independently on VFIO. Keeping in mind current QEMU
> virt - which is not the only userspace - will not do much from this info
> until we bring upheavals in virt address space management. So if I am
> not wrong, at the moment the main action to be undertaken is the
> rejection of the PCI hotplug in case we detect a collision.

I don't think so; it should be up to userspace to reject the hotplug.
If userspace doesn't have support for the regions, then that's fine --
you just end up in a situation where the CPU page table maps memory
somewhere that the device can't see. In other words, you'll end up with
spurious DMA failures, but that's exactly what happens with current systems
if you passthrough an overlapping region (Robin demonstrated this on Juno).

Additionally, you can imagine some future support where you can tell the
guest not to use certain regions of its memory for DMA. In this case, you
wouldn't want to refuse the hotplug in the case of overlapping regions.

Really, I think the kernel side just needs to enumerate the fixed reserved
regions, place the doorbell at a fixed address and then advertise these
via sysfs.

> I can respin [1]
> - studying and taking into account Robin's comments about dm_regions
> similarities
> - removing the VFIO capability chain and replacing this by a sysfs API

Ideally, this would be reusable between different SMMU drivers so the sysfs
entries have the same format etc.

> Would that be OK?

Sounds good to me. Are you in a position to prototype something on the qemu
side once we've got kernel-side agreement?

> What about Alex comments who wanted to report the usable memory ranges
> instead of unusable memory ranges?
> 
> Also did you have a chance to discuss the following items:
> 1) the VFIO irq safety assessment

The discussion really focussed on system topology, as opposed to properties
of the doorbell. Regardless of how the device talks to the doorbell, if
the doorbell can't protect against things like MSI spoofing, then it's
unsafe. My opinion is that we shouldn't allow passthrough by default on
systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
cmdline option to VFIO).

A first step would be making all this opt-in, and only supporting GICv3
ITS for now.

> 2) the MSI reserved size computation (is an arbitrary size OK?)

If we fix the base address, we could fix a size too. However, we'd still
need to enumerate the doorbells to check that they fit in the region we
have. If not, then we can warn during boot and treat it the same way as
a resource conflict (that is, reallocate the region in some deterministic
way).

Will

^ permalink raw reply

* Summary of LPC guest MSI discussion in Santa Fe
From: Will Deacon @ 2016-11-08 17:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <dae12190-1eb6-20a9-5740-9e5be8bb65fc@redhat.com>

On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
> On 08/11/2016 03:45, Will Deacon wrote:
> > Rather than treat these as separate problems, a better interface is to
> > tell userspace about a set of reserved regions, and have this include
> > the MSI doorbell, irrespective of whether or not it can be remapped.
> > Don suggested that we statically pick an address for the doorbell in a
> > similar way to x86, and have the kernel map it there. We could even pick
> > 0xfee00000. If it conflicts with a reserved region on the platform (due
> > to (4)), then we'd obviously have to (deterministically?) allocate it
> > somewhere else, but probably within the bottom 4G.
> 
> This is tentatively achieved now with
> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
> (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1264506.html)

Yup, I saw that fly by. Hopefully some of the internals can be reused
with the current thinking on user ABI.

> > The next question is how to tell userspace about all of the reserved
> > regions. Initially, the idea was to extend VFIO, however Alex pointed
> > out a horrible scenario:
> > 
> >   1. QEMU spawns a VM on system 0
> >   2. VM is migrated to system 1
> >   3. QEMU attempts to passthrough a device using PCI hotplug
> > 
> > In this scenario, the guest memory map is chosen at step (1), yet there
> > is no VFIO fd available to determine the reserved regions. Furthermore,
> > the reserved regions may vary between system 0 and system 1. This pretty
> > much rules out using VFIO to determine the reserved regions.Alex suggested
> > that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> > would solve part of the problem, but migration between systems with
> > different memory maps can still cause problems if the reserved regions
> > of the new system conflict with the guest memory map chosen by QEMU.
> 
> 
> OK so I understand we do not want anymore the VFIO chain capability API
> (patch 5 of above series) but we prefer a sysfs approach instead.

Right.

> I understand the sysfs approach which allows the userspace to get the
> info earlier and independently on VFIO. Keeping in mind current QEMU
> virt - which is not the only userspace - will not do much from this info
> until we bring upheavals in virt address space management. So if I am
> not wrong, at the moment the main action to be undertaken is the
> rejection of the PCI hotplug in case we detect a collision.

I don't think so; it should be up to userspace to reject the hotplug.
If userspace doesn't have support for the regions, then that's fine --
you just end up in a situation where the CPU page table maps memory
somewhere that the device can't see. In other words, you'll end up with
spurious DMA failures, but that's exactly what happens with current systems
if you passthrough an overlapping region (Robin demonstrated this on Juno).

Additionally, you can imagine some future support where you can tell the
guest not to use certain regions of its memory for DMA. In this case, you
wouldn't want to refuse the hotplug in the case of overlapping regions.

Really, I think the kernel side just needs to enumerate the fixed reserved
regions, place the doorbell at a fixed address and then advertise these
via sysfs.

> I can respin [1]
> - studying and taking into account Robin's comments about dm_regions
> similarities
> - removing the VFIO capability chain and replacing this by a sysfs API

Ideally, this would be reusable between different SMMU drivers so the sysfs
entries have the same format etc.

> Would that be OK?

Sounds good to me. Are you in a position to prototype something on the qemu
side once we've got kernel-side agreement?

> What about Alex comments who wanted to report the usable memory ranges
> instead of unusable memory ranges?
> 
> Also did you have a chance to discuss the following items:
> 1) the VFIO irq safety assessment

The discussion really focussed on system topology, as opposed to properties
of the doorbell. Regardless of how the device talks to the doorbell, if
the doorbell can't protect against things like MSI spoofing, then it's
unsafe. My opinion is that we shouldn't allow passthrough by default on
systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
cmdline option to VFIO).

A first step would be making all this opt-in, and only supporting GICv3
ITS for now.

> 2) the MSI reserved size computation (is an arbitrary size OK?)

If we fix the base address, we could fix a size too. However, we'd still
need to enumerate the doorbells to check that they fit in the region we
have. If not, then we can warn during boot and treat it the same way as
a resource conflict (that is, reallocate the region in some deterministic
way).

Will

^ permalink raw reply

* Question regarding PWM types in AST2500
From: Jaghathiswari Rankappagounder Natarajan @ 2016-11-08 17:54 UTC (permalink / raw)
  To: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

There are 3 types of PWM - type M, type N, type O given in aspeed

ast2500 datasheet(page 671). From this link I see that there are 3 types of

PWM : https://en.wikipedia.org/wiki/Pulse-width_modulation#Types

How to find which type is related to which definition? In the data sheet
they
have also given rising and falling bit. If the type is not fixed for a

particular definition, do we need to use these two bits to make the pwm
type
behave as per the wiki definition.

Thanks,
Jagha

[-- Attachment #2: Type: text/html, Size: 764 bytes --]

^ permalink raw reply

* [PATCH 1/2] drivers: psci: PSCI checker module
From: Lorenzo Pieralisi @ 2016-11-08 17:55 UTC (permalink / raw)
  To: linux-arm-kernel

From: Kevin Brodsky <kevin.brodsky@arm.com>

On arm and arm64, PSCI is one of the possible firmware interfaces
used for power management. This includes both turning CPUs on and off,
and suspending them (entering idle states).

This patch adds a PSCI checker module that enables basic testing of
PSCI operations during startup. There are two main tests: CPU
hotplugging and suspending.

In the hotplug tests, the hotplug API is used to turn off and on again
all CPUs in the system, and then all CPUs in each cluster, checking
the consistency of the return codes.

In the suspend tests, a high-priority thread is created on each core
and uses low-level cpuidle functionalities to enter suspend, in all
the possible states and multiple times. This should allow a maximum
number of CPUs to enter the same sleep state at the same or slightly
different time.

In essence, the suspend tests use a principle similar to that of the
intel_powerclamp driver (drivers/thermal/intel_powerclamp.c), but the
threads are only kept for the duration of the test (they are already
gone when userspace is started) and it does not require to stop/start
the tick.

While in theory power management PSCI functions (CPU_{ON,OFF,SUSPEND})
could be directly called, this proved too difficult as it would imply
the duplication of all the logic used by the kernel to allow for a
clean shutdown/bringup/suspend of the CPU (the deepest sleep states
implying potentially the shutdown of the CPU).

Note that this file cannot be compiled as a loadable module, since it
uses a number of non-exported identifiers (essentially for
PSCI-specific checks and direct use of cpuidle) and relies on the
absence of userspace to avoid races when calling hotplug and cpuidle
functions.

For now at least, CONFIG_PSCI_CHECKER is mutually exclusive with
CONFIG_TORTURE_TEST, because torture tests may also use hotplug and
cause false positives in the hotplug tests.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> [torture test config]
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
[lpieralisi: added cpuidle locking, reworded commit log/kconfig entry]
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/firmware/Kconfig        |  11 +
 drivers/firmware/Makefile       |   1 +
 drivers/firmware/psci_checker.c | 490 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 502 insertions(+)
 create mode 100644 drivers/firmware/psci_checker.c

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index bca172d..f358ec8 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -8,6 +8,17 @@ menu "Firmware Drivers"
 config ARM_PSCI_FW
 	bool
 
+config ARM_PSCI_CHECKER
+	bool "ARM PSCI checker"
+	depends on ARM_PSCI_FW && HOTPLUG_CPU && !TORTURE_TEST
+	help
+	  Run the PSCI checker during startup. This checks that hotplug and
+	  suspend operations work correctly when using PSCI.
+
+	  The torture tests may interfere with the PSCI checker by turning CPUs
+	  on and off through hotplug, so for now torture tests and PSCI checker
+	  are mutually exclusive.
+
 config ARM_SCPI_PROTOCOL
 	tristate "ARM System Control and Power Interface (SCPI) Message Protocol"
 	depends on MAILBOX
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 898ac41..cfeb7ad 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -2,6 +2,7 @@
 # Makefile for the linux kernel.
 #
 obj-$(CONFIG_ARM_PSCI_FW)	+= psci.o
+obj-$(CONFIG_ARM_PSCI_CHECKER)	+= psci_checker.o
 obj-$(CONFIG_ARM_SCPI_PROTOCOL)	+= arm_scpi.o
 obj-$(CONFIG_ARM_SCPI_POWER_DOMAIN) += scpi_pm_domain.o
 obj-$(CONFIG_DMI)		+= dmi_scan.o
diff --git a/drivers/firmware/psci_checker.c b/drivers/firmware/psci_checker.c
new file mode 100644
index 0000000..44bdb78
--- /dev/null
+++ b/drivers/firmware/psci_checker.c
@@ -0,0 +1,490 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Copyright (C) 2016 ARM Limited
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/atomic.h>
+#include <linux/completion.h>
+#include <linux/cpu.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/preempt.h>
+#include <linux/psci.h>
+#include <linux/slab.h>
+#include <linux/tick.h>
+#include <linux/topology.h>
+
+#include <asm/cpuidle.h>
+
+#include <uapi/linux/psci.h>
+
+#define NUM_SUSPEND_CYCLE (10)
+
+static unsigned int nb_available_cpus;
+static int tos_resident_cpu = -1;
+
+static atomic_t nb_active_threads;
+static struct completion suspend_threads_started =
+	COMPLETION_INITIALIZER(suspend_threads_started);
+static struct completion suspend_threads_done =
+	COMPLETION_INITIALIZER(suspend_threads_done);
+
+/*
+ * We assume that PSCI operations are used if they are available. This is not
+ * necessarily true on arm64, since the decision is based on the
+ * "enable-method" property of each CPU in the DT, but given that there is no
+ * arch-specific way to check this, we assume that the DT is sensible.
+ */
+static int psci_ops_check(void)
+{
+	int migrate_type = -1;
+	int cpu;
+
+	if (!(psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend)) {
+		pr_warn("Missing PSCI operations, aborting tests\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (psci_ops.migrate_info_type)
+		migrate_type = psci_ops.migrate_info_type();
+
+	if (migrate_type == PSCI_0_2_TOS_UP_MIGRATE ||
+	    migrate_type == PSCI_0_2_TOS_UP_NO_MIGRATE) {
+		/* There is a UP Trusted OS, find on which core it resides. */
+		for_each_online_cpu(cpu)
+			if (psci_tos_resident_on(cpu)) {
+				tos_resident_cpu = cpu;
+				break;
+			}
+		if (tos_resident_cpu == -1)
+			pr_warn("UP Trusted OS resides on no online CPU\n");
+	}
+
+	return 0;
+}
+
+static int find_clusters(const struct cpumask *cpus,
+			 const struct cpumask **clusters)
+{
+	unsigned int nb = 0;
+	cpumask_var_t tmp;
+
+	if (!alloc_cpumask_var(&tmp, GFP_KERNEL))
+		return -ENOMEM;
+	cpumask_copy(tmp, cpus);
+
+	while (!cpumask_empty(tmp)) {
+		const struct cpumask *cluster =
+			topology_core_cpumask(cpumask_any(tmp));
+
+		clusters[nb++] = cluster;
+		cpumask_andnot(tmp, tmp, cluster);
+	}
+
+	free_cpumask_var(tmp);
+	return nb;
+}
+
+/*
+ * offlined_cpus is a temporary array but passing it as an argument avoids
+ * multiple allocations.
+ */
+static unsigned int down_and_up_cpus(const struct cpumask *cpus,
+				     struct cpumask *offlined_cpus)
+{
+	int cpu;
+	int err = 0;
+
+	cpumask_clear(offlined_cpus);
+
+	/* Try to power down all CPUs in the mask. */
+	for_each_cpu(cpu, cpus) {
+		int ret = cpu_down(cpu);
+
+		/*
+		 * cpu_down() checks the number of online CPUs before the TOS
+		 * resident CPU.
+		 */
+		if (cpumask_weight(offlined_cpus) + 1 == nb_available_cpus) {
+			if (ret != -EBUSY) {
+				pr_err("Unexpected return code %d while trying "
+				       "to power down last online CPU %d\n",
+				       ret, cpu);
+				++err;
+			}
+		} else if (cpu == tos_resident_cpu) {
+			if (ret != -EPERM) {
+				pr_err("Unexpected return code %d while trying "
+				       "to power down TOS resident CPU %d\n",
+				       ret, cpu);
+				++err;
+			}
+		} else if (ret != 0) {
+			pr_err("Error occurred (%d) while trying "
+			       "to power down CPU %d\n", ret, cpu);
+			++err;
+		}
+
+		if (ret == 0)
+			cpumask_set_cpu(cpu, offlined_cpus);
+	}
+
+	/* Try to power up all the CPUs that have been offlined. */
+	for_each_cpu(cpu, offlined_cpus) {
+		int ret = cpu_up(cpu);
+
+		if (ret != 0) {
+			pr_err("Error occurred (%d) while trying "
+			       "to power up CPU %d\n", ret, cpu);
+			++err;
+		} else {
+			cpumask_clear_cpu(cpu, offlined_cpus);
+		}
+	}
+
+	/*
+	 * Something went bad at some point and some CPUs could not be turned
+	 * back on.
+	 */
+	WARN_ON(!cpumask_empty(offlined_cpus) ||
+		num_online_cpus() != nb_available_cpus);
+
+	return err;
+}
+
+static int hotplug_tests(void)
+{
+	int err;
+	cpumask_var_t offlined_cpus;
+	int i, nb_cluster;
+	const struct cpumask **clusters;
+	char *page_buf;
+
+	err = -ENOMEM;
+	if (!alloc_cpumask_var(&offlined_cpus, GFP_KERNEL))
+		return err;
+	/* We may have up to nb_available_cpus clusters. */
+	clusters = kmalloc_array(nb_available_cpus, sizeof(*clusters),
+				 GFP_KERNEL);
+	if (!clusters)
+		goto out_free_cpus;
+	page_buf = (char *)__get_free_page(GFP_KERNEL);
+	if (!page_buf)
+		goto out_free_clusters;
+
+	err = 0;
+	nb_cluster = find_clusters(cpu_online_mask, clusters);
+
+	/*
+	 * Of course the last CPU cannot be powered down and cpu_down() should
+	 * refuse doing that.
+	 */
+	pr_info("Trying to turn off and on again all CPUs\n");
+	err += down_and_up_cpus(cpu_online_mask, offlined_cpus);
+
+	/*
+	 * Take down CPUs by cluster this time. When the last CPU is turned
+	 * off, the cluster itself should shut down.
+	 */
+	for (i = 0; i < nb_cluster; ++i) {
+		int cluster_id =
+			topology_physical_package_id(cpumask_any(clusters[i]));
+		ssize_t len = cpumap_print_to_pagebuf(true, page_buf,
+						      clusters[i]);
+		/* Remove trailing newline. */
+		page_buf[len - 1] = '\0';
+		pr_info("Trying to turn off and on again cluster %d "
+			"(CPUs %s)\n", cluster_id, page_buf);
+		err += down_and_up_cpus(clusters[i], offlined_cpus);
+	}
+
+	free_page((unsigned long)page_buf);
+out_free_clusters:
+	kfree(clusters);
+out_free_cpus:
+	free_cpumask_var(offlined_cpus);
+	return err;
+}
+
+static void dummy_callback(unsigned long ignored) {}
+
+static int suspend_cpu(int index, bool broadcast)
+{
+	int ret;
+
+	arch_cpu_idle_enter();
+
+	if (broadcast) {
+		/*
+		 * The local timer will be shut down, we need to enter tick
+		 * broadcast.
+		 */
+		ret = tick_broadcast_enter();
+		if (ret) {
+			/*
+			 * In the absence of hardware broadcast mechanism,
+			 * this CPU might be used to broadcast wakeups, which
+			 * may be why entering tick broadcast has failed.
+			 * There is little the kernel can do to work around
+			 * that, so enter WFI instead (idle state 0).
+			 */
+			cpu_do_idle();
+			ret = 0;
+			goto out_arch_exit;
+		}
+	}
+
+	/*
+	 * Replicate the common ARM cpuidle enter function
+	 * (arm_enter_idle_state).
+	 */
+	ret = CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, index);
+
+	if (broadcast)
+		tick_broadcast_exit();
+
+out_arch_exit:
+	arch_cpu_idle_exit();
+
+	return ret;
+}
+
+static int suspend_test_thread(void *arg)
+{
+	int cpu = (long)arg;
+	int i, nb_suspend = 0, nb_shallow_sleep = 0, nb_err = 0;
+	struct sched_param sched_priority = { .sched_priority = MAX_RT_PRIO-1 };
+	struct cpuidle_device *dev;
+	struct cpuidle_driver *drv;
+	/* No need for an actual callback, we just want to wake up the CPU. */
+	struct timer_list wakeup_timer =
+		TIMER_INITIALIZER(dummy_callback, 0, 0);
+
+	/* Wait for the main thread to give the start signal. */
+	wait_for_completion(&suspend_threads_started);
+
+	/* Set maximum priority to preempt all other threads on this CPU. */
+	if (sched_setscheduler_nocheck(current, SCHED_FIFO, &sched_priority))
+		pr_warn("Failed to set suspend thread scheduler on CPU %d\n",
+			cpu);
+
+	dev = this_cpu_read(cpuidle_devices);
+	drv = cpuidle_get_cpu_driver(dev);
+
+	pr_info("CPU %d entering suspend cycles, states 1 through %d\n",
+		cpu, drv->state_count - 1);
+
+	for (i = 0; i < NUM_SUSPEND_CYCLE; ++i) {
+		int index;
+		/*
+		 * Test all possible states, except 0 (which is usually WFI and
+		 * doesn't use PSCI).
+		 */
+		for (index = 1; index < drv->state_count; ++index) {
+			struct cpuidle_state *state = &drv->states[index];
+			bool broadcast = state->flags & CPUIDLE_FLAG_TIMER_STOP;
+			int ret;
+
+			/*
+			 * Set the timer to wake this CPU up in some time (which
+			 * should be largely sufficient for entering suspend).
+			 * If the local tick is disabled when entering suspend,
+			 * suspend_cpu() takes care of switching to a broadcast
+			 * tick, so the timer will still wake us up.
+			 */
+			mod_timer(&wakeup_timer, jiffies +
+				  usecs_to_jiffies(state->target_residency));
+
+			/* IRQs must be disabled during suspend operations. */
+			local_irq_disable();
+
+			ret = suspend_cpu(index, broadcast);
+
+			/*
+			 * We have woken up. Re-enable IRQs to handle any
+			 * pending interrupt, do not wait until the end of the
+			 * loop.
+			 */
+			local_irq_enable();
+
+			if (ret == index) {
+				++nb_suspend;
+			} else if (ret >= 0) {
+				/* We did not enter the expected state. */
+				++nb_shallow_sleep;
+			} else {
+				pr_err("Failed to suspend CPU %d: error %d "
+				       "(requested state %d, cycle %d)\n",
+				       cpu, ret, index, i);
+				++nb_err;
+			}
+		}
+	}
+
+	/*
+	 * Disable the timer to make sure that the timer will not trigger
+	 * later.
+	 */
+	del_timer(&wakeup_timer);
+
+	if (atomic_dec_return_relaxed(&nb_active_threads) == 0)
+		complete(&suspend_threads_done);
+
+	/* Give up on RT scheduling and wait for termination. */
+	sched_priority.sched_priority = 0;
+	if (sched_setscheduler_nocheck(current, SCHED_NORMAL, &sched_priority))
+		pr_warn("Failed to set suspend thread scheduler on CPU %d\n",
+			cpu);
+	for (;;) {
+		/* Needs to be set first to avoid missing a wakeup. */
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (kthread_should_stop()) {
+			__set_current_state(TASK_RUNNING);
+			break;
+		}
+		schedule();
+	}
+
+	pr_info("CPU %d suspend test results: success %d, shallow states %d, errors %d\n",
+		cpu, nb_suspend, nb_shallow_sleep, nb_err);
+
+	return nb_err;
+}
+
+static int suspend_tests(void)
+{
+	int i, cpu, err = 0;
+	struct task_struct **threads;
+	int nb_threads = 0;
+
+	threads = kmalloc_array(nb_available_cpus, sizeof(*threads),
+				GFP_KERNEL);
+	if (!threads)
+		return -ENOMEM;
+
+	/*
+	 * Stop cpuidle to prevent the idle tasks from entering a deep sleep
+	 * mode, as it might interfere with the suspend threads on other CPUs.
+	 * This does not prevent the suspend threads from using cpuidle (only
+	 * the idle tasks check this status). Take the idle lock so that
+	 * the cpuidle driver and device look-up can be carried out safely.
+	 */
+	cpuidle_pause_and_lock();
+
+	for_each_online_cpu(cpu) {
+		struct task_struct *thread;
+		/* Check that cpuidle is available on that CPU. */
+		struct cpuidle_device *dev = per_cpu(cpuidle_devices, cpu);
+		struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
+
+		if (!dev || !drv) {
+			pr_warn("cpuidle not available on CPU %d, ignoring\n",
+				cpu);
+			continue;
+		}
+
+		thread = kthread_create_on_cpu(suspend_test_thread,
+					       (void *)(long)cpu, cpu,
+					       "psci_suspend_test");
+		if (IS_ERR(thread))
+			pr_err("Failed to create kthread on CPU %d\n", cpu);
+		else
+			threads[nb_threads++] = thread;
+	}
+
+	if (nb_threads < 1) {
+		err = -ENODEV;
+		goto out;
+	}
+
+	atomic_set(&nb_active_threads, nb_threads);
+
+	/*
+	 * Wake up the suspend threads. To avoid the main thread being preempted
+	 * before all the threads have been unparked, the suspend threads will
+	 * wait for the completion of suspend_threads_started.
+	 */
+	for (i = 0; i < nb_threads; ++i)
+		wake_up_process(threads[i]);
+	complete_all(&suspend_threads_started);
+
+	wait_for_completion(&suspend_threads_done);
+
+
+	/* Stop and destroy all threads, get return status. */
+	for (i = 0; i < nb_threads; ++i)
+		err += kthread_stop(threads[i]);
+ out:
+	cpuidle_resume_and_unlock();
+	kfree(threads);
+	return err;
+}
+
+static int __init psci_checker(void)
+{
+	int ret;
+
+	/*
+	 * Since we're in an initcall, we assume that all the CPUs that all
+	 * CPUs that can be onlined have been onlined.
+	 *
+	 * The tests assume that hotplug is enabled but nobody else is using it,
+	 * otherwise the results will be unpredictable. However, since there
+	 * is no userspace yet in initcalls, that should be fine, as long as
+	 * no torture test is running@the same time (see Kconfig).
+	 */
+	nb_available_cpus = num_online_cpus();
+
+	/* Check PSCI operations are set up and working. */
+	ret = psci_ops_check();
+	if (ret)
+		return ret;
+
+	pr_info("PSCI checker started using %u CPUs\n", nb_available_cpus);
+
+	pr_info("Starting hotplug tests\n");
+	ret = hotplug_tests();
+	if (ret == 0)
+		pr_info("Hotplug tests passed OK\n");
+	else if (ret > 0)
+		pr_err("%d error(s) encountered in hotplug tests\n", ret);
+	else {
+		pr_err("Out of memory\n");
+		return ret;
+	}
+
+	pr_info("Starting suspend tests (%d cycles per state)\n",
+		NUM_SUSPEND_CYCLE);
+	ret = suspend_tests();
+	if (ret == 0)
+		pr_info("Suspend tests passed OK\n");
+	else if (ret > 0)
+		pr_err("%d error(s) encountered in suspend tests\n", ret);
+	else {
+		switch (ret) {
+		case -ENOMEM:
+			pr_err("Out of memory\n");
+			break;
+		case -ENODEV:
+			pr_warn("Could not start suspend tests on any CPU\n");
+			break;
+		}
+	}
+
+	pr_info("PSCI checker completed\n");
+	return ret < 0 ? ret : 0;
+}
+late_initcall(psci_checker);
-- 
2.10.0

^ permalink raw reply related

* [PATCH 2/2] drivers: psci: Allow PSCI node to be disabled
From: Lorenzo Pieralisi @ 2016-11-08 17:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108175547.31146-1-lorenzo.pieralisi@arm.com>

From: Thierry Reding <treding@nvidia.com>

Allow disabling PSCI support (mostly for testing purposes) by setting
the status property to "disabled". This makes the node behave in much
the same way as proper device nodes.

Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
 drivers/firmware/psci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 8263429..6c60a50 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -630,7 +630,7 @@ int __init psci_dt_init(void)
 
 	np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
 
-	if (!np)
+	if (!np || !of_device_is_available(np))
 		return -ENODEV;
 
 	init_fn = (psci_initcall_t)matched_np->data;
-- 
2.10.0

^ permalink raw reply related

* Re: Question about 2 gp8psk patches I noticed, and possible bug.
From: Mauro Carvalho Chehab @ 2016-11-08 17:55 UTC (permalink / raw)
  To: VDR User; +Cc: linux-media
In-Reply-To: <CAA7C2qjXSkmmCB=zc7Y-Btpwzm_B=_ok0t6qMRuCy+gfrEhcMw@mail.gmail.com>

Em Sat, 5 Nov 2016 19:24:58 -0700
VDR User <user.vdr@gmail.com> escreveu:

> I have
> several different Genpix devices that use the gp8psk driver and in all
> cases the following happens when I unload it:
> 
> [494896.234616] usbcore: deregistering interface driver dvb_usb_gp8psk
> [494896.234712] ------------[ cut here ]------------
> [494896.234719] WARNING: CPU: 1 PID: 28102 at kernel/module.c:1108
> module_put+0x57/0x70
> [494896.234720] Modules linked in: dvb_usb_gp8psk(-) dvb_usb dvb_core
> nvidia_drm(PO) nvidia_modeset(PO) snd_hda_codec_hdmi snd_hda_intel
> snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd soundcore
> nvidia(PO) [last unloaded: rc_core]
> [494896.234732] CPU: 1 PID: 28102 Comm: rmmod Tainted: P        WC O
>  4.8.4-build.1 #1
> [494896.234733] Hardware name: MSI MS-7309/MS-7309, BIOS V1.12 02/23/2009
> [494896.234735]  00000000 c12ba080 00000000 00000000 c103ed6a c1616014
> 00000001 00006dc6
> [494896.234739]  c1615862 00000454 c109e8a7 c109e8a7 00000009 ffffffff
> 00000000 f13f6a10
> [494896.234743]  f5f5a600 c103ee33 00000009 00000000 00000000 c109e8a7
> f80ca4d0 c109f617
> [494896.234746] Call Trace:
> [494896.234753]  [<c12ba080>] ? dump_stack+0x44/0x64
> [494896.234756]  [<c103ed6a>] ? __warn+0xfa/0x120
> [494896.234758]  [<c109e8a7>] ? module_put+0x57/0x70
> [494896.234760]  [<c109e8a7>] ? module_put+0x57/0x70
> [494896.234762]  [<c103ee33>] ? warn_slowpath_null+0x23/0x30
> [494896.234763]  [<c109e8a7>] ? module_put+0x57/0x70
> [494896.234766]  [<f80ca4d0>] ? gp8psk_fe_set_frontend+0x460/0x460
> [dvb_usb_gp8psk]
> [494896.234769]  [<c109f617>] ? symbol_put_addr+0x27/0x50
> [494896.234771]  [<f80bc9ca>] ?
> dvb_usb_adapter_frontend_exit+0x3a/0x70 [dvb_usb]
> [494896.234773]  [<f80bb3bf>] ? dvb_usb_exit+0x2f/0xd0 [dvb_usb]
> [494896.234776]  [<c13d03bc>] ? usb_disable_endpoint+0x7c/0xb0
> [494896.234778]  [<f80bb48a>] ? dvb_usb_device_exit+0x2a/0x50 [dvb_usb]
> [494896.234780]  [<c13d2882>] ? usb_unbind_interface+0x62/0x250
> [494896.234782]  [<c136b514>] ? __pm_runtime_idle+0x44/0x70
> [494896.234785]  [<c13620d8>] ? __device_release_driver+0x78/0x120
> [494896.234787]  [<c1362907>] ? driver_detach+0x87/0x90
> [494896.234789]  [<c1361c48>] ? bus_remove_driver+0x38/0x90
> [494896.234791]  [<c13d1c18>] ? usb_deregister+0x58/0xb0
> [494896.234793]  [<c109fbb0>] ? SyS_delete_module+0x130/0x1f0
> [494896.234796]  [<c1055654>] ? task_work_run+0x64/0x80
> [494896.234798]  [<c1000fa5>] ? exit_to_usermode_loop+0x85/0x90
> [494896.234800]  [<c10013f0>] ? do_fast_syscall_32+0x80/0x130
> [494896.234803]  [<c1549f43>] ? sysenter_past_esp+0x40/0x6a
> [494896.234805] ---[ end trace 6ebc60ef3981792f ]---
> [494896.235890] dvb-usb: Genpix SkyWalker-2 DVB-S receiver
> successfully deinitialized and disconnected.

I suspect that this is due to a race condition at device unregister.
could you please try the enclosed patch?

gp8psk: unregister gp8psk-fe early

Letting gp8psk-fe to unregister late can cause race issues.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>


diff --git a/drivers/media/usb/dvb-usb/gp8psk.c b/drivers/media/usb/dvb-usb/gp8psk.c
index 2829e3082d15..04ea2bbbe5ae 100644
--- a/drivers/media/usb/dvb-usb/gp8psk.c
+++ b/drivers/media/usb/dvb-usb/gp8psk.c
@@ -270,6 +270,32 @@ static int gp8psk_usb_probe(struct usb_interface *intf,
 	return ret;
 }
 
+static void gp8psk_usb_disconnect(struct usb_interface *intf)
+{
+	struct dvb_usb_device *d = usb_get_intfdata(intf);
+	struct dvb_usb_adapter *adap;
+	int i, n;
+
+	/*
+	 * As gsp8psk-fe can call back this driver, in order to do URB
+	 * transfers, we need to manually exit the frontend earlier.
+	 */
+	for (n = 0; n < d->num_adapters_initialized; n++) {
+		adap = &d->adapter[n];
+		i = adap->num_frontends_initialized - 1;
+
+		for (; i >= 0; i--) {
+			if (adap->fe_adap[i].fe != NULL) {
+				dvb_unregister_frontend(adap->fe_adap[i].fe);
+				dvb_frontend_detach(adap->fe_adap[i].fe);
+			}
+		}
+		adap->num_frontends_initialized = 0;
+	}
+
+	dvb_usb_device_exit(intf);
+}
+
 static struct usb_device_id gp8psk_usb_table [] = {
 	    { USB_DEVICE(USB_VID_GENPIX, USB_PID_GENPIX_8PSK_REV_1_COLD) },
 	    { USB_DEVICE(USB_VID_GENPIX, USB_PID_GENPIX_8PSK_REV_1_WARM) },
@@ -338,7 +364,7 @@ static struct dvb_usb_device_properties gp8psk_properties = {
 static struct usb_driver gp8psk_usb_driver = {
 	.name		= "dvb_usb_gp8psk",
 	.probe		= gp8psk_usb_probe,
-	.disconnect = dvb_usb_device_exit,
+	.disconnect	= gp8psk_usb_disconnect,
 	.id_table	= gp8psk_usb_table,
 };
 

^ permalink raw reply related

* Re: Summary of LPC guest MSI discussion in Santa Fe
From: Will Deacon @ 2016-11-08 17:54 UTC (permalink / raw)
  To: Auger Eric
  Cc: Alex Williamson, drjones, jason, kvm, marc.zyngier, benh, joro,
	punit.agrawal, linux-kernel, arnd, diana.craciun, iommu,
	pranav.sawargaonkar, ddutile, linux-arm-kernel, jcm, tglx,
	robin.murphy, dwmw, christoffer.dall, eric.auger.pro
In-Reply-To: <dae12190-1eb6-20a9-5740-9e5be8bb65fc@redhat.com>

On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
> On 08/11/2016 03:45, Will Deacon wrote:
> > Rather than treat these as separate problems, a better interface is to
> > tell userspace about a set of reserved regions, and have this include
> > the MSI doorbell, irrespective of whether or not it can be remapped.
> > Don suggested that we statically pick an address for the doorbell in a
> > similar way to x86, and have the kernel map it there. We could even pick
> > 0xfee00000. If it conflicts with a reserved region on the platform (due
> > to (4)), then we'd obviously have to (deterministically?) allocate it
> > somewhere else, but probably within the bottom 4G.
> 
> This is tentatively achieved now with
> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
> (http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1264506.html)

Yup, I saw that fly by. Hopefully some of the internals can be reused
with the current thinking on user ABI.

> > The next question is how to tell userspace about all of the reserved
> > regions. Initially, the idea was to extend VFIO, however Alex pointed
> > out a horrible scenario:
> > 
> >   1. QEMU spawns a VM on system 0
> >   2. VM is migrated to system 1
> >   3. QEMU attempts to passthrough a device using PCI hotplug
> > 
> > In this scenario, the guest memory map is chosen at step (1), yet there
> > is no VFIO fd available to determine the reserved regions. Furthermore,
> > the reserved regions may vary between system 0 and system 1. This pretty
> > much rules out using VFIO to determine the reserved regions.Alex suggested
> > that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> > would solve part of the problem, but migration between systems with
> > different memory maps can still cause problems if the reserved regions
> > of the new system conflict with the guest memory map chosen by QEMU.
> 
> 
> OK so I understand we do not want anymore the VFIO chain capability API
> (patch 5 of above series) but we prefer a sysfs approach instead.

Right.

> I understand the sysfs approach which allows the userspace to get the
> info earlier and independently on VFIO. Keeping in mind current QEMU
> virt - which is not the only userspace - will not do much from this info
> until we bring upheavals in virt address space management. So if I am
> not wrong, at the moment the main action to be undertaken is the
> rejection of the PCI hotplug in case we detect a collision.

I don't think so; it should be up to userspace to reject the hotplug.
If userspace doesn't have support for the regions, then that's fine --
you just end up in a situation where the CPU page table maps memory
somewhere that the device can't see. In other words, you'll end up with
spurious DMA failures, but that's exactly what happens with current systems
if you passthrough an overlapping region (Robin demonstrated this on Juno).

Additionally, you can imagine some future support where you can tell the
guest not to use certain regions of its memory for DMA. In this case, you
wouldn't want to refuse the hotplug in the case of overlapping regions.

Really, I think the kernel side just needs to enumerate the fixed reserved
regions, place the doorbell at a fixed address and then advertise these
via sysfs.

> I can respin [1]
> - studying and taking into account Robin's comments about dm_regions
> similarities
> - removing the VFIO capability chain and replacing this by a sysfs API

Ideally, this would be reusable between different SMMU drivers so the sysfs
entries have the same format etc.

> Would that be OK?

Sounds good to me. Are you in a position to prototype something on the qemu
side once we've got kernel-side agreement?

> What about Alex comments who wanted to report the usable memory ranges
> instead of unusable memory ranges?
> 
> Also did you have a chance to discuss the following items:
> 1) the VFIO irq safety assessment

The discussion really focussed on system topology, as opposed to properties
of the doorbell. Regardless of how the device talks to the doorbell, if
the doorbell can't protect against things like MSI spoofing, then it's
unsafe. My opinion is that we shouldn't allow passthrough by default on
systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
cmdline option to VFIO).

A first step would be making all this opt-in, and only supporting GICv3
ITS for now.

> 2) the MSI reserved size computation (is an arbitrary size OK?)

If we fix the base address, we could fix a size too. However, we'd still
need to enumerate the doorbells to check that they fit in the region we
have. If not, then we can warn during boot and treat it the same way as
a resource conflict (that is, reallocate the region in some deterministic
way).

Will

^ permalink raw reply

* [PATCH v2] ARC: [plat-eznps] set default baud for early console
From: Noam Camus @ 2016-11-08 13:20 UTC (permalink / raw)
  To: vgupta; +Cc: linux-snps-arc, linux-kernel, Noam Camus

From: Noam Camus <noamca@mellanox.com>

For CONFIG_SERIAL_EARLYCON we need 800MHz for NPS SoC
The early console driver uses BASE_BAUD and not using dtb.

The default of 50MHz is NOT good for NPS SoC.

Signed-off-by: Noam Camus <noamca@mellanox.com>
---
 arch/arc/kernel/devtree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arc/kernel/devtree.c b/arch/arc/kernel/devtree.c
index f1e07c2..3b67f53 100644
--- a/arch/arc/kernel/devtree.c
+++ b/arch/arc/kernel/devtree.c
@@ -31,6 +31,8 @@ static void __init arc_set_early_base_baud(unsigned long dt_root)
 		arc_base_baud = 166666666;	/* Fixed 166.6MHz clk (TB10x) */
 	else if (of_flat_dt_is_compatible(dt_root, "snps,arc-sdp"))
 		arc_base_baud = 33333333;	/* Fixed 33MHz clk (AXS10x) */
+	else if (of_flat_dt_is_compatible(dt_root, "ezchip,arc-nps"))
+		arc_base_baud = 800000000;      /* Fixed 800MHz clk (NPS) */
 	else
 		arc_base_baud = 50000000;	/* Fixed default 50MHz */
 }
-- 
1.7.1

^ permalink raw reply related

* Re: net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
From: Anna Schumaker @ 2016-11-08 17:56 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Jeff Layton, Trond Myklebust, J. Bruce Fields, David S. Miller,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Adamson
In-Reply-To: <20161108174346.GA17545-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 11/08/2016 12:43 PM, Ross Zwisler wrote:
> On Tue, Nov 08, 2016 at 07:42:59AM -0500, Anna Schumaker wrote:
>> On 11/08/2016 07:09 AM, Jeff Layton wrote:
>>> On Tue, 2016-11-08 at 06:53 -0500, Jeff Layton wrote:
>>>> On Mon, 2016-11-07 at 22:42 -0700, Ross Zwisler wrote:
>>>>>
>>>>> I've got a virtual machine that has some NFS mounts, and with a newly compiled
>>>>> kernel based on v4.9-rc3 I see the following warning/info message:
>>>>>
>>>>> [   42.750181] ===============================
>>>>> [   42.750192] [ INFO: suspicious RCU usage. ]
>>>>> [   42.750203] 4.9.0-rc3-00002-g7b6e7de #3 Not tainted
>>>>> [   42.750213] -------------------------------
>>>>> [   42.750225] net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
>>>>> [   42.750235] 
>>>>> [   42.750235] other info that might help us debug this:
>>>>> [   42.750235] 
>>>>> [   42.750246] 
>>>>> [   42.750246] rcu_scheduler_active = 1, debug_locks = 0
>>>>> [   42.750257] 1 lock held by mount.nfs4/6440:
>>>>> [   42.750278]  #0: 
>>>>> [   42.750299]  (
>>>>> [   42.750319] &(&nn->nfs_client_lock)->rlock
>>>>> [   42.750340] ){+.+...}
>>>>> [   42.750362] , at: 
>>>>> [   42.750372] [<ffffffff813012b5>] nfs_get_client+0x105/0x5e0
>>>>> [   42.750383] 
>>>>> [   42.750383] stack backtrace:
>>>>> [   42.750394] CPU: 0 PID: 6440 Comm: mount.nfs4 Not tainted 4.9.0-rc3-00002-g7b6e7de #3
>>>>> [   42.750406] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS PLYDCRB1.MBH.0096.D23.1608240105 08/24/2016
>>>>> [   42.750429]  ffffc9000092fa68 ffffffff8150730f ffff88014ec8da40 0000000000000001
>>>>> [   42.750452]  ffffc9000092fa98 ffffffff810bc3f7 ffff880150b0b228 ffff88015068dbb0
>>>>> [   42.750475]  ffffc9000092fb38 ffff88014fc99180 ffffc9000092fac0 ffffffff81b243e5
>>>>> [   42.750486] Call Trace:
>>>>> [   42.750498]  [<ffffffff8150730f>] dump_stack+0x67/0x98
>>>>> [   42.750511]  [<ffffffff810bc3f7>] lockdep_rcu_suspicious+0xe7/0x120
>>>>> [   42.750524]  [<ffffffff81b243e5>] rpc_clnt_xprt_switch_has_addr+0x115/0x150
>>>>> [   42.750536]  [<ffffffff813013f4>] nfs_get_client+0x244/0x5e0
>>>>> [   42.750549]  [<ffffffff813012ac>] ? nfs_get_client+0xfc/0x5e0
>>>>> [   42.750561]  [<ffffffff813568f8>] nfs4_set_client+0x98/0x130
>>>>> [   42.750574]  [<ffffffff8135872e>] nfs4_create_server+0x13e/0x390
>>>>> [   42.750588]  [<ffffffff8134cd0e>] nfs4_remote_mount+0x2e/0x60
>>>>> [   42.750600]  [<ffffffff811f3a29>] mount_fs+0x39/0x170
>>>>> [   42.750614]  [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
>>>>> [   42.750626]  [<ffffffff8134cbec>] ? nfs_do_root_mount+0x3c/0xc0
>>>>> [   42.750639]  [<ffffffff8134cc36>] nfs_do_root_mount+0x86/0xc0
>>>>> [   42.750652]  [<ffffffff8134d014>] nfs4_try_mount+0x44/0xc0
>>>>> [   42.750664]  [<ffffffff81302097>] ? get_nfs_version+0x27/0x90
>>>>> [   42.750677]  [<ffffffff81310f8c>] nfs_fs_mount+0x4ac/0xd80
>>>>> [   42.750689]  [<ffffffff810bb938>] ? lockdep_init_map+0x88/0x1f0
>>>>> [   42.750701]  [<ffffffff81311ac0>] ? nfs_clone_super+0x130/0x130
>>>>> [   42.750713]  [<ffffffff8130f300>] ? param_set_portnr+0x70/0x70
>>>>> [   42.750726]  [<ffffffff811f3a29>] mount_fs+0x39/0x170
>>>>> [   42.750740]  [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
>>>>> [   42.750752]  [<ffffffff812176f1>] do_mount+0x1f1/0xd10
>>>>> [   42.750765]  [<ffffffff81217441>] ? copy_mount_options+0xa1/0x140
>>>>> [   42.750777]  [<ffffffff81218543>] SyS_mount+0x83/0xd0
>>>>> [   42.750790]  [<ffffffff81002abc>] do_syscall_64+0x5c/0x130
>>>>> [   42.750802]  [<ffffffff81c479a4>] entry_SYSCALL64_slow_path+0x25/0x25
>>>>>
>>>>> This rcu_dereference_check() was introduced by the following commit:
>>>>>
>>>>> commit 39e5d2df959dd4aea81fa33d765d2a5cc67a0512
>>>>> Author: Andy Adamson <andros-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>>>> Date:   Fri Sep 9 09:22:25 2016 -0400
>>>>>
>>>>>     SUNRPC search xprt switch for sockaddr
>>>>>     
>>>>>     Signed-off-by: Andy Adamson <andros-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
>>>>>     Signed-off-by: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+AlfA@public.gmane.org>
>>>>>
>>>>> Thanks,
>>>>> - Ross
>>>>
>>>> Thanks Ross,
>>
>> Hi Ross,
>>
>> Can you try this patch and let me know if it helps:
>>
>> http://git.linux-nfs.org/?p=anna/linux-nfs.git;a=commitdiff;h=bb29dd84333a96f309c6d0f88b285b5b78927058
>>
>> I'm planning on sending it to Linus soon, so it should be in rc5.
> 
> Hi Anna,
> 
> Yep, this patch makes the warning go away in my setup.

Great!  Thanks for testing!

Anna

> 
> Thanks,
> - Ross
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v3 1/6] Track the active utilisation
From: Juri Lelli @ 2016-11-08 17:56 UTC (permalink / raw)
  To: luca abeni
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Claudio Scordino,
	Steven Rostedt
In-Reply-To: <20161101221014.27eb441a@utopia>

On 01/11/16 22:10, Luca Abeni wrote:
> Hi Juri,
> 
> On Tue, 1 Nov 2016 16:45:43 +0000
> Juri Lelli <juri.lelli@arm.com> wrote:
> 
> > Hi,
> > 
> > a few nitpicks on subject and changelog and a couple of questions below.
> > 
> > Subject should be changed to something like
> > 
> >  sched/deadline: track the active utilisation
> Ok; that's easy :)
> I guess a similar change should be applied to the subjects of all the
> other patches, right?
> 

Yep. Subject have usually the form:

 <modified_file(s)>: <short title>

> 
> > 
> > On 24/10/16 16:06, Luca Abeni wrote:
> > > The active utilisation here is defined as the total utilisation of the  
> > 
> > s/The active/Active/
> > s/here//
> > s/of the active/of active/
> Ok; I'll do this in the next revision of the patchset.
> 

Thanks.

> 
> > > active (TASK_RUNNING) tasks queued on a runqueue. Hence, it is increased
> > > when a task wakes up and is decreased when a task blocks.
> > > 
> > > When a task is migrated from CPUi to CPUj, immediately subtract the task's
> > > utilisation from CPUi and add it to CPUj. This mechanism is implemented by
> > > modifying the pull and push functions.
> > > Note: this is not fully correct from the theoretical point of view
> > > (the utilisation should be removed from CPUi only at the 0 lag time),  
> > 
> > a more theoretically sound solution will follow.
> Notice that even the next patch (introducing the "inactive timer") ends up
> migrating the utilisation immediately (on tasks' migration), without waiting
> for the 0-lag time.
> This is because of the reason explained in the following paragraph:
> 

OK, but is still _more_ theoretically sound. :)

> > > but doing the right thing would be _MUCH_ more complex (leaving the
> > > timer armed when the task is on a different CPU... Inactive timers should
> > > be moved from per-task timers to per-runqueue lists of timers! Bah...)  
> > 
> > I'd remove this paragraph above.
> Ok. Re-reading the changelog, I suspect this is not the correct place for this
> comment.
> 
> 
> > > The utilisation tracking mechanism implemented in this commit can be
> > > fixed / improved by decreasing the active utilisation at the so-called
> > > "0-lag time" instead of when the task blocks.  
> > 
> > And maybe this as well, or put it as more information about the "more
> > theoretically sound" solution?
> Ok... I can remove the paragraph, or point to the next commit (which
> implements the more theoretically sound solution). Is such a "forward
> reference" in changelogs ok?
> 

I'd just say that a better solution will follow. The details about why
it's better might be then put in the changelog and as comments in the
code of the next patch.

> [...]
> > > @@ -947,14 +965,19 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
> > >  		return;
> > >  	}
> > >  
> > > +	if (p->on_rq == TASK_ON_RQ_MIGRATING)
> > > +		add_running_bw(&p->dl, &rq->dl);
> > > +
> > >  	/*
> > >  	 * If p is throttled, we do nothing. In fact, if it exhausted
> > >  	 * its budget it needs a replenishment and, since it now is on
> > >  	 * its rq, the bandwidth timer callback (which clearly has not
> > >  	 * run yet) will take care of this.
> > >  	 */
> > > -	if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH))
> > > +	if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH)) {
> > > +		add_running_bw(&p->dl, &rq->dl);  
> > 
> > Don't rememeber if we discussed this already, but do we need to add the bw here
> > even if the task is not actually enqueued until after the replenishment timer
> > fires?
> I think yes... The active utilization does not depend on the fact that the task
> is on the runqueue or not, but depends on the task's state (in GRUB parlance,
> "inactive" vs "active contending"). In other words, even when a task is throttled
> its utilization must be counted in the active utilization.
> 

OK. Could you add a comment about this point please (so that I don't
forget again :)?

> 
> [...]
> > >  	/*
> > >  	 * Since this might be the only -deadline task on the rq,
> > >  	 * this is the right place to try to pull some other one
> > > @@ -1712,6 +1748,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
> > >   */
> > >  static void switched_to_dl(struct rq *rq, struct task_struct *p)
> > >  {
> > > +	add_running_bw(&p->dl, &rq->dl);
> > >  
> > >  	/* If p is not queued we will update its parameters at next wakeup. */
> > >  	if (!task_on_rq_queued(p))  
> > 
> > Don't we also need to remove bw in task_dead_dl()?
> I think task_dead_dl() is invoked after invoking dequeue_task_dl(), which takes care
> of this... Or am I wrong? (I think I explicitly tested this, and modifications to
> task_dead_dl() turned out to be unneeded)
> 

Mmm. You explicitly check that TASK_ON_RQ_MIGRATING or DEQUEUE_SLEEP
(which btw can be actually put together with an or condition), so I
don't think that any of those turn out to be true when the task dies.
Also, AFAIU, do_exit() works on current and the TASK_DEAD case is
handled in finish_task_switch(), so I don't think we are taking care of
the "task is dying" condition.

Peter, does what I'm saying make any sense? :)

I still have to set up things here to test these patches (sorry, I was
travelling), but could you try to create some tasks and that kill them
from another shell to see if the accounting deviates or not? Or did you
already do this test?

Thanks,

- Juri

^ permalink raw reply

* Re: net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
From: Anna Schumaker @ 2016-11-08 17:56 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Jeff Layton, Trond Myklebust, J. Bruce Fields, David S. Miller,
	linux-nfs, netdev, linux-kernel, Andy Adamson
In-Reply-To: <20161108174346.GA17545@linux.intel.com>

On 11/08/2016 12:43 PM, Ross Zwisler wrote:
> On Tue, Nov 08, 2016 at 07:42:59AM -0500, Anna Schumaker wrote:
>> On 11/08/2016 07:09 AM, Jeff Layton wrote:
>>> On Tue, 2016-11-08 at 06:53 -0500, Jeff Layton wrote:
>>>> On Mon, 2016-11-07 at 22:42 -0700, Ross Zwisler wrote:
>>>>>
>>>>> I've got a virtual machine that has some NFS mounts, and with a newly compiled
>>>>> kernel based on v4.9-rc3 I see the following warning/info message:
>>>>>
>>>>> [   42.750181] ===============================
>>>>> [   42.750192] [ INFO: suspicious RCU usage. ]
>>>>> [   42.750203] 4.9.0-rc3-00002-g7b6e7de #3 Not tainted
>>>>> [   42.750213] -------------------------------
>>>>> [   42.750225] net/sunrpc/clnt.c:2773 suspicious rcu_dereference_check() usage!
>>>>> [   42.750235] 
>>>>> [   42.750235] other info that might help us debug this:
>>>>> [   42.750235] 
>>>>> [   42.750246] 
>>>>> [   42.750246] rcu_scheduler_active = 1, debug_locks = 0
>>>>> [   42.750257] 1 lock held by mount.nfs4/6440:
>>>>> [   42.750278]  #0: 
>>>>> [   42.750299]  (
>>>>> [   42.750319] &(&nn->nfs_client_lock)->rlock
>>>>> [   42.750340] ){+.+...}
>>>>> [   42.750362] , at: 
>>>>> [   42.750372] [<ffffffff813012b5>] nfs_get_client+0x105/0x5e0
>>>>> [   42.750383] 
>>>>> [   42.750383] stack backtrace:
>>>>> [   42.750394] CPU: 0 PID: 6440 Comm: mount.nfs4 Not tainted 4.9.0-rc3-00002-g7b6e7de #3
>>>>> [   42.750406] Hardware name: Intel Corporation PURLEY/PURLEY, BIOS PLYDCRB1.MBH.0096.D23.1608240105 08/24/2016
>>>>> [   42.750429]  ffffc9000092fa68 ffffffff8150730f ffff88014ec8da40 0000000000000001
>>>>> [   42.750452]  ffffc9000092fa98 ffffffff810bc3f7 ffff880150b0b228 ffff88015068dbb0
>>>>> [   42.750475]  ffffc9000092fb38 ffff88014fc99180 ffffc9000092fac0 ffffffff81b243e5
>>>>> [   42.750486] Call Trace:
>>>>> [   42.750498]  [<ffffffff8150730f>] dump_stack+0x67/0x98
>>>>> [   42.750511]  [<ffffffff810bc3f7>] lockdep_rcu_suspicious+0xe7/0x120
>>>>> [   42.750524]  [<ffffffff81b243e5>] rpc_clnt_xprt_switch_has_addr+0x115/0x150
>>>>> [   42.750536]  [<ffffffff813013f4>] nfs_get_client+0x244/0x5e0
>>>>> [   42.750549]  [<ffffffff813012ac>] ? nfs_get_client+0xfc/0x5e0
>>>>> [   42.750561]  [<ffffffff813568f8>] nfs4_set_client+0x98/0x130
>>>>> [   42.750574]  [<ffffffff8135872e>] nfs4_create_server+0x13e/0x390
>>>>> [   42.750588]  [<ffffffff8134cd0e>] nfs4_remote_mount+0x2e/0x60
>>>>> [   42.750600]  [<ffffffff811f3a29>] mount_fs+0x39/0x170
>>>>> [   42.750614]  [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
>>>>> [   42.750626]  [<ffffffff8134cbec>] ? nfs_do_root_mount+0x3c/0xc0
>>>>> [   42.750639]  [<ffffffff8134cc36>] nfs_do_root_mount+0x86/0xc0
>>>>> [   42.750652]  [<ffffffff8134d014>] nfs4_try_mount+0x44/0xc0
>>>>> [   42.750664]  [<ffffffff81302097>] ? get_nfs_version+0x27/0x90
>>>>> [   42.750677]  [<ffffffff81310f8c>] nfs_fs_mount+0x4ac/0xd80
>>>>> [   42.750689]  [<ffffffff810bb938>] ? lockdep_init_map+0x88/0x1f0
>>>>> [   42.750701]  [<ffffffff81311ac0>] ? nfs_clone_super+0x130/0x130
>>>>> [   42.750713]  [<ffffffff8130f300>] ? param_set_portnr+0x70/0x70
>>>>> [   42.750726]  [<ffffffff811f3a29>] mount_fs+0x39/0x170
>>>>> [   42.750740]  [<ffffffff81214a0b>] vfs_kern_mount+0x6b/0x150
>>>>> [   42.750752]  [<ffffffff812176f1>] do_mount+0x1f1/0xd10
>>>>> [   42.750765]  [<ffffffff81217441>] ? copy_mount_options+0xa1/0x140
>>>>> [   42.750777]  [<ffffffff81218543>] SyS_mount+0x83/0xd0
>>>>> [   42.750790]  [<ffffffff81002abc>] do_syscall_64+0x5c/0x130
>>>>> [   42.750802]  [<ffffffff81c479a4>] entry_SYSCALL64_slow_path+0x25/0x25
>>>>>
>>>>> This rcu_dereference_check() was introduced by the following commit:
>>>>>
>>>>> commit 39e5d2df959dd4aea81fa33d765d2a5cc67a0512
>>>>> Author: Andy Adamson <andros@netapp.com>
>>>>> Date:   Fri Sep 9 09:22:25 2016 -0400
>>>>>
>>>>>     SUNRPC search xprt switch for sockaddr
>>>>>     
>>>>>     Signed-off-by: Andy Adamson <andros@netapp.com>
>>>>>     Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>>>
>>>>> Thanks,
>>>>> - Ross
>>>>
>>>> Thanks Ross,
>>
>> Hi Ross,
>>
>> Can you try this patch and let me know if it helps:
>>
>> http://git.linux-nfs.org/?p=anna/linux-nfs.git;a=commitdiff;h=bb29dd84333a96f309c6d0f88b285b5b78927058
>>
>> I'm planning on sending it to Linus soon, so it should be in rc5.
> 
> Hi Anna,
> 
> Yep, this patch makes the warning go away in my setup.

Great!  Thanks for testing!

Anna

> 
> Thanks,
> - Ross
> 


^ permalink raw reply

* Re: [PATCH v2] spi: atmel: use managed resource for gpio chip select
From: Alexandre Belloni @ 2016-11-08 17:49 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: linux, Mark Brown, Boris BREZILLON, linux-spi, geert,
	Ludovic Desroches, linux-arm-kernel, linux-kernel,
	Cyrille Pitchen, Wenyou Yang
In-Reply-To: <20161108174852.14311-1-nicolas.ferre@atmel.com>

On 08/11/2016 at 18:48:52 +0100, Nicolas Ferre wrote :
> Use the managed gpio CS pin request so that we avoid having trouble
> in the cleanup code.
> In fact, if module was configured with DT, cleanup code released
> invalid pin.  Since resource wasn't freed, module cannot be reinserted.
> 
> This require to extract the gpio request call from the "setup" function
> and call it in the appropriate probe function.
> 
> Reported-by: Alexander Morozov <linux@meltdown.ru>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>

I think that's fine but I still have that item on my todo list
(discussion in july 2014 with Mark):

---
>> Mark: maybe it would make sense to do devm_gpio_request_one() in
>> of_spi_register_master(), after of_get_named_gpio.

> You need to transition all the drivers doing things manually but yes.
> As I keep saying all the GPIO handling needs to be completely
> refactored.
---

> ---
> v2: fix devm_gpio_request location: the setup code for device was not proper
>     location. Move it to the probe function and add needed DT routines to
>     handle all CS gpio specified.
> 
>  drivers/spi/spi-atmel.c | 50 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 32683a13dd60..a60925614480 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -24,6 +24,7 @@
>  
>  #include <linux/io.h>
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/pm_runtime.h>
>  
> @@ -1204,7 +1205,6 @@ static int atmel_spi_setup(struct spi_device *spi)
>  	u32			csr;
>  	unsigned int		bits = spi->bits_per_word;
>  	unsigned int		npcs_pin;
> -	int			ret;
>  
>  	as = spi_master_get_devdata(spi->master);
>  
> @@ -1247,16 +1247,9 @@ static int atmel_spi_setup(struct spi_device *spi)
>  		if (!asd)
>  			return -ENOMEM;
>  
> -		if (as->use_cs_gpios) {
> -			ret = gpio_request(npcs_pin, dev_name(&spi->dev));
> -			if (ret) {
> -				kfree(asd);
> -				return ret;
> -			}
> -
> +		if (as->use_cs_gpios)
>  			gpio_direction_output(npcs_pin,
>  					      !(spi->mode & SPI_CS_HIGH));
> -		}
>  
>  		asd->npcs_pin = npcs_pin;
>  		spi->controller_state = asd;
> @@ -1471,13 +1464,11 @@ static int atmel_spi_transfer_one_message(struct spi_master *master,
>  static void atmel_spi_cleanup(struct spi_device *spi)
>  {
>  	struct atmel_spi_device	*asd = spi->controller_state;
> -	unsigned		gpio = (unsigned long) spi->controller_data;
>  
>  	if (!asd)
>  		return;
>  
>  	spi->controller_state = NULL;
> -	gpio_free(gpio);
>  	kfree(asd);
>  }
>  
> @@ -1499,6 +1490,39 @@ static void atmel_get_caps(struct atmel_spi *as)
>  }
>  
>  /*-------------------------------------------------------------------------*/
> +static int atmel_spi_gpio_cs(struct platform_device *pdev)
> +{
> +	struct spi_master	*master = platform_get_drvdata(pdev);
> +	struct atmel_spi	*as = spi_master_get_devdata(master);
> +	struct device_node	*np = master->dev.of_node;
> +	int			i;
> +	int			ret = 0;
> +	int			nb = 0;
> +
> +	if (!as->use_cs_gpios)
> +		return 0;
> +
> +	if (!np)
> +		return 0;
> +
> +	nb = of_gpio_named_count(np, "cs-gpios");
> +	for (i = 0; i < nb; i++) {
> +		int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
> +						"cs-gpios", i);
> +
> +			if (cs_gpio == -EPROBE_DEFER)
> +				return cs_gpio;
> +
> +			if (gpio_is_valid(cs_gpio)) {
> +				ret = devm_gpio_request(&pdev->dev, cs_gpio,
> +							dev_name(&pdev->dev));
> +				if (ret)
> +					return ret;
> +			}
> +	}
> +
> +	return 0;
> +}
>  
>  static int atmel_spi_probe(struct platform_device *pdev)
>  {
> @@ -1577,6 +1601,10 @@ static int atmel_spi_probe(struct platform_device *pdev)
>  		master->num_chipselect = 4;
>  	}
>  
> +	ret = atmel_spi_gpio_cs(pdev);
> +	if (ret)
> +		goto out_unmap_regs;
> +
>  	as->use_dma = false;
>  	as->use_pdc = false;
>  	if (as->caps.has_dma_support) {
> -- 
> 2.9.0
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v9 7/7] KVM: x86: virtualize cpuid faulting
From: Thomas Gleixner @ 2016-11-08 17:53 UTC (permalink / raw)
  To: Kyle Huey
  Cc: David Matlack, Robert O'Callahan, Andy Lutomirski,
	Ingo Molnar, H. Peter Anvin, X86 ML, Paolo Bonzini,
	Radim Krčmář, Jeff Dike, Richard Weinberger,
	Alexander Viro, Shuah Khan, Dave Hansen, Borislav Petkov,
	Peter Zijlstra, Boris Ostrovsky, Len Brown, Rafael J. Wysocki,
	Dmitry Safonov
In-Reply-To: <CAP045ApKtyuWUtOpiLPnvKqwBBugm6tN+CzPLgEWgaqSx0+BeQ@mail.gmail.com>

On Tue, 8 Nov 2016, Kyle Huey wrote:
> > It will simplify the MSR get/set code, and make it easier to plumb
> > support for new bits in these MSRs.
> 
> I'm inclined to do this for MSR_PLATFORM_INFO but not
> MSR_MISC_FEATURES_ENABLES.  The former actually has other bits, and
> isn't used outside the msr handling code (yet, anyways).
> MSR_MISC_FEATURES_ENABLES doesn't have any other bits (it's actually
> not documented by Intel at all outside of that virtualization paper)
> and after masking bits in cpuid.c or adding a helper function the
> complexity would be a wash at best.

The feature MSR is also used for enabling ring3 MWAIT, which is obviously
not documented either. So there is more stuff coming along....

Thanks,

	tglx


^ permalink raw reply

* Re: ACPI module-level code (MLC) not working?
From: Peter Wu @ 2016-11-08 17:56 UTC (permalink / raw)
  To: Zheng, Lv
  Cc: Rick Kerkhof, Bartosz Skrzypczak, Moore, Robert,
	linux-acpi@vger.kernel.org
In-Reply-To: <1AE640813FDE7649BE1B193DEA596E886A263E92@SHSMSX101.ccr.corp.intel.com>

On Tue, Nov 08, 2016 at 05:35:07PM +0000, Zheng, Lv wrote:
> Hi,
> 
> > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > Subject: ACPI module-level code (MLC) not working?
> > 
> > Hi Lv,
> > 
> > According to some tests, setting acpi_gbl_parse_table_as_term_list to
> > TRUE does is not effective. The code within the If-block is still not
> > executed early enough or something else is wrong.
> > 
> > Previously Rick had an issue with an Acer Aspire V7-582PG where the dGPU
> > could not be powered off and I demonstrated an isolated test case in
> > http://www.spinics.net/lists/linux-acpi/msg70069.html
> > 
> > In Bartosz's case, the dGPU cannot be powered on (also using nouveau),
> > preventing suspend from working. Situation is as follows (tested with
> > Linux 3.16, 4.8.4, 4.9-rc2, 4.9-rc4):
> > 
> > His Lenovo IdeaPad Z510 laptop (BIOS date 2014) enables power resources
> > and related _PR3 objects under the conditional If(_OSI("Windows 2013")).
> > Both with and without acpi_gbl_parse_table_as_term_list set to TRUE, the
> > module-level code is not loaded properly. Via a SSDT override, it was
> > confirmed that removing the If conditional results in the expected
> > behavior.
> > 
> > Various details are given in https://github.com/Bumblebee-Project/bbswitch/issues/142
> > including lots of dmesg logs (see posts at the bottom).
> > With above MLC flag set (v4.9-rc4): https://pastebin.com/raw/vCEPGezX
> > With SSDT override (v4.9-rc2): https://pastebin.com/raw/3Fsf2VPU
> 
> I checked the post.
> It seems the current working way is: disabling _OSI(Windows 2013) which disables power resources.

That is how Lenovo probably intended to use it (only add Power Resources
when Windows 8 is detected).

> I have several questions related to this issue:
> 1. The following messages are prompt up when "acpi_gbl_parse_table_as_term_list = TRUE":
> [    2.519113] ACPI Error: [\_SB_.PCI0.GFX0.DD02._BCL] Namespace lookup failure, AE_NOT_FOUND (20160831/psargs-359)
> [    2.519121] ACPI Error: Method parse/execution failed [\_SB.PCI0.PEG1.PEGP.DD02._BCL] (Node ffff8802568d3cf8), AE_NOT_FOUND (20160831/psparse-543)
> How was this triggered?

This comes from acpi_video.c, ssdt4.dsl contains a \_SB.PCI0.GFX0.DD02
device with an _ADR method, but no _BCL one. It is not a regression
though, let's ignore this for now.

> 2. I noticed the following statement:
>    So, for some reason Lenovo has decided to give all tables the same name.
>    The ACPI table override functionality matches possible candidates by signature, OEM ID and OEM Revision ID which are all the same.
>    As a result, the wrong SSDT is overridden.
> Could you provide the detail of the tables from the platform?
> What is the reason of doing such kind of craps in BIOS?

I have no idea why Lenovo would do such a silly thing, but that is why
we had to patch tables.c with:

    if (existing_table->checksum != 0xAF) {
        acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
        pr_info("Skipping next table\n");
        goto next_table;
    }

This is of course an unacceptable hard-coded value, but it was needed in
as a quick hack. For a longterm solution, maybe we can name the table
files specially such that additional match conditions can be given. This
is a different issue though.

What would you like to know about the platform? The acpidump is
available at
https://bugs.launchpad.net/lpbugreporter/+bug/752542/+attachment/4773050/+files/LENOVO-20287.tar.gz

ssdt5.dsl is the file of interest, grep for "Windows 2013".

Kind regards,
Peter

> Thanks and best regards
> Lv
> 
> > 
> > If you would like a new bugzilla entry or have some patches to test, you
> > know where to find us :)
> > --
> > Kind regards,
> > Peter Wu
> > https://lekensteyn.nl

^ permalink raw reply

* [PATCH] pci: Only disable MSI/X and enable INTx if shutdown function has been called
From: Prarit Bhargava @ 2016-11-08 17:57 UTC (permalink / raw)
  To: linux-pci
  Cc: Prarit Bhargava, alex.williamson, darcari, mstowe, bhelgaas,
	lukas, keith.busch, mika.westerberg

Bjorn,

We have seen this at Red Hat on various drivers: nouveau, ahci, mei_me, and
pcieport (so far).  Google search for "unhandled irq 16" yields many results
reporting similar behavior during shutdown indicating that this problem is
widespread.  I can cause this to happen on a "stable" system by adding a 3
second delay in pci_device_shutdown() which causes the number of spurious
interrupts to exceed the 100000 limit and display the warning below for the
primarily the nouveau driver, and occasionally for the other mentioned drivers.

A patch for this was proposed and rejected here for being too risky:

https://patchwork.kernel.org/patch/5990701/

I also originally posted a patch to resolve this here:

http://marc.info/?l=linux-pci&m=147705209308588&w=2

and several other patch suggestions were made.  The problem with all of these
solutions is that there is some risk associated with them (kdump, kvm, etc.)
and they are papering over the real issue that the PCI shutdown should not
blindly switch to INTx for all devices.

I am reproposing the original suggested patch.  There is some risk associated
with this but I don't think it is any more or any less than the other patches,
and it seems like the other patches are only applying band-aids to the problem.

[Aside: Lukas Wunner asked why does this always happen on IRQ 16 (even when the
legacy device says IRQ 32 in lspci)?

The PCI irq pins A, B, C, and D are routed according to the ACPI _PRT table for
the device.  _In general_, I have noted a consistent pattern for PCI irq pins
such that

	irq pin A is IRQ 0x10 (16)
	irq pin B is IRQ 0x11 (17)
	irq pin C is IRQ 0x12 (18)
	irq pin D is IRQ 0x13 (19)

Since the device's IRQ is hooked up to pin A we're seeing the unhandled
interrupt on IRQ 16.]

I have tested this on various systems with KVM and kdump (and kdump on
KVM) and didn't see any issues.

NOTE: In my testing this resolves the problem with PCI based serial ports
cutting off their output during shutdown.  Again, this can be tracked to the
PCI shutdown path switching between MSI & INTx independently of the driver.

----8<----

The following unhandled IRQ warning is seen during shutdown:

irq 16: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.2-1.el7_UNSUPPORTED.x86_64 #1
Hardware name: Hewlett-Packard HP Z820 Workstation/158B, BIOS J63 v03.90 06/01/2016
 0000000000000000 ffff88041f803e70 ffffffff81333bd5 ffff88041cb78200
 ffff88041cb7829c ffff88041f803e98 ffffffff810d9465 ffff88041cb78200
 0000000000000000 0000000000000028 ffff88041f803ed0 ffffffff810d97bf
Call Trace:
 <IRQ>  [<ffffffff81333bd5>] dump_stack+0x63/0x8e
 [<ffffffff810d9465>] __report_bad_irq+0x35/0xd0
 [<ffffffff810d97bf>] note_interrupt+0x20f/0x260
 [<ffffffff810d6b35>] handle_irq_event_percpu+0x45/0x60
 [<ffffffff810d6b7c>] handle_irq_event+0x2c/0x50
 [<ffffffff810da31a>] handle_fasteoi_irq+0x8a/0x150
 [<ffffffff8102edfb>] handle_irq+0xab/0x130
 [<ffffffff81082391>] ? _local_bh_enable+0x21/0x50
 [<ffffffff817064ad>] do_IRQ+0x4d/0xd0
 [<ffffffff81704502>] common_interrupt+0x82/0x82
 <EOI>  [<ffffffff815d0181>] ? cpuidle_enter_state+0xc1/0x280
 [<ffffffff815d0174>] ? cpuidle_enter_state+0xb4/0x280
 [<ffffffff815d0377>] cpuidle_enter+0x17/0x20
 [<ffffffff810bf660>] cpu_startup_entry+0x220/0x3a0
 [<ffffffff816f6da7>] rest_init+0x77/0x80
 [<ffffffff81d8e147>] start_kernel+0x495/0x4a2
 [<ffffffff81d8daa0>] ? set_init_arg+0x55/0x55
 [<ffffffff81d8d120>] ? early_idt_handler_array+0x120/0x120
 [<ffffffff81d8d5d6>] x86_64_start_reservations+0x2a/0x2c
 [<ffffffff81d8d715>] x86_64_start_kernel+0x13d/0x14c

pci_device_shutdown() is called on each PCI device, and does

        if (drv && drv->shutdown)
                drv->shutdown(pci_dev);
        pci_msi_shutdown(pci_dev);
        pci_msix_shutdown(pci_dev);

The pci_msi_shutdown() and pci_msix_shutdown() functions both call
pci_intx_for_msi() which enables the INTx interrupt asynchronously of the
driver.

The problem is that the driver may not have a shutdown function and the
device remains active.  The driver continues to operate the PCI device and the
device interrupts to generate INTx.  The driver, however, has not registered a
handler for INTx and the interrupt line remains set which leads to an unhandled
IRQ warning.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: alex.williamson@redhat.com
Cc: darcari@redhat.com
Cc: mstowe@redhat.com
Cc: bhelgaas@google.com
Cc: lukas@wunner.de
Cc: keith.busch@intel.com
Cc: mika.westerberg@linux.intel.com
---
 drivers/pci/pci-driver.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 1ccce1cd6aca..87c35db5a564 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -461,10 +461,11 @@ static void pci_device_shutdown(struct device *dev)
 
 	pm_runtime_resume(dev);
 
-	if (drv && drv->shutdown)
+	if (drv && drv->shutdown) {
 		drv->shutdown(pci_dev);
-	pci_msi_shutdown(pci_dev);
-	pci_msix_shutdown(pci_dev);
+		pci_msi_shutdown(pci_dev);
+		pci_msix_shutdown(pci_dev);
+	}
 
 	/*
 	 * If this is a kexec reboot, turn off Bus Master bit on the
-- 
1.7.9.3


^ permalink raw reply related

* Re: [PATCH] gpio: tegra186: Add support for T186 GPIO
From: Thierry Reding @ 2016-11-08 17:58 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Olof Johansson, Linus Walleij, Suresh Mangipudi, Laxman Dewangan,
	Alexandre Courbot,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <08947785-2e7f-0117-2392-b6b1a774bbb8-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3954 bytes --]

On Tue, Nov 08, 2016 at 09:49:27AM -0700, Stephen Warren wrote:
> On 11/08/2016 08:55 AM, Thierry Reding wrote:
> > On Mon, Nov 07, 2016 at 05:42:33PM -0800, Olof Johansson wrote:
> > > On Mon, Nov 7, 2016 at 5:21 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > > > On Mon, Nov 07, 2016 at 08:53:37AM +0100, Linus Walleij wrote:
> > > > > On Wed, Nov 2, 2016 at 11:48 AM, Suresh Mangipudi <smangipudi@nvidia.com> wrote:
> > > > > 
> > > > > > Add GPIO driver for T186 based platforms.
> > > > > > Adds support for MAIN and AON GPIO's from T186.
> > > > > > 
> > > > > > Signed-off-by: Suresh Mangipudi <smangipudi-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > > > 
> > > > > Stephen/Thierry/Alexandre:
> > > > > Can I get your review on this Tegra thing?
> > > > 
> > > > Can we hold off on this for a bit? I've got my own implementation of
> > > > this in my Tegra186 tree because I thought nobody else was working on
> > > > it. From a brief look they seem mostly similar but there are a couple
> > > > of differences that I need to take a closer look at (and do some more
> > > > intensive testing on).
> > > 
> > > Be careful about discouraging other developers internally from
> > > participating upstream, Thierry.
> > 
> > That was certainly not my intention. I do welcome any contributions,
> > internal or external.
> > 
> > > Please don't become a bottleneck for your company's contributions.
> > > Rewriting stuff on your own isn't a scalable model.
> > 
> > Like I said, I didn't rewrite this, I merely wrote the GPIO driver
> > because there wasn't one at the time and I wasn't aware of anyone else
> > working on one. Waiting for a GPIO driver to emerge would've prevented
> > me from making progress on other fronts.
> 
> One issue here is that there are lots of patches destined for upstream in
> your own personal tree, but they aren't actually upstream yet. I think
> pushing more of your work into linux-next (and further upstream) faster
> would help people out. linux-next and other "standard" repos should be
> easily discoverable by anyway following any upstreaming HOWTOs, but those
> HOWTOs aren't going to mention your personal trees, so patches there
> effectively don't exist. Making the already extant work more discoverable
> will help prevent people duplicating work.

I had assumed that I had properly communicated what the canonical
temporary location for Tegra186 patches would be, but you're right that
it's probably easy to miss, and linux-next would be a more obvious
target.

> Related to this, I've been waiting rather a while for the Tegra186 DT
> binding patches I sent to be applied. I'd love to see them go in ASAP even
> if there's no kernel driver behind them. The bindings have been reviewed,
> ack'd, and they're in use in U-Boot already.

It's true, I've been promising this for weeks now. I'll get around to it
within the week. Please do prod me in case I don't. I promise I won't
get mad =)

> A thought on Tegra186: For a older supported SoCs, we obviously don't want
> to push changes upstream that aren't too well baked. However, for a new SoC
> that doesn't work yet, I'm tend to prioritize getting as much early work
> upstream as fast as possible (to try and unblock people working in other
> areas) over getting those patches perfect first. Release early, release
> often will help unblock people and parallelize work. Pipeline your own work
> rather than batching it all up to release at once.

I'm always hesitant to merge code that isn't functional or tested, but
perhaps I'm being a little overzealous here, and I'm evidently not doing
so great, so let me try to be more aggressive.

> P.S. don't take this email too personally or anything; I'm not trying to be
> complaining/critical or anything like that. It's just a few mild thoughts.

No offense taken, thanks for being constructive about it.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH 0/2] remoteproc: qcom: Venus firmware loader support for msm8996
From: Avaneesh Kumar Dwivedi @ 2016-11-08 17:57 UTC (permalink / raw)
  To: bjorn.andersson; +Cc: linux-arm-msm, Avaneesh Kumar Dwivedi

This Patch Series is based on https://patchwork.kernel.org/patch/9415585/

This patch series add necessary routine and data structure to support standalone
venus firmware load on msm8996. 
patch 1- It bind a private data structure to device structure so that there are minimal code
changes as new version of venus driver is supported.
patch 2- It provide clock initialization and enable/disable functionality.

below is console log on msm8996 platform with above change, this is standalone test log 
without video driver enablement.

[    2.612011]  remoteproc0: soc:vidc_tzpil@0 is available
[    2.616939]  remoteproc0: Note: remoteproc is still under development and considered experimental.
[    2.621963]  remoteproc0: THE BINARY FORMAT IS NOT YET FINALIZED, and backward compatibility isn't yet guaranteed.
[    2.631037] qcom-tz-pil soc:vidc_tzpil@0: Venus rproc probe done
[    2.641463]  remoteproc0: powering up soc:vidc_tzpil@0
[    2.641468]  remoteproc0: Booting fw image venus.mdt, size 6812
[    2.698127]  remoteproc0: remote processor soc:vidc_tzpil@0 is now up

Avaneesh Kumar Dwivedi (2):
  remoteproc: qcom: Private data support for venus rproc driver.
  remoteproc: qcom: Add clock init and enable support if needed.

 .../devicetree/bindings/remoteproc/qcom,venus.txt  |   3 +-
 drivers/remoteproc/qcom_venus_pil.c                | 103 ++++++++++++++++++++-
 2 files changed, 104 insertions(+), 2 deletions(-)

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.