Archive-only list for patches
 help / color / mirror / Atom feed
* [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops
@ 2024-06-28 16:11 Jason Gunthorpe
  2024-06-28 21:27 ` Nicolin Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2024-06-28 16:11 UTC (permalink / raw)
  To: iommu; +Cc: Kevin Tian, Nicolin Chen, patches, Yi Liu

If drivers don't do this then iommufd will oops invalidation ioctls with
something like:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
  Mem abort info:
    ESR = 0x0000000086000004
    EC = 0x21: IABT (current EL), IL = 32 bits
    SET = 0, FnV = 0
    EA = 0, S1PTW = 0
    FSC = 0x04: level 0 translation fault
  user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101059000
  [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
  Internal error: Oops: 0000000086000004 [#1] PREEMPT SMP
  Modules linked in:
  CPU: 2 PID: 371 Comm: qemu-system-aar Not tainted 6.8.0-rc7-gde77230ac23a #9
  Hardware name: linux,dummy-virt (DT)
  pstate: 81400809 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=-c)
  pc : 0x0
  lr : iommufd_hwpt_invalidate+0xa4/0x204
  sp : ffff800080f3bcc0
  x29: ffff800080f3bcf0 x28: ffff0000c369b300 x27: 0000000000000000
  x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
  x23: 0000000000000000 x22: 00000000c1e334a0 x21: ffff0000c1e334a0
  x20: ffff800080f3bd38 x19: ffff800080f3bd58 x18: 0000000000000000
  x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffff8240d6d8
  x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
  x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
  x8 : 0000001000000002 x7 : 0000fffeac1ec950 x6 : 0000000000000000
  x5 : ffff800080f3bd78 x4 : 0000000000000003 x3 : 0000000000000002
  x2 : 0000000000000000 x1 : ffff800080f3bcc8 x0 : ffff0000c6034d80
  Call trace:
   0x0
   iommufd_fops_ioctl+0x154/0x274
   __arm64_sys_ioctl+0xac/0xf0
   invoke_syscall+0x48/0x110
   el0_svc_common.constprop.0+0x40/0xe0
   do_el0_svc+0x1c/0x28
   el0_svc+0x34/0xb4
   el0t_64_sync_handler+0x120/0x12c
   el0t_64_sync+0x190/0x194

All existing drivers implement this op for nesting, this is mostly a
bisection aid.

Fixes: 8c6eabae3807 ("iommufd: Add IOMMU_HWPT_INVALIDATE")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/iommufd/hw_pagetable.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 33d142f8057d70..a9f1fe44c4c0bd 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -236,7 +236,8 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
 	}
 	hwpt->domain->owner = ops;
 
-	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
+	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED ||
+			 !hwpt->domain->ops->cache_invalidate_user)) {
 		rc = -EINVAL;
 		goto out_abort;
 	}

base-commit: f2661062f16b2de5d7b6a5c42a9a5c96326b8454
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops
  2024-06-28 16:11 [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops Jason Gunthorpe
@ 2024-06-28 21:27 ` Nicolin Chen
  2024-06-30  6:23   ` Yi Liu
  2024-07-01  9:12 ` Tian, Kevin
  2024-07-10 13:51 ` Jason Gunthorpe
  2 siblings, 1 reply; 5+ messages in thread
From: Nicolin Chen @ 2024-06-28 21:27 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: iommu, Kevin Tian, patches, Yi Liu

On Fri, Jun 28, 2024 at 01:11:11PM -0300, Jason Gunthorpe wrote:

> All existing drivers implement this op for nesting, this is mostly a
> bisection aid.
> 
> Fixes: 8c6eabae3807 ("iommufd: Add IOMMU_HWPT_INVALIDATE")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/iommufd/hw_pagetable.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
> index 33d142f8057d70..a9f1fe44c4c0bd 100644
> --- a/drivers/iommu/iommufd/hw_pagetable.c
> +++ b/drivers/iommu/iommufd/hw_pagetable.c
> @@ -236,7 +236,8 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
>  	}
>  	hwpt->domain->owner = ops;
>  
> -	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
> +	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED ||
> +			 !hwpt->domain->ops->cache_invalidate_user)) {
>  		rc = -EINVAL;
>  		goto out_abort;

Oh, we had that initially, yet lost it when splitting the series.

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops
  2024-06-28 21:27 ` Nicolin Chen
@ 2024-06-30  6:23   ` Yi Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Yi Liu @ 2024-06-30  6:23 UTC (permalink / raw)
  To: Nicolin Chen, Jason Gunthorpe; +Cc: iommu, Kevin Tian, patches

On 2024/6/29 05:27, Nicolin Chen wrote:
> On Fri, Jun 28, 2024 at 01:11:11PM -0300, Jason Gunthorpe wrote:
> 
>> All existing drivers implement this op for nesting, this is mostly a
>> bisection aid.
>>
>> Fixes: 8c6eabae3807 ("iommufd: Add IOMMU_HWPT_INVALIDATE")
>> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

>> ---
>>   drivers/iommu/iommufd/hw_pagetable.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
>> index 33d142f8057d70..a9f1fe44c4c0bd 100644
>> --- a/drivers/iommu/iommufd/hw_pagetable.c
>> +++ b/drivers/iommu/iommufd/hw_pagetable.c
>> @@ -236,7 +236,8 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
>>   	}
>>   	hwpt->domain->owner = ops;
>>   
>> -	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
>> +	if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED ||
>> +			 !hwpt->domain->ops->cache_invalidate_user)) {
>>   		rc = -EINVAL;
>>   		goto out_abort;
> 
> Oh, we had that initially, yet lost it when splitting the series.
> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

yes, it was dropped in v7 per a bisect concern as well. :(

v7: 
https://lore.kernel.org/linux-iommu/20231221153948.119007-1-yi.l.liu@intel.com/
  - Remove domain->ops->cache_invalidate_user check in hwpt alloc path due
    to failure in bisect (Baolu)

-- 
Regards,
Yi Liu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops
  2024-06-28 16:11 [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops Jason Gunthorpe
  2024-06-28 21:27 ` Nicolin Chen
@ 2024-07-01  9:12 ` Tian, Kevin
  2024-07-10 13:51 ` Jason Gunthorpe
  2 siblings, 0 replies; 5+ messages in thread
From: Tian, Kevin @ 2024-07-01  9:12 UTC (permalink / raw)
  To: Jason Gunthorpe, iommu@lists.linux.dev
  Cc: Nicolin Chen, patches@lists.linux.dev, Liu, Yi L

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Saturday, June 29, 2024 12:11 AM
> 
> If drivers don't do this then iommufd will oops invalidation ioctls with
> something like:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000000
>   Mem abort info:
>     ESR = 0x0000000086000004
>     EC = 0x21: IABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
>     FSC = 0x04: level 0 translation fault
>   user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101059000
>   [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
>   Internal error: Oops: 0000000086000004 [#1] PREEMPT SMP
>   Modules linked in:
>   CPU: 2 PID: 371 Comm: qemu-system-aar Not tainted 6.8.0-rc7-
> gde77230ac23a #9
>   Hardware name: linux,dummy-virt (DT)
>   pstate: 81400809 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=-c)
>   pc : 0x0
>   lr : iommufd_hwpt_invalidate+0xa4/0x204
>   sp : ffff800080f3bcc0
>   x29: ffff800080f3bcf0 x28: ffff0000c369b300 x27: 0000000000000000
>   x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
>   x23: 0000000000000000 x22: 00000000c1e334a0 x21: ffff0000c1e334a0
>   x20: ffff800080f3bd38 x19: ffff800080f3bd58 x18: 0000000000000000
>   x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffff8240d6d8
>   x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
>   x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
>   x8 : 0000001000000002 x7 : 0000fffeac1ec950 x6 : 0000000000000000
>   x5 : ffff800080f3bd78 x4 : 0000000000000003 x3 : 0000000000000002
>   x2 : 0000000000000000 x1 : ffff800080f3bcc8 x0 : ffff0000c6034d80
>   Call trace:
>    0x0
>    iommufd_fops_ioctl+0x154/0x274
>    __arm64_sys_ioctl+0xac/0xf0
>    invoke_syscall+0x48/0x110
>    el0_svc_common.constprop.0+0x40/0xe0
>    do_el0_svc+0x1c/0x28
>    el0_svc+0x34/0xb4
>    el0t_64_sync_handler+0x120/0x12c
>    el0t_64_sync+0x190/0x194
> 
> All existing drivers implement this op for nesting, this is mostly a
> bisection aid.
> 
> Fixes: 8c6eabae3807 ("iommufd: Add IOMMU_HWPT_INVALIDATE")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops
  2024-06-28 16:11 [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops Jason Gunthorpe
  2024-06-28 21:27 ` Nicolin Chen
  2024-07-01  9:12 ` Tian, Kevin
@ 2024-07-10 13:51 ` Jason Gunthorpe
  2 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2024-07-10 13:51 UTC (permalink / raw)
  To: iommu; +Cc: Kevin Tian, Nicolin Chen, patches, Yi Liu

On Fri, Jun 28, 2024 at 01:11:11PM -0300, Jason Gunthorpe wrote:
> If drivers don't do this then iommufd will oops invalidation ioctls with
> something like:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>   Mem abort info:
>     ESR = 0x0000000086000004
>     EC = 0x21: IABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
>     FSC = 0x04: level 0 translation fault
>   user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101059000
>   [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
>   Internal error: Oops: 0000000086000004 [#1] PREEMPT SMP
>   Modules linked in:
>   CPU: 2 PID: 371 Comm: qemu-system-aar Not tainted 6.8.0-rc7-gde77230ac23a #9
>   Hardware name: linux,dummy-virt (DT)
>   pstate: 81400809 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=-c)
>   pc : 0x0
>   lr : iommufd_hwpt_invalidate+0xa4/0x204
>   sp : ffff800080f3bcc0
>   x29: ffff800080f3bcf0 x28: ffff0000c369b300 x27: 0000000000000000
>   x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
>   x23: 0000000000000000 x22: 00000000c1e334a0 x21: ffff0000c1e334a0
>   x20: ffff800080f3bd38 x19: ffff800080f3bd58 x18: 0000000000000000
>   x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffff8240d6d8
>   x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
>   x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
>   x8 : 0000001000000002 x7 : 0000fffeac1ec950 x6 : 0000000000000000
>   x5 : ffff800080f3bd78 x4 : 0000000000000003 x3 : 0000000000000002
>   x2 : 0000000000000000 x1 : ffff800080f3bcc8 x0 : ffff0000c6034d80
>   Call trace:
>    0x0
>    iommufd_fops_ioctl+0x154/0x274
>    __arm64_sys_ioctl+0xac/0xf0
>    invoke_syscall+0x48/0x110
>    el0_svc_common.constprop.0+0x40/0xe0
>    do_el0_svc+0x1c/0x28
>    el0_svc+0x34/0xb4
>    el0t_64_sync_handler+0x120/0x12c
>    el0t_64_sync+0x190/0x194
> 
> All existing drivers implement this op for nesting, this is mostly a
> bisection aid.
> 
> Fixes: 8c6eabae3807 ("iommufd: Add IOMMU_HWPT_INVALIDATE")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/iommufd/hw_pagetable.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to iommufd for-next

Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-07-10 13:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-28 16:11 [PATCH] iommufd: Require drivers to supply the cache_invalidate_user ops Jason Gunthorpe
2024-06-28 21:27 ` Nicolin Chen
2024-06-30  6:23   ` Yi Liu
2024-07-01  9:12 ` Tian, Kevin
2024-07-10 13:51 ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox