Re: [Nouveau] [PATCH v2] drm/nouveau/secboot: remove VLA usage

[Kees Cook]

On Thu, Mar 15, 2018 at 7:05 PM, Ben Skeggs  wrote:
> On 14 March 2018 at 21:08, Thierry Reding  wrote:
>> On Tue, Mar 13, 2018 at 11:24:11AM -0500, Gustavo A. R. Silva wrote:
>>> In preparation to enabling -Wvla, remove VLA. In this particular
>>> case directly use macro NVKM_MSGQUEUE_CMDLINE_SIZE instead of local
>>> variable cmdline_size. Also, remove cmdline_size as it is not
>>> actually useful anymore.
>>>
>>> The use of stack Variable Length Arrays needs to be avoided, as they
>>> can be a vector for stack exhaustion, which can be both a runtime bug
>>> or a security flaw. Also, in general, as code evolves it is easy to
>>> lose track of how big a VLA can get. Thus, we can end up having runtime
>>> failures that are hard to debug.
>>>
>>> Also, fixed as part of the directive to remove all VLAs from
>>> the kernel: https://lkml.org/lkml/2018/3/7/621
>>>
>>> Signed-off-by: Gustavo A. R. Silva 
>>> ---
>>> Changes in v2:
>>>  - Use sizeof(buf) instead of NVKM_MSGQUEUE_CMDLINE_SIZE. This change
>>>is based on the feedback provided by David Laight. Thanks David.
>>>
>>>  drivers/gpu/drm/nouveau/nvkm/subdev/secboot/ls_ucode_msgqueue.c | 7 +++
>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> Reviewed-by: Thierry Reding 
> Thanks everyone.  I've taken the patch in my tree.

Hi!

Just checking in on this -- I don't see this patch in linux-next. Is
this queued somewhere else?

Thanks!

-Kees

-- 
Kees Cook
Pixel Security
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH 3/4] ARM: dma-mapping: Implement arch_iommu_detach_device()

[kbuild test robot]

Hi Thierry,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Thierry-Reding/drm-nouveau-tegra-Detach-from-ARM-DMA-IOMMU-mapping/20180426-140854
config: arm-omap2plus_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   arch/arm/mm/dma-mapping.c: In function 'arch_iommu_detach_device':
>> arch/arm/mm/dma-mapping.c:2380:12: error: implicit declaration of function 
>> 'arm_get_dma_map_ops'; did you mean 'arm_get_iommu_dma_map_ops'? 
>> [-Werror=implicit-function-declaration]
 dma_ops = arm_get_dma_map_ops(dev->archdata.dma_coherent);
   ^~~
   arm_get_iommu_dma_map_ops
>> arch/arm/mm/dma-mapping.c:2380:10: warning: assignment makes pointer from 
>> integer without a cast [-Wint-conversion]
 dma_ops = arm_get_dma_map_ops(dev->archdata.dma_coherent);
 ^
   arch/arm/mm/dma-mapping.c: At top level:
>> arch/arm/mm/dma-mapping.c:2402:34: error: conflicting types for 
>> 'arm_get_dma_map_ops'
static const struct dma_map_ops *arm_get_dma_map_ops(bool coherent)
 ^~~
   arch/arm/mm/dma-mapping.c:2380:12: note: previous implicit declaration of 
'arm_get_dma_map_ops' was here
 dma_ops = arm_get_dma_map_ops(dev->archdata.dma_coherent);
   ^~~
   cc1: some warnings being treated as errors

vim +2380 arch/arm/mm/dma-mapping.c

  2368  
  2369  void arch_iommu_detach_device(struct device *dev)
  2370  {
  2371  struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
  2372  const struct dma_map_ops *dma_ops;
  2373  
  2374  if (!mapping)
  2375  return;
  2376  
  2377  arm_iommu_release_mapping(mapping);
  2378  arm_iommu_detach_device(dev);
  2379  
> 2380  dma_ops = arm_get_dma_map_ops(dev->archdata.dma_coherent);
  2381  set_dma_ops(dev, dma_ops);
  2382  }
  2383  
  2384  #else
  2385  
  2386  static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, 
