Re: [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

2018-05-21 Thread Ganapatrao Kulkarni
On Mon, May 21, 2018 at 4:10 PM, Mark Rutland <mark.rutl...@arm.com> wrote:
> On Mon, May 21, 2018 at 11:37:12AM +0100, Mark Rutland wrote:
>> Hi Ganapat,
>>
>>
>> Sorry for the delay in replying; I was away most of last week.
>>
>> On Tue, May 15, 2018 at 04:03:19PM +0530, Ganapatrao Kulkarni wrote:
>> > On Sat, May 5, 2018 at 12:16 AM, Ganapatrao Kulkarni <gklkm...@gmail.com> 
>> > wrote:
>> > > On Thu, Apr 26, 2018 at 4:29 PM, Mark Rutland <mark.rutl...@arm.com> 
>> > > wrote:
>> > >> On Wed, Apr 25, 2018 at 02:30:47PM +0530, Ganapatrao Kulkarni wrote:
>>
>> > >>> +static int alloc_counter(struct thunderx2_pmu_uncore_channel 
>> > >>> *pmu_uncore)
>> > >>> +{
>> > >>> + int counter;
>> > >>> +
>> > >>> + raw_spin_lock(_uncore->lock);
>> > >>> + counter = find_first_zero_bit(pmu_uncore->counter_mask,
>> > >>> + pmu_uncore->uncore_dev->max_counters);
>> > >>> + if (counter == pmu_uncore->uncore_dev->max_counters) {
>> > >>> + raw_spin_unlock(_uncore->lock);
>> > >>> + return -ENOSPC;
>> > >>> + }
>> > >>> + set_bit(counter, pmu_uncore->counter_mask);
>> > >>> + raw_spin_unlock(_uncore->lock);
>> > >>> + return counter;
>> > >>> +}
>> > >>> +
>> > >>> +static void free_counter(struct thunderx2_pmu_uncore_channel 
>> > >>> *pmu_uncore,
>> > >>> + int counter)
>> > >>> +{
>> > >>> + raw_spin_lock(_uncore->lock);
>> > >>> + clear_bit(counter, pmu_uncore->counter_mask);
>> > >>> + raw_spin_unlock(_uncore->lock);
>> > >>> +}
>> > >>
>> > >> I don't believe that locking is required in either of these, as the perf
>> > >> core serializes pmu::add() and pmu::del(), where these get called.
>> >
>> > without this locking, i am seeing "BUG: scheduling while atomic" when
>> > i run perf with more events together than the maximum counters
>> > supported
>>
>> Did you manage to get to the bottom of this?
>>
>> Do you have a backtrace?
>>
>> It looks like in your latest posting you reserve counters through the
>> userspace ABI, which doesn't seem right to me, and I'd like to
>> understand the problem.
>
> Looks like I misunderstood -- those are still allocated kernel-side.
>
> I'll follow that up in the v5 posting.

please review v5.
>
> Thanks,
> Mark.

thanks
Ganapat
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

2018-05-21 Thread Ganapatrao Kulkarni
Hi Mark,

On Mon, May 21, 2018 at 4:25 PM, Mark Rutland <mark.rutl...@arm.com> wrote:
> On Sat, May 05, 2018 at 12:16:13AM +0530, Ganapatrao Kulkarni wrote:
>> On Thu, Apr 26, 2018 at 4:29 PM, Mark Rutland <mark.rutl...@arm.com> wrote:
>> > On Wed, Apr 25, 2018 at 02:30:47PM +0530, Ganapatrao Kulkarni wrote:
>
>> >> + *
>> >> + *  L3 Tile and DMC channel selection is through SMC call
>> >> + *  SMC call arguments,
>> >> + *   x0 = THUNDERX2_SMC_CALL_ID  (Vendor SMC call Id)
>> >> + *   x1 = THUNDERX2_SMC_SET_CHANNEL  (Id to set DMC/L3C channel)
>> >> + *   x2 = Node id
>> >
>> > How do we map Linux node IDs to the firmware's view of node IDs?
>> >
>> > I don't believe the two are necessarily the same -- Linux's node IDs are
>> > a Linux-specific construct.
>>
>> both are same, it is numa node id from ACPI/firmware.
>
> I am very wary about assuming that the Linux nid will always be the same
> as the ACPI node id.
>
> For that to *potentially* be true, this driver should depend on
> CONFIG_NUMA, NUMA must not be disabled on the command line, etc, or the
> node id will always be NUMA_NO_NODE.

ok, i can check the node id which we get from ACPI helpers in probe.
if it is NUMA_NO_NODE, I will init first socket uncore only and nid
param to fw is always zero?

>
> I would be *much* happier if we had an explicit mapping somewhere to the
> ID the FW expects.
>
>> > It would be much nicer if we could pass something based on the MPIDR,
>> > which is a known HW construct, or if this implicitly affected the
>> > current node.
>>
>> IMO,  node id is sufficient.
>
> I agree that *a* node ID is sufficient, I just don't think that we're
> guaranteed to have the specific node ID the FW wants.

for thunderx2 which is 2 socket only platform, pxm and nid should be
same(either 0 or 1)
however, i can send PXM id(node_to_pxm) to firmware to make it more sane.

>
>> > It would be vastly more sane for this to not be muxed at all. :/
>>
>> i am helpless due to crappy hw design!
>
> I'm certainly not blaming you for this! :)
>
> I hope the HW designers don't make the same mistake in future, though...
>
> Thanks,
> Mark.

thanks
Ganapat
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

2018-05-18 Thread Ganapatrao Kulkarni
On Thu, May 17, 2018 at 4:42 PM, John Garry <john.ga...@huawei.com> wrote:
> On 16/05/2018 05:55, Ganapatrao Kulkarni wrote:
>>
>> This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
>> Controller(DMC) and Level 3 Cache(L3C).
>>
>
> Hi,
>
> Just some coding comments below:
>
>> ThunderX2 has 8 independent DMC PMUs to capture performance events
>> corresponding to 8 channels of DDR4 Memory Controller and 16 independent
>> L3C PMUs to capture events corresponding to 16 tiles of L3 cache.
>> Each PMU supports up to 4 counters. All counters lack overflow interrupt
>> and are sampled periodically.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
>> ---
>>  drivers/perf/Kconfig |   8 +
>>  drivers/perf/Makefile|   1 +
>>  drivers/perf/thunderx2_pmu.c | 965
>> +++
>>  include/linux/cpuhotplug.h   |   1 +
>>  4 files changed, 975 insertions(+)
>>  create mode 100644 drivers/perf/thunderx2_pmu.c
>>
>> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
>> index 28bb5a0..eafd0fc 100644
>> --- a/drivers/perf/Kconfig
>> +++ b/drivers/perf/Kconfig
>> @@ -85,6 +85,14 @@ config QCOM_L3_PMU
>>Adds the L3 cache PMU into the perf events subsystem for
>>monitoring L3 cache events.
>>
>> +config THUNDERX2_PMU
>> +bool "Cavium ThunderX2 SoC PMU UNCORE"
>> +depends on ARCH_THUNDER2 && PERF_EVENTS && ACPI
>
>
> Is the explicit dependency for PERF_EVENTS required, since we're under the
> PERF_EVENTS menu?

not really, i can drop this.
>
> And IIRC for other perf drivers we required a dependency on ARM64 - is that
> required here also? I see arm_smccc_smc() calls in the code...

ok.
>
>
>> +   help
>> + Provides support for ThunderX2 UNCORE events.
>> + The SoC has PMU support in its L3 cache controller (L3C) and
>> + in the DDR4 Memory Controller (DMC).
>> +
>>  config XGENE_PMU
>>  depends on ARCH_XGENE
>>  bool "APM X-Gene SoC PMU"
>> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
>> index b3902bd..909f27f 100644
>> --- a/drivers/perf/Makefile
>> +++ b/drivers/perf/Makefile
>> @@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
>>  obj-$(CONFIG_HISI_PMU) += hisilicon/
>>  obj-$(CONFIG_QCOM_L2_PMU)  += qcom_l2_pmu.o
>>  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
>> +obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
>>  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
>>  obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
>> diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
>> new file mode 100644
>> index 000..0401443
>> --- /dev/null
>> +++ b/drivers/perf/thunderx2_pmu.c
>> @@ -0,0 +1,965 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * CAVIUM THUNDERX2 SoC PMU UNCORE
>> + *
>> + * Copyright (C) 2018 Cavium Inc.
>> + * Author: Ganapatrao Kulkarni <gkulka...@cavium.com>
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>
>
> Isn't this the same as the SPDX?

ok, i will remove it.
>
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* L3c and DMC has 16 and 8 channels per socket respectively.
>
>
> L3C, right?

ok
>
>> + * Each Channel supports UNCORE PMU device and consists of
>> + * 4 independent programmable counters. Counters are 32 bit
>> + * and does not support overflow interrupt, they needs to be
>
>
> /s/needs/need/, /s/does/do/

ok
>
>> + * sampled before overflow(i.e, at every 2 seconds).
>
>
> how can you ensure that this value is low enough?
>
> "I saw this comment in previous patch:
>> Given that all channels compete for access to the muxed register
>> interface, I suspect we need to

[PATCH v5 0/2] Add ThunderX2 SoC Performance Monitoring Unit driver

2018-05-15 Thread Ganapatrao Kulkarni
This patchset adds PMU driver for Cavium's ThunderX2 SoC UNCORE devices.
The SoC has PMU support in L3 cache controller (L3C) and in the
DDR4 Memory Controller (DMC).

v5:
 -Incroporated review comments from Mark Rutland[2]
v4:
 -Incroporated review comments from Mark Rutland[1]

[1] https://www.spinics.net/lists/arm-kernel/msg588563.html
[2] https://lkml.org/lkml/2018/4/26/376

v3:
 - fixed warning reported by kbuild robot

v2:
 - rebased to 4.12-rc1
 - Removed Arch VULCAN dependency.
 - update SMC call parameters as per latest firmware.

v1:
 -Initial patch

Ganapatrao Kulkarni (2):
  perf: uncore: Adding documentation for ThunderX2 pmu uncore driver
  ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

 Documentation/perf/thunderx2-pmu.txt |  66 +++
 drivers/perf/Kconfig |   8 +
 drivers/perf/Makefile|   1 +
 drivers/perf/thunderx2_pmu.c | 965 +++
 include/linux/cpuhotplug.h   |   1 +
 5 files changed, 1041 insertions(+)
 create mode 100644 Documentation/perf/thunderx2-pmu.txt
 create mode 100644 drivers/perf/thunderx2_pmu.c

-- 
2.9.4

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


[PATCH v5 1/2] perf: uncore: Adding documentation for ThunderX2 pmu uncore driver

2018-05-15 Thread Ganapatrao Kulkarni
Documentation for the UNCORE PMUs on Cavium's ThunderX2 SoC.
The SoC has PMU support in its L3 cache controller (L3C) and in the
DDR4 Memory Controller (DMC).

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
---
 Documentation/perf/thunderx2-pmu.txt | 66 
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/perf/thunderx2-pmu.txt

diff --git a/Documentation/perf/thunderx2-pmu.txt 
b/Documentation/perf/thunderx2-pmu.txt
new file mode 100644
index 000..7d89935
--- /dev/null
+++ b/Documentation/perf/thunderx2-pmu.txt
@@ -0,0 +1,66 @@
+
+Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
+==
+
+ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such
+as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC).
+
+It has 8 independent DMC PMUs to capture performance events corresponding
+to 8 channels of DDR4 Memory Controller. There are 16 independent L3C PMUs
+to capture events corresponding to 16 tiles of L3 cache. Each PMU supports
+up to 4 counters.
+
+Counters are independently programmable and can be started and stopped
+individually. Each counter can be set to sample specific perf events.
+Counters are 32 bit and do not support overflow interrupt; they are
+sampled at every 2 seconds. The Counters register access are multiplexed
+across channels of DMC and L3C. The muxing(select channel) is done through
+write to a Secure register using smcc calls.
+
+PMU UNCORE (perf) driver:
+
+The thunderx2-pmu driver registers several perf PMUs for DMC and L3C devices.
+Each of the PMUs provides description of its available events
+and configuration options in sysfs.
+   see /sys/devices/uncore_
+
+S is socket id and X represents channel number.
+Each PMU can be used to sample up to 4 events simultaneously.
+
+The "format" directory describes format of the config (event ID).
+The "events" directory provides configuration templates for all
+supported event types that can be used with perf tool.
+
+For example, "uncore_dmc_0_0/cnt_cycles/" is an
+equivalent of "uncore_dmc_0_0/config=0x1/".
+
+Each perf driver also provides a "cpumask" sysfs attribute, which contains a
+single CPU ID of the processor which is likely to be used to handle all the
+PMU events. It will be the first online CPU from the NUMA node of PMU device.
+
+Example for perf tool use:
+
+perf stat -a -e \
+uncore_dmc_0_0/cnt_cycles/,\
+uncore_dmc_0_1/cnt_cycles/,\
+uncore_dmc_0_2/cnt_cycles/,\
+uncore_dmc_0_3/cnt_cycles/,\
+uncore_dmc_0_4/cnt_cycles/,\
+uncore_dmc_0_5/cnt_cycles/,\
+uncore_dmc_0_6/cnt_cycles/,\
+uncore_dmc_0_7/cnt_cycles/ sleep 1
+
+perf stat -a -e \
+uncore_dmc_0_0/cancelled_read_txns/,\
+uncore_dmc_0_0/cnt_cycles/,\
+uncore_dmc_0_0/consumed_read_txns/,\
+uncore_dmc_0_0/data_transfers/ sleep 1
+
+perf stat -a -e \
+uncore_l3c_0_0/l3_retry/,\
+uncore_l3c_0_0/read_hit/,\
+uncore_l3c_0_0/read_request/,\
+uncore_l3c_0_0/inv_request/ sleep 1
+
+The driver does not support sampling, therefore "perf record" will
+not work. Per-task (without "-a") perf sessions are not supported.
-- 
2.9.4

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


