Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
* Christoph Hellwig wrote: > Is anyone going to pick this patch up? Sure, I've picked it up with Bjorn's Acked-by and will push it out after a bit of testing. Thanks, Ingo
Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
Is anyone going to pick this patch up?
Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
On Thu, Apr 11, 2019 at 07:18:49AM +0200, Christoph Hellwig wrote: > On Wed, Apr 10, 2019 at 04:45:01PM -0500, Bjorn Helgaas wrote: > > [+cc Keith, Jonathan (VMD guys)] > > > > I'm OK with this from a PCI perspective. It would be nice if > > > > dma_domain_list > > dma_domain_list_lock > > add_dma_domain() > > del_dma_domain() > > set_dma_domain_ops() > > > > could all be moved to vmd.c, since they're really only used there. > > I have another patch to eventually kill that, but it will need a little > more prep work and thus be delayed to the next merge window. Awesome, thanks! I really appreciate all the global cleanup work you do. Bjorn
Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
On Wed, Apr 10, 2019 at 04:45:01PM -0500, Bjorn Helgaas wrote: > [+cc Keith, Jonathan (VMD guys)] > > I'm OK with this from a PCI perspective. It would be nice if > > dma_domain_list > dma_domain_list_lock > add_dma_domain() > del_dma_domain() > set_dma_domain_ops() > > could all be moved to vmd.c, since they're really only used there. I have another patch to eventually kill that, but it will need a little more prep work and thus be delayed to the next merge window.
Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
On Wed, 2019-04-10 at 16:45 -0500, Bjorn Helgaas wrote:
> [+cc Keith, Jonathan (VMD guys)]
>
> I'm OK with this from a PCI perspective. It would be nice if
>
> dma_domain_list
> dma_domain_list_lock
> add_dma_domain()
> del_dma_domain()
> set_dma_domain_ops()
>
> could all be moved to vmd.c, since they're really only used there.
>
> But we don't really have a good way to call host bridge-specific code
> like set_dma_domain_ops() except via pcibios_add_device(). Someday
> maybe pcibios_add_device() will become a host bridge method.
I think these could have been contained to vmd.c with the host bridge
add_device callback [1] but I can't seem to find whatever came about
that discussion
[1] https://marc.info/?l=linux-pci=153337765811720=2
Either way, this looks fine
Reviewed-by: Jon Derrick
>
> Acked-by: Bjorn Helgaas
>
>
> On Wed, Apr 10, 2019 at 10:16:17AM +0200, Ingo Molnar wrote:
> > (+Cc. Patch quoted below. Acked-by from an x86 perspective.)
> >
> > * Christoph Hellwig wrote:
> >
> > > We have supported per-device dma_map_ops in generic code for a
> > > long
> > > time, and this symbol just guards the inclusion of the
> > > dma_map_ops
> > > registry used for vmd. Stop enabling it for anything but vmd.
> > >
> > > Signed-off-by: Christoph Hellwig
> > > ---
> > > arch/x86/Kconfig | 3 ---
> > > drivers/misc/mic/Kconfig | 4 ++--
> > > drivers/pci/controller/Kconfig | 1 +
> > > drivers/pci/controller/vmd.c | 7 ---
> > > 4 files changed, 3 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index 38c62ff8a3f0..d8e2e6519a61 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -28,7 +28,6 @@ config X86_64
> > > select MODULES_USE_ELF_RELA
> > > select NEED_DMA_MAP_STATE
> > > select SWIOTLB
> > > - select X86_DEV_DMA_OPS
> > > select ARCH_HAS_SYSCALL_WRAPPER
> > >
> > > #
> > > @@ -703,7 +702,6 @@ config STA2X11
> > > bool "STA2X11 Companion Chip Support"
> > > depends on X86_32_NON_STANDARD && PCI
> > > select ARCH_HAS_PHYS_TO_DMA
> > > - select X86_DEV_DMA_OPS
> > > select SWIOTLB
> > > select MFD_STA2X11
> > > select GPIOLIB
> > > @@ -2877,7 +2875,6 @@ config HAVE_ATOMIC_IOMAP
> > >
> > > config X86_DEV_DMA_OPS
> > > bool
> > > - depends on X86_64 || STA2X11
> > >
> > > config HAVE_GENERIC_GUP
> > > def_bool y
> > > diff --git a/drivers/misc/mic/Kconfig b/drivers/misc/mic/Kconfig
> > > index 242dcee14689..6736f72cc14a 100644
> > > --- a/drivers/misc/mic/Kconfig
> > > +++ b/drivers/misc/mic/Kconfig
> > > @@ -4,7 +4,7 @@ comment "Intel MIC Bus Driver"
> > >
> > > config INTEL_MIC_BUS
> > > tristate "Intel MIC Bus Driver"
> > > - depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
> > > + depends on 64BIT && PCI && X86
> > > help
> > > This option is selected by any driver which registers a
> > > device or driver on the MIC Bus, such as
> > > CONFIG_INTEL_MIC_HOST,
> > > @@ -21,7 +21,7 @@ comment "SCIF Bus Driver"
> > >
> > > config SCIF_BUS
> > > tristate "SCIF Bus Driver"
> > > - depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
> > > + depends on 64BIT && PCI && X86
> > > help
> > > This option is selected by any driver which registers a
> > > device or driver on the SCIF Bus, such as
> > > CONFIG_INTEL_MIC_HOST
> > > diff --git a/drivers/pci/controller/Kconfig
> > > b/drivers/pci/controller/Kconfig
> > > index 6012f3059acd..011c57cae4b0 100644
> > > --- a/drivers/pci/controller/Kconfig
> > > +++ b/drivers/pci/controller/Kconfig
> > > @@ -267,6 +267,7 @@ config PCIE_TANGO_SMP8759
> > >
> > > config VMD
> > > depends on PCI_MSI && X86_64 && SRCU
> > > + select X86_DEV_DMA_OPS
> > > tristate "Intel Volume Management Device Driver"
> > > ---help---
> > > Adds support for the Intel Volume Management Device (VMD).
> > > VMD is a
> > > diff --git a/drivers/pci/controller/vmd.c
> > > b/drivers/pci/controller/vmd.c
> > > index cf6816b55b5e..999a5509e57e 100644
> > > --- a/drivers/pci/controller/vmd.c
> > > +++ b/drivers/pci/controller/vmd.c
> > > @@ -95,10 +95,8 @@ struct vmd_dev {
> > > struct irq_domain *irq_domain;
> > > struct pci_bus *bus;
> > >
> > > -#ifdef CONFIG_X86_DEV_DMA_OPS
> > > struct dma_map_ops dma_ops;
> > > struct dma_domain dma_domain;
> > > -#endif
> > > };
> > >
> > > static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
> > > @@ -293,7 +291,6 @@ static struct msi_domain_info
> > > vmd_msi_domain_info = {
> > > .chip = _msi_controller,
> > > };
> > >
> > > -#ifdef CONFIG_X86_DEV_DMA_OPS
> > > /*
> > > * VMD replaces the requester ID with its own. DMA mappings for
> > > devices in a
> > > * VMD domain need to be mapped for the VMD, not the device
> > > requiring
> > > @@ -438,10 +435,6 @@ static void vmd_setup_dma_ops(struct vmd_dev
> > > *vmd)
> > > add_dma_domain(domain);
> > > }
> > > #undef ASSIGN_VMD_DMA_OPS
> > >
Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
[+cc Keith, Jonathan (VMD guys)]
I'm OK with this from a PCI perspective. It would be nice if
dma_domain_list
dma_domain_list_lock
add_dma_domain()
del_dma_domain()
set_dma_domain_ops()
could all be moved to vmd.c, since they're really only used there.
But we don't really have a good way to call host bridge-specific code
like set_dma_domain_ops() except via pcibios_add_device(). Someday
maybe pcibios_add_device() will become a host bridge method.
Acked-by: Bjorn Helgaas
On Wed, Apr 10, 2019 at 10:16:17AM +0200, Ingo Molnar wrote:
> (+Cc. Patch quoted below. Acked-by from an x86 perspective.)
>
> * Christoph Hellwig wrote:
>
> > We have supported per-device dma_map_ops in generic code for a long
> > time, and this symbol just guards the inclusion of the dma_map_ops
> > registry used for vmd. Stop enabling it for anything but vmd.
> >
> > Signed-off-by: Christoph Hellwig
> > ---
> > arch/x86/Kconfig | 3 ---
> > drivers/misc/mic/Kconfig | 4 ++--
> > drivers/pci/controller/Kconfig | 1 +
> > drivers/pci/controller/vmd.c | 7 ---
> > 4 files changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 38c62ff8a3f0..d8e2e6519a61 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -28,7 +28,6 @@ config X86_64
> > select MODULES_USE_ELF_RELA
> > select NEED_DMA_MAP_STATE
> > select SWIOTLB
> > - select X86_DEV_DMA_OPS
> > select ARCH_HAS_SYSCALL_WRAPPER
> >
> > #
> > @@ -703,7 +702,6 @@ config STA2X11
> > bool "STA2X11 Companion Chip Support"
> > depends on X86_32_NON_STANDARD && PCI
> > select ARCH_HAS_PHYS_TO_DMA
> > - select X86_DEV_DMA_OPS
> > select SWIOTLB
> > select MFD_STA2X11
> > select GPIOLIB
> > @@ -2877,7 +2875,6 @@ config HAVE_ATOMIC_IOMAP
> >
> > config X86_DEV_DMA_OPS
> > bool
> > - depends on X86_64 || STA2X11
> >
> > config HAVE_GENERIC_GUP
> > def_bool y
> > diff --git a/drivers/misc/mic/Kconfig b/drivers/misc/mic/Kconfig
> > index 242dcee14689..6736f72cc14a 100644
> > --- a/drivers/misc/mic/Kconfig
> > +++ b/drivers/misc/mic/Kconfig
> > @@ -4,7 +4,7 @@ comment "Intel MIC Bus Driver"
> >
> > config INTEL_MIC_BUS
> > tristate "Intel MIC Bus Driver"
> > - depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
> > + depends on 64BIT && PCI && X86
> > help
> > This option is selected by any driver which registers a
> > device or driver on the MIC Bus, such as CONFIG_INTEL_MIC_HOST,
> > @@ -21,7 +21,7 @@ comment "SCIF Bus Driver"
> >
> > config SCIF_BUS
> > tristate "SCIF Bus Driver"
> > - depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
> > + depends on 64BIT && PCI && X86
> > help
> > This option is selected by any driver which registers a
> > device or driver on the SCIF Bus, such as CONFIG_INTEL_MIC_HOST
> > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> > index 6012f3059acd..011c57cae4b0 100644
> > --- a/drivers/pci/controller/Kconfig
> > +++ b/drivers/pci/controller/Kconfig
> > @@ -267,6 +267,7 @@ config PCIE_TANGO_SMP8759
> >
> > config VMD
> > depends on PCI_MSI && X86_64 && SRCU
> > + select X86_DEV_DMA_OPS
> > tristate "Intel Volume Management Device Driver"
> > ---help---
> > Adds support for the Intel Volume Management Device (VMD). VMD is a
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index cf6816b55b5e..999a5509e57e 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -95,10 +95,8 @@ struct vmd_dev {
> > struct irq_domain *irq_domain;
> > struct pci_bus *bus;
> >
> > -#ifdef CONFIG_X86_DEV_DMA_OPS
> > struct dma_map_ops dma_ops;
> > struct dma_domain dma_domain;
> > -#endif
> > };
> >
> > static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
> > @@ -293,7 +291,6 @@ static struct msi_domain_info vmd_msi_domain_info = {
> > .chip = _msi_controller,
> > };
> >
> > -#ifdef CONFIG_X86_DEV_DMA_OPS
> > /*
> > * VMD replaces the requester ID with its own. DMA mappings for devices
> > in a
> > * VMD domain need to be mapped for the VMD, not the device requiring
> > @@ -438,10 +435,6 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
> > add_dma_domain(domain);
> > }
> > #undef ASSIGN_VMD_DMA_OPS
> > -#else
> > -static void vmd_teardown_dma_ops(struct vmd_dev *vmd) {}
> > -static void vmd_setup_dma_ops(struct vmd_dev *vmd) {}
> > -#endif
> >
> > static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
> > unsigned int devfn, int reg, int len)
> > --
> > 2.20.1
> >
Re: [PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
(+Cc. Patch quoted below. Acked-by from an x86 perspective.)
* Christoph Hellwig wrote:
> We have supported per-device dma_map_ops in generic code for a long
> time, and this symbol just guards the inclusion of the dma_map_ops
> registry used for vmd. Stop enabling it for anything but vmd.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/x86/Kconfig | 3 ---
> drivers/misc/mic/Kconfig | 4 ++--
> drivers/pci/controller/Kconfig | 1 +
> drivers/pci/controller/vmd.c | 7 ---
> 4 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 38c62ff8a3f0..d8e2e6519a61 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -28,7 +28,6 @@ config X86_64
> select MODULES_USE_ELF_RELA
> select NEED_DMA_MAP_STATE
> select SWIOTLB
> - select X86_DEV_DMA_OPS
> select ARCH_HAS_SYSCALL_WRAPPER
>
> #
> @@ -703,7 +702,6 @@ config STA2X11
> bool "STA2X11 Companion Chip Support"
> depends on X86_32_NON_STANDARD && PCI
> select ARCH_HAS_PHYS_TO_DMA
> - select X86_DEV_DMA_OPS
> select SWIOTLB
> select MFD_STA2X11
> select GPIOLIB
> @@ -2877,7 +2875,6 @@ config HAVE_ATOMIC_IOMAP
>
> config X86_DEV_DMA_OPS
> bool
> - depends on X86_64 || STA2X11
>
> config HAVE_GENERIC_GUP
> def_bool y
> diff --git a/drivers/misc/mic/Kconfig b/drivers/misc/mic/Kconfig
> index 242dcee14689..6736f72cc14a 100644
> --- a/drivers/misc/mic/Kconfig
> +++ b/drivers/misc/mic/Kconfig
> @@ -4,7 +4,7 @@ comment "Intel MIC Bus Driver"
>
> config INTEL_MIC_BUS
> tristate "Intel MIC Bus Driver"
> - depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
> + depends on 64BIT && PCI && X86
> help
> This option is selected by any driver which registers a
> device or driver on the MIC Bus, such as CONFIG_INTEL_MIC_HOST,
> @@ -21,7 +21,7 @@ comment "SCIF Bus Driver"
>
> config SCIF_BUS
> tristate "SCIF Bus Driver"
> - depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
> + depends on 64BIT && PCI && X86
> help
> This option is selected by any driver which registers a
> device or driver on the SCIF Bus, such as CONFIG_INTEL_MIC_HOST
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 6012f3059acd..011c57cae4b0 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -267,6 +267,7 @@ config PCIE_TANGO_SMP8759
>
> config VMD
> depends on PCI_MSI && X86_64 && SRCU
> + select X86_DEV_DMA_OPS
> tristate "Intel Volume Management Device Driver"
> ---help---
> Adds support for the Intel Volume Management Device (VMD). VMD is a
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index cf6816b55b5e..999a5509e57e 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -95,10 +95,8 @@ struct vmd_dev {
> struct irq_domain *irq_domain;
> struct pci_bus *bus;
>
> -#ifdef CONFIG_X86_DEV_DMA_OPS
> struct dma_map_ops dma_ops;
> struct dma_domain dma_domain;
> -#endif
> };
>
> static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
> @@ -293,7 +291,6 @@ static struct msi_domain_info vmd_msi_domain_info = {
> .chip = _msi_controller,
> };
>
> -#ifdef CONFIG_X86_DEV_DMA_OPS
> /*
> * VMD replaces the requester ID with its own. DMA mappings for devices in a
> * VMD domain need to be mapped for the VMD, not the device requiring
> @@ -438,10 +435,6 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
> add_dma_domain(domain);
> }
> #undef ASSIGN_VMD_DMA_OPS
> -#else
> -static void vmd_teardown_dma_ops(struct vmd_dev *vmd) {}
> -static void vmd_setup_dma_ops(struct vmd_dev *vmd) {}
> -#endif
>
> static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
> unsigned int devfn, int reg, int len)
> --
> 2.20.1
>
[PATCH 2/2] x86/pci: Clean up usage of X86_DEV_DMA_OPS
We have supported per-device dma_map_ops in generic code for a long
time, and this symbol just guards the inclusion of the dma_map_ops
registry used for vmd. Stop enabling it for anything but vmd.
Signed-off-by: Christoph Hellwig
---
arch/x86/Kconfig | 3 ---
drivers/misc/mic/Kconfig | 4 ++--
drivers/pci/controller/Kconfig | 1 +
drivers/pci/controller/vmd.c | 7 ---
4 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 38c62ff8a3f0..d8e2e6519a61 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -28,7 +28,6 @@ config X86_64
select MODULES_USE_ELF_RELA
select NEED_DMA_MAP_STATE
select SWIOTLB
- select X86_DEV_DMA_OPS
select ARCH_HAS_SYSCALL_WRAPPER
#
@@ -703,7 +702,6 @@ config STA2X11
bool "STA2X11 Companion Chip Support"
depends on X86_32_NON_STANDARD && PCI
select ARCH_HAS_PHYS_TO_DMA
- select X86_DEV_DMA_OPS
select SWIOTLB
select MFD_STA2X11
select GPIOLIB
@@ -2877,7 +2875,6 @@ config HAVE_ATOMIC_IOMAP
config X86_DEV_DMA_OPS
bool
- depends on X86_64 || STA2X11
config HAVE_GENERIC_GUP
def_bool y
diff --git a/drivers/misc/mic/Kconfig b/drivers/misc/mic/Kconfig
index 242dcee14689..6736f72cc14a 100644
--- a/drivers/misc/mic/Kconfig
+++ b/drivers/misc/mic/Kconfig
@@ -4,7 +4,7 @@ comment "Intel MIC Bus Driver"
config INTEL_MIC_BUS
tristate "Intel MIC Bus Driver"
- depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
+ depends on 64BIT && PCI && X86
help
This option is selected by any driver which registers a
device or driver on the MIC Bus, such as CONFIG_INTEL_MIC_HOST,
@@ -21,7 +21,7 @@ comment "SCIF Bus Driver"
config SCIF_BUS
tristate "SCIF Bus Driver"
- depends on 64BIT && PCI && X86 && X86_DEV_DMA_OPS
+ depends on 64BIT && PCI && X86
help
This option is selected by any driver which registers a
device or driver on the SCIF Bus, such as CONFIG_INTEL_MIC_HOST
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 6012f3059acd..011c57cae4b0 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -267,6 +267,7 @@ config PCIE_TANGO_SMP8759
config VMD
depends on PCI_MSI && X86_64 && SRCU
+ select X86_DEV_DMA_OPS
tristate "Intel Volume Management Device Driver"
---help---
Adds support for the Intel Volume Management Device (VMD). VMD is a
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index cf6816b55b5e..999a5509e57e 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -95,10 +95,8 @@ struct vmd_dev {
struct irq_domain *irq_domain;
struct pci_bus *bus;
-#ifdef CONFIG_X86_DEV_DMA_OPS
struct dma_map_ops dma_ops;
struct dma_domain dma_domain;
-#endif
};
static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
@@ -293,7 +291,6 @@ static struct msi_domain_info vmd_msi_domain_info = {
.chip = _msi_controller,
};
-#ifdef CONFIG_X86_DEV_DMA_OPS
/*
* VMD replaces the requester ID with its own. DMA mappings for devices in a
* VMD domain need to be mapped for the VMD, not the device requiring
@@ -438,10 +435,6 @@ static void vmd_setup_dma_ops(struct vmd_dev *vmd)
add_dma_domain(domain);
}
#undef ASSIGN_VMD_DMA_OPS
-#else
-static void vmd_teardown_dma_ops(struct vmd_dev *vmd) {}
-static void vmd_setup_dma_ops(struct vmd_dev *vmd) {}
-#endif
static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
unsigned int devfn, int reg, int len)
--
2.20.1
