Re: [PATCH 3/4] powerpc/powernv: remove unused NPU DMA code
On Thu, Jun 27, 2019 at 09:22:40AM +0200, Christoph Hellwig wrote: > On Thu, Jun 27, 2019 at 10:21:55AM +1000, Alexey Kardashevskiy wrote: > > > Which comment? Last time I asked you complaint "it is still used in > > > exactly the same way as before" which you later clarified that you > > > have a hidden out of tree user somewhere, and you only objected to > > > > It is not hidden, anyone can download and inspect that GPL driver. > > For one no one has ever posted a link. And second as mentioned > countless times it doesn't matter, it only matters if it is in mainline, > or as a special exception actively trying to go mainline. > > > > the word "dead". That has been fixed and there were no further > > > comments. > > > > You still have it in the cover letter so at very least 3/4 is not a part > > of this patchset then. > > > > And I still want to see a formal statement about out-of-tree drivers > > support/tolerance. If you manage to remove this code, I'll have to post > > a revert (again and again) but I would rather know the exact list of > > what we do and what we do not do about such drivers and if the list 1) > > exists 2) is reasonable then I could try to come up with a better > > solution or point others to the policy and push them to do the right > > thing. Right now it is just you pretending that the nVidia driver does > > not exist, this is not helping. Thanks, > > We had that discussion at kernel summit and it was reported. Anyway, > adding Greg, who usually has some pretty good prewritten letters for > this kind of thing. I used to have one but it's been so long since anyone tried to even think about defending the removal of functions that are not used in the kernel tree anymore, that I can't seem to find it anymore :) Christoph is completely correct here, if it isn't in the tree, it doesn't matter. We have made this "formal" statement again and again over the years, starting with the old "stable api nonsense" document that is in the kernel tree itself. And he is also correct in that we talked about this specific issue, in detail, at the maintainers summit last year, see lwn.net for the details if you somehow missed it then. thanks, greg k-h
Re: [PATCH 3/4] powerpc/powernv: remove unused NPU DMA code
On Thu, Jun 27, 2019 at 10:21:55AM +1000, Alexey Kardashevskiy wrote: > > Which comment? Last time I asked you complaint "it is still used in > > exactly the same way as before" which you later clarified that you > > have a hidden out of tree user somewhere, and you only objected to > > It is not hidden, anyone can download and inspect that GPL driver. For one no one has ever posted a link. And second as mentioned countless times it doesn't matter, it only matters if it is in mainline, or as a special exception actively trying to go mainline. > > the word "dead". That has been fixed and there were no further > > comments. > > You still have it in the cover letter so at very least 3/4 is not a part > of this patchset then. > > And I still want to see a formal statement about out-of-tree drivers > support/tolerance. If you manage to remove this code, I'll have to post > a revert (again and again) but I would rather know the exact list of > what we do and what we do not do about such drivers and if the list 1) > exists 2) is reasonable then I could try to come up with a better > solution or point others to the policy and push them to do the right > thing. Right now it is just you pretending that the nVidia driver does > not exist, this is not helping. Thanks, We had that discussion at kernel summit and it was reported. Anyway, adding Greg, who usually has some pretty good prewritten letters for this kind of thing.
Re: [PATCH 3/4] powerpc/powernv: remove unused NPU DMA code
On 26/06/2019 17:49, Christoph Hellwig wrote: > On Wed, Jun 26, 2019 at 10:44:38AM +1000, Alexey Kardashevskiy wrote: >> >> >> On 26/06/2019 00:52, Christoph Hellwig wrote: >>> None of these routines were ever used anywhere in the kernel tree >>> since they were added to the kernel. >> >> >> So none of my comments has been addressed. Nice. > > Which comment? Last time I asked you complaint "it is still used in > exactly the same way as before" which you later clarified that you > have a hidden out of tree user somewhere, and you only objected to It is not hidden, anyone can download and inspect that GPL driver. > the word "dead". That has been fixed and there were no further > comments. You still have it in the cover letter so at very least 3/4 is not a part of this patchset then. And I still want to see a formal statement about out-of-tree drivers support/tolerance. If you manage to remove this code, I'll have to post a revert (again and again) but I would rather know the exact list of what we do and what we do not do about such drivers and if the list 1) exists 2) is reasonable then I could try to come up with a better solution or point others to the policy and push them to do the right thing. Right now it is just you pretending that the nVidia driver does not exist, this is not helping. Thanks, -- Alexey
Re: [PATCH 3/4] powerpc/powernv: remove unused NPU DMA code
On Wed, Jun 26, 2019 at 10:44:38AM +1000, Alexey Kardashevskiy wrote: > > > On 26/06/2019 00:52, Christoph Hellwig wrote: > > None of these routines were ever used anywhere in the kernel tree > > since they were added to the kernel. > > > So none of my comments has been addressed. Nice. Which comment? Last time I asked you complaint "it is still used in exactly the same way as before" which you later clarified that you have a hidden out of tree user somewhere, and you only objected to the word "dead". That has been fixed and there were no further comments.
Re: [PATCH 3/4] powerpc/powernv: remove unused NPU DMA code
On 26/06/2019 00:52, Christoph Hellwig wrote:
> None of these routines were ever used anywhere in the kernel tree
> since they were added to the kernel.
So none of my comments has been addressed. Nice.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/powerpc/include/asm/book3s/64/mmu.h | 2 -
> arch/powerpc/include/asm/powernv.h | 22 -
> arch/powerpc/mm/book3s64/mmu_context.c | 1 -
> arch/powerpc/platforms/powernv/npu-dma.c | 556 ---
> 4 files changed, 581 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h
> b/arch/powerpc/include/asm/book3s/64/mmu.h
> index 74d24201fc4f..23b83d3593e2 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> @@ -116,8 +116,6 @@ typedef struct {
> /* Number of users of the external (Nest) MMU */
> atomic_t copros;
>
> - /* NPU NMMU context */
> - struct npu_context *npu_context;
> struct hash_mm_context *hash_context;
>
> unsigned long vdso_base;
> diff --git a/arch/powerpc/include/asm/powernv.h
> b/arch/powerpc/include/asm/powernv.h
> index 05b552418519..40f868c5e93c 100644
> --- a/arch/powerpc/include/asm/powernv.h
> +++ b/arch/powerpc/include/asm/powernv.h
> @@ -11,35 +11,13 @@
> #define _ASM_POWERNV_H
>
> #ifdef CONFIG_PPC_POWERNV
> -#define NPU2_WRITE 1
> extern void powernv_set_nmmu_ptcr(unsigned long ptcr);
> -extern struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
> - unsigned long flags,
> - void (*cb)(struct npu_context *, void *),
> - void *priv);
> -extern void pnv_npu2_destroy_context(struct npu_context *context,
> - struct pci_dev *gpdev);
> -extern int pnv_npu2_handle_fault(struct npu_context *context, uintptr_t *ea,
> - unsigned long *flags, unsigned long *status,
> - int count);
>
> void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val);
>
> void pnv_tm_init(void);
> #else
> static inline void powernv_set_nmmu_ptcr(unsigned long ptcr) { }
> -static inline struct npu_context *pnv_npu2_init_context(struct pci_dev
> *gpdev,
> - unsigned long flags,
> - struct npu_context *(*cb)(struct npu_context *, void *),
> - void *priv) { return ERR_PTR(-ENODEV); }
> -static inline void pnv_npu2_destroy_context(struct npu_context *context,
> - struct pci_dev *gpdev) { }
> -
> -static inline int pnv_npu2_handle_fault(struct npu_context *context,
> - uintptr_t *ea, unsigned long *flags,
> - unsigned long *status, int count) {
> - return -ENODEV;
> -}
>
> static inline void pnv_tm_init(void) { }
> #endif
> diff --git a/arch/powerpc/mm/book3s64/mmu_context.c
> b/arch/powerpc/mm/book3s64/mmu_context.c
> index cb2b08635508..0dd3e631cf3e 100644
> --- a/arch/powerpc/mm/book3s64/mmu_context.c
> +++ b/arch/powerpc/mm/book3s64/mmu_context.c
> @@ -140,7 +140,6 @@ static int radix__init_new_context(struct mm_struct *mm)
>*/
> asm volatile("ptesync;isync" : : : "memory");
>
> - mm->context.npu_context = NULL;
> mm->context.hash_context = NULL;
>
> return index;
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c
> b/arch/powerpc/platforms/powernv/npu-dma.c
> index dc1058efc24f..72b7441029ca 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -22,12 +22,6 @@
>
> #include "pci.h"
>
> -/*
> - * spinlock to protect initialisation of an npu_context for a particular
> - * mm_struct.
> - */
> -static DEFINE_SPINLOCK(npu_context_lock);
> -
> static struct pci_dev *get_pci_dev(struct device_node *dn)
> {
> struct pci_dn *pdn = PCI_DN(dn);
> @@ -375,15 +369,6 @@ struct npu_comp {
> /* 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;
> -
> struct npu_comp npucomp;
> };
>
> @@ -640,534 +625,8 @@ struct iommu_table_group
> *pnv_npu_compound_attach(struct pnv_ioda_pe *pe)
> }
> #endif /* CONFIG_IOMMU_API */
>
> -/* Maximum number of nvlinks per npu */
> -#define NV_MAX_LINKS 6
> -
> -/* Maximum index of npu2 hosts in the system. Always < NV_MAX_NPUS */
> -static int max_npu2_index;
> -
> -struct npu_context {
> - struct mm_struct *mm;
> - struct pci_dev *npdev[NV_MAX_NPUS][NV_MAX_LINKS];
> - struct mmu_notifier mn;
> - struct kref kref;
> - bool nmmu_flush;
> -
> - /* Callback to stop translation requests on a given GPU */
> - void (*release_cb)(struct
