Messages in this thread | | | Subject | Re: [PATCH v5 0/2] perf: riscv: Preliminary Perf Event Support on RISC-V | From | Atish Patra <> | Date | Tue, 24 Apr 2018 18:15:49 -0700 |
| |
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 ;)
| |