Haren Myneni <haren@xxxxxxxxxxxxxxxxxx> writes:
> On 06/01/2018 12:41 AM, Stewart Smith wrote:
>> Haren Myneni <haren@xxxxxxxxxxxxxxxxxx> writes:
>>> NX increments readOffset by FIFO size in receive FIFO control register
>>> when CRB is read. But the index in RxFIFO has to match with the
>>> corresponding entry in FIFO maintained by VAS in kernel. Otherwise NX
>>> may be processing incorrect CRBs and can cause CRB timeout.
>>>
>>> VAS FIFO offset is 0 when the receive window is opened during
>>> initialization. When the module is reloaded or in kexec boot, readOffset
>>> in FIFO control register may not match with VAS entry. This patch adds
>>> nx_coproc_init OPAL call to reset readOffset and queued entries in FIFO
>>> control register for both high and normal FIFOs.
>>>
>>> Signed-off-by: Haren Myneni <haren@xxxxxxxxxx>
>>>
>>> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
>>> index d886a5b..ff61e4b 100644
>>> --- a/arch/powerpc/include/asm/opal-api.h
>>> +++ b/arch/powerpc/include/asm/opal-api.h
>>> @@ -206,7 +206,8 @@
>>> #define OPAL_NPU_TL_SET 161
>>> #define OPAL_PCI_GET_PBCQ_TUNNEL_BAR 164
>>> #define OPAL_PCI_SET_PBCQ_TUNNEL_BAR 165
>>> -#define OPAL_LAST 165
>>> +#define OPAL_NX_COPROC_INIT 167
>>> +#define OPAL_LAST 167
>>>
>>> /* Device tree flags */
>>>
>>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>>> index 7159e1a..d79eb82 100644
>>> --- a/arch/powerpc/include/asm/opal.h
>>> +++ b/arch/powerpc/include/asm/opal.h
>>> @@ -288,6 +288,7 @@ int64_t opal_imc_counters_init(uint32_t type, uint64_t address,
>>> int opal_get_power_shift_ratio(u32 handle, int token, u32 *psr);
>>> int opal_set_power_shift_ratio(u32 handle, int token, u32 psr);
>>> int opal_sensor_group_clear(u32 group_hndl, int token);
>>> +int opal_nx_coproc_init(uint32_t chip_id, uint32_t ct);
>>>
>>> s64 opal_signal_system_reset(s32 cpu);
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
>>> index 3da30c2..c7541a9 100644
>>> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
>>> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
>>> @@ -325,3 +325,4 @@ OPAL_CALL(opal_npu_spa_clear_cache, OPAL_NPU_SPA_CLEAR_CACHE);
>>> OPAL_CALL(opal_npu_tl_set, OPAL_NPU_TL_SET);
>>> OPAL_CALL(opal_pci_get_pbcq_tunnel_bar, OPAL_PCI_GET_PBCQ_TUNNEL_BAR);
>>> OPAL_CALL(opal_pci_set_pbcq_tunnel_bar, OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
>>> +OPAL_CALL(opal_nx_coproc_init, OPAL_NX_COPROC_INIT);
>>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>>> index 48fbb41..5e13908 100644
>>> --- a/arch/powerpc/platforms/powernv/opal.c
>>> +++ b/arch/powerpc/platforms/powernv/opal.c
>>> @@ -1035,3 +1035,5 @@ void powernv_set_nmmu_ptcr(unsigned long ptcr)
>>> EXPORT_SYMBOL_GPL(opal_int_set_mfrr);
>>> EXPORT_SYMBOL_GPL(opal_int_eoi);
>>> EXPORT_SYMBOL_GPL(opal_error_code);
>>> +/* Export the below symbol for NX compression */
>>> +EXPORT_SYMBOL(opal_nx_coproc_init);
>>> diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
>>> index 1e87637..6c4784d 100644
>>> --- a/drivers/crypto/nx/nx-842-powernv.c
>>> +++ b/drivers/crypto/nx/nx-842-powernv.c
>>> @@ -24,6 +24,8 @@
>>> #include <asm/icswx.h>
>>> #include <asm/vas.h>
>>> #include <asm/reg.h>
>>> +#include <asm/opal-api.h>
>>> +#include <asm/opal.h>
>>>
>>> MODULE_LICENSE("GPL");
>>> MODULE_AUTHOR("Dan Streetman <ddstreet@xxxxxxxx>");
>>> @@ -803,9 +805,26 @@ static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
>>> if (!coproc)
>>> return -ENOMEM;
>>>
>>> - if (!strcmp(priority, "High"))
>>> + if (!strcmp(priority, "High")) {
>>> + /*
>>> + * (lpid, pid, tid) combination has to be unique for each
>>> + * coprocessor instance in the system. So to make it
>>> + * unique, skiboot uses coprocessor type such as 842 or
>>> + * GZIP for pid and provides this value to kernel in pid
>>> + * device-tree property.
>>> + *
>>> + * Initialize each NX instance for both high and normal
>>> + * priority FIFOs.
>>> + */
>>> + ret = opal_nx_coproc_init(chip_id, pid);
>>> + if (ret) {
>>> + pr_err("Failed to initialize NX coproc: %d\n", ret);
>>> + ret = opal_error_code(ret);
>>> + goto err_out;
>>> + }
>>> +
>>> coproc->ct = VAS_COP_TYPE_842_HIPRI;
>>
>> I think this should be called for all priority queues as it would be at
>> least theoretically possible to only have Normal priority queues, in
>> which case this patch wouldn't fix the problem.
>
> device tree exports separate nodes for high and normal priority FIFOs
> per each NX instance. But NX init OPAL function is called once per
> each NX instance when high-FIFO device node is parsed to reset high
> and normal FIFO control registers. As you see in skiboot patch, resets
> both priority registers. Thought we should minimize the number of OPAL
> calla execution and also should have generic NX init OPAL call. We can
> extend this call to reset default values for any other registers if
> needed.
This code does'nt do that though, it calls it for "high" priority, so if
for whatever reason there was a DT without a "High" priority node there,
it'd not call it.
> If you prefer calling separately based on priority, we can have
> opal_nx_coproc_FIFO_init(chip_id, pid, priority). Please let me know.
I think the call is okay, it's just that if it needs to be called once
per accellerator, then we should ensure it does that.
--
Stewart Smith
OPAL Architect, IBM.
