Re: [PATCH kernel v3 11/22] powerpc/pseries/npu: Enable platform support

From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: "Alex Williamson" <alex.williamson@redhat.com>,
	"Jose Ricardo Ziviani" <joserz@linux.ibm.com>,
	"Sam Bobroff" <sbobroff@linux.ibm.com>,
	"Alistair Popple" <alistair@popple.id.au>,
	linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org,
	"Piotr Jaroszynski" <pjaroszynski@nvidia.com>,
	"Oliver O'Halloran" <oohall@gmail.com>,
	"Andrew Donnellan" <andrew.donnellan@au1.ibm.com>,
	"Leonardo Augusto Guimarães Garcia" <lagarcia@br.ibm.com>,
	"Reza Arbab" <arbab@linux.ibm.com>
Subject: Re: [PATCH kernel v3 11/22] powerpc/pseries/npu: Enable platform support
Date: Mon, 19 Nov 2018 18:50:58 +1100	[thread overview]
Message-ID: <c8dee175-d33a-232d-5237-9233b74e0697@ozlabs.ru> (raw)
In-Reply-To: <20181116052534.GD23632@umbus>

On 16/11/2018 16:25, David Gibson wrote:
> On Tue, Nov 13, 2018 at 07:28:12PM +1100, Alexey Kardashevskiy wrote:
>> We already changed NPU API for GPUs to not to call OPAL and the remaining
>> bit is initializing NPU structures.
>>
>> This uses a new QEMU capability which marks NPU-enabled vPHBs as
>> "IBM,npu-vphb" and initializes an NPU structure per vPHB.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>  arch/powerpc/platforms/pseries/pci.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
>> index 41d8a4d..a50d5e4 100644
>> --- a/arch/powerpc/platforms/pseries/pci.c
>> +++ b/arch/powerpc/platforms/pseries/pci.c
>> @@ -29,6 +29,7 @@
>>  #include <asm/pci-bridge.h>
>>  #include <asm/prom.h>
>>  #include <asm/ppc-pci.h>
>> +#include <asm/pci.h>
>>  #include "pseries.h"
>>  
>>  #if 0
>> @@ -237,6 +238,8 @@ static void __init pSeries_request_regions(void)
>>  
>>  void __init pSeries_final_fixup(void)
>>  {
>> +	struct pci_controller *hose;
>> +
>>  	pSeries_request_regions();
>>  
>>  	eeh_probe_devices();
>> @@ -246,6 +249,9 @@ void __init pSeries_final_fixup(void)
>>  	ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
>>  	ppc_md.pcibios_sriov_disable = pseries_pcibios_sriov_disable;
>>  #endif
>> +	list_for_each_entry(hose, &hose_list, list_node)
>> +		if (of_device_is_compatible(hose->dn, "IBM,npu-vphb"))
>> +			pnv_npu2_init(hose);
> 
> I take it from this the NPUs are showing up with a compatible property
> that lists the normal PHB value as well as IBM,npu-vphb.  Since AIUI
> the NPUs act quite differently from other (real) PHBs this seems
> bogus.  Shouldn't they be probed separately?

First, bad naming, will think of better one.

"IBM,npu-vphb" is an extra compatible type for otherwise usual pseries
PHB. The  differences are:

1. Initialize an "NPU" (not a NVLink2 bridge but a proper NPU) per a PHB
for context manipulation for a GPU;

2. Kill the default DMA window.

When a GPU is passed to a guest, it looks like:

aik@u1804kvm:~$ lspci
00:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
Controller (rev 03)
00:01.0 Ethernet controller: Red Hat, Inc Virtio network device
00:02.0 3D controller: NVIDIA Corporation GV100 [Tesla V100 SXM2] (rev a1)
00:03.0 Bridge: IBM Device 04ea (rev 01)
00:04.0 Bridge: IBM Device 04ea (rev 01)

So there are:
- one "struct npu" associated with the pseries PHB (not presented in
lspci but there /proc/device-tree/npuphb0/ with link@0 and link@1)

- 2 NVLink2 bridges, presented in lspci.

-- 
Alexey