[GIT PULL 0/7] intel_th: Fixes for v5.4

[GIT PULL 0/7] intel_th: Fixes for v5.4

[Alexander Shishkin]

Hi Greg,

These are the fixes I have for the v5.4 cycle. They are 4 fixes for the
buffer management code that went in in the v5.4-rc1 and 2 new PCI IDs.
The IDs are CC'd to stable. All checked with static checkers and Andy
Shevchenko, who was kind enough to look them over. Signed tag at the URL
below. Individual patches follow. Please consider pulling or applying.
Thanks!

The following changes since commit 54ecb8f7028c5eb3d740bb82b0f1d90f2df63c5c:

  Linux 5.4-rc1 (2019-09-30 10:35:40 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm.git intel_th-fixes-for-greg-20191028

for you to fetch changes up to 379815d9f34ef62f7f268a25c96b321396a7cbe5:

  intel_th: pci: Add Jasper Lake PCH support (2019-10-28 08:54:27 +0200)

----------------------------------------------------------------
intel_th: Fixes for v5.4

These are:
 * several fixes for buffer management added in v5.4
 * two new PCI IDs

----------------------------------------------------------------
Alexander Shishkin (4):
      intel_th: gth: Fix the window switching sequence
      intel_th: msu: Fix an uninitialized mutex
      intel_th: pci: Add Comet Lake PCH support
      intel_th: pci: Add Jasper Lake PCH support

Colin Ian King (2):
      intel_th: msu: Fix missing allocation failure check on a kstrndup
      intel_th: msu: Fix overflow in shift of an unsigned int

Wei Yongjun (1):
      intel_th: msu: Fix possible memory leak in mode_store()

 drivers/hwtracing/intel_th/gth.c |  3 +++
 drivers/hwtracing/intel_th/msu.c | 11 ++++++++---
 drivers/hwtracing/intel_th/pci.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 3 deletions(-)
The following changes since commit 54ecb8f7028c5eb3d740bb82b0f1d90f2df63c5c:

  Linux 5.4-rc1 (2019-09-30 10:35:40 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm.git tags/intel_th-fixes-for-greg-20191028

for you to fetch changes up to 379815d9f34ef62f7f268a25c96b321396a7cbe5:

  intel_th: pci: Add Jasper Lake PCH support (2019-10-28 08:54:27 +0200)

----------------------------------------------------------------
intel_th: Fixes for v5.4

These are:
 * several fixes for buffer management added in v5.4
 * two new PCI IDs

----------------------------------------------------------------
Alexander Shishkin (4):
      intel_th: gth: Fix the window switching sequence
      intel_th: msu: Fix an uninitialized mutex
      intel_th: pci: Add Comet Lake PCH support
      intel_th: pci: Add Jasper Lake PCH support

Colin Ian King (2):
      intel_th: msu: Fix missing allocation failure check on a kstrndup
      intel_th: msu: Fix overflow in shift of an unsigned int

Wei Yongjun (1):
      intel_th: msu: Fix possible memory leak in mode_store()

 drivers/hwtracing/intel_th/gth.c |  3 +++
 drivers/hwtracing/intel_th/msu.c | 11 ++++++++---
 drivers/hwtracing/intel_th/pci.c | 10 ++++++++++
 3 files changed, 21 insertions(+), 3 deletions(-)

-- 
2.23.0

[GIT PULL 1/7] intel_th: gth: Fix the window switching sequence

[Alexander Shishkin]

Commit 8116db57cf16 ("intel_th: Add switch triggering support") added
a trigger assertion of the CTS, but forgot to de-assert it at the end
of the sequence. This results in window switches randomly not happening.

Fix that by de-asserting the trigger at the end of the window switch
sequence.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hwtracing/intel_th/gth.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/intel_th/gth.c b/drivers/hwtracing/intel_th/gth.c
index fa9d34af87ac..f72803a02391 100644
--- a/drivers/hwtracing/intel_th/gth.c
+++ b/drivers/hwtracing/intel_th/gth.c
@@ -626,6 +626,9 @@ static void intel_th_gth_switch(struct intel_th_device *thdev,
 	if (!count)
 		dev_dbg(&thdev->dev, "timeout waiting for CTS Trigger\n");
 
+	/* De-assert the trigger */
+	iowrite32(0, gth->base + REG_CTS_CTL);
+
 	intel_th_gth_stop(gth, output, false);
 	intel_th_gth_start(gth, output);
 }
-- 
2.23.0

[GIT PULL 2/7] intel_th: msu: Fix an uninitialized mutex

[Alexander Shishkin]

Commit 615c164da0eb ("intel_th: msu: Introduce buffer interface") added a
mutex that it forgot to initialize, resulting in a lockdep splat.

Fix that by initializing the mutex statically.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hwtracing/intel_th/msu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index fc9f15f36ad4..51021020fa3f 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -164,7 +164,7 @@ struct msc {
 };
 
 static LIST_HEAD(msu_buffer_list);
-static struct mutex msu_buffer_mutex;
+static DEFINE_MUTEX(msu_buffer_mutex);
 
 /**
  * struct msu_buffer_entry - internal MSU buffer bookkeeping
-- 
2.23.0

[GIT PULL 3/7] intel_th: msu: Fix missing allocation failure check on a kstrndup

[Alexander Shishkin]

From: Colin Ian King <colin.king@canonical.com>

Commit 615c164da0eb ("intel_th: msu: Introduce buffer interface") forgot
to add a NULL pointer check for the value returned from kstrdup(), which
will be troublesome if the allocation fails.

Fix that by adding the check.

Addresses-Coverity: ("Dereference null return")
Fixes: 615c164da0eb ("intel_th: msu: Introduce buffer interface")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
[alexander.shishkin: amended the commit message]
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/lkml/20190726120421.9650-1-colin.king@canonical.com/
---
 drivers/hwtracing/intel_th/msu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 51021020fa3f..201a166fdff5 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -1848,6 +1848,9 @@ mode_store(struct device *dev, struct device_attribute *attr, const char *buf,
 		len = cp - buf;
 
 	mode = kstrndup(buf, len, GFP_KERNEL);
+	if (!mode)
+		return -ENOMEM;
+
 	i = match_string(msc_mode, ARRAY_SIZE(msc_mode), mode);
 	if (i >= 0)
 		goto found;
-- 
2.23.0

[GIT PULL 4/7] intel_th: msu: Fix overflow in shift of an unsigned int

[Alexander Shishkin]

From: Colin Ian King <colin.king@canonical.com>

The shift of the unsigned int win->nr_blocks by PAGE_SHIFT may
potentially overflow. Note that the intended return of this shift
is expected to be a size_t however the shift is being performed as
an unsigned int.  Fix this by casting win->nr_blocks to a size_t
before performing the shift.

Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 615c164da0eb ("intel_th: msu: Introduce buffer interface")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/lkml/20190726113151.8967-1-colin.king@canonical.com/
---
 drivers/hwtracing/intel_th/msu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 201a166fdff5..9dc9ae87b5e5 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -327,7 +327,7 @@ static size_t msc_win_total_sz(struct msc_window *win)
 		struct msc_block_desc *bdesc = sg_virt(sg);
 
 		if (msc_block_wrapped(bdesc))
-			return win->nr_blocks << PAGE_SHIFT;
+			return (size_t)win->nr_blocks << PAGE_SHIFT;
 
 		size += msc_total_sz(bdesc);
 		if (msc_block_last_written(bdesc))
-- 
2.23.0

[GIT PULL 5/7] intel_th: msu: Fix possible memory leak in mode_store()

[Alexander Shishkin]

From: Wei Yongjun <weiyongjun1@huawei.com>

'mode' is malloced in mode_store() and should be freed before leaving
from the error handling cases, otherwise it will cause memory leak.

Fixes: 615c164da0eb ("intel_th: msu: Introduce buffer interface")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/lkml/20190801013825.182543-1-weiyongjun1@huawei.com/
---
 drivers/hwtracing/intel_th/msu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 9dc9ae87b5e5..6d240dfae9d9 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -1852,8 +1852,10 @@ mode_store(struct device *dev, struct device_attribute *attr, const char *buf,
 		return -ENOMEM;
 
 	i = match_string(msc_mode, ARRAY_SIZE(msc_mode), mode);
-	if (i >= 0)
+	if (i >= 0) {
+		kfree(mode);
 		goto found;
+	}
 
 	/* Buffer sinks only work with a usable IRQ */
 	if (!msc->do_irq) {
-- 
2.23.0

[GIT PULL 6/7] intel_th: pci: Add Comet Lake PCH support

[Alexander Shishkin]

This adds support for Intel TH on Comet Lake PCH.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index 91dfeba62485..4088e1865b07 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -199,6 +199,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x02a6),
 		.driver_data = (kernel_ulong_t)&intel_th_2x,
 	},
+	{
+		/* Comet Lake PCH */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x06a6),
+		.driver_data = (kernel_ulong_t)&intel_th_2x,
+	},
 	{
 		/* Ice Lake NNPI */
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x45c5),
-- 
2.23.0