u64 size,
  2387  const struct iommu_ops *iommu)
  2388  {
  2389  return false;
  2390  }
  2391  
  2392  static void arm_teardown_iommu_dma_ops(struct device *dev) { }
  2393  
  2394  #define arm_get_iommu_dma_map_ops arm_get_dma_map_ops
  2395  
  2396  void arch_iommu_detach_device(struct device *dev)
  2397  {
  2398  }
  2399  
  2400  #endif  /* CONFIG_ARM_DMA_USE_IOMMU */
  2401  
> 2402  static const struct dma_map_ops *arm_get_dma_map_ops(bool coherent)
  2403  {
  2404  return coherent ? _coherent_dma_ops : _dma_ops;
  2405  }
  2406  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 1/5] drm/nouveau: tegra: Detach from ARM DMA/IOMMU mapping

[Thierry Reding]

On Thu, Apr 26, 2018 at 03:59:04PM +0300, Mikko Perttunen wrote:
> On 26.04.2018 15:41, Thierry Reding wrote:
> > On Wed, Apr 25, 2018 at 09:28:49AM -0600, Jordan Crouse wrote:
> > > On Wed, Apr 25, 2018 at 12:10:47PM +0200, Thierry Reding wrote:
> > > > From: Thierry Reding 
> > > > 
> > > > Depending on the kernel configuration, early ARM architecture setup code
> > > > may have attached the GPU to a DMA/IOMMU mapping that transparently uses
> > > > the IOMMU to back the DMA API. Tegra requires special handling for IOMMU
> > > > backed buffers (a special bit in the GPU's MMU page tables indicates the
> > > > memory path to take: via the SMMU or directly to the memory controller).
> > > > Transparently backing DMA memory with an IOMMU prevents Nouveau from
> > > > properly handling such memory accesses and causes memory access faults.
> > > > 
> > > > As a side-note: buffers other than those allocated in instance memory
> > > > don't need to be physically contiguous from the GPU's perspective since
> > > > the GPU can map them into contiguous buffers using its own MMU. Mapping
> > > > these buffers through the IOMMU is unnecessary and will even lead to
> > > > performance degradation because of the additional translation.
> > > > 
> > > > Signed-off-by: Thierry Reding 
> > > > ---
> > > > I had already sent this out independently to fix a regression that was
> > > > introduced in v4.16, but then Christoph pointed out that it should've
> > > > been sent to a wider audience and should use a core API rather than
> > > > calling into architecture code directly.
> > > > 
> > > > I've added it to this series for easier reference and to show the need
> > > > for the new API.
> > > 
> > > This is good stuff, I am struggling with something similar on ARM64. One
> > > problem that I wasn't able to fully solve cleanly was that for arm-smmu
> > > the SMMU HW resources are not released until the domain itself is 
> > > destroyed
> > > and I never quite figured out a way to swap the default domain cleanly.
> > > 
> > > This is a problem for the MSM GPU because not only do we run our own 
> > > IOMMU as
> > > you do we also have a hardware dependency to use context bank 0 to
> > > asynchronously switch the pagetable during rendering.
> > > 
> > > I'm not sure if this is a problem you have encountered.
> > 
> > I don't think I have. Recent chips have similar capabilities, but they
> > don't have the restriction to a context bank, as far as I understand.
> > Adding Mikko who's had more exposure to this.
> 
> IIRC the only way I've gotten Host1x to work on Tegra186 with IOMMU enabled
> is doing the equivalent of this patch (or actually using the DMA API, which
> may be possible but has some potential issues).
> 
> As you said, we don't have a limitation regarding the context bank or
> similar - Host1x handles context switching by changing the sent stream ID on
> the fly (which is quite difficult to deal with from kernel point of view as
> well), and the actual GPU has its own MMU.

