* Re: [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks with PVR checks
From: Pratik Sampat @ 2020-07-21 10:24 UTC (permalink / raw)
To: Nicholas Piggin, benh, ego, linux-kernel, linuxppc-dev, mikey,
mpe, paulus, pratik.r.sampat, svaidy
In-Reply-To: <1595203067.oropk0x5c8.astroid@bobo.none>
On 20/07/20 5:30 am, Nicholas Piggin wrote:
> Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
>> As the idle framework's architecture is incomplete, hence instead of
>> checking for just the processor type advertised in the device tree CPU
>> features; check for the Processor Version Register (PVR) so that finer
>> granularity can be leveraged while making processor checks.
>>
>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/idle.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
>> index 2dd467383a88..f62904f70fc6 100644
>> --- a/arch/powerpc/platforms/powernv/idle.c
>> +++ b/arch/powerpc/platforms/powernv/idle.c
>> @@ -92,7 +92,7 @@ static int pnv_save_sprs_for_deep_states(void)
>> if (rc != 0)
>> return rc;
>>
>> - if (cpu_has_feature(CPU_FTR_ARCH_300)) {
>> + if (pvr_version_is(PVR_POWER9)) {
>> rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
>> if (rc)
>> return rc;
>> @@ -116,7 +116,7 @@ static int pnv_save_sprs_for_deep_states(void)
>> return rc;
>>
>> /* Only p8 needs to set extra HID regiters */
>> - if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
>> + if (!pvr_version_is(PVR_POWER9)) {
>>
>> rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
>> if (rc != 0)
> What I think you should do is keep using CPU_FTR_ARCH_300 for this stuff
> which is written for power9 and we know is running on power9, because
> that's a faster test (static branch and does not have to read PVR. And
> then...
>
>> @@ -1205,7 +1205,7 @@ static void __init pnv_probe_idle_states(void)
>> return;
>> }
>>
>> - if (cpu_has_feature(CPU_FTR_ARCH_300))
>> + if (pvr_version_is(PVR_POWER9))
>> pnv_power9_idle_init();
>>
>> for (i = 0; i < nr_pnv_idle_states; i++)
> Here is where you would put the version check. Once we have code that
> can also handle P10 (either by testing CPU_FTR_ARCH_31, or by adding
> an entirely new power10 idle function), then you can add the P10 version
> check here.
Sure, it makes sense to make this check on the top level function and
retain CPU_FTR_ARCH_300 lower in the calls for speed.
I'll make that change.
Thanks
Pratik
> Thanks,
> Nick
>
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries: Avoid using addr_to_pfn in realmode
From: Nicholas Piggin @ 2020-07-21 10:08 UTC (permalink / raw)
To: Ganesh Goudar, linuxppc-dev, mpe; +Cc: aneesh.kumar, mahesh
In-Reply-To: <20200720080335.21049-1-ganeshgr@linux.ibm.com>
Excerpts from Ganesh Goudar's message of July 20, 2020 6:03 pm:
> When an UE or memory error exception is encountered the MCE handler
> tries to find the pfn using addr_to_pfn() which takes effective
> address as an argument, later pfn is used to poison the page where
> memory error occurred, recent rework in this area made addr_to_pfn
> to run in realmode, which can be fatal as it may try to access
> memory outside RMO region.
>
> To fix this have separate functions for realmode and virtual mode
> handling and let addr_to_pfn to run in virtual mode.
You didn't really explain what you moved around. You added some
helper functions, but what does it actually do differently now? Can you
explain that in the changelog?
Thanks,
Nick
>
> Without this fix following kernel crash is seen on hitting UE.
>
> [ 485.128036] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 485.128040] LE SMP NR_CPUS=2048 NUMA pSeries
> [ 485.128047] Modules linked in:
> [ 485.128067] CPU: 15 PID: 6536 Comm: insmod Kdump: loaded Tainted: G OE 5.7.0 #22
> [ 485.128074] NIP: c00000000009b24c LR: c0000000000398d8 CTR: c000000000cd57c0
> [ 485.128078] REGS: c000000003f1f970 TRAP: 0300 Tainted: G OE (5.7.0)
> [ 485.128082] MSR: 8000000000001003 <SF,ME,RI,LE> CR: 28008284 XER: 00000001
> [ 485.128088] CFAR: c00000000009b190 DAR: c0000001fab00000 DSISR: 40000000 IRQMASK: 1
> [ 485.128088] GPR00: 0000000000000001 c000000003f1fbf0 c000000001634300 0000b0fa01000000
> [ 485.128088] GPR04: d000000002220000 0000000000000000 00000000fab00000 0000000000000022
> [ 485.128088] GPR08: c0000001fab00000 0000000000000000 c0000001fab00000 c000000003f1fc14
> [ 485.128088] GPR12: 0000000000000008 c000000003ff5880 d000000002100008 0000000000000000
> [ 485.128088] GPR16: 000000000000ff20 000000000000fff1 000000000000fff2 d0000000021a1100
> [ 485.128088] GPR20: d000000002200000 c00000015c893c50 c000000000d49b28 c00000015c893c50
> [ 485.128088] GPR24: d0000000021a0d08 c0000000014e5da8 d0000000021a0818 000000000000000a
> [ 485.128088] GPR28: 0000000000000008 000000000000000a c0000000017e2970 000000000000000a
> [ 485.128125] NIP [c00000000009b24c] __find_linux_pte+0x11c/0x310
> [ 485.128130] LR [c0000000000398d8] addr_to_pfn+0x138/0x170
> [ 485.128133] Call Trace:
> [ 485.128135] Instruction dump:
> [ 485.128138] 3929ffff 7d4a3378 7c883c36 7d2907b4 794a1564 7d294038 794af082 3900ffff
> [ 485.128144] 79291f24 790af00e 78e70020 7d095214 <7c69502a> 2fa30000 419e011c 70690040
> [ 485.128152] ---[ end trace d34b27e29ae0e340 ]---
>
> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> ---
> V2: Leave bare metal code and save_mce_event as is.
>
> V3: Have separate functions for realmode and virtual mode handling.
> ---
> arch/powerpc/platforms/pseries/ras.c | 119 ++++++++++++++++-----------
> 1 file changed, 70 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index f3736fcd98fc..32fe3fad86b8 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -522,18 +522,55 @@ int pSeries_system_reset_exception(struct pt_regs *regs)
> return 0; /* need to perform reset */
> }
>
> +static int mce_handle_err_realmode(int disposition, u8 error_type)
> +{
> +#ifdef CONFIG_PPC_BOOK3S_64
> + if (disposition == RTAS_DISP_NOT_RECOVERED) {
> + switch (error_type) {
> + case MC_ERROR_TYPE_SLB:
> + case MC_ERROR_TYPE_ERAT:
> + /*
> + * Store the old slb content in paca before flushing.
> + * Print this when we go to virtual mode.
> + * There are chances that we may hit MCE again if there
> + * is a parity error on the SLB entry we trying to read
> + * for saving. Hence limit the slb saving to single
> + * level of recursion.
> + */
> + if (local_paca->in_mce == 1)
> + slb_save_contents(local_paca->mce_faulty_slbs);
> + flush_and_reload_slb();
> + disposition = RTAS_DISP_FULLY_RECOVERED;
> + break;
> + default:
> + break;
> + }
> + } else if (disposition == RTAS_DISP_LIMITED_RECOVERY) {
> + /* Platform corrected itself but could be degraded */
> + pr_err("MCE: limited recovery, system may be degraded\n");
> + disposition = RTAS_DISP_FULLY_RECOVERED;
> + }
> +#endif
> + return disposition;
> +}
>
> -static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
> +static int mce_handle_err_virtmode(struct pt_regs *regs,
> + struct rtas_error_log *errp,
> + struct pseries_mc_errorlog *mce_log,
> + int disposition)
> {
> struct mce_error_info mce_err = { 0 };
> - unsigned long eaddr = 0, paddr = 0;
> - struct pseries_errorlog *pseries_log;
> - struct pseries_mc_errorlog *mce_log;
> - int disposition = rtas_error_disposition(errp);
> int initiator = rtas_error_initiator(errp);
> int severity = rtas_error_severity(errp);
> + unsigned long eaddr = 0, paddr = 0;
> u8 error_type, err_sub_type;
>
> + if (!mce_log)
> + goto out;
> +
> + error_type = mce_log->error_type;
> + err_sub_type = rtas_mc_error_sub_type(mce_log);
> +
> if (initiator == RTAS_INITIATOR_UNKNOWN)
> mce_err.initiator = MCE_INITIATOR_UNKNOWN;
> else if (initiator == RTAS_INITIATOR_CPU)
> @@ -572,18 +609,7 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
> mce_err.error_type = MCE_ERROR_TYPE_UNKNOWN;
> mce_err.error_class = MCE_ECLASS_UNKNOWN;
>
> - if (!rtas_error_extended(errp))
> - goto out;
> -
> - pseries_log = get_pseries_errorlog(errp, PSERIES_ELOG_SECT_ID_MCE);
> - if (pseries_log == NULL)
> - goto out;
> -
> - mce_log = (struct pseries_mc_errorlog *)pseries_log->data;
> - error_type = mce_log->error_type;
> - err_sub_type = rtas_mc_error_sub_type(mce_log);
> -
> - switch (mce_log->error_type) {
> + switch (error_type) {
> case MC_ERROR_TYPE_UE:
> mce_err.error_type = MCE_ERROR_TYPE_UE;
> mce_common_process_ue(regs, &mce_err);
> @@ -683,37 +709,32 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
> mce_err.error_type = MCE_ERROR_TYPE_UNKNOWN;
> break;
> }
> +out:
> + save_mce_event(regs, disposition == RTAS_DISP_FULLY_RECOVERED,
> + &mce_err, regs->nip, eaddr, paddr);
> + return disposition;
> +}
>
> -#ifdef CONFIG_PPC_BOOK3S_64
> - if (disposition == RTAS_DISP_NOT_RECOVERED) {
> - switch (error_type) {
> - case MC_ERROR_TYPE_SLB:
> - case MC_ERROR_TYPE_ERAT:
> - /*
> - * Store the old slb content in paca before flushing.
> - * Print this when we go to virtual mode.
> - * There are chances that we may hit MCE again if there
> - * is a parity error on the SLB entry we trying to read
> - * for saving. Hence limit the slb saving to single
> - * level of recursion.
> - */
> - if (local_paca->in_mce == 1)
> - slb_save_contents(local_paca->mce_faulty_slbs);
> - flush_and_reload_slb();
> - disposition = RTAS_DISP_FULLY_RECOVERED;
> - break;
> - default:
> - break;
> - }
> - } else if (disposition == RTAS_DISP_LIMITED_RECOVERY) {
> - /* Platform corrected itself but could be degraded */
> - printk(KERN_ERR "MCE: limited recovery, system may "
> - "be degraded\n");
> - disposition = RTAS_DISP_FULLY_RECOVERED;
> - }
> -#endif
> +static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
> +{
> + struct pseries_errorlog *pseries_log;
> + struct pseries_mc_errorlog *mce_log = NULL;
> + int disposition = rtas_error_disposition(errp);
> + u8 error_type, err_sub_type;
> +
> + if (!rtas_error_extended(errp))
> + goto out;
> +
> + pseries_log = get_pseries_errorlog(errp, PSERIES_ELOG_SECT_ID_MCE);
> + if (!pseries_log)
> + goto out;
> +
> + mce_log = (struct pseries_mc_errorlog *)pseries_log->data;
> + error_type = mce_log->error_type;
> + err_sub_type = rtas_mc_error_sub_type(mce_log);
> +
> + disposition = mce_handle_err_realmode(disposition, error_type);
>
> -out:
> /*
> * Enable translation as we will be accessing per-cpu variables
> * in save_mce_event() which may fall outside RMO region, also
> @@ -724,10 +745,10 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
> * Note: All the realmode handling like flushing SLB entries for
> * SLB multihit is done by now.
> */
> +out:
> mtmsr(mfmsr() | MSR_IR | MSR_DR);
> - save_mce_event(regs, disposition == RTAS_DISP_FULLY_RECOVERED,
> - &mce_err, regs->nip, eaddr, paddr);
> -
> + disposition = mce_handle_err_virtmode(regs, errp, mce_log,
> + disposition);
> return disposition;
> }
>
> --
> 2.17.2
>
>
^ permalink raw reply
* Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode
From: Nicholas Piggin @ 2020-07-21 10:04 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Jens Axboe, linux-arch, Arnd Bergmann, Peter Zijlstra, x86,
linux-kernel, Andy Lutomirski, linux-mm, Andy Lutomirski,
linuxppc-dev
In-Reply-To: <2055788870.20749.1595263590675.JavaMail.zimbra@efficios.com>
Excerpts from Mathieu Desnoyers's message of July 21, 2020 2:46 am:
> ----- On Jul 19, 2020, at 11:03 PM, Nicholas Piggin npiggin@gmail.com wrote:
>
>> Excerpts from Mathieu Desnoyers's message of July 17, 2020 11:42 pm:
>>> ----- On Jul 16, 2020, at 7:26 PM, Nicholas Piggin npiggin@gmail.com wrote:
>>> [...]
>>>>
>>>> membarrier does replace barrier instructions on remote CPUs, which do
>>>> order accesses performed by the kernel on the user address space. So
>>>> membarrier should too I guess.
>>>>
>>>> Normal process context accesses like read(2) will do so because they
>>>> don't get filtered out from IPIs, but kernel threads using the mm may
>>>> not.
>>>
>>> But it should not be an issue, because membarrier's ordering is only with
>>> respect
>>> to submit and completion of io_uring requests, which are performed through
>>> system calls from the context of user-space threads, which are called from the
>>> right mm.
>>
>> Is that true? Can io completions be written into an address space via a
>> kernel thread? I don't know the io_uring code well but it looks like
>> that's asynchonously using the user mm context.
>
> Indeed, the io completion appears to be signaled asynchronously between kernel
> and user-space.
Yep, many other places do similar with use_mm.
[snip]
> So as far as membarrier memory ordering dependencies are concerned, it relies
> on the store-release/load-acquire dependency chain in the completion queue to
> order against anything that was done prior to the completed requests.
>
> What is in-flight while the requests are being serviced provides no memory
> ordering guarantee whatsoever.
Yeah you're probably right in this case I think. Quite likely most kernel
tasks that asynchronously write to user memory would at least have some
kind of producer-consumer barriers.
But is that restriction of all async modifications documented and enforced
anywhere?
>> How about other memory accesses via kthread_use_mm? Presumably there is
>> still ordering requirement there for membarrier,
>
> Please provide an example case with memory accesses via kthread_use_mm where
> ordering matters to support your concern.
I think the concern Andy raised with io_uring was less a specific
problem he saw and more a general concern that we have these memory
accesses which are not synchronized with membarrier.
>> so I really think
>> it's a fragile interface with no real way for the user to know how
>> kernel threads may use its mm for any particular reason, so membarrier
>> should synchronize all possible kernel users as well.
>
> I strongly doubt so, but perhaps something should be clarified in the documentation
> if you have that feeling.
I'd rather go the other way and say if you have reasoning or numbers for
why PF_KTHREAD is an important optimisation above rq->curr == rq->idle
then we could think about keeping this subtlety with appropriate
documentation added, otherwise we can just kill it and remove all doubt.
That being said, the x86 sync core gap that I imagined could be fixed
by changing to rq->curr == rq->idle test does not actually exist because
the global membarrier does not have a sync core option. So fixing the
exit_lazy_tlb points that this series does *should* fix that. So
PF_KTHREAD may be less problematic than I thought from implementation
point of view, only semantics.
Thanks,
Nick
^ permalink raw reply
* [PATCH trivial] ppc64/mm: remove comment that is no longer valid
From: Santosh Sivaraj @ 2020-07-21 9:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Santosh Sivaraj
hash_low_64.S was removed in [1] and since flush_hash_page is not called
from any assembly routine.
[1]: commit a43c0eb8364c0 ("powerpc/mm: Convert 4k insert from asm to C")
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/mm/book3s64/hash_utils.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 468169e33c86f..90ee0be3281a9 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1706,10 +1706,6 @@ unsigned long pte_get_hash_gslot(unsigned long vpn, unsigned long shift,
return gslot;
}
-/*
- * WARNING: This is called from hash_low_64.S, if you change this prototype,
- * do not forget to update the assembly call site !
- */
void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize, int ssize,
unsigned long flags)
{
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v4 09/10] powerpc/watchpoint: Return available watchpoints dynamically
From: Ravi Bangoria @ 2020-07-21 8:15 UTC (permalink / raw)
To: Jordan Niethe
Cc: Christophe Leroy, miltonm, mikey, Ravi Bangoria, peterz, oleg,
Nicholas Piggin, linux-kernel, Paul Mackerras, jolsa, fweisbec,
pedromfc, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <CACzsE9q5YtT_bXOpw9cri_UCxziW_FRbCpcViANaZwui0hjDqw@mail.gmail.com>
>>>> @@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
>>>>
>>>> static inline int nr_wp_slots(void)
>>>> {
>>>> - return HBP_NUM_MAX;
>>>> + return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
>>> So it'd be something like:
>>> + return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_MAX : 1;
>>> But thinking that there might be more slots added in the future, it
>>> may be better to make the number of slots a variable that is set
>>> during the init and then have this function return that.
>>
>> Not sure I follow. What do you mean by setting number of slots a
>> variable that is set during the init?
> Sorry I was unclear there.
> I was just looking and saw arm also has a variable number of hw breakpoints.
> If we did something like how they handle it, it might look something like:
>
> static int num_wp_slots __ro_after_init;
>
> int nr_wp_slots(void) {
> return num_wp_slots;
> }
>
> static int __init arch_hw_breakpoint_init(void) {
> num_wp_slots = work out how many wp_slots
> }
> arch_initcall(arch_hw_breakpoint_init);
>
> Then we wouldn't have to calculate everytime nr_wp_slots() is called.
> In the future if more wp's are added nr_wp_slots() will get more complicated.
> But just an idea, feel free to ignore.
Ok I got the idea. But ARM arch_hw_breakpoint_init() is much more complex
compared to our nr_wp_slots(). I don't see any benefit by making our code
like ARM.
Thanks for the idea though :)
Ravi
^ permalink raw reply
* Re: [PATCH v4 05/10] powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
From: Ravi Bangoria @ 2020-07-21 7:51 UTC (permalink / raw)
To: Jordan Niethe
Cc: Christophe Leroy, apopple, mikey, miltonm, peterz, oleg,
Nicholas Piggin, linux-kernel, Paul Mackerras, jolsa, fweisbec,
pedromfc, naveen.n.rao, linuxppc-dev, mingo, Ravi Bangoria
In-Reply-To: <CACzsE9oE+OMnWEXvbZZbq35YzpSzCbBHWEJcjtCgkcq-YrABng@mail.gmail.com>
On 7/17/20 11:14 AM, Jordan Niethe wrote:
> On Fri, Jul 17, 2020 at 2:10 PM Ravi Bangoria
> <ravi.bangoria@linux.ibm.com> wrote:
>>
>> Add new device-tree feature for 2nd DAWR. If this feature is present,
>> 2nd DAWR is supported, otherwise not.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/cputable.h | 7 +++++--
>> arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++++++
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
>> index e506d429b1af..3445c86e1f6f 100644
>> --- a/arch/powerpc/include/asm/cputable.h
>> +++ b/arch/powerpc/include/asm/cputable.h
>> @@ -214,6 +214,7 @@ static inline void cpu_feature_keys_init(void) { }
>> #define CPU_FTR_P9_TLBIE_ERAT_BUG LONG_ASM_CONST(0x0001000000000000)
>> #define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002000000000000)
>> #define CPU_FTR_ARCH_31 LONG_ASM_CONST(0x0004000000000000)
>> +#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008000000000000)
>>
>> #ifndef __ASSEMBLY__
>>
>> @@ -497,14 +498,16 @@ static inline void cpu_feature_keys_init(void) { }
>> #define CPU_FTRS_POSSIBLE \
>> (CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
>> CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
>> - CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
>> + CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
>> + CPU_FTR_DAWR1)
>> #else
>> #define CPU_FTRS_POSSIBLE \
>> (CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
>> CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
>> CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
>> CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
>> - CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
>> + CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | \
>> + CPU_FTR_DAWR1)
> Instead of putting CPU_FTR_DAWR1 into CPU_FTRS_POSSIBLE should it go
> into CPU_FTRS_POWER10?
> Then it will be picked up by CPU_FTRS_POSSIBLE.
I remember a discussion about this with Mikey and we decided to do it
this way. Obviously, the purpose is to make CPU_FTR_DAWR1 independent of
CPU_FTRS_POWER10 because DAWR1 is an optional feature in p10. I fear
including CPU_FTR_DAWR1 in CPU_FTRS_POWER10 can make it forcefully enabled
even when device-tree property is not present or pa-feature bit it not set,
because we do:
{ /* 3.1-compliant processor, i.e. Power10 "architected" mode */
.pvr_mask = 0xffffffff,
.pvr_value = 0x0f000006,
.cpu_name = "POWER10 (architected)",
.cpu_features = CPU_FTRS_POWER10,
>> #endif /* CONFIG_CPU_LITTLE_ENDIAN */
>> #endif
>> #else
>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> index ac650c233cd9..c78cd3596ec4 100644
>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> @@ -574,6 +574,12 @@ static int __init feat_enable_mma(struct dt_cpu_feature *f)
>> return 1;
>> }
>>
>> +static int __init feat_enable_debug_facilities_v31(struct dt_cpu_feature *f)
>> +{
>> + cur_cpu_spec->cpu_features |= CPU_FTR_DAWR1;
>> + return 1;
>> +}
>> +
>> struct dt_cpu_feature_match {
>> const char *name;
>> int (*enable)(struct dt_cpu_feature *f);
>> @@ -649,6 +655,7 @@ static struct dt_cpu_feature_match __initdata
>> {"wait-v3", feat_enable, 0},
>> {"prefix-instructions", feat_enable, 0},
>> {"matrix-multiply-assist", feat_enable_mma, 0},
>> + {"debug-facilities-v31", feat_enable_debug_facilities_v31, 0},
> Since all feat_enable_debug_facilities_v31() does is set
> CPU_FTR_DAWR1, if you just have:
> {"debug-facilities-v31", feat_enable, CPU_FTR_DAWR1},
> I think cpufeatures_process_feature() should set it in for you at this point:
> if (m->enable(f)) {
> cur_cpu_spec->cpu_features |= m->cpu_ftr_bit_mask;
> break;
> }
Yes, that seems a better option.
Thanks,
Ravi
^ permalink raw reply
* Re: [RFC PATCH] powerpc/pseries/svm: capture instruction faulting on MMIO access, in sprg0 register
From: Laurent Dufour @ 2020-07-21 7:22 UTC (permalink / raw)
To: Segher Boessenkool
Cc: aik, Ram Pai, kvm-ppc, bharata, sathnaga, sukadev, linuxppc-dev,
bauerman, david
In-Reply-To: <20200720202452.GN30544@gate.crashing.org>
Le 20/07/2020 à 22:24, Segher Boessenkool a écrit :
> On Mon, Jul 20, 2020 at 03:10:41PM -0500, Segher Boessenkool wrote:
>> On Mon, Jul 20, 2020 at 11:39:56AM +0200, Laurent Dufour wrote:
>>> Le 16/07/2020 à 10:32, Ram Pai a écrit :
>>>> + if (is_secure_guest()) { \
>>>> + __asm__ __volatile__("mfsprg0 %3;" \
>>>> + "lnia %2;" \
>>>> + "ld %2,12(%2);" \
>>>> + "mtsprg0 %2;" \
>>>> + "sync;" \
>>>> + #insn" %0,%y1;" \
>>>> + "twi 0,%0,0;" \
>>>> + "isync;" \
>>>> + "mtsprg0 %3" \
>>>> + : "=r" (ret) \
>>>> + : "Z" (*addr), "r" (0), "r" (0) \
>>>
>>> I'm wondering if SPRG0 is restored to its original value.
>>> You're using the same register (r0) for parameters 2 and 3, so when doing
>>> lnia %2, you're overwriting the SPRG0 value you saved in r0 just earlier.
>>
>> It is putting the value 0 in the registers the compiler chooses for
>> operands 2 and 3. But operand 3 is written, while the asm says it is an
>> input. It needs an earlyclobber as well.
Oh nice, I was not aware that compiler may choose registers this way.
Good to know, thanks for the explanation.
>>> It may be clearer to use explicit registers for %2 and %3 and to mark them
>>> as modified for the compiler.
>>
>> That is not a good idea, imnsho.
>
> (The explicit register number part, I mean; operand 2 should be an
> output as well, yes.)
Sure if the compiler can choose the registers that's far better.
Cheers,
Laurent.
^ permalink raw reply
* Re: [PATCH 09/20] Documentation: i2c: eliminate duplicated word
From: Wolfram Sang @ 2020-07-21 6:27 UTC (permalink / raw)
To: Randy Dunlap
Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
Liviu Dudau, dri-devel, Douglas Anderson, Paul Cercueil, keyrings,
Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
Halil Pasic, Jarkko Sakkinen, James Wang, linux-input,
Mali DP Maintainers, Derek Kiernan, linux-mips, Dragan Cvetic,
Wu Hao, Tony Krowiak, linux-kbuild, James E.J. Bottomley,
Jiri Kosina, Hannes Reinecke, linux-block, Thomas Bogendoerfer,
Jacek Anaszewski, linux-mm, Dan Williams, Andrew Morton,
Mimi Zohar, Jens Axboe, Michal Marek, Martin K. Petersen,
Pierre Morel, linux-kernel, Daniel Vetter, Jason Wessel,
Paolo Bonzini, linux-integrity, linuxppc-dev, Mike Rapoport,
Dan Murphy
In-Reply-To: <20200707180414.10467-10-rdunlap@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 341 bytes --]
On Tue, Jul 07, 2020 at 11:04:03AM -0700, Randy Dunlap wrote:
> Drop doubled word "new".
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: Wolfram Sang <wsa@kernel.org>
> Cc: linux-i2c@vger.kernel.org
Reviewed-by: Wolfram Sang <wsa@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [v3 15/15] tools/perf: Add perf tools support for extended regs in power10
From: kajoljain @ 2020-07-21 6:04 UTC (permalink / raw)
To: Athira Rajeev, mpe, acme, jolsa
Cc: ego, mikey, maddy, kvm, kvm-ppc, svaidyan, linuxppc-dev
In-Reply-To: <1594996707-3727-16-git-send-email-atrajeev@linux.vnet.ibm.com>
On 7/17/20 8:08 PM, Athira Rajeev wrote:
> Added support for supported regs which are new in power10
> ( MMCR3, SIER2, SIER3 ) to sample_reg_mask in the tool side
> to use with `-I?` option. Also added PVR check to send extended
> mask for power10 at kernel while capturing extended regs in
> each sample.
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> tools/arch/powerpc/include/uapi/asm/perf_regs.h | 6 ++++++
> tools/perf/arch/powerpc/include/perf_regs.h | 3 +++
> tools/perf/arch/powerpc/util/perf_regs.c | 6 ++++++
> 3 files changed, 15 insertions(+)
>
Reviewed-by: Kajol Jain <kjain@linux.ibm.com>
Thanks,
Kajol Jain
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index 225c64c..bdf5f10 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -52,6 +52,9 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_MMCR0,
> PERF_REG_POWERPC_MMCR1,
> PERF_REG_POWERPC_MMCR2,
> + PERF_REG_POWERPC_MMCR3,
> + PERF_REG_POWERPC_SIER2,
> + PERF_REG_POWERPC_SIER3,
> /* Max regs without the extended regs */
> PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> };
> @@ -60,6 +63,9 @@ enum perf_event_powerpc_regs {
>
> /* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300 */
> #define PERF_REG_PMU_MASK_300 (((1ULL << (PERF_REG_POWERPC_MMCR2 + 1)) - 1) - PERF_REG_PMU_MASK)
> +/* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31 */
> +#define PERF_REG_PMU_MASK_31 (((1ULL << (PERF_REG_POWERPC_SIER3 + 1)) - 1) - PERF_REG_PMU_MASK)
>
> #define PERF_REG_MAX_ISA_300 (PERF_REG_POWERPC_MMCR2 + 1)
> +#define PERF_REG_MAX_ISA_31 (PERF_REG_POWERPC_SIER3 + 1)
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index 46ed00d..63f3ac9 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -68,6 +68,9 @@
> [PERF_REG_POWERPC_MMCR0] = "mmcr0",
> [PERF_REG_POWERPC_MMCR1] = "mmcr1",
> [PERF_REG_POWERPC_MMCR2] = "mmcr2",
> + [PERF_REG_POWERPC_MMCR3] = "mmcr3",
> + [PERF_REG_POWERPC_SIER2] = "sier2",
> + [PERF_REG_POWERPC_SIER3] = "sier3",
> };
>
> static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index d64ba0c..2b6d470 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -14,6 +14,7 @@
> #include <linux/kernel.h>
>
> #define PVR_POWER9 0x004E
> +#define PVR_POWER10 0x0080
>
> const struct sample_reg sample_reg_masks[] = {
> SMPL_REG(r0, PERF_REG_POWERPC_R0),
> @@ -64,6 +65,9 @@
> SMPL_REG(mmcr0, PERF_REG_POWERPC_MMCR0),
> SMPL_REG(mmcr1, PERF_REG_POWERPC_MMCR1),
> SMPL_REG(mmcr2, PERF_REG_POWERPC_MMCR2),
> + SMPL_REG(mmcr3, PERF_REG_POWERPC_MMCR3),
> + SMPL_REG(sier2, PERF_REG_POWERPC_SIER2),
> + SMPL_REG(sier3, PERF_REG_POWERPC_SIER3),
> SMPL_REG_END
> };
>
> @@ -194,6 +198,8 @@ uint64_t arch__intr_reg_mask(void)
> version = (((mfspr(SPRN_PVR)) >> 16) & 0xFFFF);
> if (version == PVR_POWER9)
> extended_mask = PERF_REG_PMU_MASK_300;
> + else if (version == PVR_POWER10)
> + extended_mask = PERF_REG_PMU_MASK_31;
> else
> return mask;
>
>
^ permalink raw reply
* Re: [v3 14/15] powerpc/perf: Add extended regs support for power10 platform
From: kajoljain @ 2020-07-21 6:03 UTC (permalink / raw)
To: Athira Rajeev, mpe, acme, jolsa
Cc: ego, mikey, maddy, kvm, kvm-ppc, svaidyan, linuxppc-dev
In-Reply-To: <1594996707-3727-15-git-send-email-atrajeev@linux.vnet.ibm.com>
On 7/17/20 8:08 PM, Athira Rajeev wrote:
> Include capability flag `PERF_PMU_CAP_EXTENDED_REGS` for power10
> and expose MMCR3, SIER2, SIER3 registers as part of extended regs.
> Also introduce `PERF_REG_PMU_MASK_31` to define extended mask
> value at runtime for power10
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> [Fix build failure on PPC32 platform]
> Suggested-by: Ryan Grimm <grimm@linux.ibm.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
> arch/powerpc/include/uapi/asm/perf_regs.h | 6 ++++++
> arch/powerpc/perf/perf_regs.c | 12 +++++++++++-
> arch/powerpc/perf/power10-pmu.c | 6 ++++++
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
Reviewed-by: Kajol Jain <kjain@linux.ibm.com>
Thanks,
Kajol Jain
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index 225c64c..bdf5f10 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -52,6 +52,9 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_MMCR0,
> PERF_REG_POWERPC_MMCR1,
> PERF_REG_POWERPC_MMCR2,
> + PERF_REG_POWERPC_MMCR3,
> + PERF_REG_POWERPC_SIER2,
> + PERF_REG_POWERPC_SIER3,
> /* Max regs without the extended regs */
> PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> };
> @@ -60,6 +63,9 @@ enum perf_event_powerpc_regs {
>
> /* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300 */
> #define PERF_REG_PMU_MASK_300 (((1ULL << (PERF_REG_POWERPC_MMCR2 + 1)) - 1) - PERF_REG_PMU_MASK)
> +/* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31 */
> +#define PERF_REG_PMU_MASK_31 (((1ULL << (PERF_REG_POWERPC_SIER3 + 1)) - 1) - PERF_REG_PMU_MASK)
>
> #define PERF_REG_MAX_ISA_300 (PERF_REG_POWERPC_MMCR2 + 1)
> +#define PERF_REG_MAX_ISA_31 (PERF_REG_POWERPC_SIER3 + 1)
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index b0cf68f..11b90d5 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -81,6 +81,14 @@ static u64 get_ext_regs_value(int idx)
> return mfspr(SPRN_MMCR1);
> case PERF_REG_POWERPC_MMCR2:
> return mfspr(SPRN_MMCR2);
> +#ifdef CONFIG_PPC64
> + case PERF_REG_POWERPC_MMCR3:
> + return mfspr(SPRN_MMCR3);
> + case PERF_REG_POWERPC_SIER2:
> + return mfspr(SPRN_SIER2);
> + case PERF_REG_POWERPC_SIER3:
> + return mfspr(SPRN_SIER3);
> +#endif
> default: return 0;
> }
> }
> @@ -89,7 +97,9 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
> {
> u64 PERF_REG_EXTENDED_MAX;
>
> - if (cpu_has_feature(CPU_FTR_ARCH_300))
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + PERF_REG_EXTENDED_MAX = PERF_REG_MAX_ISA_31;
> + else if (cpu_has_feature(CPU_FTR_ARCH_300))
> PERF_REG_EXTENDED_MAX = PERF_REG_MAX_ISA_300;
>
> if (idx == PERF_REG_POWERPC_SIER &&
> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index b02aabb..f066ed9 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -87,6 +87,8 @@
> #define POWER10_MMCRA_IFM3 0x00000000C0000000UL
> #define POWER10_MMCRA_BHRB_MASK 0x00000000C0000000UL
>
> +extern u64 PERF_REG_EXTENDED_MASK;
> +
> /* Table of alternatives, sorted by column 0 */
> static const unsigned int power10_event_alternatives[][MAX_ALT] = {
> { PM_RUN_CYC_ALT, PM_RUN_CYC },
> @@ -397,6 +399,7 @@ static void power10_config_bhrb(u64 pmu_bhrb_filter)
> .cache_events = &power10_cache_events,
> .attr_groups = power10_pmu_attr_groups,
> .bhrb_nr = 32,
> + .capabilities = PERF_PMU_CAP_EXTENDED_REGS,
> };
>
> int init_power10_pmu(void)
> @@ -408,6 +411,9 @@ int init_power10_pmu(void)
> strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power10"))
> return -ENODEV;
>
> + /* Set the PERF_REG_EXTENDED_MASK here */
> + PERF_REG_EXTENDED_MASK = PERF_REG_PMU_MASK_31;
> +
> rc = register_power_pmu(&power10_pmu);
> if (rc)
> return rc;
>
^ permalink raw reply
* Re: [v3 13/15] tools/perf: Add perf tools support for extended register capability in powerpc
From: kajoljain @ 2020-07-21 6:03 UTC (permalink / raw)
To: Athira Rajeev, mpe, acme, jolsa
Cc: ego, mikey, maddy, kvm, kvm-ppc, svaidyan, linuxppc-dev
In-Reply-To: <1594996707-3727-14-git-send-email-atrajeev@linux.vnet.ibm.com>
On 7/17/20 8:08 PM, Athira Rajeev wrote:
> From: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>
> Add extended regs to sample_reg_mask in the tool side to use
> with `-I?` option. Perf tools side uses extended mask to display
> the platform supported register names (with -I? option) to the user
> and also send this mask to the kernel to capture the extended registers
> in each sample. Hence decide the mask value based on the processor
> version.
>
> Currently definitions for `mfspr`, `SPRN_PVR` are part of
> `arch/powerpc/util/header.c`. Move this to a header file so that
> these definitions can be re-used in other source files as well.
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> [Decide extended mask at run time based on platform]
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> tools/arch/powerpc/include/uapi/asm/perf_regs.h | 14 ++++++-
> tools/perf/arch/powerpc/include/perf_regs.h | 5 ++-
> tools/perf/arch/powerpc/util/header.c | 9 +----
> tools/perf/arch/powerpc/util/perf_regs.c | 49 +++++++++++++++++++++++++
> tools/perf/arch/powerpc/util/utils_header.h | 15 ++++++++
> 5 files changed, 82 insertions(+), 10 deletions(-)
> create mode 100644 tools/perf/arch/powerpc/util/utils_header.h
>
Reviewed-by: Kajol Jain <kjain@linux.ibm.com>
Thanks,
Kajol Jain
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index f599064..225c64c 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -48,6 +48,18 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_DSISR,
> PERF_REG_POWERPC_SIER,
> PERF_REG_POWERPC_MMCRA,
> - PERF_REG_POWERPC_MAX,
> + /* Extended registers */
> + PERF_REG_POWERPC_MMCR0,
> + PERF_REG_POWERPC_MMCR1,
> + PERF_REG_POWERPC_MMCR2,
> + /* Max regs without the extended regs */
> + PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> };
> +
> +#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
> +
> +/* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300 */
> +#define PERF_REG_PMU_MASK_300 (((1ULL << (PERF_REG_POWERPC_MMCR2 + 1)) - 1) - PERF_REG_PMU_MASK)
> +
> +#define PERF_REG_MAX_ISA_300 (PERF_REG_POWERPC_MMCR2 + 1)
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index e18a355..46ed00d 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -64,7 +64,10 @@
> [PERF_REG_POWERPC_DAR] = "dar",
> [PERF_REG_POWERPC_DSISR] = "dsisr",
> [PERF_REG_POWERPC_SIER] = "sier",
> - [PERF_REG_POWERPC_MMCRA] = "mmcra"
> + [PERF_REG_POWERPC_MMCRA] = "mmcra",
> + [PERF_REG_POWERPC_MMCR0] = "mmcr0",
> + [PERF_REG_POWERPC_MMCR1] = "mmcr1",
> + [PERF_REG_POWERPC_MMCR2] = "mmcr2",
> };
>
> static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
> index d487007..1a95017 100644
> --- a/tools/perf/arch/powerpc/util/header.c
> +++ b/tools/perf/arch/powerpc/util/header.c
> @@ -7,17 +7,10 @@
> #include <string.h>
> #include <linux/stringify.h>
> #include "header.h"
> +#include "utils_header.h"
> #include "metricgroup.h"
> #include <api/fs/fs.h>
>
> -#define mfspr(rn) ({unsigned long rval; \
> - asm volatile("mfspr %0," __stringify(rn) \
> - : "=r" (rval)); rval; })
> -
> -#define SPRN_PVR 0x11F /* Processor Version Register */
> -#define PVR_VER(pvr) (((pvr) >> 16) & 0xFFFF) /* Version field */
> -#define PVR_REV(pvr) (((pvr) >> 0) & 0xFFFF) /* Revison field */
> -
> int
> get_cpuid(char *buffer, size_t sz)
> {
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index 0a52429..d64ba0c 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -6,9 +6,15 @@
>
> #include "../../../util/perf_regs.h"
> #include "../../../util/debug.h"
> +#include "../../../util/event.h"
> +#include "../../../util/header.h"
> +#include "../../../perf-sys.h"
> +#include "utils_header.h"
>
> #include <linux/kernel.h>
>
> +#define PVR_POWER9 0x004E
> +
> const struct sample_reg sample_reg_masks[] = {
> SMPL_REG(r0, PERF_REG_POWERPC_R0),
> SMPL_REG(r1, PERF_REG_POWERPC_R1),
> @@ -55,6 +61,9 @@
> SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
> SMPL_REG(sier, PERF_REG_POWERPC_SIER),
> SMPL_REG(mmcra, PERF_REG_POWERPC_MMCRA),
> + SMPL_REG(mmcr0, PERF_REG_POWERPC_MMCR0),
> + SMPL_REG(mmcr1, PERF_REG_POWERPC_MMCR1),
> + SMPL_REG(mmcr2, PERF_REG_POWERPC_MMCR2),
> SMPL_REG_END
> };
>
> @@ -163,3 +172,43 @@ int arch_sdt_arg_parse_op(char *old_op, char **new_op)
>
> return SDT_ARG_VALID;
> }
> +
> +uint64_t arch__intr_reg_mask(void)
> +{
> + struct perf_event_attr attr = {
> + .type = PERF_TYPE_HARDWARE,
> + .config = PERF_COUNT_HW_CPU_CYCLES,
> + .sample_type = PERF_SAMPLE_REGS_INTR,
> + .precise_ip = 1,
> + .disabled = 1,
> + .exclude_kernel = 1,
> + };
> + int fd;
> + u32 version;
> + u64 extended_mask = 0, mask = PERF_REGS_MASK;
> +
> + /*
> + * Get the PVR value to set the extended
> + * mask specific to platform.
> + */
> + version = (((mfspr(SPRN_PVR)) >> 16) & 0xFFFF);
> + if (version == PVR_POWER9)
> + extended_mask = PERF_REG_PMU_MASK_300;
> + else
> + return mask;
> +
> + attr.sample_regs_intr = extended_mask;
> + attr.sample_period = 1;
> + event_attr_init(&attr);
> +
> + /*
> + * check if the pmu supports perf extended regs, before
> + * returning the register mask to sample.
> + */
> + fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
> + if (fd != -1) {
> + close(fd);
> + mask |= extended_mask;
> + }
> + return mask;
> +}
> diff --git a/tools/perf/arch/powerpc/util/utils_header.h b/tools/perf/arch/powerpc/util/utils_header.h
> new file mode 100644
> index 0000000..5788eb1
> --- /dev/null
> +++ b/tools/perf/arch/powerpc/util/utils_header.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PERF_UTIL_HEADER_H
> +#define __PERF_UTIL_HEADER_H
> +
> +#include <linux/stringify.h>
> +
> +#define mfspr(rn) ({unsigned long rval; \
> + asm volatile("mfspr %0," __stringify(rn) \
> + : "=r" (rval)); rval; })
> +
> +#define SPRN_PVR 0x11F /* Processor Version Register */
> +#define PVR_VER(pvr) (((pvr) >> 16) & 0xFFFF) /* Version field */
> +#define PVR_REV(pvr) (((pvr) >> 0) & 0xFFFF) /* Revison field */
> +
> +#endif /* __PERF_UTIL_HEADER_H */
>
^ permalink raw reply
* Re: [v3 12/15] powerpc/perf: Add support for outputting extended regs in perf intr_regs
From: kajoljain @ 2020-07-21 6:02 UTC (permalink / raw)
To: Athira Rajeev, mpe, acme, jolsa
Cc: ego, mikey, maddy, kvm, kvm-ppc, svaidyan, linuxppc-dev
In-Reply-To: <1594996707-3727-13-git-send-email-atrajeev@linux.vnet.ibm.com>
On 7/17/20 8:08 PM, Athira Rajeev wrote:
> From: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>
> Add support for perf extended register capability in powerpc.
> The capability flag PERF_PMU_CAP_EXTENDED_REGS, is used to indicate the
> PMU which support extended registers. The generic code define the mask
> of extended registers as 0 for non supported architectures.
>
> Patch adds extended regs support for power9 platform by
> exposing MMCR0, MMCR1 and MMCR2 registers.
>
> REG_RESERVED mask needs update to include extended regs.
> `PERF_REG_EXTENDED_MASK`, contains mask value of the supported registers,
> is defined at runtime in the kernel based on platform since the supported
> registers may differ from one processor version to another and hence the
> MASK value.
>
> with patch
> ----------
>
> available registers: r0 r1 r2 r3 r4 r5 r6 r7 r8 r9 r10 r11
> r12 r13 r14 r15 r16 r17 r18 r19 r20 r21 r22 r23 r24 r25 r26
> r27 r28 r29 r30 r31 nip msr orig_r3 ctr link xer ccr softe
> trap dar dsisr sier mmcra mmcr0 mmcr1 mmcr2
>
> PERF_RECORD_SAMPLE(IP, 0x1): 4784/4784: 0 period: 1 addr: 0
> ... intr regs: mask 0xffffffffffff ABI 64-bit
> .... r0 0xc00000000012b77c
> .... r1 0xc000003fe5e03930
> .... r2 0xc000000001b0e000
> .... r3 0xc000003fdcddf800
> .... r4 0xc000003fc7880000
> .... r5 0x9c422724be
> .... r6 0xc000003fe5e03908
> .... r7 0xffffff63bddc8706
> .... r8 0x9e4
> .... r9 0x0
> .... r10 0x1
> .... r11 0x0
> .... r12 0xc0000000001299c0
> .... r13 0xc000003ffffc4800
> .... r14 0x0
> .... r15 0x7fffdd8b8b00
> .... r16 0x0
> .... r17 0x7fffdd8be6b8
> .... r18 0x7e7076607730
> .... r19 0x2f
> .... r20 0xc00000001fc26c68
> .... r21 0xc0002041e4227e00
> .... r22 0xc00000002018fb60
> .... r23 0x1
> .... r24 0xc000003ffec4d900
> .... r25 0x80000000
> .... r26 0x0
> .... r27 0x1
> .... r28 0x1
> .... r29 0xc000000001be1260
> .... r30 0x6008010
> .... r31 0xc000003ffebb7218
> .... nip 0xc00000000012b910
> .... msr 0x9000000000009033
> .... orig_r3 0xc00000000012b86c
> .... ctr 0xc0000000001299c0
> .... link 0xc00000000012b77c
> .... xer 0x0
> .... ccr 0x28002222
> .... softe 0x1
> .... trap 0xf00
> .... dar 0x0
> .... dsisr 0x80000000000
> .... sier 0x0
> .... mmcra 0x80000000000
> .... mmcr0 0x82008090
> .... mmcr1 0x1e000000
> .... mmcr2 0x0
> ... thread: perf:4784
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> [Defined PERF_REG_EXTENDED_MASK at run time to add support for different platforms ]
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
Patch looks good to me.
Reviewed-by: Kajol Jain <kjain@linux.ibm.com>
Thanks,
Kajol Jain
> arch/powerpc/include/asm/perf_event_server.h | 8 +++++++
> arch/powerpc/include/uapi/asm/perf_regs.h | 14 +++++++++++-
> arch/powerpc/perf/core-book3s.c | 1 +
> arch/powerpc/perf/perf_regs.c | 34 +++++++++++++++++++++++++---
> arch/powerpc/perf/power9-pmu.c | 6 +++++
> 5 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 832450a..bf85d1a 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -15,6 +15,9 @@
> #define MAX_EVENT_ALTERNATIVES 8
> #define MAX_LIMITED_HWCOUNTERS 2
>
> +extern u64 PERF_REG_EXTENDED_MASK;
> +#define PERF_REG_EXTENDED_MASK PERF_REG_EXTENDED_MASK
> +
> struct perf_event;
>
> struct mmcr_regs {
> @@ -62,6 +65,11 @@ struct power_pmu {
> int *blacklist_ev;
> /* BHRB entries in the PMU */
> int bhrb_nr;
> + /*
> + * set this flag with `PERF_PMU_CAP_EXTENDED_REGS` if
> + * the pmu supports extended perf regs capability
> + */
> + int capabilities;
> };
>
> /*
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index f599064..225c64c 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -48,6 +48,18 @@ enum perf_event_powerpc_regs {
> PERF_REG_POWERPC_DSISR,
> PERF_REG_POWERPC_SIER,
> PERF_REG_POWERPC_MMCRA,
> - PERF_REG_POWERPC_MAX,
> + /* Extended registers */
> + PERF_REG_POWERPC_MMCR0,
> + PERF_REG_POWERPC_MMCR1,
> + PERF_REG_POWERPC_MMCR2,
> + /* Max regs without the extended regs */
> + PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> };
> +
> +#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
> +
> +/* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300 */
> +#define PERF_REG_PMU_MASK_300 (((1ULL << (PERF_REG_POWERPC_MMCR2 + 1)) - 1) - PERF_REG_PMU_MASK)
> +
> +#define PERF_REG_MAX_ISA_300 (PERF_REG_POWERPC_MMCR2 + 1)
> #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 31c0535..d5a9529 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2316,6 +2316,7 @@ int register_power_pmu(struct power_pmu *pmu)
> pmu->name);
>
> power_pmu.attr_groups = ppmu->attr_groups;
> + power_pmu.capabilities |= (ppmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS);
>
> #ifdef MSR_HV
> /*
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index a213a0a..b0cf68f 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -13,9 +13,11 @@
> #include <asm/ptrace.h>
> #include <asm/perf_regs.h>
>
> +u64 PERF_REG_EXTENDED_MASK;
> +
> #define PT_REGS_OFFSET(id, r) [id] = offsetof(struct pt_regs, r)
>
> -#define REG_RESERVED (~((1ULL << PERF_REG_POWERPC_MAX) - 1))
> +#define REG_RESERVED (~(PERF_REG_EXTENDED_MASK | PERF_REG_PMU_MASK))
>
> static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
> PT_REGS_OFFSET(PERF_REG_POWERPC_R0, gpr[0]),
> @@ -69,10 +71,26 @@
> PT_REGS_OFFSET(PERF_REG_POWERPC_MMCRA, dsisr),
> };
>
> +/* Function to return the extended register values */
> +static u64 get_ext_regs_value(int idx)
> +{
> + switch (idx) {
> + case PERF_REG_POWERPC_MMCR0:
> + return mfspr(SPRN_MMCR0);
> + case PERF_REG_POWERPC_MMCR1:
> + return mfspr(SPRN_MMCR1);
> + case PERF_REG_POWERPC_MMCR2:
> + return mfspr(SPRN_MMCR2);
> + default: return 0;
> + }
> +}
> +
> u64 perf_reg_value(struct pt_regs *regs, int idx)
> {
> - if (WARN_ON_ONCE(idx >= PERF_REG_POWERPC_MAX))
> - return 0;
> + u64 PERF_REG_EXTENDED_MAX;
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_300))
> + PERF_REG_EXTENDED_MAX = PERF_REG_MAX_ISA_300;
>
> if (idx == PERF_REG_POWERPC_SIER &&
> (IS_ENABLED(CONFIG_FSL_EMB_PERF_EVENT) ||
> @@ -85,6 +103,16 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
> IS_ENABLED(CONFIG_PPC32)))
> return 0;
>
> + if (idx >= PERF_REG_POWERPC_MAX && idx < PERF_REG_EXTENDED_MAX)
> + return get_ext_regs_value(idx);
> +
> + /*
> + * If the idx is referring to value beyond the
> + * supported registers, return 0 with a warning
> + */
> + if (WARN_ON_ONCE(idx >= PERF_REG_EXTENDED_MAX))
> + return 0;
> +
> return regs_get_register(regs, pt_regs_offset[idx]);
> }
>
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 05dae38..2a57e93 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -90,6 +90,8 @@ enum {
> #define POWER9_MMCRA_IFM3 0x00000000C0000000UL
> #define POWER9_MMCRA_BHRB_MASK 0x00000000C0000000UL
>
> +extern u64 PERF_REG_EXTENDED_MASK;
> +
> /* Nasty Power9 specific hack */
> #define PVR_POWER9_CUMULUS 0x00002000
>
> @@ -434,6 +436,7 @@ static void power9_config_bhrb(u64 pmu_bhrb_filter)
> .cache_events = &power9_cache_events,
> .attr_groups = power9_pmu_attr_groups,
> .bhrb_nr = 32,
> + .capabilities = PERF_PMU_CAP_EXTENDED_REGS,
> };
>
> int init_power9_pmu(void)
> @@ -457,6 +460,9 @@ int init_power9_pmu(void)
> }
> }
>
> + /* Set the PERF_REG_EXTENDED_MASK here */
> + PERF_REG_EXTENDED_MASK = PERF_REG_PMU_MASK_300;
> +
> rc = register_power_pmu(&power9_pmu);
> if (rc)
> return rc;
>
^ permalink raw reply
* ASMedia ASM2142 USB host controller tries to DMA to address zero when doing bulk reads from multiple devices
From: Forest Crossman @ 2020-07-21 5:49 UTC (permalink / raw)
To: linux-usb; +Cc: linuxppc-dev
Hello, again!
After fixing the issue in my previous thread using this patch[1], I
decided to do some stress-testing of the controller to make sure it
could handle my intended workloads and that there were no further DMA
address issues that would need to be fixed. Unfortunately, it looks
like there's still more work to be done: when I try to do long bulk
reads from multiple devices simultaneously, eventually the host
controller sends a DMA write to address zero, which then triggers EEH
in my POWER9 system, causing the controller card to get hotplug-reset,
which of course kills the disk-reading processes. For more details on
the EEH errors, you can see my kernel's EEH message log[2]. The
results of the various tests I performed are listed below.
Test results (all failures are due to DMA writes to address zero, all
hubs are USB 3.0/3.1 Gen1 only, and all disks are accessed via the
usb-storage driver):
- Reading simultaneously from two or more disks behind a hub connected
to one port on the host controller:
- FAIL after 20-50 GB of data transferred for each device.
- Reading simultaneously from two disks, each connected directly to
one port on the host controller:
- FAIL after about 800 GB of data transferred for each device.
- Reading from one disk behind a hub connected to one port on the host
controller:
- OK for at least 2.7 TB of data transferred (I didn't test the
whole 8 TB disk).
- Writing simultaneously to two FL2000 dongles (using osmo-fl2k's
"fl2k_test"), each connected directly to one port on the host
controller:
- OK, was able to write several dozen terabytes to each device over
the course of a little over 21 hours.
Seeing how simultaneous writes to multiple devices and reads from
single devices both seem to work fine, I assume that means this is
being caused by some race condition in the host controller firmware
when it responds to multiple read requests. I also assume we're not
going to be able to convince ASMedia to both fix the bug in their
firmware and release the details on how to flash it from Linux, so I
guess we'll just have to figure out how to make the driver talk to the
controller in a way that avoids triggering the bad DMA write. As
before, I decided to try a little kernel hacking of my own before
sending this email, and tried separately enabling the
XHCI_BROKEN_STREAMS and XHCI_ASMEDIA_MODIFY_FLOWCONTROL quirks in an
attempt to fix this. As you might expect since you're reading this
message, neither of those quirks fixed the issue, nor did they even
make the transfers last any longer before failing.
So now I've reached the limits of my understanding, and I need some
help devising a fix. If anyone has any comments to that effect, or any
questions about my hardware configuration, testing methodology, etc.,
please don't hesitate to air them. Also, if anyone needs me to perform
additional tests, or collect more log information, I'd be happy to do
that as well.
Thanks in advance for your help,
Forest
[1]: https://patchwork.kernel.org/patch/11669989/
[2]: https://paste.debian.net/hidden/2a442aa6
^ permalink raw reply
* Re: [PATCH 2/2] KVM: PPC: Book3S HV: rework secure mem slot dropping
From: Paul Mackerras @ 2020-07-21 5:46 UTC (permalink / raw)
To: Laurent Dufour
Cc: Ram Pai, linux-kernel, kvm-ppc, bharata, sathnaga, sukadev,
linuxppc-dev, bauerman
In-Reply-To: <0588d16a-8548-0f55-1132-400807a390a1@linux.ibm.com>
On Wed, Jul 08, 2020 at 02:16:36PM +0200, Laurent Dufour wrote:
> Le 08/07/2020 à 13:25, Bharata B Rao a écrit :
> > On Fri, Jul 03, 2020 at 05:59:14PM +0200, Laurent Dufour wrote:
> > > When a secure memslot is dropped, all the pages backed in the secure device
> > > (aka really backed by secure memory by the Ultravisor) should be paged out
> > > to a normal page. Previously, this was achieved by triggering the page
> > > fault mechanism which is calling kvmppc_svm_page_out() on each pages.
> > >
> > > This can't work when hot unplugging a memory slot because the memory slot
> > > is flagged as invalid and gfn_to_pfn() is then not trying to access the
> > > page, so the page fault mechanism is not triggered.
> > >
> > > Since the final goal is to make a call to kvmppc_svm_page_out() it seems
> > > simpler to directly calling it instead of triggering such a mechanism. This
> > > way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
> > > memslot.
> >
> > Yes, this appears much simpler.
>
> Thanks Bharata for reviewing this.
>
> >
> > >
> > > Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
> > > the call to __kvmppc_svm_page_out() is made.
> > > As __kvmppc_svm_page_out needs the vma pointer to migrate the pages, the
> > > VMA is fetched in a lazy way, to not trigger find_vma() all the time. In
> > > addition, the mmap_sem is help in read mode during that time, not in write
> > > mode since the virual memory layout is not impacted, and
> > > kvm->arch.uvmem_lock prevents concurrent operation on the secure device.
> > >
> > > Cc: Ram Pai <linuxram@us.ibm.com>
> > > Cc: Bharata B Rao <bharata@linux.ibm.com>
> > > Cc: Paul Mackerras <paulus@ozlabs.org>
> > > Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> > > ---
> > > arch/powerpc/kvm/book3s_hv_uvmem.c | 54 ++++++++++++++++++++----------
> > > 1 file changed, 37 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
> > > index 852cc9ae6a0b..479ddf16d18c 100644
> > > --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
> > > +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
> > > @@ -533,35 +533,55 @@ static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,
> > > * fault on them, do fault time migration to replace the device PTEs in
> > > * QEMU page table with normal PTEs from newly allocated pages.
> > > */
> > > -void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
> > > +void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *slot,
> > > struct kvm *kvm, bool skip_page_out)
> > > {
> > > int i;
> > > struct kvmppc_uvmem_page_pvt *pvt;
> > > - unsigned long pfn, uvmem_pfn;
> > > - unsigned long gfn = free->base_gfn;
> > > + struct page *uvmem_page;
> > > + struct vm_area_struct *vma = NULL;
> > > + unsigned long uvmem_pfn, gfn;
> > > + unsigned long addr, end;
> > > +
> > > + down_read(&kvm->mm->mmap_sem);
> >
> > You should be using mmap_read_lock(kvm->mm) with recent kernels.
>
> Absolutely, shame on me, I reviewed Michel's series about that!
>
> Paul, Michael, could you fix that when pulling this patch or should I sent a
> whole new series?
Given that Ram has reworked his series, I think it would be best if
you rebase on top of his new series and re-send your series.
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: Use feature flag CPU_FTR_P9_TIDR when accessing TIDR
From: David Gibson @ 2020-07-21 5:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, linuxppc-dev, Cédric Le Goater, kvm
In-Reply-To: <20200721050445.GA3878639@thinks.paulus.ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]
On Tue, Jul 21, 2020 at 03:04:45PM +1000, Paul Mackerras wrote:
> On Tue, Jun 23, 2020 at 06:50:27PM +0200, Cédric Le Goater wrote:
> > The TIDR register is only available on POWER9 systems and code
> > accessing this register is not always protected by the CPU_FTR_P9_TIDR
> > flag. Fix that to make sure POWER10 systems won't use it as TIDR has
> > been removed.
>
> I'm concerned about what this patch would do if we are trying to
> migrate from a P9 guest to a guest on P10 in P9-compat mode, in that
> the destination QEMU would get an error on doing the SET_ONE_REG for
> the TIDR. I don't think the lack of TIDR is worth failing the
> migration for given that TIDR only actually does anything if you are
> using an accelerator, and KVM has never supported use of accelerators
> in guests. I'm cc'ing David Gibson for his comments on the
> compatibility and migration issues.
Having thought about this a bit more, I don't think it matters. We're
going to have to update qemu to handle POWER10 anyway. If this causes
a problem, this would just add one small thing to whatever we need to
fix there.
> In any case, given that both move to and move from TIDR will be no-ops
> on P10 (for privileged code), I don't think there is a great urgency
> for this patch.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: Use feature flag CPU_FTR_P9_TIDR when accessing TIDR
From: Paul Mackerras @ 2020-07-21 5:04 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev, kvm-ppc, kvm, David Gibson
In-Reply-To: <20200623165027.271215-1-clg@kaod.org>
On Tue, Jun 23, 2020 at 06:50:27PM +0200, Cédric Le Goater wrote:
> The TIDR register is only available on POWER9 systems and code
> accessing this register is not always protected by the CPU_FTR_P9_TIDR
> flag. Fix that to make sure POWER10 systems won't use it as TIDR has
> been removed.
I'm concerned about what this patch would do if we are trying to
migrate from a P9 guest to a guest on P10 in P9-compat mode, in that
the destination QEMU would get an error on doing the SET_ONE_REG for
the TIDR. I don't think the lack of TIDR is worth failing the
migration for given that TIDR only actually does anything if you are
using an accelerator, and KVM has never supported use of accelerators
in guests. I'm cc'ing David Gibson for his comments on the
compatibility and migration issues.
In any case, given that both move to and move from TIDR will be no-ops
on P10 (for privileged code), I don't think there is a great urgency
for this patch.
Paul.
^ permalink raw reply
* Re: [PATCH v4 5/7] powerpc/iommu: Move iommu_table cleaning routine to iommu_table_clean
From: Alexey Kardashevskiy @ 2020-07-21 4:59 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200716071658.467820-6-leobras.c@gmail.com>
On 16/07/2020 17:16, Leonardo Bras wrote:
> Move the part of iommu_table_free() that does struct iommu_table cleaning
> into iommu_table_clean, so we can invoke it separately.
>
> This new function is useful for cleaning struct iommu_table before
> initializing it again with a new DMA window, without having it freed and
> allocated again.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/kernel/iommu.c | 30 ++++++++++++++++++------------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 9704f3f76e63..c3242253a4e7 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -735,21 +735,10 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid,
> return tbl;
> }
>
> -static void iommu_table_free(struct kref *kref)
> +static void iommu_table_clean(struct iommu_table *tbl)
iommu_table_free() + iommu_init_table() + set_iommu_table_base() should
work too, why new helper?
There is also iommu_table_clear() which does a different thing so you
need a better name.
Second, iommu_table_free() would print a warning if any IOMMU page is in
use and it would be ok as we would only see this when hot-unplugging a
PE because we always kept the default window.
Btw you must be seeing these warnings now every time you create DDW with
these patches as at least the first page is reserved, do not you?
Since we are replacing a table for a device which is still in the
system, we should not try messing with its DMA if it already has
mappings so the warning should become an error preventing DDW. It is
rather hard to trigger in practice but I could hack a driver to ask for
32bit DMA mask first, map few pages and then ask for 64bit DMA mask, it
is not illegal, I think. So this needs a new helper - "bool
iommu_table_in_use(tbl)" - to use in enable_ddw(). Or I am overthinking
this?... Thanks,
> {
> unsigned long bitmap_sz;
> unsigned int order;
> - struct iommu_table *tbl;
> -
> - tbl = container_of(kref, struct iommu_table, it_kref);
> -
> - if (tbl->it_ops->free)
> - tbl->it_ops->free(tbl);
> -
> - if (!tbl->it_map) {
> - kfree(tbl);
> - return;
> - }
>
> iommu_table_release_pages(tbl);
>
> @@ -763,6 +752,23 @@ static void iommu_table_free(struct kref *kref)
> /* free bitmap */
> order = get_order(bitmap_sz);
> free_pages((unsigned long) tbl->it_map, order);
> +}
> +
> +static void iommu_table_free(struct kref *kref)
> +{
> + struct iommu_table *tbl;
> +
> + tbl = container_of(kref, struct iommu_table, it_kref);
> +
> + if (tbl->it_ops->free)
> + tbl->it_ops->free(tbl);
> +
> + if (!tbl->it_map) {
> + kfree(tbl);
> + return;
> + }
> +
> + iommu_table_clean(tbl);
>
> /* free table */
> kfree(tbl);
>
--
Alexey
^ permalink raw reply
* Re: [v3 02/15] KVM: PPC: Book3S HV: Cleanup updates for kvm vcpu MMCR
From: Paul Mackerras @ 2020-07-21 3:54 UTC (permalink / raw)
To: Athira Rajeev
Cc: ego, mikey, maddy, kvm, kvm-ppc, svaidyan, acme, jolsa,
linuxppc-dev
In-Reply-To: <1594996707-3727-3-git-send-email-atrajeev@linux.vnet.ibm.com>
On Fri, Jul 17, 2020 at 10:38:14AM -0400, Athira Rajeev wrote:
> Currently `kvm_vcpu_arch` stores all Monitor Mode Control registers
> in a flat array in order: mmcr0, mmcr1, mmcra, mmcr2, mmcrs
> Split this to give mmcra and mmcrs its own entries in vcpu and
> use a flat array for mmcr0 to mmcr2. This patch implements this
> cleanup to make code easier to read.
Changing the way KVM stores these values internally is fine, but
changing the user ABI is not. This part:
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 264e266..e55d847 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -510,8 +510,8 @@ struct kvm_ppc_cpu_char {
>
> #define KVM_REG_PPC_MMCR0 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
> #define KVM_REG_PPC_MMCR1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
> -#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
> -#define KVM_REG_PPC_MMCR2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x13)
> +#define KVM_REG_PPC_MMCR2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
> +#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x13)
means that existing userspace programs that used to work would now be
broken. That is not acceptable (breaking the user ABI is only ever
acceptable with a very compelling reason). So NAK to this part of the
patch.
Regards,
Paul.
^ permalink raw reply
* Re: [PATCH v3 0/4] powerpc/mm/radix: Memory unplug fixes
From: Aneesh Kumar K.V @ 2020-07-21 4:42 UTC (permalink / raw)
To: Michael Ellerman, Nathan Lynch; +Cc: linuxppc-dev, Bharata B Rao
In-Reply-To: <87tuy1sksv.fsf@mpe.ellerman.id.au>
On 7/21/20 7:15 AM, Michael Ellerman wrote:
> Nathan Lynch <nathanl@linux.ibm.com> writes:
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>> This is the next version of the fixes for memory unplug on radix.
>>> The issues and the fix are described in the actual patches.
>>
>> I guess this isn't actually causing problems at runtime right now, but I
>> notice calls to resize_hpt_for_hotplug() from arch_add_memory() and
>> arch_remove_memory(), which ought to be mmu-agnostic:
>>
>> int __ref arch_add_memory(int nid, u64 start, u64 size,
>> struct mhp_params *params)
>> {
>> unsigned long start_pfn = start >> PAGE_SHIFT;
>> unsigned long nr_pages = size >> PAGE_SHIFT;
>> int rc;
>>
>> resize_hpt_for_hotplug(memblock_phys_mem_size());
>>
>> start = (unsigned long)__va(start);
>> rc = create_section_mapping(start, start + size, nid,
>> params->pgprot);
>> ...
>
> Hmm well spotted.
>
> That does return early if the ops are not setup:
>
> int resize_hpt_for_hotplug(unsigned long new_mem_size)
> {
> unsigned target_hpt_shift;
>
> if (!mmu_hash_ops.resize_hpt)
> return 0;
>
>
> And:
>
> void __init hpte_init_pseries(void)
> {
> ...
> if (firmware_has_feature(FW_FEATURE_HPT_RESIZE))
> mmu_hash_ops.resize_hpt = pseries_lpar_resize_hpt;
>
> And that comes in via ibm,hypertas-functions:
>
> {FW_FEATURE_HPT_RESIZE, "hcall-hpt-resize"},
>
>
> But firmware is not necessarily going to add/remove that call based on
> whether we're using hash/radix.
>
We are good there because hpte_init_pseries is only called for hash
translation.
early_init_mmu()
-> hash__early_init_mmu
-> hpte_init_pseries
-> mmu_hash_ops.resize_hpt = pseries_lpar_resize_hpt;
> So I think a follow-up patch is needed to make this more robust.
>
> Aneesh/Bharata what platform did you test this series on? I'm curious
> how this didn't break.
All the changes are tested with kvm.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 09/10] powerpc/watchpoint: Return available watchpoints dynamically
From: Jordan Niethe @ 2020-07-21 4:41 UTC (permalink / raw)
To: Ravi Bangoria
Cc: Christophe Leroy, miltonm, mikey, peterz, oleg, Nicholas Piggin,
linux-kernel, Paul Mackerras, jolsa, fweisbec, pedromfc,
naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <ccfcf488-0ec9-1737-8368-a848de1d72d1@linux.ibm.com>
On Tue, Jul 21, 2020 at 1:57 PM Ravi Bangoria
<ravi.bangoria@linux.ibm.com> wrote:
>
>
>
> On 7/20/20 9:12 AM, Jordan Niethe wrote:
> > On Fri, Jul 17, 2020 at 2:11 PM Ravi Bangoria
> > <ravi.bangoria@linux.ibm.com> wrote:
> >>
> >> So far Book3S Powerpc supported only one watchpoint. Power10 is
> >> introducing 2nd DAWR. Enable 2nd DAWR support for Power10.
> >> Availability of 2nd DAWR will depend on CPU_FTR_DAWR1.
> >>
> >> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> >> ---
> >> arch/powerpc/include/asm/cputable.h | 4 +++-
> >> arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
> >> 2 files changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> >> index 3445c86e1f6f..36a0851a7a9b 100644
> >> --- a/arch/powerpc/include/asm/cputable.h
> >> +++ b/arch/powerpc/include/asm/cputable.h
> >> @@ -633,7 +633,9 @@ enum {
> >> * Maximum number of hw breakpoint supported on powerpc. Number of
> >> * breakpoints supported by actual hw might be less than this.
> >> */
> >> -#define HBP_NUM_MAX 1
> >> +#define HBP_NUM_MAX 2
> >> +#define HBP_NUM_ONE 1
> >> +#define HBP_NUM_TWO 2
> > I wonder if these defines are necessary - has it any advantage over
> > just using the literal?
>
> No, not really. Initially I had something like:
>
> #define HBP_NUM_MAX 2
> #define HBP_NUM_P8_P9 1
> #define HBP_NUM_P10 2
>
> But then I thought it's also not right. So I made it _ONE and _TWO.
> Now the function that decides nr watchpoints dynamically (nr_wp_slots)
> is in different file, I thought to keep it like this so it would be
> easier to figure out why _MAX is 2.
>
> >>
> >> #endif /* !__ASSEMBLY__ */
> >>
> >> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> >> index cb424799da0d..d4eab1694bcd 100644
> >> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> >> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> >> @@ -5,10 +5,11 @@
> >> * Copyright 2010, IBM Corporation.
> >> * Author: K.Prasad <prasad@linux.vnet.ibm.com>
> >> */
> >> -
> > Was removing this line deliberate?
>
> Nah. Will remove that hunk.
>
> >> #ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
> >> #define _PPC_BOOK3S_64_HW_BREAKPOINT_H
> >>
> >> +#include <asm/cpu_has_feature.h>
> >> +
> >> #ifdef __KERNEL__
> >> struct arch_hw_breakpoint {
> >> unsigned long address;
> >> @@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
> >>
> >> static inline int nr_wp_slots(void)
> >> {
> >> - return HBP_NUM_MAX;
> >> + return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
> > So it'd be something like:
> > + return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_MAX : 1;
> > But thinking that there might be more slots added in the future, it
> > may be better to make the number of slots a variable that is set
> > during the init and then have this function return that.
>
> Not sure I follow. What do you mean by setting number of slots a
> variable that is set during the init?
Sorry I was unclear there.
I was just looking and saw arm also has a variable number of hw breakpoints.
If we did something like how they handle it, it might look something like:
static int num_wp_slots __ro_after_init;
int nr_wp_slots(void) {
return num_wp_slots;
}
static int __init arch_hw_breakpoint_init(void) {
num_wp_slots = work out how many wp_slots
}
arch_initcall(arch_hw_breakpoint_init);
Then we wouldn't have to calculate everytime nr_wp_slots() is called.
In the future if more wp's are added nr_wp_slots() will get more complicated.
But just an idea, feel free to ignore.
>
> Thanks,
> Ravi
^ permalink raw reply
* Re: [PATCH v2 13/16] scripts/kallsyms: move ignored symbol types to is_ignored_symbol()
From: Finn Thain @ 2020-07-21 4:00 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linuxppc-dev, Linux Kernel Mailing List,
Linux Kbuild mailing list
In-Reply-To: <CAK7LNATVDfHJG=+G6DqkR-vSyVhb8KmxcT0ae+ukdi_otthi8A@mail.gmail.com>
On Mon, 20 Jul 2020, Masahiro Yamada wrote:
>
> I got a similar report before.
>
> I'd like to know whether or not
> this is the same issue as fixed by
> 7883a14339299773b2ce08dcfd97c63c199a9289
>
The problem can be observed with 3d77e6a8804ab ("Linux 5.7").
So it appears that 7883a14339299 ("scripts/kallsyms: fix wrong
kallsyms_relative_base") is not sufficient to fix it.
>
> Does your problem happen on the latest kernel?
Unfortunately this cross compiler (gcc 4.6.4) is too old to build
v5.8-rc1 or later. I will have to upgrade.
> Which version of binutils are you using?
>
This toolchain uses binutils 2.22.
In case it helps,
$ powerpc-linux-gnu-nm -n vmlinux |head
w mach_chrp
00000005 a LG_CACHELINE_BYTES
00000005 a LG_CACHELINE_BYTES
00000005 a LG_CACHELINE_BYTES
0000000c a Hash_bits
0000001f a CACHELINE_MASK
0000001f a CACHELINE_MASK
0000001f a CACHELINE_MASK
00000020 a CACHELINE_BYTES
00000020 a CACHELINE_BYTES
00000020 a CACHELINE_BYTES
00000020 a reg
0003ffc0 a Hash_msk
c0000000 T _start
c0000000 A _stext
c0000000 A _text
c000000c T __start
c0000054 t __after_mmu_off
c0000090 t turn_on_mmu
c00000c4 T __secondary_hold
c00000dc T __secondary_hold_spinloop
c00000e0 T __secondary_hold_acknowledge
c0000100 t Reset
^ permalink raw reply
* Re: [PATCH v4 09/10] powerpc/watchpoint: Return available watchpoints dynamically
From: Ravi Bangoria @ 2020-07-21 3:57 UTC (permalink / raw)
To: Jordan Niethe
Cc: Christophe Leroy, apopple, mikey, miltonm, peterz, oleg,
Nicholas Piggin, linux-kernel, Paul Mackerras, jolsa, fweisbec,
pedromfc, naveen.n.rao, linuxppc-dev, mingo, Ravi Bangoria
In-Reply-To: <CACzsE9r0acLUkV35mVxy1AEK_xObs0yz+fD6UdbNdc6uz=Buqw@mail.gmail.com>
On 7/20/20 9:12 AM, Jordan Niethe wrote:
> On Fri, Jul 17, 2020 at 2:11 PM Ravi Bangoria
> <ravi.bangoria@linux.ibm.com> wrote:
>>
>> So far Book3S Powerpc supported only one watchpoint. Power10 is
>> introducing 2nd DAWR. Enable 2nd DAWR support for Power10.
>> Availability of 2nd DAWR will depend on CPU_FTR_DAWR1.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/cputable.h | 4 +++-
>> arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
>> index 3445c86e1f6f..36a0851a7a9b 100644
>> --- a/arch/powerpc/include/asm/cputable.h
>> +++ b/arch/powerpc/include/asm/cputable.h
>> @@ -633,7 +633,9 @@ enum {
>> * Maximum number of hw breakpoint supported on powerpc. Number of
>> * breakpoints supported by actual hw might be less than this.
>> */
>> -#define HBP_NUM_MAX 1
>> +#define HBP_NUM_MAX 2
>> +#define HBP_NUM_ONE 1
>> +#define HBP_NUM_TWO 2
> I wonder if these defines are necessary - has it any advantage over
> just using the literal?
No, not really. Initially I had something like:
#define HBP_NUM_MAX 2
#define HBP_NUM_P8_P9 1
#define HBP_NUM_P10 2
But then I thought it's also not right. So I made it _ONE and _TWO.
Now the function that decides nr watchpoints dynamically (nr_wp_slots)
is in different file, I thought to keep it like this so it would be
easier to figure out why _MAX is 2.
>>
>> #endif /* !__ASSEMBLY__ */
>>
>> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
>> index cb424799da0d..d4eab1694bcd 100644
>> --- a/arch/powerpc/include/asm/hw_breakpoint.h
>> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
>> @@ -5,10 +5,11 @@
>> * Copyright 2010, IBM Corporation.
>> * Author: K.Prasad <prasad@linux.vnet.ibm.com>
>> */
>> -
> Was removing this line deliberate?
Nah. Will remove that hunk.
>> #ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
>> #define _PPC_BOOK3S_64_HW_BREAKPOINT_H
>>
>> +#include <asm/cpu_has_feature.h>
>> +
>> #ifdef __KERNEL__
>> struct arch_hw_breakpoint {
>> unsigned long address;
>> @@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
>>
>> static inline int nr_wp_slots(void)
>> {
>> - return HBP_NUM_MAX;
>> + return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
> So it'd be something like:
> + return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_MAX : 1;
> But thinking that there might be more slots added in the future, it
> may be better to make the number of slots a variable that is set
> during the init and then have this function return that.
Not sure I follow. What do you mean by setting number of slots a
variable that is set during the init?
Thanks,
Ravi
^ permalink raw reply
* [PATCH 1/2] ASoC: fsl-asoc-card: Support configuring dai fmt from DT
From: Shengjiu Wang @ 2020-07-21 3:41 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
Support same propeties as simple card for configuring fmt
from DT.
In order to make this change compatible with old DT, these
properties are optional.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl-asoc-card.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index ee80d02b56c6..4848ba61d083 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -531,11 +531,14 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
struct device_node *cpu_np, *codec_np, *asrc_np;
struct device_node *np = pdev->dev.of_node;
struct platform_device *asrc_pdev = NULL;
+ struct device_node *bitclkmaster = NULL;
+ struct device_node *framemaster = NULL;
struct platform_device *cpu_pdev;
struct fsl_asoc_card_priv *priv;
struct device *codec_dev = NULL;
const char *codec_dai_name;
const char *codec_dev_name;
+ unsigned int daifmt;
u32 width;
int ret;
@@ -667,6 +670,31 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
goto asrc_fail;
}
+ /* Format info from DT is optional. */
+ daifmt = snd_soc_of_parse_daifmt(np, NULL,
+ &bitclkmaster, &framemaster);
+ daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+ if (bitclkmaster || framemaster) {
+ if (codec_np == bitclkmaster)
+ daifmt |= (codec_np == framemaster) ?
+ SND_SOC_DAIFMT_CBM_CFM : SND_SOC_DAIFMT_CBM_CFS;
+ else
+ daifmt |= (codec_np == framemaster) ?
+ SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;
+
+ /* Override dai_fmt with value from DT */
+ priv->dai_fmt = daifmt;
+ }
+
+ /* Change direction according to format */
+ if (priv->dai_fmt & SND_SOC_DAIFMT_CBM_CFM) {
+ priv->cpu_priv.sysclk_dir[TX] = SND_SOC_CLOCK_IN;
+ priv->cpu_priv.sysclk_dir[RX] = SND_SOC_CLOCK_IN;
+ }
+
+ of_node_put(bitclkmaster);
+ of_node_put(framemaster);
+
if (!fsl_asoc_card_is_ac97(priv) && !codec_dev) {
dev_err(&pdev->dev, "failed to find codec device\n");
ret = -EPROBE_DEFER;
--
2.27.0
^ permalink raw reply related
* [PATCH 2/2] ASoC: bindings: fsl-asoc-card: Support properties for configuring dai fmt
From: Shengjiu Wang @ 2020-07-21 3:41 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, robh+dt, devicetree
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1595302910-19688-1-git-send-email-shengjiu.wang@nxp.com>
In order to support configuring dai fmt through DT, add some properties.
These properiese are same as the properties in simple card.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index 8a6a3d0fda5e..63ebf52b43e8 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -71,6 +71,11 @@ Optional properties:
- hp-det-gpio : The GPIO that detect headphones are plugged in
- mic-det-gpio : The GPIO that detect microphones are plugged in
+ - bitclock-master : Indicates dai-link bit clock master; for details see simple-card.yaml.
+ - frame-master : Indicates dai-link frame master; for details see simple-card.yaml.
+ - dai-format : audio format, for details see simple-card.yaml.
+ - frame-inversion : dai-link uses frame clock inversion, for details see simple-card.yaml.
+ - bitclock-inversion : dai-link uses bit clock inversion, for details see simple-card.yaml.
Optional unless SSI is selected as a CPU DAI:
--
2.27.0
^ permalink raw reply related
* Re: [v3 01/15] powerpc/perf: Update cpu_hw_event to use `struct` for storing MMCR registers
From: Jordan Niethe @ 2020-07-21 3:42 UTC (permalink / raw)
To: Athira Rajeev
Cc: Gautham R Shenoy, mikey, maddy, kvm, kvm-ppc, svaidyan, acme,
jolsa, linuxppc-dev
In-Reply-To: <1594996707-3727-2-git-send-email-atrajeev@linux.vnet.ibm.com>
On Sat, Jul 18, 2020 at 12:48 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
> core-book3s currently uses array to store the MMCR registers as part
> of per-cpu `cpu_hw_events`. This patch does a clean up to use `struct`
> to store mmcr regs instead of array. This will make code easier to read
> and reduces chance of any subtle bug that may come in the future, say
> when new registers are added. Patch updates all relevant code that was
> using MMCR array ( cpuhw->mmcr[x]) to use newly introduced `struct`.
> This includes the PMU driver code for supported platforms (power5
> to power9) and ISA macros for counter support functions.
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/perf_event_server.h | 10 ++++--
> arch/powerpc/perf/core-book3s.c | 53 +++++++++++++---------------
> arch/powerpc/perf/isa207-common.c | 20 +++++------
> arch/powerpc/perf/isa207-common.h | 4 +--
> arch/powerpc/perf/mpc7450-pmu.c | 21 +++++++----
> arch/powerpc/perf/power5+-pmu.c | 17 ++++-----
> arch/powerpc/perf/power5-pmu.c | 17 ++++-----
> arch/powerpc/perf/power6-pmu.c | 16 ++++-----
> arch/powerpc/perf/power7-pmu.c | 17 ++++-----
> arch/powerpc/perf/ppc970-pmu.c | 24 ++++++-------
> 10 files changed, 105 insertions(+), 94 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 3e9703f..f9a3668 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -17,6 +17,12 @@
>
> struct perf_event;
>
> +struct mmcr_regs {
> + unsigned long mmcr0;
> + unsigned long mmcr1;
> + unsigned long mmcr2;
> + unsigned long mmcra;
> +};
> /*
> * This struct provides the constants and functions needed to
> * describe the PMU on a particular POWER-family CPU.
> @@ -28,7 +34,7 @@ struct power_pmu {
> unsigned long add_fields;
> unsigned long test_adder;
> int (*compute_mmcr)(u64 events[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[],
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> struct perf_event *pevents[]);
> int (*get_constraint)(u64 event_id, unsigned long *mskp,
> unsigned long *valp);
> @@ -41,7 +47,7 @@ struct power_pmu {
> unsigned long group_constraint_val;
> u64 (*bhrb_filter_map)(u64 branch_sample_type);
> void (*config_bhrb)(u64 pmu_bhrb_filter);
> - void (*disable_pmc)(unsigned int pmc, unsigned long mmcr[]);
> + void (*disable_pmc)(unsigned int pmc, struct mmcr_regs *mmcr);
> int (*limited_pmc_event)(u64 event_id);
> u32 flags;
> const struct attribute_group **attr_groups;
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index cd6a742..18b1b6a 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -37,12 +37,7 @@ struct cpu_hw_events {
> struct perf_event *event[MAX_HWEVENTS];
> u64 events[MAX_HWEVENTS];
> unsigned int flags[MAX_HWEVENTS];
> - /*
> - * The order of the MMCR array is:
> - * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
> - * - 32-bit, MMCR0, MMCR1, MMCR2
> - */
> - unsigned long mmcr[4];
> + struct mmcr_regs mmcr;
> struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS];
> u8 limited_hwidx[MAX_LIMITED_HWCOUNTERS];
> u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
> @@ -121,7 +116,7 @@ static void ebb_event_add(struct perf_event *event) { }
> static void ebb_switch_out(unsigned long mmcr0) { }
> static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
> {
> - return cpuhw->mmcr[0];
> + return cpuhw->mmcr.mmcr0;
> }
>
> static inline void power_pmu_bhrb_enable(struct perf_event *event) {}
> @@ -590,7 +585,7 @@ static void ebb_switch_out(unsigned long mmcr0)
>
> static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
> {
> - unsigned long mmcr0 = cpuhw->mmcr[0];
> + unsigned long mmcr0 = cpuhw->mmcr.mmcr0;
>
> if (!ebb)
> goto out;
> @@ -624,7 +619,7 @@ static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
> * unfreeze counters, it should not set exclude_xxx in its events and
> * instead manage the MMCR2 entirely by itself.
> */
> - mtspr(SPRN_MMCR2, cpuhw->mmcr[3] | current->thread.mmcr2);
> + mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2 | current->thread.mmcr2);
> out:
> return mmcr0;
> }
> @@ -1232,9 +1227,9 @@ static void power_pmu_disable(struct pmu *pmu)
> /*
> * Disable instruction sampling if it was enabled
> */
> - if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
> + if (cpuhw->mmcr.mmcra & MMCRA_SAMPLE_ENABLE) {
> mtspr(SPRN_MMCRA,
> - cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
> + cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
> mb();
> isync();
> }
> @@ -1308,18 +1303,18 @@ static void power_pmu_enable(struct pmu *pmu)
> * (possibly updated for removal of events).
> */
> if (!cpuhw->n_added) {
> - mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
> - mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);
> + mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
> + mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);
> goto out_enable;
> }
>
> /*
> * Clear all MMCR settings and recompute them for the new set of events.
> */
> - memset(cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
> + memset(&cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
>
> if (ppmu->compute_mmcr(cpuhw->events, cpuhw->n_events, hwc_index,
> - cpuhw->mmcr, cpuhw->event)) {
> + &cpuhw->mmcr, cpuhw->event)) {
> /* shouldn't ever get here */
> printk(KERN_ERR "oops compute_mmcr failed\n");
> goto out;
> @@ -1333,11 +1328,11 @@ static void power_pmu_enable(struct pmu *pmu)
> */
> event = cpuhw->event[0];
> if (event->attr.exclude_user)
> - cpuhw->mmcr[0] |= MMCR0_FCP;
> + cpuhw->mmcr.mmcr0 |= MMCR0_FCP;
> if (event->attr.exclude_kernel)
> - cpuhw->mmcr[0] |= freeze_events_kernel;
> + cpuhw->mmcr.mmcr0 |= freeze_events_kernel;
> if (event->attr.exclude_hv)
> - cpuhw->mmcr[0] |= MMCR0_FCHV;
> + cpuhw->mmcr.mmcr0 |= MMCR0_FCHV;
> }
>
> /*
> @@ -1346,12 +1341,12 @@ static void power_pmu_enable(struct pmu *pmu)
> * Then unfreeze the events.
> */
> ppc_set_pmu_inuse(1);
> - mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
> - mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);
> - mtspr(SPRN_MMCR0, (cpuhw->mmcr[0] & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))
> + mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
> + mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);
> + mtspr(SPRN_MMCR0, (cpuhw->mmcr.mmcr0 & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))
> | MMCR0_FC);
> if (ppmu->flags & PPMU_ARCH_207S)
> - mtspr(SPRN_MMCR2, cpuhw->mmcr[3]);
> + mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2);
>
> /*
> * Read off any pre-existing events that need to move
> @@ -1402,7 +1397,7 @@ static void power_pmu_enable(struct pmu *pmu)
> perf_event_update_userpage(event);
> }
> cpuhw->n_limited = n_lim;
> - cpuhw->mmcr[0] |= MMCR0_PMXE | MMCR0_FCECE;
> + cpuhw->mmcr.mmcr0 |= MMCR0_PMXE | MMCR0_FCECE;
>
> out_enable:
> pmao_restore_workaround(ebb);
> @@ -1418,9 +1413,9 @@ static void power_pmu_enable(struct pmu *pmu)
> /*
> * Enable instruction sampling if necessary
> */
> - if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
> + if (cpuhw->mmcr.mmcra & MMCRA_SAMPLE_ENABLE) {
> mb();
> - mtspr(SPRN_MMCRA, cpuhw->mmcr[2]);
> + mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra);
> }
>
> out:
> @@ -1550,7 +1545,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
> cpuhw->flags[i-1] = cpuhw->flags[i];
> }
> --cpuhw->n_events;
> - ppmu->disable_pmc(event->hw.idx - 1, cpuhw->mmcr);
> + ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
> if (event->hw.idx) {
> write_pmc(event->hw.idx, 0);
> event->hw.idx = 0;
> @@ -1571,7 +1566,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
> }
> if (cpuhw->n_events == 0) {
> /* disable exceptions if no events are running */
> - cpuhw->mmcr[0] &= ~(MMCR0_PMXE | MMCR0_FCECE);
> + cpuhw->mmcr.mmcr0 &= ~(MMCR0_PMXE | MMCR0_FCECE);
> }
>
> if (has_branch_stack(event))
> @@ -2240,7 +2235,7 @@ static void __perf_event_interrupt(struct pt_regs *regs)
> * XXX might want to use MSR.PM to keep the events frozen until
> * we get back out of this interrupt.
> */
> - write_mmcr0(cpuhw, cpuhw->mmcr[0]);
> + write_mmcr0(cpuhw, cpuhw->mmcr.mmcr0);
>
> if (nmi)
> nmi_exit();
> @@ -2262,7 +2257,7 @@ static int power_pmu_prepare_cpu(unsigned int cpu)
>
> if (ppmu) {
> memset(cpuhw, 0, sizeof(*cpuhw));
> - cpuhw->mmcr[0] = MMCR0_FC;
> + cpuhw->mmcr.mmcr0 = MMCR0_FC;
> }
> return 0;
> }
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 4c86da5..2fe63f2 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -363,7 +363,7 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
> }
>
> int isa207_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[],
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> struct perf_event *pevents[])
> {
> unsigned long mmcra, mmcr1, mmcr2, unit, combine, psel, cache, val;
> @@ -464,30 +464,30 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
> }
>
> /* Return MMCRx values */
> - mmcr[0] = 0;
> + mmcr->mmcr0 = 0;
>
> /* pmc_inuse is 1-based */
> if (pmc_inuse & 2)
> - mmcr[0] = MMCR0_PMC1CE;
> + mmcr->mmcr0 = MMCR0_PMC1CE;
>
> if (pmc_inuse & 0x7c)
> - mmcr[0] |= MMCR0_PMCjCE;
> + mmcr->mmcr0 |= MMCR0_PMCjCE;
>
> /* If we're not using PMC 5 or 6, freeze them */
> if (!(pmc_inuse & 0x60))
> - mmcr[0] |= MMCR0_FC56;
> + mmcr->mmcr0 |= MMCR0_FC56;
>
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcra;
> - mmcr[3] = mmcr2;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcra = mmcra;
> + mmcr->mmcr2 = mmcr2;
>
> return 0;
> }
>
> -void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +void isa207_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> if (pmc <= 3)
> - mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
> + mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
> }
>
> static int find_alternative(u64 event, const unsigned int ev_alt[][MAX_ALT], int size)
> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> index 63fd4f3..df968fd 100644
> --- a/arch/powerpc/perf/isa207-common.h
> +++ b/arch/powerpc/perf/isa207-common.h
> @@ -217,9 +217,9 @@
>
> int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp);
> int isa207_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[],
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> struct perf_event *pevents[]);
> -void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[]);
> +void isa207_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr);
> int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
> const unsigned int ev_alt[][MAX_ALT]);
> void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
> index 4d5ef92..826de25 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -257,7 +257,7 @@ static int mpc7450_get_alternatives(u64 event, unsigned int flags, u64 alt[])
> * Compute MMCR0/1/2 values for a set of events.
> */
> static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],
> - unsigned long mmcr[],
> + struct mmcr_regs *mmcr,
> struct perf_event *pevents[])
> {
> u8 event_index[N_CLASSES][N_COUNTER];
> @@ -321,9 +321,16 @@ static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],
> mmcr0 |= MMCR0_PMCnCE;
>
> /* Return MMCRx values */
> - mmcr[0] = mmcr0;
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcr2;
> + mmcr->mmcr0 = mmcr0;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcr2 = mmcr2;
Will this mmcr->mmcr2 be used anywere, or will it always use mmcr->mmcra?
> + /*
> + * 32-bit doesn't have an MMCRA and uses SPRN_MMCR2 to define
> + * SPRN_MMCRA. So assign mmcra of cpu_hw_events with `mmcr2`
> + * value to ensure that any write to this SPRN_MMCRA will
> + * use mmcr2 value.
> + */
Something like this comment would probably be useful to near the struct mmcr.
> + mmcr->mmcra = mmcr2;
> return 0;
> }
>
> @@ -331,12 +338,12 @@ static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],
> * Disable counting by a PMC.
> * Note that the pmc argument is 0-based here, not 1-based.
> */
> -static void mpc7450_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +static void mpc7450_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> if (pmc <= 1)
> - mmcr[0] &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
> + mmcr->mmcr0 &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
> else
> - mmcr[1] &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
> + mmcr->mmcr1 &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
> }
>
> static int mpc7450_generic_events[] = {
> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
> index f857454..5f0821e 100644
> --- a/arch/powerpc/perf/power5+-pmu.c
> +++ b/arch/powerpc/perf/power5+-pmu.c
> @@ -448,7 +448,8 @@ static int power5p_marked_instr_event(u64 event)
> }
>
> static int power5p_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> + struct perf_event *pevents[])
> {
> unsigned long mmcr1 = 0;
> unsigned long mmcra = 0;
> @@ -586,20 +587,20 @@ static int power5p_compute_mmcr(u64 event[], int n_ev,
> }
>
> /* Return MMCRx values */
> - mmcr[0] = 0;
> + mmcr->mmcr0 = 0;
> if (pmc_inuse & 1)
> - mmcr[0] = MMCR0_PMC1CE;
> + mmcr->mmcr0 = MMCR0_PMC1CE;
> if (pmc_inuse & 0x3e)
> - mmcr[0] |= MMCR0_PMCjCE;
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcra;
> + mmcr->mmcr0 |= MMCR0_PMCjCE;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcra = mmcra;
> return 0;
> }
>
> -static void power5p_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +static void power5p_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> if (pmc <= 3)
> - mmcr[1] &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
> + mmcr->mmcr1 &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
> }
>
> static int power5p_generic_events[] = {
> diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
> index da52eca..426021d 100644
> --- a/arch/powerpc/perf/power5-pmu.c
> +++ b/arch/powerpc/perf/power5-pmu.c
> @@ -379,7 +379,8 @@ static int power5_marked_instr_event(u64 event)
> }
>
> static int power5_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> + struct perf_event *pevents[])
> {
> unsigned long mmcr1 = 0;
> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
> @@ -528,20 +529,20 @@ static int power5_compute_mmcr(u64 event[], int n_ev,
> }
>
> /* Return MMCRx values */
> - mmcr[0] = 0;
> + mmcr->mmcr0 = 0;
> if (pmc_inuse & 1)
> - mmcr[0] = MMCR0_PMC1CE;
> + mmcr->mmcr0 = MMCR0_PMC1CE;
> if (pmc_inuse & 0x3e)
> - mmcr[0] |= MMCR0_PMCjCE;
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcra;
> + mmcr->mmcr0 |= MMCR0_PMCjCE;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcra = mmcra;
> return 0;
> }
>
> -static void power5_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +static void power5_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> if (pmc <= 3)
> - mmcr[1] &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
> + mmcr->mmcr1 &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
> }
>
> static int power5_generic_events[] = {
> diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
> index 3929cac..e343a51 100644
> --- a/arch/powerpc/perf/power6-pmu.c
> +++ b/arch/powerpc/perf/power6-pmu.c
> @@ -171,7 +171,7 @@ static int power6_marked_instr_event(u64 event)
> * Assign PMC numbers and compute MMCR1 value for a set of events
> */
> static int p6_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
> + unsigned int hwc[], struct mmcr_regs *mmcr, struct perf_event *pevents[])
> {
> unsigned long mmcr1 = 0;
> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
> @@ -243,13 +243,13 @@ static int p6_compute_mmcr(u64 event[], int n_ev,
> if (pmc < 4)
> mmcr1 |= (unsigned long)psel << MMCR1_PMCSEL_SH(pmc);
> }
> - mmcr[0] = 0;
> + mmcr->mmcr0 = 0;
> if (pmc_inuse & 1)
> - mmcr[0] = MMCR0_PMC1CE;
> + mmcr->mmcr0 = MMCR0_PMC1CE;
> if (pmc_inuse & 0xe)
> - mmcr[0] |= MMCR0_PMCjCE;
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcra;
> + mmcr->mmcr0 |= MMCR0_PMCjCE;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcra = mmcra;
> return 0;
> }
>
> @@ -457,11 +457,11 @@ static int p6_get_alternatives(u64 event, unsigned int flags, u64 alt[])
> return nalt;
> }
>
> -static void p6_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +static void p6_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> /* Set PMCxSEL to 0 to disable PMCx */
> if (pmc <= 3)
> - mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
> + mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
> }
>
> static int power6_generic_events[] = {
> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index a137813..3152336 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -242,7 +242,8 @@ static int power7_marked_instr_event(u64 event)
> }
>
> static int power7_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> + struct perf_event *pevents[])
> {
> unsigned long mmcr1 = 0;
> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
> @@ -298,20 +299,20 @@ static int power7_compute_mmcr(u64 event[], int n_ev,
> }
>
> /* Return MMCRx values */
> - mmcr[0] = 0;
> + mmcr->mmcr0 = 0;
> if (pmc_inuse & 1)
> - mmcr[0] = MMCR0_PMC1CE;
> + mmcr->mmcr0 = MMCR0_PMC1CE;
> if (pmc_inuse & 0x3e)
> - mmcr[0] |= MMCR0_PMCjCE;
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcra;
> + mmcr->mmcr0 |= MMCR0_PMCjCE;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcra = mmcra;
> return 0;
> }
>
> -static void power7_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +static void power7_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> if (pmc <= 3)
> - mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
> + mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
> }
>
> static int power7_generic_events[] = {
> diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
> index 4035d93..89a90ab 100644
> --- a/arch/powerpc/perf/ppc970-pmu.c
> +++ b/arch/powerpc/perf/ppc970-pmu.c
> @@ -253,7 +253,8 @@ static int p970_get_alternatives(u64 event, unsigned int flags, u64 alt[])
> }
>
> static int p970_compute_mmcr(u64 event[], int n_ev,
> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
> + unsigned int hwc[], struct mmcr_regs *mmcr,
> + struct perf_event *pevents[])
> {
> unsigned long mmcr0 = 0, mmcr1 = 0, mmcra = 0;
> unsigned int pmc, unit, byte, psel;
> @@ -393,27 +394,26 @@ static int p970_compute_mmcr(u64 event[], int n_ev,
> mmcra |= 0x2000; /* mark only one IOP per PPC instruction */
>
> /* Return MMCRx values */
> - mmcr[0] = mmcr0;
> - mmcr[1] = mmcr1;
> - mmcr[2] = mmcra;
> + mmcr->mmcr0 = mmcr0;
> + mmcr->mmcr1 = mmcr1;
> + mmcr->mmcra = mmcra;
> return 0;
> }
>
> -static void p970_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +static void p970_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
> {
> - int shift, i;
> + int shift;
>
> + /*
> + * Setting the PMCxSEL field to 0x08 disables PMC x.
> + */
> if (pmc <= 1) {
> shift = MMCR0_PMC1SEL_SH - 7 * pmc;
> - i = 0;
> + mmcr->mmcr0 = (mmcr->mmcr0 & ~(0x1fUL << shift)) | (0x08UL << shift);
> } else {
> shift = MMCR1_PMC3SEL_SH - 5 * (pmc - 2);
> - i = 1;
> + mmcr->mmcr1 = (mmcr->mmcr1 & ~(0x1fUL << shift)) | (0x08UL << shift);
> }
> - /*
> - * Setting the PMCxSEL field to 0x08 disables PMC x.
> - */
> - mmcr[i] = (mmcr[i] & ~(0x1fUL << shift)) | (0x08UL << shift);
> }
>
> static int ppc970_generic_events[] = {
> --
> 1.8.3.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox