LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Atish Patra <atish.patra@wdc.com>
To: Alan Kao <alankao@andestech.com>
Cc: Palmer Dabbelt <palmer@sifive.com>,
	"albert@sifive.com" <albert@sifive.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"acme@kernel.org" <acme@kernel.org>,
	"alexander.shishkin@linux.intel.com"
	<alexander.shishkin@linux.intel.com>,
	"jolsa@redhat.com" <jolsa@redhat.com>,
	"namhyung@kernel.org" <namhyung@kernel.org>,
	"sols@sifive.com" <sols@sifive.com>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"greentime@andestech.com" <greentime@andestech.com>,
	"nickhu@andestech.com" <nickhu@andestech.com>
Subject: Re: [PATCH v5 0/2] perf: riscv: Preliminary Perf Event Support on RISC-V
Date: Wed, 25 Apr 2018 01:09:57 -0700	[thread overview]
Message-ID: <51b78a28-bd21-678e-f6b4-5418a53ad920@wdc.com> (raw)
In-Reply-To: <20180425031936.GA7303@andestech.com>

On 4/24/18 8:19 PM, Alan Kao wrote:
> Hi Atish, Palmer,
> 
> On Tue, Apr 24, 2018 at 06:15:49PM -0700, Atish Patra wrote:
>> On 4/24/18 5:29 PM, Palmer Dabbelt wrote:
>>> On Tue, 24 Apr 2018 15:16:16 PDT (-0700), atish.patra@wdc.com wrote:
>>>> On 4/24/18 12:44 PM, Palmer Dabbelt wrote:
>>>>> On Tue, 24 Apr 2018 12:27:26 PDT (-0700), atish.patra@wdc.com wrote:
>>>>>> On 4/24/18 11:07 AM, Atish Patra wrote:
>>>>>>> On 4/19/18 4:28 PM, Alan Kao wrote:
>>>>>>> However, I got an rcu-stall for the test "47: Event times".
>>>>>>> # ./perf test -v 47
>>>>>> Got it working. The test tries to attach the event to CPU0 which doesn't
>>>>>> exist in HighFive Unleashed. Changing it to cpu1 works.
>>>>>>
>>>>>> diff --git a/tools/perf/tests/event-times.c b/tools/perf/tests/event-times.c
>>>>>> index 1a2686f..eb11632f 100644
>>>>>> --- a/tools/perf/tests/event-times.c
>>>>>> +++ b/tools/perf/tests/event-times.c
>>>>>> @@ -113,9 +113,9 @@ static int attach__cpu_disabled(struct perf_evlist
>>>>>> *evlist)
>>>>>>            struct cpu_map *cpus;
>>>>>>            int err;
>>>>>>
>>>>>> -       pr_debug("attaching to CPU 0 as enabled\n");
>>>>>> +       pr_debug("attaching to CPU 1 as disabled\n");
>>>>>>
>>>>>> -       cpus = cpu_map__new("0");
>>>>>> +       cpus = cpu_map__new("1");
>>>>>>            if (cpus == NULL) {
>>>>>>                    pr_debug("failed to call cpu_map__new\n");
>>>>>>                    return -1;
>>>>>> @@ -142,9 +142,9 @@ static int attach__cpu_enabled(struct perf_evlist
>>>>>> *evlist)
>>>>>>            struct cpu_map *cpus;
>>>>>>            int err;
>>>>>>
>>>>>> -       pr_debug("attaching to CPU 0 as enabled\n");
>>>>>> +       pr_debug("attaching to CPU 1 as enabled\n");
>>>>>>
>>>>>> -       cpus = cpu_map__new("0");
>>>>>> +       cpus = cpu_map__new("1");
>>>>>>            if (cpus == NULL) {
>>>>>>                    pr_debug("failed to call cpu_map__new\n");
>>>>>>                    return -1;
>>>>>>
>>>>>>
>>>>>> Palmer,
>>>>>> Would it be better to officially document it somewhere that CPU0 doesn't
>>>>>> exist in the HighFive Unleashed board ?
>>>>>> I fear that there will be other standard tests/code path that may fail
>>>>>> because of inherent assumption of cpu0 presence.
>>>>>
>>>>> I think the best way to fix this is to just have BBL (or whatever the
>>>>> bootloader is) renumber the CPUs so they're contiguous and begin with 0.
>>>>
>>>> Do you mean BBL will update the device tree that kernel eventually parse
>>>> and set the hart id?
>>>> Sounds good to me unless it acts as a big hack in future boot loaders.
>>>
>>> Right now the machine-mode and supervisor-mode hart IDs are logically separate:
>>> the bootloader just provides the hart ID as a register argument when starting
>>> the kernel.
>>
>> Yes.
>>
>>    BBL already needs to enumerate the harts by looking through the
>>> device tree for various other reasons (at least to mask off the harts that
>>> Linux doesn't support), so it's not that much effort to just maintain a mapping
>> >from supervisor-mode hart IDs to machine-mode hart IDs.
>>>
>>
>> But Linux also parses the device tree to get hart ID in
>> riscv_of_processor_hart(). This is used to setup the possible/present cpu
>> map in setup_smp().
>>
>> Thus, Linux also need to see a device tree with cpu0-3 instead of cpu1-4.
>> Otherwise, present cpu map will be incorrect. Isn't it ?
>>
>>> I have some patches floating around that do this, but appear to do it
>>> incorrectly enough that nothing boots so maybe I'm missing something that makes
>>> this complicated :).
>>>
>>
>> Just a wild guess: May be the because of the above reason ;)
>>
> 
> Thanks for the test and discussion.  It looks like am implementation issue from
> Unleash, so ... is there anything I should fix and provide a further patch?
> 
Nope. Nothing for this issue.
The fix for this issue should be either in bbl or kernel as a separate 
patch.

  reply	other threads:[~2018-04-25  8:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-19 23:27 Alan Kao
2018-04-19 23:27 ` [PATCH v5 1/2] perf: riscv: preliminary RISC-V support Alan Kao
2018-04-19 23:27 ` [PATCH v5 2/2] perf: riscv: Add Document for Future Porting Guide Alan Kao
2018-04-24 18:07 ` [PATCH v5 0/2] perf: riscv: Preliminary Perf Event Support on RISC-V Atish Patra
2018-04-24 19:27   ` Atish Patra
2018-04-24 19:44     ` Palmer Dabbelt
2018-04-24 22:16       ` Atish Patra
2018-04-25  0:29         ` Palmer Dabbelt
2018-04-25  1:15           ` Atish Patra
2018-04-25  3:19             ` Alan Kao
2018-04-25  8:09               ` Atish Patra [this message]
2018-04-25 16:39               ` Palmer Dabbelt
2018-06-06  3:16                 ` Alan Kao
2018-04-25 16:16             ` Palmer Dabbelt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51b78a28-bd21-678e-f6b4-5418a53ad920@wdc.com \
    --to=atish.patra@wdc.com \
    --cc=acme@kernel.org \
    --cc=alankao@andestech.com \
    --cc=albert@sifive.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=greentime@andestech.com \
    --cc=jolsa@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=nickhu@andestech.com \
    --cc=palmer@sifive.com \
    --cc=peterz@infradead.org \
    --cc=sols@sifive.com \
    --subject='Re: [PATCH v5 0/2] perf: riscv: Preliminary Perf Event Support on RISC-V' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).