One instance where we still need the system MMU for GPU is to implement
support for big pages, which is required in order to do compression and
better performance in some other use-cases. I don't think we'll need
anything fancy like context switching in that case, though, because we
would use the SMMU exclusively to make sparse allocations look
contiguous to the GPU, so all of the per-process protection would still
be achieved via the GPU MMU.

Thierry


signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 1/5] drm/nouveau: tegra: Detach from ARM DMA/IOMMU mapping

[Mikko Perttunen]

On 26.04.2018 15:41, Thierry Reding wrote:

On Wed, Apr 25, 2018 at 09:28:49AM -0600, Jordan Crouse wrote:

On Wed, Apr 25, 2018 at 12:10:47PM +0200, Thierry Reding wrote:

From: Thierry Reding 

Depending on the kernel configuration, early ARM architecture setup code
may have attached the GPU to a DMA/IOMMU mapping that transparently uses
the IOMMU to back the DMA API. Tegra requires special handling for IOMMU
backed buffers (a special bit in the GPU's MMU page tables indicates the
memory path to take: via the SMMU or directly to the memory controller).
Transparently backing DMA memory with an IOMMU prevents Nouveau from
properly handling such memory accesses and causes memory access faults.

As a side-note: buffers other than those allocated in instance memory
don't need to be physically contiguous from the GPU's perspective since
the GPU can map them into contiguous buffers using its own MMU. Mapping
these buffers through the IOMMU is unnecessary and will even lead to
performance degradation because of the additional translation.

Signed-off-by: Thierry Reding 
---
I had already sent this out independently to fix a regression that was
introduced in v4.16, but then Christoph pointed out that it should've
been sent to a wider audience and should use a core API rather than
calling into architecture code directly.

I've added it to this series for easier reference and to show the need
for the new API.


This is good stuff, I am struggling with something similar on ARM64. One
problem that I wasn't able to fully solve cleanly was that for arm-smmu
the SMMU HW resources are not released until the domain itself is destroyed
and I never quite figured out a way to swap the default domain cleanly.

This is a problem for the MSM GPU because not only do we run our own IOMMU as
you do we also have a hardware dependency to use context bank 0 to
asynchronously switch the pagetable during rendering.

I'm not sure if this is a problem you have encountered.


I don't think I have. Recent chips have similar capabilities, but they
don't have the restriction to a context bank, as far as I understand.
Adding Mikko who's had more exposure to this.


IIRC the only way I've gotten Host1x to work on Tegra186 with IOMMU 
enabled is doing the equivalent of this patch (or actually using the DMA 
API, which may be possible but has some potential issues).


As you said, we don't have a limitation regarding the context bank or 
similar - Host1x handles context switching by changing the sent stream 
ID on the fly (which is quite difficult to deal with from kernel point 
of view as well), and the actual GPU has its own MMU.


Thanks,
Mikko




In any event, this code
gets us a little bit further down the path and at least there is somebody else
out there in the cold dark world that understands my pain. :)


This doesn't actually fix anything on 64-bit ARM, and oddly enough I
haven't run into this issue myself on 64-bit ARM either. I think the
reason is that I haven't tested Nouveau on Tegra186 yet, which is the
first SoC which has an ARM SMMU. On prior 64-bit ARM chips we've used
the custom Tegra SMMU and that driver simply forbids creating any DMA
domains, so it will allow only explicit usage of the IOMMU API. There
is code in the generic DMA/IOMMU integration layer to not use the DMA
API with non-DMA IOMMU domains, but that's not true on 32-bit ARM,
unfortunately. It's entirely possible that Tegra186 will show exactly
the same problem that you are describing.

We do use the IOMMU API explicitly in the Tegra DRM driver as well,
and that is something that I've tested on Tegra186 and that I know to
be working. However, the reason why it works there is that the IOMMU
group will contain multiple display controllers, which will again
trigger a special case that will prevent the DMA/IOMMU integration
from setting up a DMA domain for use with those devices.


