On 05/12/2018 16:14, David Gibson wrote:
> On Fri, Nov 23, 2018 at 04:52:49PM +1100, Alexey Kardashevskiy wrote:
>> The powernv PCI code stores NPU data in the pnv_phb struct. The latter
>> is referenced by pci_controller::private_data. We are going to have NPU2
>> support in the pseries platform as well but it does not store any
>> private_data in in the pci_controller struct; and even if it did,
>> it would be a different data structure.
>>
>> This makes npu a pointer and stores it one level higher in
>> the pci_controller struct.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
>> ---
>> Changes:
>> v4:
>> * changed subj from "powerpc/powernv: Detach npu struct from pnv_phb"
>> * got rid of global list of npus - store them now in pci_controller
>> * got rid of npdev_to_npu() helper
>> ---
>> arch/powerpc/include/asm/pci-bridge.h | 1 +
>> arch/powerpc/platforms/powernv/pci.h | 16 -----
>> arch/powerpc/platforms/powernv/npu-dma.c | 81 ++++++++++++++++++------
>> 3 files changed, 64 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>> index 94d4490..aee4fcc 100644
>> --- a/arch/powerpc/include/asm/pci-bridge.h
>> +++ b/arch/powerpc/include/asm/pci-bridge.h
>> @@ -129,6 +129,7 @@ struct pci_controller {
>> #endif /* CONFIG_PPC64 */
>>
>> void *private_data;
>> + struct npu *npu;
>> };
>>
>> /* These are used for config access before all the PCI probing
>> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
>> index 2131373..f2d50974 100644
>> --- a/arch/powerpc/platforms/powernv/pci.h
>> +++ b/arch/powerpc/platforms/powernv/pci.h
>> @@ -8,9 +8,6 @@
>>
>> struct pci_dn;
>>
>> -/* Maximum possible number of ATSD MMIO registers per NPU */
>> -#define NV_NMMU_ATSD_REGS 8
>> -
>> enum pnv_phb_type {
>> PNV_PHB_IODA1 = 0,
>> PNV_PHB_IODA2 = 1,
>> @@ -176,19 +173,6 @@ struct pnv_phb {
>> unsigned int diag_data_size;
>> u8 *diag_data;
>>
>> - /* Nvlink2 data */
>> - struct npu {
>> - int index;
>> - __be64 *mmio_atsd_regs[NV_NMMU_ATSD_REGS];
>> - unsigned int mmio_atsd_count;
>> -
>> - /* Bitmask for MMIO register usage */
>> - unsigned long mmio_atsd_usage;
>> -
>> - /* Do we need to explicitly flush the nest mmu? */
>> - bool nmmu_flush;
>> - } npu;
>> -
>> int p2p_target_count;
>> };
>>
>> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
>> index 91d488f..7dd5c0e5 100644
>> --- a/arch/powerpc/platforms/powernv/npu-dma.c
>> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
>> @@ -327,6 +327,25 @@ struct pnv_ioda_pe *pnv_pci_npu_setup_iommu(struct pnv_ioda_pe *npe)
>> return gpe;
>> }
>>
>> +/*
>> + * NPU2 ATS
>> + */
>> +/* Maximum possible number of ATSD MMIO registers per NPU */
>> +#define NV_NMMU_ATSD_REGS 8
>> +
>> +/* An NPU descriptor, valid for POWER9 only */
>> +struct npu {
>> + int index;
>> + __be64 *mmio_atsd_regs[NV_NMMU_ATSD_REGS];
>> + unsigned int mmio_atsd_count;
>> +
>> + /* Bitmask for MMIO register usage */
>> + unsigned long mmio_atsd_usage;
>> +
>> + /* Do we need to explicitly flush the nest mmu? */
>> + bool nmmu_flush;
>> +};
>> +
>> /* Maximum number of nvlinks per npu */
>> #define NV_MAX_LINKS 6
>>
>> @@ -478,7 +497,6 @@ static void acquire_atsd_reg(struct npu_context *npu_context,
>> int i, j;
>> struct npu *npu;
>> struct pci_dev *npdev;
>> - struct pnv_phb *nphb;
>>
>> for (i = 0; i <= max_npu2_index; i++) {
>> mmio_atsd_reg[i].reg = -1;
>> @@ -493,8 +511,10 @@ static void acquire_atsd_reg(struct npu_context *npu_context,
>> if (!npdev)
>> continue;
>>
>> - nphb = pci_bus_to_host(npdev->bus)->private_data;
>> - npu = &nphb->npu;
>> + npu = pci_bus_to_host(npdev->bus)->npu;
>> + if (!npu)
>> + continue;
>
> This patch changes a bunch of places that used to unconditionally
> locate an NPU now have a failure path.
>
> Given that this used to always have an NPU, doesn't that mean that if
> the NPU is not present something has already gone wrong, and we should
> WARN_ON() or something?
That means this is a leftover since I dropped that npdev_to_npu helper
which could help but there was no real value in it. I'll remove the
check here in the next respin.
I'll probably add checks for npu!=NULL where we used to have
firmware_has_feature(FW_FEATURE_OPAL) in 05/19.
--
Alexey
