| Message ID | 1412941908-5850-3-git-send-email-stefano.stabellini@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > Introduce a function to check whether a device is dma coherent. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > CC: linux@arm.linux.org.uk > CC: catalin.marinas@arm.com > CC: will.deacon@arm.com > CC: linux-arm-kernel@lists.infradead.org > --- > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > 2 files changed, 11 insertions(+) > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > index c45b61a..bededbb 100644 > --- a/arch/arm/include/asm/dma-mapping.h > +++ b/arch/arm/include/asm/dma-mapping.h > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > set_dma_ops(dev, &arm_coherent_dma_ops); > return 0; > } > + > +static inline bool is_dma_coherent(struct device *dev) > +{ > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > +} Hmm, what about the IOMMU ops? Will
On Fri, 10 Oct 2014, Will Deacon wrote: > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > Introduce a function to check whether a device is dma coherent. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > CC: linux@arm.linux.org.uk > > CC: catalin.marinas@arm.com > > CC: will.deacon@arm.com > > CC: linux-arm-kernel@lists.infradead.org > > --- > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > 2 files changed, 11 insertions(+) > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > index c45b61a..bededbb 100644 > > --- a/arch/arm/include/asm/dma-mapping.h > > +++ b/arch/arm/include/asm/dma-mapping.h > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > set_dma_ops(dev, &arm_coherent_dma_ops); > > return 0; > > } > > + > > +static inline bool is_dma_coherent(struct device *dev) > > +{ > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > +} > > Hmm, what about the IOMMU ops? Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? Do you have any better suggestions?
On Fri, 10 Oct 2014, Stefano Stabellini wrote: > On Fri, 10 Oct 2014, Will Deacon wrote: > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > Introduce a function to check whether a device is dma coherent. > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > CC: linux@arm.linux.org.uk > > > CC: catalin.marinas@arm.com > > > CC: will.deacon@arm.com > > > CC: linux-arm-kernel@lists.infradead.org > > > --- > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > 2 files changed, 11 insertions(+) > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > index c45b61a..bededbb 100644 > > > --- a/arch/arm/include/asm/dma-mapping.h > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > return 0; > > > } > > > + > > > +static inline bool is_dma_coherent(struct device *dev) > > > +{ > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > +} > > > > Hmm, what about the IOMMU ops? > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > Do you have any better suggestions? ping?
On Mon, Oct 13, 2014 at 12:16:14PM +0100, Stefano Stabellini wrote: > On Fri, 10 Oct 2014, Stefano Stabellini wrote: > > On Fri, 10 Oct 2014, Will Deacon wrote: > > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > > Introduce a function to check whether a device is dma coherent. > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > CC: linux@arm.linux.org.uk > > > > CC: catalin.marinas@arm.com > > > > CC: will.deacon@arm.com > > > > CC: linux-arm-kernel@lists.infradead.org > > > > --- > > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > > 2 files changed, 11 insertions(+) > > > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > > index c45b61a..bededbb 100644 > > > > --- a/arch/arm/include/asm/dma-mapping.h > > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > > return 0; > > > > } > > > > + > > > > +static inline bool is_dma_coherent(struct device *dev) > > > > +{ > > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > > +} > > > > > > Hmm, what about the IOMMU ops? > > > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > > Do you have any better suggestions? > > ping? Without any clear idea about why this is needed or how it's used, I don't have any better ideas. Will
On Mon, 13 Oct 2014, Will Deacon wrote: > On Mon, Oct 13, 2014 at 12:16:14PM +0100, Stefano Stabellini wrote: > > On Fri, 10 Oct 2014, Stefano Stabellini wrote: > > > On Fri, 10 Oct 2014, Will Deacon wrote: > > > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > > > Introduce a function to check whether a device is dma coherent. > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > CC: linux@arm.linux.org.uk > > > > > CC: catalin.marinas@arm.com > > > > > CC: will.deacon@arm.com > > > > > CC: linux-arm-kernel@lists.infradead.org > > > > > --- > > > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > > > 2 files changed, 11 insertions(+) > > > > > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > > > index c45b61a..bededbb 100644 > > > > > --- a/arch/arm/include/asm/dma-mapping.h > > > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > > > return 0; > > > > > } > > > > > + > > > > > +static inline bool is_dma_coherent(struct device *dev) > > > > > +{ > > > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > > > +} > > > > > > > > Hmm, what about the IOMMU ops? > > > > > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > > > Do you have any better suggestions? > > > > ping? > > Without any clear idea about why this is needed or how it's used, I don't > have any better ideas. Thanks for the quick reply. It is used in dom0 to figure out whether the device is not coherent: in that case Dom0 is going to issue an hypercall and Xen is going to execute the required cache maintenance operations on Dom0's behalf. In practice for this use case iommu_dma_ops is not a problem, as the device is never going to appear as being behind an SMMU in dom0.
On Mon, 13 Oct 2014, Stefano Stabellini wrote: > On Mon, 13 Oct 2014, Will Deacon wrote: > > On Mon, Oct 13, 2014 at 12:16:14PM +0100, Stefano Stabellini wrote: > > > On Fri, 10 Oct 2014, Stefano Stabellini wrote: > > > > On Fri, 10 Oct 2014, Will Deacon wrote: > > > > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > > > > Introduce a function to check whether a device is dma coherent. > > > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > CC: linux@arm.linux.org.uk > > > > > > CC: catalin.marinas@arm.com > > > > > > CC: will.deacon@arm.com > > > > > > CC: linux-arm-kernel@lists.infradead.org > > > > > > --- > > > > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > > > > 2 files changed, 11 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > > > > index c45b61a..bededbb 100644 > > > > > > --- a/arch/arm/include/asm/dma-mapping.h > > > > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > > > > return 0; > > > > > > } > > > > > > + > > > > > > +static inline bool is_dma_coherent(struct device *dev) > > > > > > +{ > > > > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > > > > +} > > > > > > > > > > Hmm, what about the IOMMU ops? > > > > > > > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > > > > Do you have any better suggestions? > > > > > > ping? > > > > Without any clear idea about why this is needed or how it's used, I don't > > have any better ideas. > > Thanks for the quick reply. > > It is used in dom0 to figure out whether the device is not coherent: in > that case Dom0 is going to issue an hypercall and Xen is going to > execute the required cache maintenance operations on Dom0's behalf. > > In practice for this use case iommu_dma_ops is not a problem, as the > device is never going to appear as being behind an SMMU in dom0. re-ping?
On Wed, 22 Oct 2014, Stefano Stabellini wrote: > On Mon, 13 Oct 2014, Stefano Stabellini wrote: > > On Mon, 13 Oct 2014, Will Deacon wrote: > > > On Mon, Oct 13, 2014 at 12:16:14PM +0100, Stefano Stabellini wrote: > > > > On Fri, 10 Oct 2014, Stefano Stabellini wrote: > > > > > On Fri, 10 Oct 2014, Will Deacon wrote: > > > > > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > > > > > Introduce a function to check whether a device is dma coherent. > > > > > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > > CC: linux@arm.linux.org.uk > > > > > > > CC: catalin.marinas@arm.com > > > > > > > CC: will.deacon@arm.com > > > > > > > CC: linux-arm-kernel@lists.infradead.org > > > > > > > --- > > > > > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > > > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > > > > > 2 files changed, 11 insertions(+) > > > > > > > > > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > > > > > index c45b61a..bededbb 100644 > > > > > > > --- a/arch/arm/include/asm/dma-mapping.h > > > > > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > > > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > > > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > > > > > return 0; > > > > > > > } > > > > > > > + > > > > > > > +static inline bool is_dma_coherent(struct device *dev) > > > > > > > +{ > > > > > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > > > > > +} > > > > > > > > > > > > Hmm, what about the IOMMU ops? > > > > > > > > > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > > > > > Do you have any better suggestions? > > > > > > > > ping? > > > > > > Without any clear idea about why this is needed or how it's used, I don't > > > have any better ideas. > > > > Thanks for the quick reply. > > > > It is used in dom0 to figure out whether the device is not coherent: in > > that case Dom0 is going to issue an hypercall and Xen is going to > > execute the required cache maintenance operations on Dom0's behalf. > > > > In practice for this use case iommu_dma_ops is not a problem, as the > > device is never going to appear as being behind an SMMU in dom0. > > re-ping? I don't mean to be pushy but this series is required to fix a regression that causes dom0 to crash when running as dom0 on Xen on ARM if non-coherent devices are present on the platform. We should aim at getting it in 3.18 or we are going to release a broken kernel. I am thinking of reposting the series keeping the ugly is_dma_coherent function in a Xen specific source file as a static function and then fix it up properly in common code later with your help?
On Thu, Oct 23, 2014 at 03:22:27PM +0100, Stefano Stabellini wrote: > On Wed, 22 Oct 2014, Stefano Stabellini wrote: > > On Mon, 13 Oct 2014, Stefano Stabellini wrote: > > > On Mon, 13 Oct 2014, Will Deacon wrote: > > > > On Mon, Oct 13, 2014 at 12:16:14PM +0100, Stefano Stabellini wrote: > > > > > On Fri, 10 Oct 2014, Stefano Stabellini wrote: > > > > > > On Fri, 10 Oct 2014, Will Deacon wrote: > > > > > > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > > > > > > Introduce a function to check whether a device is dma coherent. > > > > > > > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > > > CC: linux@arm.linux.org.uk > > > > > > > > CC: catalin.marinas@arm.com > > > > > > > > CC: will.deacon@arm.com > > > > > > > > CC: linux-arm-kernel@lists.infradead.org > > > > > > > > --- > > > > > > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > > > > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > > > > > > 2 files changed, 11 insertions(+) > > > > > > > > > > > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > > > > > > index c45b61a..bededbb 100644 > > > > > > > > --- a/arch/arm/include/asm/dma-mapping.h > > > > > > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > > > > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > > > > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > > > > > > return 0; > > > > > > > > } > > > > > > > > + > > > > > > > > +static inline bool is_dma_coherent(struct device *dev) > > > > > > > > +{ > > > > > > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > > > > > > +} > > > > > > > > > > > > > > Hmm, what about the IOMMU ops? > > > > > > > > > > > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > > > > > > Do you have any better suggestions? > > > > > > > > > > ping? > > > > > > > > Without any clear idea about why this is needed or how it's used, I don't > > > > have any better ideas. > > > > > > Thanks for the quick reply. > > > > > > It is used in dom0 to figure out whether the device is not coherent: in > > > that case Dom0 is going to issue an hypercall and Xen is going to > > > execute the required cache maintenance operations on Dom0's behalf. > > > > > > In practice for this use case iommu_dma_ops is not a problem, as the > > > device is never going to appear as being behind an SMMU in dom0. > > > > re-ping? > > I don't mean to be pushy but this series is required to fix a regression > that causes dom0 to crash when running as dom0 on Xen on ARM if > non-coherent devices are present on the platform. Sorry for ignoring it but I don't fully understand how Xen works, especially this DMA stuff. I recall some blogs you wrote but I don't have time to re-read them (and I'm on holiday next week). I think a better way would be some Xen hook around set_arch_dma_coherent_ops(). Does Xen have its own device tracking structures? If not, you may be able to add another bitfield to the kernel one.
On Fri, 24 Oct 2014, Catalin Marinas wrote: > On Thu, Oct 23, 2014 at 03:22:27PM +0100, Stefano Stabellini wrote: > > On Wed, 22 Oct 2014, Stefano Stabellini wrote: > > > On Mon, 13 Oct 2014, Stefano Stabellini wrote: > > > > On Mon, 13 Oct 2014, Will Deacon wrote: > > > > > On Mon, Oct 13, 2014 at 12:16:14PM +0100, Stefano Stabellini wrote: > > > > > > On Fri, 10 Oct 2014, Stefano Stabellini wrote: > > > > > > > On Fri, 10 Oct 2014, Will Deacon wrote: > > > > > > > > On Fri, Oct 10, 2014 at 12:51:44PM +0100, Stefano Stabellini wrote: > > > > > > > > > Introduce a function to check whether a device is dma coherent. > > > > > > > > > > > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > > > > CC: linux@arm.linux.org.uk > > > > > > > > > CC: catalin.marinas@arm.com > > > > > > > > > CC: will.deacon@arm.com > > > > > > > > > CC: linux-arm-kernel@lists.infradead.org > > > > > > > > > --- > > > > > > > > > arch/arm/include/asm/dma-mapping.h | 6 ++++++ > > > > > > > > > arch/arm64/include/asm/dma-mapping.h | 5 +++++ > > > > > > > > > 2 files changed, 11 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h > > > > > > > > > index c45b61a..bededbb 100644 > > > > > > > > > --- a/arch/arm/include/asm/dma-mapping.h > > > > > > > > > +++ b/arch/arm/include/asm/dma-mapping.h > > > > > > > > > @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) > > > > > > > > > set_dma_ops(dev, &arm_coherent_dma_ops); > > > > > > > > > return 0; > > > > > > > > > } > > > > > > > > > + > > > > > > > > > +static inline bool is_dma_coherent(struct device *dev) > > > > > > > > > +{ > > > > > > > > > + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); > > > > > > > > > +} > > > > > > > > > > > > > > > > Hmm, what about the IOMMU ops? > > > > > > > > > > > > > > Maybe I should check __generic_dma_ops(dev) != &arm_dma_ops? > > > > > > > Do you have any better suggestions? > > > > > > > > > > > > ping? > > > > > > > > > > Without any clear idea about why this is needed or how it's used, I don't > > > > > have any better ideas. > > > > > > > > Thanks for the quick reply. > > > > > > > > It is used in dom0 to figure out whether the device is not coherent: in > > > > that case Dom0 is going to issue an hypercall and Xen is going to > > > > execute the required cache maintenance operations on Dom0's behalf. > > > > > > > > In practice for this use case iommu_dma_ops is not a problem, as the > > > > device is never going to appear as being behind an SMMU in dom0. > > > > > > re-ping? > > > > I don't mean to be pushy but this series is required to fix a regression > > that causes dom0 to crash when running as dom0 on Xen on ARM if > > non-coherent devices are present on the platform. > > Sorry for ignoring it but I don't fully understand how Xen works, > especially this DMA stuff. I recall some blogs you wrote but I don't > have time to re-read them (and I'm on holiday next week). No worries. > I think a better way would be some Xen hook around > set_arch_dma_coherent_ops(). Does Xen have its own device tracking > structures? If not, you may be able to add another bitfield to the > kernel one. We don't have an additional device tracking struct on Xen. I agree that a new bit somewhere would be the best solution, but I am not sure where. Maybe in dev_archdata under arm and arm64? After all it is already used to keep pointers to dma and coherency related structures. However given the timing constraints I hope you would be OK with the suboptimal solution for now and create a common is_dma_coherent function in 3.19?
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h index c45b61a..bededbb 100644 --- a/arch/arm/include/asm/dma-mapping.h +++ b/arch/arm/include/asm/dma-mapping.h @@ -126,6 +126,12 @@ static inline int set_arch_dma_coherent_ops(struct device *dev) set_dma_ops(dev, &arm_coherent_dma_ops); return 0; } + +static inline bool is_dma_coherent(struct device *dev) +{ + return (__generic_dma_ops(dev) == &arm_coherent_dma_ops); +} + #define set_arch_dma_coherent_ops(dev) set_arch_dma_coherent_ops(dev) static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h index dc82e52..d51d67e 100644 --- a/arch/arm64/include/asm/dma-mapping.h +++ b/arch/arm64/include/asm/dma-mapping.h @@ -52,6 +52,11 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops) dev->archdata.dma_ops = ops; } +static inline bool is_dma_coherent(struct device *dev) +{ + return (__generic_dma_ops(dev) == &coherent_swiotlb_dma_ops); +} + #include <asm-generic/dma-mapping-common.h> static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
Introduce a function to check whether a device is dma coherent. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> CC: linux@arm.linux.org.uk CC: catalin.marinas@arm.com CC: will.deacon@arm.com CC: linux-arm-kernel@lists.infradead.org --- arch/arm/include/asm/dma-mapping.h | 6 ++++++ arch/arm64/include/asm/dma-mapping.h | 5 +++++ 2 files changed, 11 insertions(+)