Re: [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

2018-05-15 Thread Ganapatrao Kulkarni
Hi Mark,


On Sat, May 5, 2018 at 12:16 AM, Ganapatrao Kulkarni <gklkm...@gmail.com> wrote:
> Hi Mark,
>
> On Thu, Apr 26, 2018 at 4:29 PM, Mark Rutland <mark.rutl...@arm.com> wrote:
>> Hi,
>>
>> On Wed, Apr 25, 2018 at 02:30:47PM +0530, Ganapatrao Kulkarni wrote:
>>> +
>>> +/* L3c and DMC has 16 and 8 channels per socket respectively.
>>> + * Each Channel supports UNCORE PMU device and consists of
>>> + * 4 independent programmable counters. Counters are 32 bit
>>> + * and does not support overflow interrupt, they needs to be
>>> + * sampled before overflow(i.e, at every 2 seconds).
>>> + */
>>> +
>>> +#define UNCORE_MAX_COUNTERS  4
>>> +#define UNCORE_L3_MAX_TILES  16
>>> +#define UNCORE_DMC_MAX_CHANNELS  8
>>> +
>>> +#define UNCORE_HRTIMER_INTERVAL  (2 * NSEC_PER_SEC)
>>
>> How was a period of two seconds chosen?
>
> It has been suggested from hw team  to sample before  3 to 4 seconds.
>
>>
>> What's the maximum clock speed for the L3C and DMC?
>
> L3C at 1.5GHz and DMC at 1.2GHz
>>
>> Given that all channels compete for access to the muxed register
>> interface, I suspect we need to try more often than once every 2
>> seconds...
>
> 2 seconds seems to be sufficient. So far testing looks good.
>
>>
>> [...]
>>
>>> +struct active_timer {
>>> + struct perf_event *event;
>>> + struct hrtimer hrtimer;
>>> +};
>>> +
>>> +/*
>>> + * pmu on each socket has 2 uncore devices(dmc and l3),
>>> + * each uncore device has up to 16 channels, each channel can sample
>>> + * events independently with counters up to 4.
>>> + *
>>> + * struct thunderx2_pmu_uncore_channel created per channel.
>>> + * struct thunderx2_pmu_uncore_dev per uncore device.
>>> + */
>>> +struct thunderx2_pmu_uncore_channel {
>>> + struct thunderx2_pmu_uncore_dev *uncore_dev;
>>> + struct pmu pmu;
>>
>> Can we put the pmu first in the struct, please?
>
> ok
>>
>>> + int counter;
>>
>> AFAICT, this counter field is never used.
>
> hmm ok, will remove.
>>
>>> + int channel;
>>> + DECLARE_BITMAP(counter_mask, UNCORE_MAX_COUNTERS);
>>> + struct active_timer *active_timers;
>>
>> You should only need a single timer per channel, rather than one per
>> event.
>>
>> I think you can get rid of the active_timer structure, and have:
>>
>> struct perf_event *events[UNCORE_MAX_COUNTERS];
>> struct hrtimer timer;
>>
>
> thanks, will change as suggested.
>
>>> + /* to sync counter alloc/release */
>>> + raw_spinlock_t lock;
>>> +};
>>> +
>>> +struct thunderx2_pmu_uncore_dev {
>>> + char *name;
>>> + struct device *dev;
>>> + enum thunderx2_uncore_type type;
>>> + unsigned long base;
>>
>> This should be:
>>
>> void __iomem *base;
>
> ok
>>
>> [...]
>>
>>> +static ssize_t cpumask_show(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct cpumask cpu_mask;
>>> + struct thunderx2_pmu_uncore_channel *pmu_uncore =
>>> + pmu_to_thunderx2_pmu_uncore(dev_get_drvdata(dev));
>>> +
>>> + /* Pick first online cpu from the node */
>>> + cpumask_clear(_mask);
>>> + cpumask_set_cpu(cpumask_first(
>>> + cpumask_of_node(pmu_uncore->uncore_dev->node)),
>>> + _mask);
>>> +
>>> + return cpumap_print_to_pagebuf(true, buf, _mask);
>>> +}
>>
>> AFAICT cpumask_of_node() returns a mask that can include offline CPUs.
>>
>> Regardless, I don't think that you can keep track of the management CPU
>> this way. Please keep track of the CPU the PMU should be managed by,
>> either with a cpumask or int embedded within the PMU structure.
>
> thanks, will add hotplug callbacks.
>>
>> At hotplug time, you'll need to update the management CPU, calling
>> perf_pmu_migrate_context() when it is offlined.
>
> ok
>>
>> [...]
>>
>>> +static int alloc_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore)
>>> +{
>>> + int counter;
>>> +
>>> + raw_spin_lock(_uncore->lock);
>>> + counter = find_first_zero_bit(pmu_un

Re: [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

2018-05-04 Thread Ganapatrao Kulkarni
Hi Mark,

On Thu, Apr 26, 2018 at 4:29 PM, Mark Rutland <mark.rutl...@arm.com> wrote:
> Hi,
>
> On Wed, Apr 25, 2018 at 02:30:47PM +0530, Ganapatrao Kulkarni wrote:
>> +
>> +/* L3c and DMC has 16 and 8 channels per socket respectively.
>> + * Each Channel supports UNCORE PMU device and consists of
>> + * 4 independent programmable counters. Counters are 32 bit
>> + * and does not support overflow interrupt, they needs to be
>> + * sampled before overflow(i.e, at every 2 seconds).
>> + */
>> +
>> +#define UNCORE_MAX_COUNTERS  4
>> +#define UNCORE_L3_MAX_TILES  16
>> +#define UNCORE_DMC_MAX_CHANNELS  8
>> +
>> +#define UNCORE_HRTIMER_INTERVAL  (2 * NSEC_PER_SEC)
>
> How was a period of two seconds chosen?

It has been suggested from hw team  to sample before  3 to 4 seconds.

>
> What's the maximum clock speed for the L3C and DMC?

L3C at 1.5GHz and DMC at 1.2GHz
>
> Given that all channels compete for access to the muxed register
> interface, I suspect we need to try more often than once every 2
> seconds...

2 seconds seems to be sufficient. So far testing looks good.

>
> [...]
>
>> +struct active_timer {
>> + struct perf_event *event;
>> + struct hrtimer hrtimer;
>> +};
>> +
>> +/*
>> + * pmu on each socket has 2 uncore devices(dmc and l3),
>> + * each uncore device has up to 16 channels, each channel can sample
>> + * events independently with counters up to 4.
>> + *
>> + * struct thunderx2_pmu_uncore_channel created per channel.
>> + * struct thunderx2_pmu_uncore_dev per uncore device.
>> + */
>> +struct thunderx2_pmu_uncore_channel {
>> + struct thunderx2_pmu_uncore_dev *uncore_dev;
>> + struct pmu pmu;
>
> Can we put the pmu first in the struct, please?

ok
>
>> + int counter;
>
> AFAICT, this counter field is never used.

hmm ok, will remove.
>
>> + int channel;
>> + DECLARE_BITMAP(counter_mask, UNCORE_MAX_COUNTERS);
>> + struct active_timer *active_timers;
>
> You should only need a single timer per channel, rather than one per
> event.
>
> I think you can get rid of the active_timer structure, and have:
>
> struct perf_event *events[UNCORE_MAX_COUNTERS];
> struct hrtimer timer;
>

thanks, will change as suggested.

>> + /* to sync counter alloc/release */
>> + raw_spinlock_t lock;
>> +};
>> +
>> +struct thunderx2_pmu_uncore_dev {
>> + char *name;
>> + struct device *dev;
>> + enum thunderx2_uncore_type type;
>> + unsigned long base;
>
> This should be:
>
> void __iomem *base;

ok
>
> [...]
>
>> +static ssize_t cpumask_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct cpumask cpu_mask;
>> + struct thunderx2_pmu_uncore_channel *pmu_uncore =
>> + pmu_to_thunderx2_pmu_uncore(dev_get_drvdata(dev));
>> +
>> + /* Pick first online cpu from the node */
>> + cpumask_clear(_mask);
>> + cpumask_set_cpu(cpumask_first(
>> + cpumask_of_node(pmu_uncore->uncore_dev->node)),
>> + _mask);
>> +
>> + return cpumap_print_to_pagebuf(true, buf, _mask);
>> +}
>
> AFAICT cpumask_of_node() returns a mask that can include offline CPUs.
>
> Regardless, I don't think that you can keep track of the management CPU
> this way. Please keep track of the CPU the PMU should be managed by,
> either with a cpumask or int embedded within the PMU structure.

thanks, will add hotplug callbacks.
>
> At hotplug time, you'll need to update the management CPU, calling
> perf_pmu_migrate_context() when it is offlined.

ok
>
> [...]
>
>> +static int alloc_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore)
>> +{
>> + int counter;
>> +
>> + raw_spin_lock(_uncore->lock);
>> + counter = find_first_zero_bit(pmu_uncore->counter_mask,
>> + pmu_uncore->uncore_dev->max_counters);
>> + if (counter == pmu_uncore->uncore_dev->max_counters) {
>> + raw_spin_unlock(_uncore->lock);
>> + return -ENOSPC;
>> + }
>> + set_bit(counter, pmu_uncore->counter_mask);
>> + raw_spin_unlock(_uncore->lock);
>> + return counter;
>> +}
>> +
>> +static void free_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore,
>> + int counter)
>> +{
>> +  

Re: [PATCH v4 1/2] perf: uncore: Adding documentation for ThunderX2 pmu uncore driver

2018-04-26 Thread Ganapatrao Kulkarni
On Fri, Apr 27, 2018 at 2:25 AM, Randy Dunlap <rdun...@infradead.org> wrote:
> Hi,
>
> Just a few typo corrections...
>
> On 04/25/2018 02:00 AM, Ganapatrao Kulkarni wrote:
>> Documentation for the UNCORE PMUs on Cavium's ThunderX2 SoC.
>> The SoC has PMU support in its L3 cache controller (L3C) and in the
>> DDR4 Memory Controller (DMC).
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
>> ---
>>  Documentation/perf/thunderx2-pmu.txt | 66 
>> 
>>  1 file changed, 66 insertions(+)
>>  create mode 100644 Documentation/perf/thunderx2-pmu.txt
>>
>> diff --git a/Documentation/perf/thunderx2-pmu.txt 
>> b/Documentation/perf/thunderx2-pmu.txt
>> new file mode 100644
>> index 000..9e9f535
>> --- /dev/null
>> +++ b/Documentation/perf/thunderx2-pmu.txt
>> @@ -0,0 +1,66 @@
>> +
>> +Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
>> +==
>> +
>> +ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such
>> +as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC).
>> +
>> +It has 8 independent DMC PMUs to capture performance events corresponding
>> +to 8 channels of DDR4 Memory Controller. There are 16 independent L3C PMUs
>> +to capture events corresponding to 16 tiles of L3 cache. Each PMU supports
>> +up to 4 counters.
>> +
>> +Counters are independent programmable and can be started and stopped
>
> independently

thanks, will update.
>
>> +individually. Each counter can be set to sample specific perf events.
>> +Counters are 32 bit and does not support overflow interrupt, they are
>
>do notinterrupt; they are
>
>
>> +sampled at every 2 seconds. The Counters register access are multiplexed
>> +across channels of DMC and L3C. The muxing(select channel) is done through
>> +write to a Secure register using smcc calls.
>> +
>> +PMU UNCORE (perf) driver:
>> +
>> +The thunderx2-pmu driver registers several perf PMUs for DMC and L3C 
>> devices.
>> +Each of the PMU provides description of its available events
>
> of the PMUs
>
>> +and configuration options in sysfs.
>> + see /sys/devices/uncore_
>> +
>> +S is socket id and X represents channel number.
>> +Each PMU can be used to sample up to 4 events simultaneously.
>> +
>> +The "format" directory describes format of the config (event ID).
>> +The "events" directory provides configuration templates for all
>> +supported event types that can be used with perf tool.
>> +
>> +For example, "uncore_dmc_0_0/cnt_cycles/" is an
>> +equivalent of "uncore_dmc_0_0/config=0x1/".
>> +
>> +Each perf driver also provides a "cpumask" sysfs attribute, which contains a
>> +single CPU ID of the processor which is likely to be used to handle all the
>> +PMU events. It will be the first online CPU from the NUMA node of PMU 
>> device.
>> +
>> +Example for perf tool use:
>> +
>> +perf stat -a -e \
>> +uncore_dmc_0_0/cnt_cycles/,\
>> +uncore_dmc_0_1/cnt_cycles/,\
>> +uncore_dmc_0_2/cnt_cycles/,\
>> +uncore_dmc_0_3/cnt_cycles/,\
>> +uncore_dmc_0_4/cnt_cycles/,\
>> +uncore_dmc_0_5/cnt_cycles/,\
>> +uncore_dmc_0_6/cnt_cycles/,\
>> +uncore_dmc_0_7/cnt_cycles/ sleep 1
>> +
>> +perf stat -a -e \
>> +uncore_dmc_0_0/cancelled_read_txns/,\
>> +uncore_dmc_0_0/cnt_cycles/,\
>> +uncore_dmc_0_0/consumed_read_txns/,\
>> +uncore_dmc_0_0/data_transfers/ sleep 1
>> +
>> +perf stat -a -e \
>> +uncore_l3c_0_0/l3_retry/,\
>> +uncore_l3c_0_0/read_hit/,\
>> +uncore_l3c_0_0/read_request/,\
>> +uncore_l3c_0_0/inv_request/ sleep 1
>> +
>> +The driver does not support sampling, therefore "perf record" will
>> +not work. Per-task (without "-a") perf sessions are not supported.
>>
>
>
> --
> ~Randy

thanks
Ganapat
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/2] perf: uncore: Adding documentation for ThunderX2 pmu uncore driver

2018-04-25 Thread Ganapatrao Kulkarni
Documentation for the UNCORE PMUs on Cavium's ThunderX2 SoC.
The SoC has PMU support in its L3 cache controller (L3C) and in the
DDR4 Memory Controller (DMC).

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
---
 Documentation/perf/thunderx2-pmu.txt | 66 
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/perf/thunderx2-pmu.txt

diff --git a/Documentation/perf/thunderx2-pmu.txt 
b/Documentation/perf/thunderx2-pmu.txt
new file mode 100644
index 000..9e9f535
--- /dev/null
+++ b/Documentation/perf/thunderx2-pmu.txt
@@ -0,0 +1,66 @@
+
+Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
+==
+
+ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such
+as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC).
+
+It has 8 independent DMC PMUs to capture performance events corresponding
+to 8 channels of DDR4 Memory Controller. There are 16 independent L3C PMUs
+to capture events corresponding to 16 tiles of L3 cache. Each PMU supports
+up to 4 counters.
+
+Counters are independent programmable and can be started and stopped
+individually. Each counter can be set to sample specific perf events.
+Counters are 32 bit and does not support overflow interrupt, they are
+sampled at every 2 seconds. The Counters register access are multiplexed
+across channels of DMC and L3C. The muxing(select channel) is done through
+write to a Secure register using smcc calls.
+
+PMU UNCORE (perf) driver:
+
+The thunderx2-pmu driver registers several perf PMUs for DMC and L3C devices.
+Each of the PMU provides description of its available events
+and configuration options in sysfs.
+   see /sys/devices/uncore_
+
+S is socket id and X represents channel number.
+Each PMU can be used to sample up to 4 events simultaneously.
+
+The "format" directory describes format of the config (event ID).
+The "events" directory provides configuration templates for all
+supported event types that can be used with perf tool.
+
+For example, "uncore_dmc_0_0/cnt_cycles/" is an
+equivalent of "uncore_dmc_0_0/config=0x1/".
+
+Each perf driver also provides a "cpumask" sysfs attribute, which contains a
+single CPU ID of the processor which is likely to be used to handle all the
+PMU events. It will be the first online CPU from the NUMA node of PMU device.
+
+Example for perf tool use:
+
+perf stat -a -e \
+uncore_dmc_0_0/cnt_cycles/,\
+uncore_dmc_0_1/cnt_cycles/,\
+uncore_dmc_0_2/cnt_cycles/,\
+uncore_dmc_0_3/cnt_cycles/,\
+uncore_dmc_0_4/cnt_cycles/,\
+uncore_dmc_0_5/cnt_cycles/,\
+uncore_dmc_0_6/cnt_cycles/,\
+uncore_dmc_0_7/cnt_cycles/ sleep 1
+
+perf stat -a -e \
+uncore_dmc_0_0/cancelled_read_txns/,\
+uncore_dmc_0_0/cnt_cycles/,\
+uncore_dmc_0_0/consumed_read_txns/,\
+uncore_dmc_0_0/data_transfers/ sleep 1
+
+perf stat -a -e \
+uncore_l3c_0_0/l3_retry/,\
+uncore_l3c_0_0/read_hit/,\
+uncore_l3c_0_0/read_request/,\
+uncore_l3c_0_0/inv_request/ sleep 1
+
+The driver does not support sampling, therefore "perf record" will
+not work. Per-task (without "-a") perf sessions are not supported.
-- 
2.9.4

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


[PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

2018-04-25 Thread Ganapatrao Kulkarni
This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
Controller(DMC) and Level 3 Cache(L3C).

ThunderX2 has 8 independent DMC PMUs to capture performance events
corresponding to 8 channels of DDR4 Memory Controller and 16 independent
L3C PMUs to capture events corresponding to 16 tiles of L3 cache.
Each PMU supports up to 4 counters. All counters lack overflow interrupt
and are sampled periodically.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
---
 drivers/perf/Kconfig |   8 +
 drivers/perf/Makefile|   1 +
 drivers/perf/thunderx2_pmu.c | 958 +++
 3 files changed, 967 insertions(+)
 create mode 100644 drivers/perf/thunderx2_pmu.c

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 28bb5a0..eafd0fc 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -85,6 +85,14 @@ config QCOM_L3_PMU
   Adds the L3 cache PMU into the perf events subsystem for
   monitoring L3 cache events.
 
+config THUNDERX2_PMU
+bool "Cavium ThunderX2 SoC PMU UNCORE"
+depends on ARCH_THUNDER2 && PERF_EVENTS && ACPI
+   help
+ Provides support for ThunderX2 UNCORE events.
+ The SoC has PMU support in its L3 cache controller (L3C) and
+ in the DDR4 Memory Controller (DMC).
+
 config XGENE_PMU
 depends on ARCH_XGENE
 bool "APM X-Gene SoC PMU"
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index b3902bd..909f27f 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
 obj-$(CONFIG_HISI_PMU) += hisilicon/
 obj-$(CONFIG_QCOM_L2_PMU)  += qcom_l2_pmu.o
 obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
+obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
 obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
new file mode 100644
index 000..92d19b5
--- /dev/null
+++ b/drivers/perf/thunderx2_pmu.c
@@ -0,0 +1,958 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CAVIUM THUNDERX2 SoC PMU UNCORE
+ *
+ * Copyright (C) 2018 Cavium Inc.
+ * Author: Ganapatrao Kulkarni <gkulka...@cavium.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* L3c and DMC has 16 and 8 channels per socket respectively.
+ * Each Channel supports UNCORE PMU device and consists of
+ * 4 independent programmable counters. Counters are 32 bit
+ * and does not support overflow interrupt, they needs to be
+ * sampled before overflow(i.e, at every 2 seconds).
+ */
+
+#define UNCORE_MAX_COUNTERS4
+#define UNCORE_L3_MAX_TILES16
+#define UNCORE_DMC_MAX_CHANNELS8
+
+#define UNCORE_HRTIMER_INTERVAL(2 * NSEC_PER_SEC)
+#define GET_EVENTID(ev)((ev->hw.config) & 0x1ff)
+#define GET_COUNTERID(ev)  ((ev->hw.idx) & 0xf)
+#define GET_CHANNELID(pmu_uncore)  (pmu_uncore->channel)
+
+#define DMC_COUNTER_CTL0x234
+#define DMC_COUNTER_DATA   0x240
+#define L3C_COUNTER_CTL0xA8
+#define L3C_COUNTER_DATA   0xAC
+
+#define THUNDERX2_SMC_CALL_ID  0xC200FF00
+#define THUNDERX2_SMC_SET_CHANNEL  0xB010
+
+
+enum thunderx2_uncore_l3_events {
+   L3_EVENT_NONE,
+   L3_EVENT_NBU_CANCEL,
+   L3_EVENT_DIB_RETRY,
+   L3_EVENT_DOB_RETRY,
+   L3_EVENT_DIB_CREDIT_RETRY,
+   L3_EVENT_DOB_CREDIT_RETRY,
+   L3_EVENT_FORCE_RETRY,
+   L3_EVENT_IDX_CONFLICT_RETRY,
+   L3_EVENT_EVICT_CONFLICT_RETRY,
+   L3_EVENT_BANK_CONFLICT_RETRY,
+   L3_EVENT_FILL_ENTRY_RETRY,
+   L3_EVENT_EVICT_NOT_READY_RETRY,
+   L3_EVENT_L3_RETRY,
+   L3_EVENT_READ_REQ,
+   L3_EVENT_WRITE_BACK_REQ,
+   L3_EVENT_INVALIDATE_NWRITE_REQ,
+   L3_EVENT_INV_REQ,
+   L3_EVENT_SELF_REQ,
+   L3_EVENT_REQ,
+   L3_EVENT_EVICT_REQ,
+   L3_EVENT_INVALIDATE_NWRITE_HIT,
+   L3_EVENT_INVALIDATE_HIT,
+   L3_EVENT_SELF_HIT,
+   L3_EVENT_READ_HIT,
+   L3_EVENT_MAX,
+};
+
+enum thunderx2_uncore_dmc_events {
+   DMC_EVENT_NONE,
+   DMC_EVENT_COUNT_CYCLES,
+   DMC_EVENT_RES2,
+   DMC_EVENT_RES3,
+   DMC_EVENT_RES4,
+   DMC_EVENT_RES5,

[PATCH v4 0/2] Add ThunderX2 SoC Performance Monitoring Unit driver

2018-04-25 Thread Ganapatrao Kulkarni
This patchset adds PMU driver for Cavium's ThunderX2 SoC UNCORE devices.
The SoC has PMU support in L3 cache controller (L3C) and in the
DDR4 Memory Controller (DMC).

v4:
 -Incroporated review comments from Mark Rutland[1]

[1] https://www.spinics.net/lists/arm-kernel/msg588563.html

v3:
 - fixed warning reported by kbuild robot

v2:
 - rebased to 4.12-rc1
 - Removed Arch VULCAN dependency.
 - update SMC call parameters as per latest firmware.

v1:
 -Initial patch

Ganapatrao Kulkarni (2):
  perf: uncore: Adding documentation for ThunderX2 pmu uncore driver
  ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver

 Documentation/perf/thunderx2-pmu.txt |  66 +++
 drivers/perf/Kconfig |   8 +
 drivers/perf/Makefile|   1 +
 drivers/perf/thunderx2_pmu.c | 958 +++
 4 files changed, 1033 insertions(+)
 create mode 100644 Documentation/perf/thunderx2-pmu.txt
 create mode 100644 drivers/perf/thunderx2_pmu.c

-- 
2.9.4

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


Re: [PATCH v2] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

2018-01-19 Thread Ganapatrao Kulkarni
On Fri, Jan 19, 2018 at 5:53 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
> On 18/01/18 05:28, Ganapatrao Kulkarni wrote:
>> This erratum is observed on the ThunderX2 GICv3 ITS. When a
>> MOVI command is used to change affinity of a LPI to a collection/cpu
>> on another node, the LPI is not delivered to the cpu.
>> An additional INV command is required after the MOVI to deliver
>> the LPI to the new destination.
>>
>> If we add INV after MOVI, there is a chance that we lose LPIs which
>> are raised when the affinity is changed. So for now, adding workaround fix
>> to disable inter node affinity change.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
>> ---
>>
>> v2: Added workaround to avoid inter node affinity change.
>>
>> v1: Initial patch
>>
>>  Documentation/arm64/silicon-errata.txt |  1 +
>>  arch/arm64/Kconfig | 10 ++
>>  drivers/irqchip/irq-gic-v3-its.c   | 21 -
>>  3 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt 
>> b/Documentation/arm64/silicon-errata.txt
>> index fc1c884..fb27cb5 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -63,6 +63,7 @@ stable kernels.
>>  | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456 
>>|
>>  | Cavium | ThunderX Core   | #30115  | CAVIUM_ERRATUM_30115 
>>|
>>  | Cavium | ThunderX SMMUv2 | #27704  | N/A  
>>|
>> +| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174   
>>|
>>  | Cavium | ThunderX2 SMMUv3| #74 | N/A  
>>|
>>  | Cavium | ThunderX2 SMMUv3| #126| N/A  
>>|
>>  || | |  
>>|
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index c9a7e9e..0dbf3bd 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -461,6 +461,16 @@ config ARM64_ERRATUM_843419
>>
>> If unsure, say Y.
>>
>> +config CAVIUM_ERRATUM_174
>> + bool "Cavium ThunderX2 erratum 174"
>> + default y
>> + help
>> +   Cavium ThunderX2 dual socket systems may loose interrupts
>> +   on affinity change to a cpu on other node.
>> +   This workaround fix avoids inter node affinity change.
>> +
>> +   If unsure, say Y.
>> +
>>  config CAVIUM_ERRATUM_22375
>>   bool "Cavium erratum 22375, 24313"
>>   default y
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 06f025f..b0cb528 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -46,6 +46,7 @@
>>  #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING(1ULL << 0)
>>  #define ITS_FLAGS_WORKAROUND_CAVIUM_22375(1ULL << 1)
>>  #define ITS_FLAGS_WORKAROUND_CAVIUM_23144(1ULL << 2)
>> +#define ITS_FLAGS_WORKAROUND_CAVIUM_174  (1ULL << 3)
>
> Instead of inventing a new flag, please rename the existing one to
> ITS_FLAG_WORKAROUND_RESTRICT_NODE_AFFINITY (or something similar). There
> is really no need to have two flags that do the exact same thing,

#23144 is used to restrict ITS to collection mapping too,
where as 174 is only restricts cross node affinity.
Having said that, Since we are restricting affinity in #174,
i see there is no use of having ITS to other node collection mapping.
There should not be any issue if we club flag. I will post this change
in next version.

>
>>
>>  #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING  (1 << 0)
>>
>> @@ -1102,7 +1103,8 @@ static int its_set_affinity(struct irq_data *d, const 
>> struct cpumask *mask_val,
>>   return -EINVAL;
>>
>> /* lpi cannot be routed to a redistributor that is on a foreign node 
>> */
>> - if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
>> + if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144 ||
>> + its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_174) 
>> {
>>   if (its_dev->its->numa_node >= 0) {
>>   cpu_mask = cpumask_of_node(its_dev->its->numa_node);
>>   if (!cp

[PATCH v2] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

2018-01-17 Thread Ganapatrao Kulkarni
This erratum is observed on the ThunderX2 GICv3 ITS. When a
MOVI command is used to change affinity of a LPI to a collection/cpu
on another node, the LPI is not delivered to the cpu.
An additional INV command is required after the MOVI to deliver
the LPI to the new destination.

If we add INV after MOVI, there is a chance that we lose LPIs which
are raised when the affinity is changed. So for now, adding workaround fix
to disable inter node affinity change.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
---

v2: Added workaround to avoid inter node affinity change.

v1: Initial patch

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 10 ++
 drivers/irqchip/irq-gic-v3-its.c   | 21 -
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index fc1c884..fb27cb5 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,6 +63,7 @@ stable kernels.
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX Core   | #30115  | CAVIUM_ERRATUM_30115
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
+| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174  
|
 | Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
 | Cavium | ThunderX2 SMMUv3| #126| N/A 
|
 || | | 
|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c9a7e9e..0dbf3bd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -461,6 +461,16 @@ config ARM64_ERRATUM_843419
 
  If unsure, say Y.
 
+config CAVIUM_ERRATUM_174
+   bool "Cavium ThunderX2 erratum 174"
+   default y
+   help
+ Cavium ThunderX2 dual socket systems may loose interrupts
+ on affinity change to a cpu on other node.
+ This workaround fix avoids inter node affinity change.
+
+ If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
bool "Cavium erratum 22375, 24313"
default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 06f025f..b0cb528 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -46,6 +46,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING  (1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375  (1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144  (1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_CAVIUM_174(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 
@@ -1102,7 +1103,8 @@ static int its_set_affinity(struct irq_data *d, const 
struct cpumask *mask_val,
return -EINVAL;
 
/* lpi cannot be routed to a redistributor that is on a foreign node */
-   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
+   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144 ||
+   its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_174) {
if (its_dev->its->numa_node >= 0) {
cpu_mask = cpumask_of_node(its_dev->its->numa_node);
if (!cpumask_intersects(mask_val, cpu_mask))
@@ -2904,6 +2906,15 @@ static int its_force_quiescent(void __iomem *base)
}
 }
 
+static bool __maybe_unused its_enable_quirk_cavium_174(void *data)
+{
+   struct its_node *its = data;
+
+   its->flags |= ITS_FLAGS_WORKAROUND_CAVIUM_174;
+
+   return true;
+}
+
 static bool __maybe_unused its_enable_quirk_cavium_22375(void *data)
 {
struct its_node *its = data;
@@ -3031,6 +3042,14 @@ static const struct gic_quirk its_quirks[] = {
.init   = its_enable_quirk_hip07_161600802,
},
 #endif
+#ifdef CONFIG_CAVIUM_ERRATUM_174
+   {
+   .desc   = "ITS: Cavium ThunderX2 erratum 174",
+   .iidr   = 0x13f,/* ThunderX2 pass A1/A2/B0 */
+   .mask   = 0x,
+   .init   = its_enable_quirk_cavium_174,
+   },
+#endif
{
}
 };
-- 
2.9.4

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


Re: [PATCH] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

2018-01-03 Thread Ganapatrao Kulkarni
On Wed, Jan 3, 2018 at 5:06 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
> On 03/01/18 11:20, Ganapatrao Kulkarni wrote:
>> On Wed, Jan 3, 2018 at 3:43 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
>>> On 03/01/18 09:35, Ganapatrao Kulkarni wrote:
>>>> Hi Marc,
>>>>
>>>> On Wed, Jan 3, 2018 at 2:17 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
>>>>> On 03/01/18 06:32, Ganapatrao Kulkarni wrote:
>>>>>> When an interrupt is moved across node collections on ThunderX2
>>>>>
>>>>> node collections?
>>>>
>>>> ok, i will rephrase it.
>>>>  i was intended to say cross NUMA node collection/cpu affinity change.
>>>>
>>>>>
>>>>>> multi Socket platform, an interrupt stops routed to new collection
>>>>>> and results in loss of interrupts.
>>>>>>
>>>>>> Adding workaround to issue INV after MOVI for cross-node collection
>>>>>> move to flush out the cached entry.
>>>>>>
>>>>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
>>>>>> ---
>>>>>>  Documentation/arm64/silicon-errata.txt |  1 +
>>>>>>  arch/arm64/Kconfig | 11 +++
>>>>>>  drivers/irqchip/irq-gic-v3-its.c   | 24 
>>>>>>  3 files changed, 36 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/arm64/silicon-errata.txt 
>>>>>> b/Documentation/arm64/silicon-errata.txt
>>>>>> index fc1c884..fb27cb5 100644
>>>>>> --- a/Documentation/arm64/silicon-errata.txt
>>>>>> +++ b/Documentation/arm64/silicon-errata.txt
>>>>>> @@ -63,6 +63,7 @@ stable kernels.
>>>>>>  | Cavium | ThunderX Core   | #27456  | 
>>>>>> CAVIUM_ERRATUM_27456|
>>>>>>  | Cavium | ThunderX Core   | #30115  | 
>>>>>> CAVIUM_ERRATUM_30115|
>>>>>>  | Cavium | ThunderX SMMUv2 | #27704  | N/A  
>>>>>>|
>>>>>> +| Cavium | ThunderX2 ITS   | #174| 
>>>>>> CAVIUM_ERRATUM_174  |
>>>>>>  | Cavium | ThunderX2 SMMUv3| #74 | N/A  
>>>>>>|
>>>>>>  | Cavium | ThunderX2 SMMUv3| #126| N/A  
>>>>>>|
>>>>>>  || | |  
>>>>>>|
>>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>>> index c9a7e9e..71a7e30 100644
>>>>>> --- a/arch/arm64/Kconfig
>>>>>> +++ b/arch/arm64/Kconfig
>>>>>> @@ -461,6 +461,17 @@ config ARM64_ERRATUM_843419
>>>>>>
>>>>>> If unsure, say Y.
>>>>>>
>>>>>> +config CAVIUM_ERRATUM_174
>>>>>> + bool "Cavium ThunderX2 erratum 174"
>>>>>> + depends on NUMA
>>>>>
>>>>> Why? This system will be affected no matter whether NUMA is selected or 
>>>>> not.
>>>>
>>>> it does not makes sense to enable on non-NUMA/single socket platforms.
>>>> By default NUMA is enabled on ThunderX2 dual socket platforms.
>>>
>>> 
>>> config ARCH_THUNDER2
>>> bool "Cavium ThunderX2 Server Processors"
>>> select GPIOLIB
>>> help
>>>   This enables support for Cavium's ThunderX2 CN99XX family of
>>>   server processors.
>>> 
>>>
>>> Do you see any NUMA here? I can perfectly compile a kernel with both
>>> sockets, and not using NUMA. NUMA has to do with memory, and not interrupts.
>>
>> ok,  i will remote it.
>>>
>>>>
>>>>>
>>>>>> + default y
>>>>>> + help
>>>>>> +   LPI stops routed to redistributors after inter node collection
>>>>>> +   move in ITS. Enable workaround to invalidate ITS entry after
>>>>>> +   inter-node collection move.
>>>>>
>>>>> That's a very terse description. Nobody knows what an LPI, a
>>>>> redis

Re: [PATCH] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

2018-01-03 Thread Ganapatrao Kulkarni
On Wed, Jan 3, 2018 at 3:43 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
> On 03/01/18 09:35, Ganapatrao Kulkarni wrote:
>> Hi Marc,
>>
>> On Wed, Jan 3, 2018 at 2:17 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
>>> On 03/01/18 06:32, Ganapatrao Kulkarni wrote:
>>>> When an interrupt is moved across node collections on ThunderX2
>>>
>>> node collections?
>>
>> ok, i will rephrase it.
>>  i was intended to say cross NUMA node collection/cpu affinity change.
>>
>>>
>>>> multi Socket platform, an interrupt stops routed to new collection
>>>> and results in loss of interrupts.
>>>>
>>>> Adding workaround to issue INV after MOVI for cross-node collection
>>>> move to flush out the cached entry.
>>>>
>>>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
>>>> ---
>>>>  Documentation/arm64/silicon-errata.txt |  1 +
>>>>  arch/arm64/Kconfig | 11 +++
>>>>  drivers/irqchip/irq-gic-v3-its.c   | 24 
>>>>  3 files changed, 36 insertions(+)
>>>>
>>>> diff --git a/Documentation/arm64/silicon-errata.txt 
>>>> b/Documentation/arm64/silicon-errata.txt
>>>> index fc1c884..fb27cb5 100644
>>>> --- a/Documentation/arm64/silicon-errata.txt
>>>> +++ b/Documentation/arm64/silicon-errata.txt
>>>> @@ -63,6 +63,7 @@ stable kernels.
>>>>  | Cavium | ThunderX Core   | #27456  | 
>>>> CAVIUM_ERRATUM_27456|
>>>>  | Cavium | ThunderX Core   | #30115  | 
>>>> CAVIUM_ERRATUM_30115|
>>>>  | Cavium | ThunderX SMMUv2 | #27704  | N/A
>>>>  |
>>>> +| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174 
>>>>  |
>>>>  | Cavium | ThunderX2 SMMUv3| #74 | N/A
>>>>  |
>>>>  | Cavium | ThunderX2 SMMUv3| #126| N/A
>>>>  |
>>>>  || | |
>>>>  |
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index c9a7e9e..71a7e30 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -461,6 +461,17 @@ config ARM64_ERRATUM_843419
>>>>
>>>> If unsure, say Y.
>>>>
>>>> +config CAVIUM_ERRATUM_174
>>>> + bool "Cavium ThunderX2 erratum 174"
>>>> + depends on NUMA
>>>
>>> Why? This system will be affected no matter whether NUMA is selected or not.
>>
>> it does not makes sense to enable on non-NUMA/single socket platforms.
>> By default NUMA is enabled on ThunderX2 dual socket platforms.
>
> 
> config ARCH_THUNDER2
> bool "Cavium ThunderX2 Server Processors"
> select GPIOLIB
> help
>   This enables support for Cavium's ThunderX2 CN99XX family of
>   server processors.
> 
>
> Do you see any NUMA here? I can perfectly compile a kernel with both
> sockets, and not using NUMA. NUMA has to do with memory, and not interrupts.

ok,  i will remote it.
>
>>
>>>
>>>> + default y
>>>> + help
>>>> +   LPI stops routed to redistributors after inter node collection
>>>> +   move in ITS. Enable workaround to invalidate ITS entry after
>>>> +   inter-node collection move.
>>>
>>> That's a very terse description. Nobody knows what an LPI, a
>>> redistributor or a collection is. Please explain what the erratum is in
>>> layman's terms (Cavium ThunderX2 systems may loose interrupts on
>>> affinity change) so that people understand whether or not they are affected.
>>
>> ok, i will rephrase it in next version.
>>>
>>>> +
>>>> +   If unsure, say Y.
>>>> +
>>>>  config CAVIUM_ERRATUM_22375
>>>>   bool "Cavium erratum 22375, 24313"
>>>>   default y
>>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>>>> b/drivers/irqchip/irq-gic-v3-its.c
>>>> index 06f025f..d8b9c96 100644
>>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>>>> @@ -46,6 +46,7 @@
>>>>  #define 

Re: [PATCH] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

2018-01-03 Thread Ganapatrao Kulkarni
Hi Marc,

On Wed, Jan 3, 2018 at 2:17 PM, Marc Zyngier <marc.zyng...@arm.com> wrote:
> On 03/01/18 06:32, Ganapatrao Kulkarni wrote:
>> When an interrupt is moved across node collections on ThunderX2
>
> node collections?

ok, i will rephrase it.
 i was intended to say cross NUMA node collection/cpu affinity change.

>
>> multi Socket platform, an interrupt stops routed to new collection
>> and results in loss of interrupts.
>>
>> Adding workaround to issue INV after MOVI for cross-node collection
>> move to flush out the cached entry.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
>> ---
>>  Documentation/arm64/silicon-errata.txt |  1 +
>>  arch/arm64/Kconfig | 11 +++
>>  drivers/irqchip/irq-gic-v3-its.c   | 24 
>>  3 files changed, 36 insertions(+)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt 
>> b/Documentation/arm64/silicon-errata.txt
>> index fc1c884..fb27cb5 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -63,6 +63,7 @@ stable kernels.
>>  | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456 
>>|
>>  | Cavium | ThunderX Core   | #30115  | CAVIUM_ERRATUM_30115 
>>|
>>  | Cavium | ThunderX SMMUv2 | #27704  | N/A  
>>|
>> +| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174   
>>|
>>  | Cavium | ThunderX2 SMMUv3| #74 | N/A  
>>|
>>  | Cavium | ThunderX2 SMMUv3| #126| N/A  
>>|
>>  || | |  
>>|
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index c9a7e9e..71a7e30 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -461,6 +461,17 @@ config ARM64_ERRATUM_843419
>>
>> If unsure, say Y.
>>
>> +config CAVIUM_ERRATUM_174
>> + bool "Cavium ThunderX2 erratum 174"
>> + depends on NUMA
>
> Why? This system will be affected no matter whether NUMA is selected or not.

it does not makes sense to enable on non-NUMA/single socket platforms.
By default NUMA is enabled on ThunderX2 dual socket platforms.

>
>> + default y
>> + help
>> +   LPI stops routed to redistributors after inter node collection
>> +   move in ITS. Enable workaround to invalidate ITS entry after
>> +   inter-node collection move.
>
> That's a very terse description. Nobody knows what an LPI, a
> redistributor or a collection is. Please explain what the erratum is in
> layman's terms (Cavium ThunderX2 systems may loose interrupts on
> affinity change) so that people understand whether or not they are affected.

ok, i will rephrase it in next version.
>
>> +
>> +   If unsure, say Y.
>> +
>>  config CAVIUM_ERRATUM_22375
>>   bool "Cavium erratum 22375, 24313"
>>   default y
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 06f025f..d8b9c96 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -46,6 +46,7 @@
>>  #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING(1ULL << 0)
>>  #define ITS_FLAGS_WORKAROUND_CAVIUM_22375(1ULL << 1)
>>  #define ITS_FLAGS_WORKAROUND_CAVIUM_23144(1ULL << 2)
>> +#define ITS_FLAGS_WORKAROUND_CAVIUM_174  (1ULL << 3)
>>
>>  #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING  (1 << 0)
>>
>> @@ -1119,6 +1120,12 @@ static int its_set_affinity(struct irq_data *d, const 
>> struct cpumask *mask_val,
>>   if (cpu != its_dev->event_map.col_map[id]) {
>>   target_col = _dev->its->collections[cpu];
>>   its_send_movi(its_dev, target_col, id);
>> + if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_174) {
>> + /* Issue INV for cross node collection move. */
>> + if (cpu_to_node(cpu) !=
>> + cpu_to_node(its_dev->event_map.col_map[id]))
>> + its_send_inv(its_dev, id);
>> + }
>
> What happens if an interrupt happens after the MOV, but before the INV?

there can be drop,  if interrupt happens before INV, however, it is
highly unlikely that we will hit the issue since MOVI a

[PATCH] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

2018-01-02 Thread Ganapatrao Kulkarni
When an interrupt is moved across node collections on ThunderX2
multi Socket platform, an interrupt stops routed to new collection
and results in loss of interrupts.

Adding workaround to issue INV after MOVI for cross-node collection
move to flush out the cached entry.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulka...@cavium.com>
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 11 +++
 drivers/irqchip/irq-gic-v3-its.c   | 24 
 3 files changed, 36 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index fc1c884..fb27cb5 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,6 +63,7 @@ stable kernels.
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX Core   | #30115  | CAVIUM_ERRATUM_30115
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
+| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174  
|
 | Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
 | Cavium | ThunderX2 SMMUv3| #126| N/A 
|
 || | | 
|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c9a7e9e..71a7e30 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -461,6 +461,17 @@ config ARM64_ERRATUM_843419
 
  If unsure, say Y.
 
+config CAVIUM_ERRATUM_174
+   bool "Cavium ThunderX2 erratum 174"
+   depends on NUMA
+   default y
+   help
+ LPI stops routed to redistributors after inter node collection
+ move in ITS. Enable workaround to invalidate ITS entry after
+ inter-node collection move.
+
+ If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
bool "Cavium erratum 22375, 24313"
default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 06f025f..d8b9c96 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -46,6 +46,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING  (1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375  (1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144  (1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_CAVIUM_174(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 
@@ -1119,6 +1120,12 @@ static int its_set_affinity(struct irq_data *d, const 
struct cpumask *mask_val,
if (cpu != its_dev->event_map.col_map[id]) {
target_col = _dev->its->collections[cpu];
its_send_movi(its_dev, target_col, id);
+   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_174) {
+   /* Issue INV for cross node collection move. */
+   if (cpu_to_node(cpu) !=
+   cpu_to_node(its_dev->event_map.col_map[id]))
+   its_send_inv(its_dev, id);
+   }
its_dev->event_map.col_map[id] = cpu;
irq_data_update_effective_affinity(d, cpumask_of(cpu));
}
@@ -2904,6 +2911,15 @@ static int its_force_quiescent(void __iomem *base)
}
 }
 
+static bool __maybe_unused its_enable_quirk_cavium_174(void *data)
+{
+   struct its_node *its = data;
+
+   its->flags |= ITS_FLAGS_WORKAROUND_CAVIUM_174;
+
+   return true;
+}
+
 static bool __maybe_unused its_enable_quirk_cavium_22375(void *data)
 {
struct its_node *its = data;
@@ -3031,6 +3047,14 @@ static const struct gic_quirk its_quirks[] = {
.init   = its_enable_quirk_hip07_161600802,
},
 #endif
+#ifdef CONFIG_CAVIUM_ERRATUM_174
+   {
+   .desc   = "ITS: Cavium ThunderX2 erratum 174",
+   .iidr   = 0x13f,/* ThunderX2 pass A1/A2/B0 */
+   .mask   = 0x,
+   .init   = its_enable_quirk_cavium_174,
+   },
+#endif
{
}
 };
-- 
2.9.4

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