For your interest, here was my half-hearted attempt to avoid creating DMA
domains in the first place based on a blacklist to try to spur a bit of
discussion: https://patchwork.freedesktop.org/series/41573/


This looks very interesting and simple, but I can imagine that it will
see significant pushback from the ARM SMMU maintainers (if not complete
silence), because it adds SoC-specific knowledge to an otherwise fully
generic driver. Having the GPU driver explicitly detach from the IOMMU
domain sounds like a better option, but I don't see how that would help
with the context bank issue that you're seeing.

One other possibility that I can imagine is to add something to struct
device that could be used by arch_setup_dma_ops() to not attach any of
the IOMMU-backed DMA operations to the device. Unfortunately that code
is called before the driver's ->probe() implementation is called, so a
driver doesn't have an opportunity to set it. Something like
of_dma_configure() could still set that up, perhaps based on some DT
property, though I can already hear 

Re: [Nouveau] [PATCH] drm: Don't pass the index to drm_property_add_enum()

[Lisovskiy, Stanislav]

Reviewed-by: Stanislav Lisovskiy 

Best Regards,

Lisovskiy Stanislav

Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo


From: Intel-gfx [intel-gfx-boun...@lists.freedesktop.org] on behalf of 
Lisovskiy, Stanislav [stanislav.lisovs...@intel.com]
Sent: Monday, April 23, 2018 4:59 PM
To: Ville Syrjala; dri-de...@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org; intel-...@lists.freedesktop.org; Ben Skeggs
Subject: Re: [Intel-gfx] [PATCH] drm: Don't pass the index to 
drm_property_add_enum()

Acked-by: Stanislav Lisovskiy 

Best Regards,

Lisovskiy Stanislav

Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo>


From: Ville Syrjala [ville.syrj...@linux.intel.com]
Sent: Friday, March 16, 2018 9:04 PM
To: dri-de...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org; Patrik Jakobsson; Ben Skeggs; 
nouveau@lists.freedesktop.org
Subject: [PATCH] drm: Don't pass the index to drm_property_add_enum()

From: Ville Syrjälä 

drm_property_add_enum() can calculate the index itself just fine,
so no point in having the caller pass it in.

