[Skiboot] [PATCH 6/6] npu2: Add support for relaxed-ordering mode

[Reza Arbab]

On Mon, Jul 30, 2018 at 04:37:33PM +1000, Alistair Popple wrote:
>On Friday, 27 July 2018 9:47:42 AM AEST Reza Arbab wrote: 
>> +/*
>> + * Enable or disable relaxed ordering on all nvlinks on a given NPU. May leave
>> + * relaxed ordering partially enabled if there are insufficient HW resources to
>> + * enable it on all links.
>> + */
>
>This comment isn't correct as it does so for all nvlinks on all NPUs. Whomever
>wrote that originaly clearly didn't know what he was talking about. He also
>can't spell "originally" correctly.

Changed to "for a given PEC" in v2.

>> +static int npu2_check_relaxed_ordering(struct phb *phb __unused,
>> +				       struct pci_device *pd, void *enable)
>> +{
>> +	/*
>> +	 * IBM PCIe bridge devices (ie. the root ports) can always allow relaxed
>> +	 * ordering
>> +	 */
>> +	if (pd->vdid == 0x04c11014)
>> +		pd->allow_relaxed_ordering = true;
>> +
>> +	PCINOTICE(phb, pd->bdfn, "Checking relaxed ordering config\n");
>> +	if (pd->allow_relaxed_ordering)
>> +		return 0;
>> +
>> +	PCINOTICE(phb, pd->bdfn, "Relaxed ordering not allowed\n");
>
>It's probably best to drop the log level here. Maybe PCIDBG?

No problem.

>> +static int64_t opal_npu_set_relaxed_order(uint64_t phb_id, uint16_t bdfn,
>> +					  bool request_enabled)
>> +{
>> +	struct phb *phb = pci_get_phb(phb_id);
>> +	struct phb4 *phb4;
>> +	uint32_t chip_id, pec;
>> +	struct pci_device *pd;
>> +	bool enable = true;
>> +
>> +	if (!phb || phb->phb_type != phb_type_pcie_v4)
>> +		return OPAL_PARAMETER;
>> +
>> +	phb4 = phb_to_phb4(phb);
>> +	pec = phb4->pec;
>> +	chip_id = phb4->chip_id;
>> +
>> +	/* Can chip_id be packed into NPU2_RELAXED_ORDERING_SOURCE_GRPCHP? */
>> +	if (chip_id & 0x64)
>
>Actually this negates my comments above on gcid validation, but perhaps a
>comment in npu2_relaxed_ordering_source_grpchp(uint32_t gcid) mentioning that
>the caller needs to ensure the gcid is within a valid range wouldn't hurt.

I reworked this a bit so npu2_relaxed_ordering_source_grpchp() is where 
the argument gets validated now.

-- 
Reza Arbab