[GIT PULL 7/7] intel_th: pci: Add Jasper Lake PCH support

[Alexander Shishkin]

This adds support for Intel TH on Jasper Lake PCH.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index 4088e1865b07..03ca5b1bef9f 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -214,6 +214,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa0a6),
 		.driver_data = (kernel_ulong_t)&intel_th_2x,
 	},
+	{
+		/* Jasper Lake PCH */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4da6),
+		.driver_data = (kernel_ulong_t)&intel_th_2x,
+	},
 	{ 0 },
 };
 
-- 
2.23.0

Re: [GIT PULL 1/7] intel_th: gth: Fix the window switching sequence

[Greg Kroah-Hartman]

On Mon, Oct 28, 2019 at 09:06:45AM +0200, Alexander Shishkin wrote:
> Commit 8116db57cf16 ("intel_th: Add switch triggering support") added
> a trigger assertion of the CTS, but forgot to de-assert it at the end
> of the sequence. This results in window switches randomly not happening.
> 
> Fix that by de-asserting the trigger at the end of the window switch
> sequence.
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/hwtracing/intel_th/gth.c | 3 +++
>  1 file changed, 3 insertions(+)

Shouldn't this have a Fixes: tag and a cc: stable@ in it?

I can add it if you say it's ok to.

thanks,