Cc: Patrik Jakobsson 
Cc: Ben Skeggs 
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_connector.c   |  6 +++---
 drivers/gpu/drm/drm_property.c| 27 +--
 drivers/gpu/drm/gma500/cdv_device.c   |  4 ++--
 drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c |  5 ++---
 drivers/gpu/drm/nouveau/nouveau_display.c |  4 +---
 include/drm/drm_property.h|  2 +-
 7 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde897cd80..dfc8ca1e9413 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1069,7 +1069,7 @@ int drm_mode_create_tv_properties(struct drm_device *dev,
goto nomem;

for (i = 0; i < num_modes; i++)
-   drm_property_add_enum(dev->mode_config.tv_mode_property, i,
+   drm_property_add_enum(dev->mode_config.tv_mode_property,
  i, modes[i]);

dev->mode_config.tv_brightness_property =
@@ -1156,7 +1156,7 @@ int drm_connector_attach_scaling_mode_property(struct 
drm_connector *connector,
 {
struct drm_device *dev = connector->dev;
struct drm_property *scaling_mode_property;
-   int i, j = 0;
+   int i;
const unsigned valid_scaling_mode_mask =
(1U << ARRAY_SIZE(drm_scaling_mode_enum_list)) - 1;

@@ -1177,7 +1177,7 @@ int drm_connector_attach_scaling_mode_property(struct 
drm_connector *connector,
if (!(BIT(i) & scaling_mode_mask))
continue;

-   ret = drm_property_add_enum(scaling_mode_property, j++,
+   ret = drm_property_add_enum(scaling_mode_property,
drm_scaling_mode_enum_list[i].type,
drm_scaling_mode_enum_list[i].name);

diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 8f4672daac7f..1f8031e30f53 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -169,9 +169,9 @@ struct drm_property *drm_property_create_enum(struct 
drm_device *dev,
return NULL;

for (i = 0; i < num_values; i++) {
-   ret = drm_property_add_enum(property, i,
- props[i].type,
- props[i].name);
+   ret = drm_property_add_enum(property,
+   props[i].type,
+   props[i].name);
if (ret) {
drm_property_destroy(dev, property);
return NULL;
@@ -209,7 +209,7 @@ struct drm_property *drm_property_create_bitmask(struct 
drm_device *dev,
 uint64_t supported_bits)
 {
struct drm_property *property;
-   int i, ret, index = 0;
+   int i, ret;
int num_values = hweight64(supported_bits);

flags |= DRM_MODE_PROP_BITMASK;
@@ -221,14 +221,9 @@ struct drm_property *drm_property_create_bitmask(struct 
drm_device *dev,
if (!(supported_bits & (1ULL << props[i].type)))
continue;

-   if (WARN_ON(index >= num_values)) {
-   drm_property_destroy(dev, property);
-   return NULL;
-   }
-
-   ret = drm_property_add_enum(property, index++,
- 

Re: [Nouveau] [PATCH v2 1/5] drm/nouveau: tegra: Detach from ARM DMA/IOMMU mapping

[Thierry Reding]

On Wed, Apr 25, 2018 at 09:28:49AM -0600, Jordan Crouse wrote:
> On Wed, Apr 25, 2018 at 12:10:47PM +0200, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > Depending on the kernel configuration, early ARM architecture setup code
> > may have attached the GPU to a DMA/IOMMU mapping that transparently uses
> > the IOMMU to back the DMA API. Tegra requires special handling for IOMMU
> > backed buffers (a special bit in the GPU's MMU page tables indicates the
> > memory path to take: via the SMMU or directly to the memory controller).
> > Transparently backing DMA memory with an IOMMU prevents Nouveau from
> > properly handling such memory accesses and causes memory access faults.
> > 
> > As a side-note: buffers other than those allocated in instance memory
> > don't need to be physically contiguous from the GPU's perspective since
> > the GPU can map them into contiguous buffers using its own MMU. Mapping
> > these buffers through the IOMMU is unnecessary and will even lead to
> > performance degradation because of the additional translation.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> > I had already sent this out independently to fix a regression that was
> > introduced in v4.16, but then Christoph pointed out that it should've
> > been sent to a wider audience and should use a core API rather than
> > calling into architecture code directly.
> > 
> > I've added it to this series for easier reference and to show the need
> > for the new API.
> 
> This is good stuff, I am struggling with something similar on ARM64. One
> problem that I wasn't able to fully solve cleanly was that for arm-smmu 
> the SMMU HW resources are not released until the domain itself is destroyed
> and I never quite figured out a way to swap the default domain cleanly.
> 
> This is a problem for the MSM GPU because not only do we run our own IOMMU as
> you do we also have a hardware dependency to use context bank 0 to
> asynchronously switch the pagetable during rendering.
> 
> I'm not sure if this is a problem you have encountered.

I don't think I have. Recent chips have similar capabilities, but they
don't have the restriction to a context bank, as far as I understand.
Adding Mikko who's had more exposure to this.

> In any event, this code
> gets us a little bit further down the path and at least there is somebody else
> out there in the cold dark world that understands my pain. :)

This doesn't actually fix anything on 64-bit ARM, and oddly enough I
haven't run into this issue myself on 64-bit ARM either. I think the
reason is that I haven't tested Nouveau on Tegra186 yet, which is the
first SoC which has an ARM SMMU. On prior 64-bit ARM chips we've used
the custom Tegra SMMU and that driver simply forbids creating any DMA
domains, so it will allow only explicit usage of the IOMMU API. There
is code in the generic DMA/IOMMU integration layer to not use the DMA
API with non-DMA IOMMU domains, but that's not true on 32-bit ARM,
unfortunately. It's entirely possible that Tegra186 will show exactly
the same problem that you are describing.

We do use the IOMMU API explicitly in the Tegra DRM driver as well,
and that is something that I've tested on Tegra186 and that I know to
be working. However, the reason why it works there is that the IOMMU
group will contain multiple display controllers, which will again
trigger a special case that will prevent the DMA/IOMMU integration
from setting up a DMA domain for use with those devices.

> For your interest, here was my half-hearted attempt to avoid creating DMA
> domains in the first place based on a blacklist to try to spur a bit of
> discussion: https://patchwork.freedesktop.org/series/41573/

This looks very interesting and simple, but I can imagine that it will
see significant pushback from the ARM SMMU maintainers (if not complete
silence), because it adds SoC-specific knowledge to an otherwise fully
generic driver. Having the GPU driver explicitly detach from the IOMMU
domain sounds like a better option, but I don't see how that would help
with the context bank issue that you're seeing.

One other possibility that I can imagine is to add something to struct
device that could be used by arch_setup_dma_ops() to not attach any of
the IOMMU-backed DMA operations to the device. Unfortunately that code
is called before the driver's ->probe() implementation is called, so a
driver doesn't have an opportunity to set it. Something like
of_dma_configure() could still set that up, perhaps based on some DT
property, though I can already hear the NAK from DT maintainers because
this is, after all, policy, not hardware description.

The last solution that I can think of that might allow us to do this is
to add a flag to struct device_driver (bool explicit_iommu?) that will
be used by the DMA/IOMMU setup code to decide whether or not to attach
to the IOMMU automatically.

Though, again, I'm not sure that would actually solve your bank problem.

Re: [Nouveau] [PATCH v2 3/5] ARM: dma-mapping: Implement arch_iommu_detach_device()

[Thierry Reding]

On Wed, Apr 25, 2018 at 08:20:49AM -0700, Christoph Hellwig wrote:
> > +void arch_iommu_detach_device(struct device *dev)
> > +{
> > +#ifdef CONFIG_ARM_DMA_USE_IOMMU
> > +   struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
> > +   const struct dma_map_ops *dma_ops;
> > +
> > +   if (!mapping)
> > +   return;
> > +
> > +   arm_iommu_release_mapping(mapping);
> > +   arm_iommu_detach_device(dev);
> > +
> > +   dma_ops = arm_get_dma_map_ops(dev->archdata.dma_coherent);
> > +   set_dma_ops(dev, dma_ops);
> 
> Why not simply:
> 
>   set_dma_ops(dev, arm_get_dma_map_ops(dev->archdata.dma_coherent));

I had that initially, but it looked cluttered to me, so I split it up.
I don't care much either way, so I can revert to that if you prefer.

Thierry


signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 2/5] dma-mapping: Introduce dma_iommu_detach_device() API

[Thierry Reding]

On Wed, Apr 25, 2018 at 08:19:34AM -0700, Christoph Hellwig wrote:
> On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > The dma_iommu_detach_device() API can be used by drivers to forcibly
> > detach a device from an IOMMU that architecture code might have attached
> > to. This is useful for drivers that need explicit control over the IOMMU
> > using the IOMMU API directly.
> 
> Given that no one else implements it making it a generic API seems
> rather confusing.  For now I'd rename it to
> arm_dma_iommu_detach_device() and only implement it in arm.

That'd be suboptimal because this code is used on both 32-bit and 64-bit
ARM. If we make the function 32-bit ARM specific then the driver code
would need to use an #ifdef to make sure compilation doesn't break on
64-bit ARM.

Thierry


signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH v2 1/5] drm/nouveau: tegra: Detach from ARM DMA/IOMMU mapping

[Thierry Reding]

On Wed, Apr 25, 2018 at 08:18:15AM -0700, Christoph Hellwig wrote:
> The series seems to miss a cover letter.
> 
> Also I really think this patch original patch shouldn't be in the proper
> series.

I added a note explaining why I included this. Not everyone in this
discussion had seen the patch and therefore may not be aware of the
problem that the series attempts to fix. I agree that it shouldn't
be merged as part of the series, though.

Thierry


signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau