| Message ID | 20231228170504.720794-3-haifeng.zhao@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix vt-d hard lockup when hotplug ATS capable device | expand |
On 12/29/2023 1:05 AM, Ethan Zhao wrote: > When the ATS Invalidation request timeout happens, the qi_submit_sync() > will restart and loop for the invalidation request forever till it is > done, it will block another Invalidation thread such as the fq_timer > to issue invalidation request, cause the system lockup as following > > [exception RIP: native_queued_spin_lock_slowpath+92] > > RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 > > RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 > > RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 > > R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 > > R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 > > ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 #12 [ffffb202f268cdc8] native_queued_spin_lock_slowpath at ffffffffa9d1025c #13 [ffffb202f268cdc8] do_raw_spin_lock at ffffffffa9d121f1 #14 [ffffb202f268cdd8] _raw_spin_lock_irqsave at ffffffffaa51795b #15 [ffffb202f268cdf8] iommu_flush_dev_iotlb at ffffffffaa20df48 #16 [ffffb202f268ce28] iommu_flush_iova at ffffffffaa20e182 #17 [ffffb202f268ce60] iova_domain_flush at ffffffffaa220e27 #18 [ffffb202f268ce70] fq_flush_timeout at ffffffffaa221c9d #19 [ffffb202f268cea8] call_timer_fn at ffffffffa9d46661 #20 [ffffb202f268cf08] run_timer_softirq at ffffffffa9d47933 #21 [ffffb202f268cf98] __softirqentry_text_start at ffffffffaa8000e0 #22 [ffffb202f268cff0] asm_call_sysvec_on_stack at ffffffffaa60114f This part get lost perhpas I append "----" here. Thanks, Ethan > > (the left part of exception see the hotplug case of ATS capable device) > > If one endpoint device just no response to the ATS Invalidation request, > but is not gone, it will bring down the whole system, to avoid such > case, don't try the timeout ATS Invalidation request forever. > > Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> > --- > drivers/iommu/intel/dmar.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c > index 0a8d628a42ee..9edb4b44afca 100644 > --- a/drivers/iommu/intel/dmar.c > +++ b/drivers/iommu/intel/dmar.c > @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc, > reclaim_free_desc(qi); > raw_spin_unlock_irqrestore(&qi->q_lock, flags); > > - if (rc == -EAGAIN) > + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE) > goto restart; > > if (iotlb_start_ktime)
On 12/29/23 1:05 AM, Ethan Zhao wrote: > When the ATS Invalidation request timeout happens, the qi_submit_sync() > will restart and loop for the invalidation request forever till it is > done, it will block another Invalidation thread such as the fq_timer > to issue invalidation request, cause the system lockup as following > > [exception RIP: native_queued_spin_lock_slowpath+92] > > RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 > > RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 > > RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 > > R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 > > R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 > > ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 > > (the left part of exception see the hotplug case of ATS capable device) > > If one endpoint device just no response to the ATS Invalidation request, > but is not gone, it will bring down the whole system, to avoid such > case, don't try the timeout ATS Invalidation request forever. > > Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> > --- > drivers/iommu/intel/dmar.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c > index 0a8d628a42ee..9edb4b44afca 100644 > --- a/drivers/iommu/intel/dmar.c > +++ b/drivers/iommu/intel/dmar.c > @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc, > reclaim_free_desc(qi); > raw_spin_unlock_irqrestore(&qi->q_lock, flags); > > - if (rc == -EAGAIN) > + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE) > goto restart; > > if (iotlb_start_ktime) Above is also unnecessary if qi_check_fault() returns -ETIMEDOUT, instead of -EAGAIN. Or did I miss anything? Best regards, baolu
On 1/10/2024 1:28 PM, Baolu Lu wrote: > On 12/29/23 1:05 AM, Ethan Zhao wrote: >> When the ATS Invalidation request timeout happens, the qi_submit_sync() >> will restart and loop for the invalidation request forever till it is >> done, it will block another Invalidation thread such as the fq_timer >> to issue invalidation request, cause the system lockup as following >> >> [exception RIP: native_queued_spin_lock_slowpath+92] >> >> RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 >> >> RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 >> >> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 >> >> RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 >> >> R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 >> >> R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 >> >> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 >> >> (the left part of exception see the hotplug case of ATS capable device) >> >> If one endpoint device just no response to the ATS Invalidation request, >> but is not gone, it will bring down the whole system, to avoid such >> case, don't try the timeout ATS Invalidation request forever. >> >> Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> >> --- >> drivers/iommu/intel/dmar.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c >> index 0a8d628a42ee..9edb4b44afca 100644 >> --- a/drivers/iommu/intel/dmar.c >> +++ b/drivers/iommu/intel/dmar.c >> @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, >> struct qi_desc *desc, >> reclaim_free_desc(qi); >> raw_spin_unlock_irqrestore(&qi->q_lock, flags); >> - if (rc == -EAGAIN) >> + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != >> QI_DEIOTLB_TYPE) >> goto restart; >> if (iotlb_start_ktime) > > Above is also unnecessary if qi_check_fault() returns -ETIMEDOUT, > instead of -EAGAIN. Or did I miss anything? It is pro if we fold it into qi_check_fault(), the con is we have to add more parameter to qi_check_fault(), no need check invalidation type of QI_DIOTLB_TYPE&QI_DEIOTLB_TYPE in qi_check_fault() ? Thanks, Ethan > > Best regards, > baolu
On 1/10/24 4:40 PM, Ethan Zhao wrote: > > On 1/10/2024 1:28 PM, Baolu Lu wrote: >> On 12/29/23 1:05 AM, Ethan Zhao wrote: >>> When the ATS Invalidation request timeout happens, the qi_submit_sync() >>> will restart and loop for the invalidation request forever till it is >>> done, it will block another Invalidation thread such as the fq_timer >>> to issue invalidation request, cause the system lockup as following >>> >>> [exception RIP: native_queued_spin_lock_slowpath+92] >>> >>> RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 >>> >>> RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 >>> >>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 >>> >>> RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 >>> >>> R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 >>> >>> R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 >>> >>> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 >>> >>> (the left part of exception see the hotplug case of ATS capable device) >>> >>> If one endpoint device just no response to the ATS Invalidation request, >>> but is not gone, it will bring down the whole system, to avoid such >>> case, don't try the timeout ATS Invalidation request forever. >>> >>> Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> >>> --- >>> drivers/iommu/intel/dmar.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c >>> index 0a8d628a42ee..9edb4b44afca 100644 >>> --- a/drivers/iommu/intel/dmar.c >>> +++ b/drivers/iommu/intel/dmar.c >>> @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, >>> struct qi_desc *desc, >>> reclaim_free_desc(qi); >>> raw_spin_unlock_irqrestore(&qi->q_lock, flags); >>> - if (rc == -EAGAIN) >>> + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != >>> QI_DEIOTLB_TYPE) >>> goto restart; >>> if (iotlb_start_ktime) >> >> Above is also unnecessary if qi_check_fault() returns -ETIMEDOUT, >> instead of -EAGAIN. Or did I miss anything? > > It is pro if we fold it into qi_check_fault(), the con is we have to add > > more parameter to qi_check_fault(), no need check invalidation type > > of QI_DIOTLB_TYPE&QI_DEIOTLB_TYPE in qi_check_fault() ? No need to check the request type as multiple requests might be batched together in a single call. This is also the reason why I asked you to add a flag bit to this helper and make the intention explicit, say, "This includes requests to interact with a PCI endpoint. The device may become unavailable at any time, so do not attempt to retry if ITE is detected and the device has gone away." Best regards, baolu
On 1/11/2024 10:31 AM, Baolu Lu wrote: > On 1/10/24 4:40 PM, Ethan Zhao wrote: >> >> On 1/10/2024 1:28 PM, Baolu Lu wrote: >>> On 12/29/23 1:05 AM, Ethan Zhao wrote: >>>> When the ATS Invalidation request timeout happens, the >>>> qi_submit_sync() >>>> will restart and loop for the invalidation request forever till it is >>>> done, it will block another Invalidation thread such as the fq_timer >>>> to issue invalidation request, cause the system lockup as following >>>> >>>> [exception RIP: native_queued_spin_lock_slowpath+92] >>>> >>>> RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 >>>> >>>> RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 >>>> >>>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 >>>> >>>> RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 >>>> >>>> R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 >>>> >>>> R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 >>>> >>>> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 >>>> >>>> (the left part of exception see the hotplug case of ATS capable >>>> device) >>>> >>>> If one endpoint device just no response to the ATS Invalidation >>>> request, >>>> but is not gone, it will bring down the whole system, to avoid such >>>> case, don't try the timeout ATS Invalidation request forever. >>>> >>>> Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> >>>> --- >>>> drivers/iommu/intel/dmar.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c >>>> index 0a8d628a42ee..9edb4b44afca 100644 >>>> --- a/drivers/iommu/intel/dmar.c >>>> +++ b/drivers/iommu/intel/dmar.c >>>> @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, >>>> struct qi_desc *desc, >>>> reclaim_free_desc(qi); >>>> raw_spin_unlock_irqrestore(&qi->q_lock, flags); >>>> - if (rc == -EAGAIN) >>>> + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != >>>> QI_DEIOTLB_TYPE) >>>> goto restart; >>>> if (iotlb_start_ktime) >>> >>> Above is also unnecessary if qi_check_fault() returns -ETIMEDOUT, >>> instead of -EAGAIN. Or did I miss anything? >> >> It is pro if we fold it into qi_check_fault(), the con is we have to add >> >> more parameter to qi_check_fault(), no need check invalidation type >> >> of QI_DIOTLB_TYPE&QI_DEIOTLB_TYPE in qi_check_fault() ? > > No need to check the request type as multiple requests might be batched > together in a single call. This is also the reason why I asked you to > add a flag bit to this helper and make the intention explicit, say, > > "This includes requests to interact with a PCI endpoint. The device may > become unavailable at any time, so do not attempt to retry if ITE is > detected and the device has gone away." That is to say, the usage of this function finally becomes that way, the user space interface could submit request with mixed iotlb & devtlb invalidation together in the queue or seperated iotlb/devtlb invalidation. we depend on caller to pass the QI_OPT_CHECK_ENDPOINT as option bit to bail out even there is other iotlb invalidation in the same batch ? then is user's call to choose retry the iotbl /devtlb invalidation or not. if the caller hits the case the endpoint dead, the caller will get -ETIMEDOUT/ -ENOTCONN as returned value, but no real ITE in its interested list, to tell userland user what happened, we fake a DMA_FSTS_ITE for user ? given we wouldn't read a ITE from DMA_FSTS_REG that moment. 1. checking the first request for devTLB invalidation will miss chance to check endpoint state if the iotlb & devtlb invalidation were mixed. here explict option bit would be better. while valid pdev does the same thing. so if pdev passed, no need to check for QI_DIOTLB_TYPE || QI_EIOTLB_TYPE in qi_submit_sync() & qi_check_fault(). 2. seems not perfect to drop or retry whole batch of request if there is devtlb invalidation within the batch, let caller to choose the later action is simpler than making the qi_submit_sync() too complex. 3. fake a DMA_FSTS_ITE for user's interested list on behalf of hardware is better than no error/ fault feedback to user even it is predicted not happened yet. my cents. Thanks, Ethan > > Best regards, > baolu
On 1/11/2024 11:44 AM, Ethan Zhao wrote: > > On 1/11/2024 10:31 AM, Baolu Lu wrote: >> On 1/10/24 4:40 PM, Ethan Zhao wrote: >>> >>> On 1/10/2024 1:28 PM, Baolu Lu wrote: >>>> On 12/29/23 1:05 AM, Ethan Zhao wrote: >>>>> When the ATS Invalidation request timeout happens, the >>>>> qi_submit_sync() >>>>> will restart and loop for the invalidation request forever till it is >>>>> done, it will block another Invalidation thread such as the fq_timer >>>>> to issue invalidation request, cause the system lockup as following >>>>> >>>>> [exception RIP: native_queued_spin_lock_slowpath+92] >>>>> >>>>> RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 >>>>> >>>>> RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 >>>>> >>>>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 >>>>> >>>>> RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 >>>>> >>>>> R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 >>>>> >>>>> R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 >>>>> >>>>> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 >>>>> >>>>> (the left part of exception see the hotplug case of ATS capable >>>>> device) >>>>> >>>>> If one endpoint device just no response to the ATS Invalidation >>>>> request, >>>>> but is not gone, it will bring down the whole system, to avoid such >>>>> case, don't try the timeout ATS Invalidation request forever. >>>>> >>>>> Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> >>>>> --- >>>>> drivers/iommu/intel/dmar.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c >>>>> index 0a8d628a42ee..9edb4b44afca 100644 >>>>> --- a/drivers/iommu/intel/dmar.c >>>>> +++ b/drivers/iommu/intel/dmar.c >>>>> @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu >>>>> *iommu, struct qi_desc *desc, >>>>> reclaim_free_desc(qi); >>>>> raw_spin_unlock_irqrestore(&qi->q_lock, flags); >>>>> - if (rc == -EAGAIN) >>>>> + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != >>>>> QI_DEIOTLB_TYPE) >>>>> goto restart; >>>>> if (iotlb_start_ktime) >>>> >>>> Above is also unnecessary if qi_check_fault() returns -ETIMEDOUT, >>>> instead of -EAGAIN. Or did I miss anything? >>> >>> It is pro if we fold it into qi_check_fault(), the con is we have to >>> add >>> >>> more parameter to qi_check_fault(), no need check invalidation type >>> >>> of QI_DIOTLB_TYPE&QI_DEIOTLB_TYPE in qi_check_fault() ? >> >> No need to check the request type as multiple requests might be batched >> together in a single call. This is also the reason why I asked you to >> add a flag bit to this helper and make the intention explicit, say, >> >> "This includes requests to interact with a PCI endpoint. The device may >> become unavailable at any time, so do not attempt to retry if ITE is >> detected and the device has gone away." > > That is to say, the usage of this function finally becomes that way, > > the user space interface could submit request with mixed iotlb & devtlb > > invalidation together in the queue or seperated iotlb/devtlb > invalidation. > > we depend on caller to pass the QI_OPT_CHECK_ENDPOINT as option > > bit to bail out even there is other iotlb invalidation in the same > batch ? > > then is user's call to choose retry the iotbl /devtlb invalidation or > not. > > if the caller hits the case the endpoint dead, the caller will get > -ETIMEDOUT/ > > -ENOTCONN as returned value, but no real ITE in its interested list, to > > tell userland user what happened, we fake a DMA_FSTS_ITE for user ? > > given we wouldn't read a ITE from DMA_FSTS_REG that moment. > > > 1. checking the first request for devTLB invalidation will miss chance to > > check endpoint state if the iotlb & devtlb invalidation were mixed. > > here explict option bit would be better. while valid pdev does the > > same thing. so if pdev passed, no need to check for QI_DIOTLB_TYPE > > || QI_EIOTLB_TYPE in qi_submit_sync() & qi_check_fault(). > > > 2. seems not perfect to drop or retry whole batch of request if there is > > devtlb invalidation within the batch, let caller to choose the later > action > > is simpler than making the qi_submit_sync() too complex. > > > 3. fake a DMA_FSTS_ITE for user's interested list on behalf of hardware > > is better than no error/ fault feedback to user even it is predicted > not > > happened yet. > > See Intel VT-d spec r4.1, section 4.3 & section 6.5.2.10 We should keep the original retry logic intact, in order to not break the fault handling flow. only breaks the loop when endpoint device is gone with returned error code to reflect the reality. not -ETIMEOUT, that is not triggered yet, but will hit ITE later about previous request, and software should handle it smoothly to let the other subsequent requests could be done in next try. Thanks, Ethan > my cents. > > > Thanks, > > Ethan > > > >> >> Best regards, >> baolu >
On 12/29/2023 1:05 AM, Ethan Zhao wrote: > When the ATS Invalidation request timeout happens, the qi_submit_sync() > will restart and loop for the invalidation request forever till it is > done, it will block another Invalidation thread such as the fq_timer > to issue invalidation request, cause the system lockup as following > > [exception RIP: native_queued_spin_lock_slowpath+92] > > RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 > > RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 > > RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 > > R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 > > R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 > > ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 > > (the left part of exception see the hotplug case of ATS capable device) > > If one endpoint device just no response to the ATS Invalidation request, > but is not gone, it will bring down the whole system, to avoid such > case, don't try the timeout ATS Invalidation request forever. > > Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> > --- > drivers/iommu/intel/dmar.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c > index 0a8d628a42ee..9edb4b44afca 100644 > --- a/drivers/iommu/intel/dmar.c > +++ b/drivers/iommu/intel/dmar.c > @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc, > reclaim_free_desc(qi); > raw_spin_unlock_irqrestore(&qi->q_lock, flags); > > - if (rc == -EAGAIN) > + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE) > goto restart; > > if (iotlb_start_ktime) mark, only break the loop when the sid of ITE is the same as current target pdev. need check the target dev is pf or vf. The ITE is possible left by previous devtlb invalidation request for other device. Thanks, Ethan
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c index 0a8d628a42ee..9edb4b44afca 100644 --- a/drivers/iommu/intel/dmar.c +++ b/drivers/iommu/intel/dmar.c @@ -1453,7 +1453,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc, reclaim_free_desc(qi); raw_spin_unlock_irqrestore(&qi->q_lock, flags); - if (rc == -EAGAIN) + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE) goto restart; if (iotlb_start_ktime)
When the ATS Invalidation request timeout happens, the qi_submit_sync() will restart and loop for the invalidation request forever till it is done, it will block another Invalidation thread such as the fq_timer to issue invalidation request, cause the system lockup as following [exception RIP: native_queued_spin_lock_slowpath+92] RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002 RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0 RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000 R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000 R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980 ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 (the left part of exception see the hotplug case of ATS capable device) If one endpoint device just no response to the ATS Invalidation request, but is not gone, it will bring down the whole system, to avoid such case, don't try the timeout ATS Invalidation request forever. Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> --- drivers/iommu/intel/dmar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)