Re: [PATCH v2 14/22] gpu: host1x: Forbid relocation address shifting in the firewall

2017-06-14 Thread Dmitry Osipenko 
On 14.06.2017 09:50, Mikko Perttunen wrote:
> On 14.06.2017 02:15, Dmitry Osipenko wrote:
>> Incorrectly shifted relocation address will cause a lower memory corruption
>> and likely a hang on a write or a read of an arbitrary data in case of IOMMU
>> absent. As of now there is no use for the address shifting (at least on
>> Tegra20) and adding a proper shifting / sizes validation is much more work.
>> Let's forbid it in the firewall till a proper validation is implemented.
>>
>> Signed-off-by: Dmitry Osipenko 
>> Reviewed-by: Erik Faye-Lund 
>> ---
>>  drivers/gpu/host1x/dev.c |  4 
>>  drivers/gpu/host1x/dev.h |  1 +
>>  drivers/gpu/host1x/job.c | 24 
>>  3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
>> index 6a805ed908c3..21da331ce092 100644
>> --- a/drivers/gpu/host1x/dev.c
>> +++ b/drivers/gpu/host1x/dev.c
>> @@ -72,6 +72,7 @@ static const struct host1x_info host1x01_info = {
>>  .init = host1x01_init,
>>  .sync_offset = 0x3000,
>>  .dma_mask = DMA_BIT_MASK(32),
>> +.version = 1,
>>  };
>>
>>  static const struct host1x_info host1x02_info = {
>> @@ -82,6 +83,7 @@ static const struct host1x_info host1x02_info = {
>>  .init = host1x02_init,
>>  .sync_offset = 0x3000,
>>  .dma_mask = DMA_BIT_MASK(32),
>> +.version = 2,
>>  };
>>
>>  static const struct host1x_info host1x04_info = {
>> @@ -92,6 +94,7 @@ static const struct host1x_info host1x04_info = {
>>  .init = host1x04_init,
>>  .sync_offset = 0x2100,
>>  .dma_mask = DMA_BIT_MASK(34),
>> +.version = 4,
>>  };
>>
>>  static const struct host1x_info host1x05_info = {
>> @@ -102,6 +105,7 @@ static const struct host1x_info host1x05_info = {
>>  .init = host1x05_init,
>>  .sync_offset = 0x2100,
>>  .dma_mask = DMA_BIT_MASK(34),
>> +.version = 5,
>>  };
>>
>>  static const struct of_device_id host1x_of_match[] = {
>> diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
>> index 229d08b6a45e..c6997651b336 100644
>> --- a/drivers/gpu/host1x/dev.h
>> +++ b/drivers/gpu/host1x/dev.h
>> @@ -100,6 +100,7 @@ struct host1x_info {
>>  int (*init)(struct host1x *host1x); /* initialize per SoC ops */
>>  unsigned int sync_offset; /* offset of syncpoint registers */
>>  u64 dma_mask; /* mask of addressable memory */
>> +unsigned int version; /* host1x's version */
>>  };
>>
>>  struct host1x {
>> diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
>> index 4208329ca2af..7825643da324 100644
>> --- a/drivers/gpu/host1x/job.c
>> +++ b/drivers/gpu/host1x/job.c
>> @@ -333,6 +333,27 @@ static bool check_reloc(struct host1x_reloc *reloc,
>> struct host1x_bo *cmdbuf,
>>  return true;
>>  }
>>
>> +static bool check_reloc_shift(struct device *dev, struct host1x_reloc 
>> *reloc)
>> +{
>> +struct host1x *host = dev_get_drvdata(dev->parent);
>> +
>> +/* skip newer Tegra's since IOMMU is supposed to be used by
>> + * them and not all address registers with their shifts are
>> + * publicly documented */
>> +if (host->info->version > 1)
>> +return true;
> 
> I don't like the firewall working differently per SoC - IOMMU can still be
> disabled on later SoCs and in this case there would be a security hole even if
> the firewall is enabled.
> 

Yes, it would be a security hole like it is is right now. Unfortunately we can't
fix it on later SoC's without knowing all the address registers and their 
shifts.

>> +
>> +/* skip Tegra30 with IOMMU enabled */
>> +if (host->domain)
>> +return true;
>> +
> 
> So if we want to be able to test the firewall on all SoCs just having this 
> check
> here for all SoCs would be better.
> 

I'm not sure what you are meaning here. You'll get a wrong address resolved in
HW if that address needs to be shifted.

As I wrote before, on later SoC's firewall could be used as a debug feature
without the shifts being validated.

>> +/* relocation shift value validation isn't implemented yet */
>> +if (reloc->shift)
>> +return false;
>> +
>> +return true;
>> +}
>> +
>>  struct host1x_firewall {
>>  struct host1x_job *job;
>>  struct device *dev;
>> @@ -359,6 +380,9 @@ static int check_register(struct host1x_firewall *fw,
>> unsigned long offset)
>>  if (!check_reloc(fw->reloc, fw->cmdbuf, fw->offset))
>>  return -EINVAL;
>>
>> +if (!check_reloc_shift(fw->dev, fw->reloc))
>> +return -EINVAL;
>> +
>>  fw->num_relocs--;
>>  fw->reloc++;
>>  }
>>


-- 
Dmitry
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 14/22] gpu: host1x: Forbid relocation address shifting in the firewall

2017-06-14 Thread Mikko Perttunen 

On 06/14/2017 05:49 PM, Dmitry Osipenko wrote:

On 14.06.2017 14:47, Mikko Perttunen wrote:



On 14.06.2017 12:06, Dmitry Osipenko wrote:

On 14.06.2017 09:50, Mikko Perttunen wrote:

On 14.06.2017 02:15, Dmitry Osipenko wrote:

Incorrectly shifted relocation address will cause a lower memory corruption
and likely a hang on a write or a read of an arbitrary data in case of IOMMU
absent. As of now there is no use for the address shifting (at least on
Tegra20) and adding a proper shifting / sizes validation is much more work.
Let's forbid it in the firewall till a proper validation is implemented.

Signed-off-by: Dmitry Osipenko 
Reviewed-by: Erik Faye-Lund 
---
  drivers/gpu/host1x/dev.c |  4 
  drivers/gpu/host1x/dev.h |  1 +
  drivers/gpu/host1x/job.c | 24 
  3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 6a805ed908c3..21da331ce092 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -72,6 +72,7 @@ static const struct host1x_info host1x01_info = {
  .init = host1x01_init,
  .sync_offset = 0x3000,
  .dma_mask = DMA_BIT_MASK(32),
+.version = 1,
  };

  static const struct host1x_info host1x02_info = {
@@ -82,6 +83,7 @@ static const struct host1x_info host1x02_info = {
  .init = host1x02_init,
  .sync_offset = 0x3000,
  .dma_mask = DMA_BIT_MASK(32),
+.version = 2,
  };

  static const struct host1x_info host1x04_info = {
@@ -92,6 +94,7 @@ static const struct host1x_info host1x04_info = {
  .init = host1x04_init,
  .sync_offset = 0x2100,
  .dma_mask = DMA_BIT_MASK(34),
+.version = 4,
  };

  static const struct host1x_info host1x05_info = {
@@ -102,6 +105,7 @@ static const struct host1x_info host1x05_info = {
  .init = host1x05_init,
  .sync_offset = 0x2100,
  .dma_mask = DMA_BIT_MASK(34),
+.version = 5,
  };

  static const struct of_device_id host1x_of_match[] = {
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index 229d08b6a45e..c6997651b336 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -100,6 +100,7 @@ struct host1x_info {
  int (*init)(struct host1x *host1x); /* initialize per SoC ops */
  unsigned int sync_offset; /* offset of syncpoint registers */
  u64 dma_mask; /* mask of addressable memory */
+unsigned int version; /* host1x's version */
  };

  struct host1x {
diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 4208329ca2af..7825643da324 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -333,6 +333,27 @@ static bool check_reloc(struct host1x_reloc *reloc,
struct host1x_bo *cmdbuf,
  return true;
  }

+static bool check_reloc_shift(struct device *dev, struct host1x_reloc *reloc)
+{
+struct host1x *host = dev_get_drvdata(dev->parent);
+
+/* skip newer Tegra's since IOMMU is supposed to be used by
+ * them and not all address registers with their shifts are
+ * publicly documented */
+if (host->info->version > 1)
+return true;


I don't like the firewall working differently per SoC - IOMMU can still be
disabled on later SoCs and in this case there would be a security hole even if
the firewall is enabled.



Yes, it would be a security hole like it is is right now. Unfortunately we can't
fix it on later SoC's without knowing all the address registers and their 
shifts.


Indeed, we can't fix it, but I was thinking that having the firewall enabled in
the kernel should always mean that the system is safe, even if IOMMU is
disabled, and even if it means that you cannot really do anything useful with it
(at least running host1x_test should be possible without shifts, though). Thus
no matter what the hardware revision, we should reject anything we cannot ensure
as safe.




+
+/* skip Tegra30 with IOMMU enabled */
+if (host->domain)
+return true;
+


So if we want to be able to test the firewall on all SoCs just having this check
here for all SoCs would be better.



I'm not sure what you are meaning here. You'll get a wrong address resolved in
HW if that address needs to be shifted.


So we have two choices here:
a) If IOMMU is enabled, skip the shift check and return true always. This means
that we still ensure that the system is safe, since with IOMMU enabled it is
safe even if the firewall cannot ensure that the job is proper. This is what I
was trying to say above.
b) Always return false if there is a shift, on any SoC and even if IOMMU is
enabled. This would harmonize the behavior of the firewall across systems.


Okay, I think b) is a better choice, actually that's what V1 of the patch did.
Preserving behaviour regardless of the IOMMU state is reasonable and this is an
opensource driver that is supposed to be used with the opensource userspace
after all.

Later I'd want to stricter the firewall checks to disallow any unknown register
accesses,

Re: [PATCH v2 14/22] gpu: host1x: Forbid relocation address shifting in the firewall

2017-06-14 Thread Mikko Perttunen 

On 14.06.2017 02:15, Dmitry Osipenko wrote:

Incorrectly shifted relocation address will cause a lower memory corruption
and likely a hang on a write or a read of an arbitrary data in case of IOMMU
absent. As of now there is no use for the address shifting (at least on
Tegra20) and adding a proper shifting / sizes validation is much more work.
Let's forbid it in the firewall till a proper validation is implemented.

Signed-off-by: Dmitry Osipenko 
Reviewed-by: Erik Faye-Lund 
---
 drivers/gpu/host1x/dev.c |  4 
 drivers/gpu/host1x/dev.h |  1 +
 drivers/gpu/host1x/job.c | 24 
 3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 6a805ed908c3..21da331ce092 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -72,6 +72,7 @@ static const struct host1x_info host1x01_info = {
.init = host1x01_init,
.sync_offset = 0x3000,
.dma_mask = DMA_BIT_MASK(32),
+   .version = 1,
 };

 static const struct host1x_info host1x02_info = {
@@ -82,6 +83,7 @@ static const struct host1x_info host1x02_info = {
.init = host1x02_init,
.sync_offset = 0x3000,
.dma_mask = DMA_BIT_MASK(32),
+   .version = 2,
 };

 static const struct host1x_info host1x04_info = {
@@ -92,6 +94,7 @@ static const struct host1x_info host1x04_info = {
.init = host1x04_init,
.sync_offset = 0x2100,
.dma_mask = DMA_BIT_MASK(34),
+   .version = 4,
 };

 static const struct host1x_info host1x05_info = {
@@ -102,6 +105,7 @@ static const struct host1x_info host1x05_info = {
.init = host1x05_init,
.sync_offset = 0x2100,
.dma_mask = DMA_BIT_MASK(34),
+   .version = 5,
 };

 static const struct of_device_id host1x_of_match[] = {
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index 229d08b6a45e..c6997651b336 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -100,6 +100,7 @@ struct host1x_info {
int (*init)(struct host1x *host1x); /* initialize per SoC ops */
unsigned int sync_offset; /* offset of syncpoint registers */
u64 dma_mask; /* mask of addressable memory */
+   unsigned int version; /* host1x's version */
 };

 struct host1x {
diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 4208329ca2af..7825643da324 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -333,6 +333,27 @@ static bool check_reloc(struct host1x_reloc *reloc, struct 
host1x_bo *cmdbuf,
return true;
 }

+static bool check_reloc_shift(struct device *dev, struct host1x_reloc *reloc)
+{
+   struct host1x *host = dev_get_drvdata(dev->parent);
+
+   /* skip newer Tegra's since IOMMU is supposed to be used by
+* them and not all address registers with their shifts are
+* publicly documented */
+   if (host->info->version > 1)
+   return true;


I don't like the firewall working differently per SoC - IOMMU can still 
be disabled on later SoCs and in this case there would be a security 
hole even if the firewall is enabled.



+
+   /* skip Tegra30 with IOMMU enabled */
+   if (host->domain)
+   return true;
+


So if we want to be able to test the firewall on all SoCs just having 
this check here for all SoCs would be better.


Thanks,
Mikko


+   /* relocation shift value validation isn't implemented yet */
+   if (reloc->shift)
+   return false;
+
+   return true;
+}
+
 struct host1x_firewall {
struct host1x_job *job;
struct device *dev;
@@ -359,6 +380,9 @@ static int check_register(struct host1x_firewall *fw, 
unsigned long offset)
if (!check_reloc(fw->reloc, fw->cmdbuf, fw->offset))
return -EINVAL;

+   if (!check_reloc_shift(fw->dev, fw->reloc))
+   return -EINVAL;
+
fw->num_relocs--;
fw->reloc++;
}


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 14/22] gpu: host1x: Forbid relocation address shifting in the firewall

2017-06-14 Thread Mikko Perttunen 



On 14.06.2017 12:06, Dmitry Osipenko wrote:

On 14.06.2017 09:50, Mikko Perttunen wrote:

On 14.06.2017 02:15, Dmitry Osipenko wrote:

Incorrectly shifted relocation address will cause a lower memory corruption
and likely a hang on a write or a read of an arbitrary data in case of IOMMU
absent. As of now there is no use for the address shifting (at least on
Tegra20) and adding a proper shifting / sizes validation is much more work.
Let's forbid it in the firewall till a proper validation is implemented.

Signed-off-by: Dmitry Osipenko 
Reviewed-by: Erik Faye-Lund 
---
 drivers/gpu/host1x/dev.c |  4 
 drivers/gpu/host1x/dev.h |  1 +
 drivers/gpu/host1x/job.c | 24 
 3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 6a805ed908c3..21da331ce092 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -72,6 +72,7 @@ static const struct host1x_info host1x01_info = {
 .init = host1x01_init,
 .sync_offset = 0x3000,
 .dma_mask = DMA_BIT_MASK(32),
+.version = 1,
 };

 static const struct host1x_info host1x02_info = {
@@ -82,6 +83,7 @@ static const struct host1x_info host1x02_info = {
 .init = host1x02_init,
 .sync_offset = 0x3000,
 .dma_mask = DMA_BIT_MASK(32),
+.version = 2,
 };

 static const struct host1x_info host1x04_info = {
@@ -92,6 +94,7 @@ static const struct host1x_info host1x04_info = {
 .init = host1x04_init,
 .sync_offset = 0x2100,
 .dma_mask = DMA_BIT_MASK(34),
+.version = 4,
 };

 static const struct host1x_info host1x05_info = {
@@ -102,6 +105,7 @@ static const struct host1x_info host1x05_info = {
 .init = host1x05_init,
 .sync_offset = 0x2100,
 .dma_mask = DMA_BIT_MASK(34),
+.version = 5,
 };

 static const struct of_device_id host1x_of_match[] = {
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index 229d08b6a45e..c6997651b336 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -100,6 +100,7 @@ struct host1x_info {
 int (*init)(struct host1x *host1x); /* initialize per SoC ops */
 unsigned int sync_offset; /* offset of syncpoint registers */
 u64 dma_mask; /* mask of addressable memory */
+unsigned int version; /* host1x's version */
 };

 struct host1x {
diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 4208329ca2af..7825643da324 100644
--- a/drivers/gpu/host1x/job.c
+++ b/drivers/gpu/host1x/job.c
@@ -333,6 +333,27 @@ static bool check_reloc(struct host1x_reloc *reloc,
struct host1x_bo *cmdbuf,
 return true;
 }

+static bool check_reloc_shift(struct device *dev, struct host1x_reloc *reloc)
+{
+struct host1x *host = dev_get_drvdata(dev->parent);
+
+/* skip newer Tegra's since IOMMU is supposed to be used by
+ * them and not all address registers with their shifts are
+ * publicly documented */
+if (host->info->version > 1)
+return true;


I don't like the firewall working differently per SoC - IOMMU can still be
disabled on later SoCs and in this case there would be a security hole even if
the firewall is enabled.



Yes, it would be a security hole like it is is right now. Unfortunately we can't
fix it on later SoC's without knowing all the address registers and their 
shifts.


Indeed, we can't fix it, but I was thinking that having the firewall 
enabled in the kernel should always mean that the system is safe, even 
if IOMMU is disabled, and even if it means that you cannot really do 
anything useful with it (at least running host1x_test should be possible 
without shifts, though). Thus no matter what the hardware revision, we 
should reject anything we cannot ensure as safe.





+
+/* skip Tegra30 with IOMMU enabled */
+if (host->domain)
+return true;
+


So if we want to be able to test the firewall on all SoCs just having this check
here for all SoCs would be better.



I'm not sure what you are meaning here. You'll get a wrong address resolved in
HW if that address needs to be shifted.


So we have two choices here:
a) If IOMMU is enabled, skip the shift check and return true always. 
This means that we still ensure that the system is safe, since with 
IOMMU enabled it is safe even if the firewall cannot ensure that the job 
is proper. This is what I was trying to say above.
b) Always return false if there is a shift, on any SoC and even if IOMMU 
is enabled. This would harmonize the behavior of the firewall across 
systems.




As I wrote before, on later SoC's firewall could be used as a debug feature
without the shifts being validated.


+/* relocation shift value validation isn't implemented yet */
+if (reloc->shift)
+return false;
+
+return true;
+}
+
 struct host1x_firewall {
 struct host1x_job *job;
 struct device *dev;
@@ -359,6 +380,9 @@ static int check_register(struct host1x_firewall *fw,
unsigned long offset)
 if

Re: [PATCH v2 14/22] gpu: host1x: Forbid relocation address shifting in the firewall

2017-06-14 Thread Dmitry Osipenko 
On 14.06.2017 14:47, Mikko Perttunen wrote:
> 
> 
> On 14.06.2017 12:06, Dmitry Osipenko wrote:
>> On 14.06.2017 09:50, Mikko Perttunen wrote:
>>> On 14.06.2017 02:15, Dmitry Osipenko wrote:
 Incorrectly shifted relocation address will cause a lower memory corruption
 and likely a hang on a write or a read of an arbitrary data in case of 
 IOMMU
 absent. As of now there is no use for the address shifting (at least on
 Tegra20) and adding a proper shifting / sizes validation is much more work.
 Let's forbid it in the firewall till a proper validation is implemented.

 Signed-off-by: Dmitry Osipenko 
 Reviewed-by: Erik Faye-Lund 
 ---
  drivers/gpu/host1x/dev.c |  4 
  drivers/gpu/host1x/dev.h |  1 +
  drivers/gpu/host1x/job.c | 24 
  3 files changed, 29 insertions(+)

 diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
 index 6a805ed908c3..21da331ce092 100644
 --- a/drivers/gpu/host1x/dev.c
 +++ b/drivers/gpu/host1x/dev.c
 @@ -72,6 +72,7 @@ static const struct host1x_info host1x01_info = {
  .init = host1x01_init,
  .sync_offset = 0x3000,
  .dma_mask = DMA_BIT_MASK(32),
 +.version = 1,
  };

  static const struct host1x_info host1x02_info = {
 @@ -82,6 +83,7 @@ static const struct host1x_info host1x02_info = {
  .init = host1x02_init,
  .sync_offset = 0x3000,
  .dma_mask = DMA_BIT_MASK(32),
 +.version = 2,
  };

  static const struct host1x_info host1x04_info = {
 @@ -92,6 +94,7 @@ static const struct host1x_info host1x04_info = {
  .init = host1x04_init,
  .sync_offset = 0x2100,
  .dma_mask = DMA_BIT_MASK(34),
 +.version = 4,
  };

  static const struct host1x_info host1x05_info = {
 @@ -102,6 +105,7 @@ static const struct host1x_info host1x05_info = {
  .init = host1x05_init,
  .sync_offset = 0x2100,
  .dma_mask = DMA_BIT_MASK(34),
 +.version = 5,
  };

  static const struct of_device_id host1x_of_match[] = {
 diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
 index 229d08b6a45e..c6997651b336 100644
 --- a/drivers/gpu/host1x/dev.h
 +++ b/drivers/gpu/host1x/dev.h
 @@ -100,6 +100,7 @@ struct host1x_info {
  int (*init)(struct host1x *host1x); /* initialize per SoC ops */
  unsigned int sync_offset; /* offset of syncpoint registers */
  u64 dma_mask; /* mask of addressable memory */
 +unsigned int version; /* host1x's version */
  };

  struct host1x {
 diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
 index 4208329ca2af..7825643da324 100644
 --- a/drivers/gpu/host1x/job.c
 +++ b/drivers/gpu/host1x/job.c
 @@ -333,6 +333,27 @@ static bool check_reloc(struct host1x_reloc *reloc,
 struct host1x_bo *cmdbuf,
  return true;
  }

 +static bool check_reloc_shift(struct device *dev, struct host1x_reloc 
 *reloc)
 +{
 +struct host1x *host = dev_get_drvdata(dev->parent);
 +
 +/* skip newer Tegra's since IOMMU is supposed to be used by
 + * them and not all address registers with their shifts are
 + * publicly documented */
 +if (host->info->version > 1)
 +return true;
>>>
>>> I don't like the firewall working differently per SoC - IOMMU can still be
>>> disabled on later SoCs and in this case there would be a security hole even 
>>> if
>>> the firewall is enabled.
>>>
>>
>> Yes, it would be a security hole like it is is right now. Unfortunately we 
>> can't
>> fix it on later SoC's without knowing all the address registers and their 
>> shifts.
> 
> Indeed, we can't fix it, but I was thinking that having the firewall enabled 
> in
> the kernel should always mean that the system is safe, even if IOMMU is
> disabled, and even if it means that you cannot really do anything useful with 
> it
> (at least running host1x_test should be possible without shifts, though). Thus
> no matter what the hardware revision, we should reject anything we cannot 
> ensure
> as safe.
> 
>>
 +
 +/* skip Tegra30 with IOMMU enabled */
 +if (host->domain)
 +return true;
 +
>>>
>>> So if we want to be able to test the firewall on all SoCs just having this 
>>> check
>>> here for all SoCs would be better.
>>>
>>
>> I'm not sure what you are meaning here. You'll get a wrong address resolved 
>> in
>> HW if that address needs to be shifted.
> 
> So we have two choices here:
> a) If IOMMU is enabled, skip the shift check and return true always. This 
> means
> that we still ensure that the system is safe, since with IOMMU enabled it is
> safe even if the firewall cannot ensure that the job is proper. This is what I
> was trying to say above.
> b) Always return false if