greg k-h

Re: [GIT PULL 2/7] intel_th: msu: Fix an uninitialized mutex

[Greg Kroah-Hartman]

On Mon, Oct 28, 2019 at 09:06:46AM +0200, Alexander Shishkin wrote:
> Commit 615c164da0eb ("intel_th: msu: Introduce buffer interface") added a
> mutex that it forgot to initialize, resulting in a lockdep splat.
> 
> Fix that by initializing the mutex statically.
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/hwtracing/intel_th/msu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Again, no fixes or stable?

Re: [GIT PULL 1/7] intel_th: gth: Fix the window switching sequence

[Alexander Shishkin]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Mon, Oct 28, 2019 at 09:06:45AM +0200, Alexander Shishkin wrote:
>> Commit 8116db57cf16 ("intel_th: Add switch triggering support") added
>> a trigger assertion of the CTS, but forgot to de-assert it at the end
>> of the sequence. This results in window switches randomly not happening.
>> 
>> Fix that by de-asserting the trigger at the end of the window switch
>> sequence.
>> 
>> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/hwtracing/intel_th/gth.c | 3 +++
>>  1 file changed, 3 insertions(+)
>
> Shouldn't this have a Fixes: tag and a cc: stable@ in it?
>
> I can add it if you say it's ok to.

Fixes: yes, but cc: stable shouldn't be required if this goes into 5.4,
because the buggy commit is in 5.4-rc1.

Thank you,
--
Alex

Re: [GIT PULL 2/7] intel_th: msu: Fix an uninitialized mutex

[Alexander Shishkin]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Mon, Oct 28, 2019 at 09:06:46AM +0200, Alexander Shishkin wrote:
>> Commit 615c164da0eb ("intel_th: msu: Introduce buffer interface") added a
>> mutex that it forgot to initialize, resulting in a lockdep splat.
>> 
>> Fix that by initializing the mutex statically.
>> 
>> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/hwtracing/intel_th/msu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Again, no fixes or stable?

Same as the other one: fixes makes sense, but cc: stable -- not so
much.

Thanks,
--
Alex

Re: [GIT PULL 1/7] intel_th: gth: Fix the window switching sequence

[Greg Kroah-Hartman]

On Mon, Nov 04, 2019 at 08:25:39AM +0200, Alexander Shishkin wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> > On Mon, Oct 28, 2019 at 09:06:45AM +0200, Alexander Shishkin wrote:
> >> Commit 8116db57cf16 ("intel_th: Add switch triggering support") added
> >> a trigger assertion of the CTS, but forgot to de-assert it at the end
> >> of the sequence. This results in window switches randomly not happening.
> >> 
> >> Fix that by de-asserting the trigger at the end of the window switch
> >> sequence.
> >> 
> >> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> ---
> >>  drivers/hwtracing/intel_th/gth.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >
> > Shouldn't this have a Fixes: tag and a cc: stable@ in it?
> >
> > I can add it if you say it's ok to.
> 
> Fixes: yes, but cc: stable shouldn't be required if this goes into 5.4,
> because the buggy commit is in 5.4-rc1.

8116db57cf16 ("intel_th: Add switch triggering support") was in the 5.2
kernel release.

Re: [GIT PULL 1/7] intel_th: gth: Fix the window switching sequence

[Alexander Shishkin]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Mon, Nov 04, 2019 at 08:25:39AM +0200, Alexander Shishkin wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> 
>> > On Mon, Oct 28, 2019 at 09:06:45AM +0200, Alexander Shishkin wrote:
>> >> Commit 8116db57cf16 ("intel_th: Add switch triggering support") added
>> >> a trigger assertion of the CTS, but forgot to de-assert it at the end
>> >> of the sequence. This results in window switches randomly not happening.
>> >> 
>> >> Fix that by de-asserting the trigger at the end of the window switch
>> >> sequence.
>> >> 
>> >> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> >> ---
>> >>  drivers/hwtracing/intel_th/gth.c | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >
>> > Shouldn't this have a Fixes: tag and a cc: stable@ in it?
>> >
>> > I can add it if you say it's ok to.
>> 
>> Fixes: yes, but cc: stable shouldn't be required if this goes into 5.4,
>> because the buggy commit is in 5.4-rc1.
>
> 8116db57cf16 ("intel_th: Add switch triggering support") was in the 5.2
> kernel release.

Ah true. Thank you for checking that. Can you then also add a CC:
stable?

I'll be sure to check these things next time.

Thanks,
--
Alex

Re: [GIT PULL 0/7] intel_th: Fixes for v5.4

[Greg Kroah-Hartman]

On Mon, Oct 28, 2019 at 09:06:44AM +0200, Alexander Shishkin wrote:
> Hi Greg,
> 
> These are the fixes I have for the v5.4 cycle. They are 4 fixes for the
> buffer management code that went in in the v5.4-rc1 and 2 new PCI IDs.
> The IDs are CC'd to stable. All checked with static checkers and Andy
> Shevchenko, who was kind enough to look them over. Signed tag at the URL
> below. Individual patches follow. Please consider pulling or applying.
> Thanks!

Applied them by hand due to the fixes needed for the first two.

thanks,

greg k-h