* [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump
@ 2021-01-19 18:25 Tina Zhang
2021-01-20 2:34 ` Lu Baolu
0 siblings, 1 reply; 5+ messages in thread
From: Tina Zhang @ 2021-01-19 18:25 UTC (permalink / raw)
Cc: iommu, Yi Sun
irq_remapping_cap() was introduced to detect whether irq remapping
supports new features, such as VT-d Posted-Interrupts", according to
commit 959c870f7305 ("iommu, x86: Provide irq_remapping_cap() interface").
The VT-d Posted-Interrupts support can be disabled by the command
line parameter "intremap=nopost".
So, it's better to use irq_remapping_cap() to check if the VT-d
Posted-Interrupts is enabled before any Posted Interrupt Descriptor
info dump.
Cc: Lu Baolu <baolu.lu@linux.intel.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Sohil Mehta <sohil.mehta@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Reported-by: Yi Sun <yi.sun@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
---
drivers/iommu/intel/debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index efea7f02abd9..87a4a76866f4 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -516,7 +516,8 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused)
seq_puts(m, "****\n\n");
for_each_active_iommu(iommu, drhd) {
- if (!cap_pi_support(iommu->cap))
+ if (!irq_remapping_cap(IRQ_POSTING_CAP) ||
+ !cap_pi_support(iommu->cap))
continue;
seq_printf(m, "Posted Interrupt supported on IOMMU: %s\n",
--
2.25.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump
2021-01-19 18:25 [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump Tina Zhang
@ 2021-01-20 2:34 ` Lu Baolu
2021-01-20 8:41 ` Zhang, Tina
0 siblings, 1 reply; 5+ messages in thread
From: Lu Baolu @ 2021-01-20 2:34 UTC (permalink / raw)
To: Tina Zhang; +Cc: iommu, Yi Sun
On 1/20/21 2:25 AM, Tina Zhang wrote:
> irq_remapping_cap() was introduced to detect whether irq remapping
> supports new features, such as VT-d Posted-Interrupts", according to
> commit 959c870f7305 ("iommu, x86: Provide irq_remapping_cap() interface").
>
> The VT-d Posted-Interrupts support can be disabled by the command
> line parameter "intremap=nopost".
>
> So, it's better to use irq_remapping_cap() to check if the VT-d
> Posted-Interrupts is enabled before any Posted Interrupt Descriptor
> info dump.
>
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Sohil Mehta <sohil.mehta@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Reported-by: Yi Sun <yi.sun@intel.com>
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> ---
> drivers/iommu/intel/debugfs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
> index efea7f02abd9..87a4a76866f4 100644
> --- a/drivers/iommu/intel/debugfs.c
> +++ b/drivers/iommu/intel/debugfs.c
> @@ -516,7 +516,8 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused)
> seq_puts(m, "****\n\n");
>
> for_each_active_iommu(iommu, drhd) {
> - if (!cap_pi_support(iommu->cap))
> + if (!irq_remapping_cap(IRQ_POSTING_CAP) ||
> + !cap_pi_support(iommu->cap))
With irq_remapping_cap(IRQ_POSTING_CAP), do you still need
cap_pi_support(iommu->cap)?
> continue;
>
> seq_printf(m, "Posted Interrupt supported on IOMMU: %s\n",
>
Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump
2021-01-20 2:34 ` Lu Baolu
@ 2021-01-20 8:41 ` Zhang, Tina
2021-01-20 11:37 ` Lu Baolu
0 siblings, 1 reply; 5+ messages in thread
From: Zhang, Tina @ 2021-01-20 8:41 UTC (permalink / raw)
To: Lu Baolu; +Cc: iommu, Sun, Yi
> -----Original Message-----
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Wednesday, January 20, 2021 10:35 AM
> To: Zhang, Tina <tina.zhang@intel.com>
> Cc: baolu.lu@linux.intel.com; iommu@lists.linux-foundation.org; Joerg
> Roedel <joro@8bytes.org>; Mehta, Sohil <sohil.mehta@intel.com>; Jacob
> Pan <jacob.jun.pan@linux.intel.com>; Sun, Yi <yi.sun@intel.com>
> Subject: Re: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before
> PI info dump
>
> On 1/20/21 2:25 AM, Tina Zhang wrote:
> > irq_remapping_cap() was introduced to detect whether irq remapping
> > supports new features, such as VT-d Posted-Interrupts", according to
> > commit 959c870f7305 ("iommu, x86: Provide irq_remapping_cap()
> interface").
> >
> > The VT-d Posted-Interrupts support can be disabled by the command line
> > parameter "intremap=nopost".
> >
> > So, it's better to use irq_remapping_cap() to check if the VT-d
> > Posted-Interrupts is enabled before any Posted Interrupt Descriptor
> > info dump.
> >
> > Cc: Lu Baolu <baolu.lu@linux.intel.com>
> > Cc: Joerg Roedel <joro@8bytes.org>
> > Cc: Sohil Mehta <sohil.mehta@intel.com>
> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Reported-by: Yi Sun <yi.sun@intel.com>
> > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > ---
> > drivers/iommu/intel/debugfs.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel/debugfs.c
> > b/drivers/iommu/intel/debugfs.c index efea7f02abd9..87a4a76866f4
> > 100644
> > --- a/drivers/iommu/intel/debugfs.c
> > +++ b/drivers/iommu/intel/debugfs.c
> > @@ -516,7 +516,8 @@ static int ir_translation_struct_show(struct seq_file
> *m, void *unused)
> > seq_puts(m, "****\n\n");
> >
> > for_each_active_iommu(iommu, drhd) {
> > - if (!cap_pi_support(iommu->cap))
> > + if (!irq_remapping_cap(IRQ_POSTING_CAP) ||
> > + !cap_pi_support(iommu->cap))
>
> With irq_remapping_cap(IRQ_POSTING_CAP), do you still need
> cap_pi_support(iommu->cap)?
I guess yes. The "iommu->cap" value comes from the iommu reg. Current code seems to use cap_pi_suport() to check if the iommu hardware supports PI capability, meanwhile using irq_remapping_cap() to see if the vt-d PI support is enabled by user.
So, the problem here is if a user explicitly disables the vt-d PI support by "intremap=nopost", it would be very confused that the PI descriptor related info can still get dump.
Thanks,
Tina
>
> > continue;
> >
> > seq_printf(m, "Posted Interrupt supported on
> IOMMU: %s\n",
> >
>
> Best regards,
> baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump
2021-01-20 8:41 ` Zhang, Tina
@ 2021-01-20 11:37 ` Lu Baolu
2021-01-20 12:21 ` Zhang, Tina
0 siblings, 1 reply; 5+ messages in thread
From: Lu Baolu @ 2021-01-20 11:37 UTC (permalink / raw)
To: Zhang, Tina; +Cc: iommu, Sun, Yi
On 2021/1/20 16:41, Zhang, Tina wrote:
>
>
>> -----Original Message-----
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Wednesday, January 20, 2021 10:35 AM
>> To: Zhang, Tina <tina.zhang@intel.com>
>> Cc: baolu.lu@linux.intel.com; iommu@lists.linux-foundation.org; Joerg
>> Roedel <joro@8bytes.org>; Mehta, Sohil <sohil.mehta@intel.com>; Jacob
>> Pan <jacob.jun.pan@linux.intel.com>; Sun, Yi <yi.sun@intel.com>
>> Subject: Re: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before
>> PI info dump
>>
>> On 1/20/21 2:25 AM, Tina Zhang wrote:
>>> irq_remapping_cap() was introduced to detect whether irq remapping
>>> supports new features, such as VT-d Posted-Interrupts", according to
>>> commit 959c870f7305 ("iommu, x86: Provide irq_remapping_cap()
>> interface").
>>>
>>> The VT-d Posted-Interrupts support can be disabled by the command line
>>> parameter "intremap=nopost".
>>>
>>> So, it's better to use irq_remapping_cap() to check if the VT-d
>>> Posted-Interrupts is enabled before any Posted Interrupt Descriptor
>>> info dump.
>>>
>>> Cc: Lu Baolu <baolu.lu@linux.intel.com>
>>> Cc: Joerg Roedel <joro@8bytes.org>
>>> Cc: Sohil Mehta <sohil.mehta@intel.com>
>>> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>> Reported-by: Yi Sun <yi.sun@intel.com>
>>> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
>>> ---
>>> drivers/iommu/intel/debugfs.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iommu/intel/debugfs.c
>>> b/drivers/iommu/intel/debugfs.c index efea7f02abd9..87a4a76866f4
>>> 100644
>>> --- a/drivers/iommu/intel/debugfs.c
>>> +++ b/drivers/iommu/intel/debugfs.c
>>> @@ -516,7 +516,8 @@ static int ir_translation_struct_show(struct seq_file
>> *m, void *unused)
>>> seq_puts(m, "****\n\n");
>>>
>>> for_each_active_iommu(iommu, drhd) {
>>> - if (!cap_pi_support(iommu->cap))
>>> + if (!irq_remapping_cap(IRQ_POSTING_CAP) ||
>>> + !cap_pi_support(iommu->cap))
>>
>> With irq_remapping_cap(IRQ_POSTING_CAP), do you still need
>> cap_pi_support(iommu->cap)?
>
> I guess yes. The "iommu->cap" value comes from the iommu reg. Current code seems to use cap_pi_suport() to check if the iommu hardware supports PI capability, meanwhile using irq_remapping_cap() to see if the vt-d PI support is enabled by user.
>
> So, the problem here is if a user explicitly disables the vt-d PI support by "intremap=nopost", it would be very confused that the PI descriptor related info can still get dump.
I don't worry about dump hardware data even it's not enabled. But I do
care that the table is not allocated (due to not enabled) but the code
still tries to dump it, hence result in some kinds of NULL pointer or
wild pointer referencing.
Best regards,
baolu
>
> Thanks,
> Tina
>
>>
>>> continue;
>>>
>>> seq_printf(m, "Posted Interrupt supported on
>> IOMMU: %s\n",
>>>
>>
>> Best regards,
>> baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump
2021-01-20 11:37 ` Lu Baolu
@ 2021-01-20 12:21 ` Zhang, Tina
0 siblings, 0 replies; 5+ messages in thread
From: Zhang, Tina @ 2021-01-20 12:21 UTC (permalink / raw)
To: Lu Baolu; +Cc: iommu, Sun, Yi
> -----Original Message-----
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Wednesday, January 20, 2021 7:37 PM
> To: Zhang, Tina <tina.zhang@intel.com>
> Cc: baolu.lu@linux.intel.com; iommu@lists.linux-foundation.org; Joerg
> Roedel <joro@8bytes.org>; Mehta, Sohil <sohil.mehta@intel.com>; Jacob
> Pan <jacob.jun.pan@linux.intel.com>; Sun, Yi <yi.sun@intel.com>
> Subject: Re: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before
> PI info dump
>
> On 2021/1/20 16:41, Zhang, Tina wrote:
> >
> >
> >> -----Original Message-----
> >> From: Lu Baolu <baolu.lu@linux.intel.com>
> >> Sent: Wednesday, January 20, 2021 10:35 AM
> >> To: Zhang, Tina <tina.zhang@intel.com>
> >> Cc: baolu.lu@linux.intel.com; iommu@lists.linux-foundation.org; Joerg
> >> Roedel <joro@8bytes.org>; Mehta, Sohil <sohil.mehta@intel.com>; Jacob
> >> Pan <jacob.jun.pan@linux.intel.com>; Sun, Yi <yi.sun@intel.com>
> >> Subject: Re: [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap
> >> before PI info dump
> >>
> >> On 1/20/21 2:25 AM, Tina Zhang wrote:
> >>> irq_remapping_cap() was introduced to detect whether irq remapping
> >>> supports new features, such as VT-d Posted-Interrupts", according to
> >>> commit 959c870f7305 ("iommu, x86: Provide irq_remapping_cap()
> >> interface").
> >>>
> >>> The VT-d Posted-Interrupts support can be disabled by the command
> >>> line parameter "intremap=nopost".
> >>>
> >>> So, it's better to use irq_remapping_cap() to check if the VT-d
> >>> Posted-Interrupts is enabled before any Posted Interrupt Descriptor
> >>> info dump.
> >>>
> >>> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> >>> Cc: Joerg Roedel <joro@8bytes.org>
> >>> Cc: Sohil Mehta <sohil.mehta@intel.com>
> >>> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >>> Reported-by: Yi Sun <yi.sun@intel.com>
> >>> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> >>> ---
> >>> drivers/iommu/intel/debugfs.c | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/iommu/intel/debugfs.c
> >>> b/drivers/iommu/intel/debugfs.c index efea7f02abd9..87a4a76866f4
> >>> 100644
> >>> --- a/drivers/iommu/intel/debugfs.c
> >>> +++ b/drivers/iommu/intel/debugfs.c
> >>> @@ -516,7 +516,8 @@ static int ir_translation_struct_show(struct
> >>> seq_file
> >> *m, void *unused)
> >>> seq_puts(m, "****\n\n");
> >>>
> >>> for_each_active_iommu(iommu, drhd) {
> >>> - if (!cap_pi_support(iommu->cap))
> >>> + if (!irq_remapping_cap(IRQ_POSTING_CAP) ||
> >>> + !cap_pi_support(iommu->cap))
> >>
> >> With irq_remapping_cap(IRQ_POSTING_CAP), do you still need
> >> cap_pi_support(iommu->cap)?
> >
> > I guess yes. The "iommu->cap" value comes from the iommu reg. Current
> code seems to use cap_pi_suport() to check if the iommu hardware supports
> PI capability, meanwhile using irq_remapping_cap() to see if the vt-d PI
> support is enabled by user.
> >
> > So, the problem here is if a user explicitly disables the vt-d PI support by
> "intremap=nopost", it would be very confused that the PI descriptor related
> info can still get dump.
>
> I don't worry about dump hardware data even it's not enabled. But I do care
> that the table is not allocated (due to not enabled) but the code still tries to
> dump it, hence result in some kinds of NULL pointer or wild pointer
> referencing.
Oh, I see.
BR,
Tina
>
> Best regards,
> baolu
>
> >
> > Thanks,
> > Tina
> >
> >>
> >>> continue;
> >>>
> >>> seq_printf(m, "Posted Interrupt supported on
> >> IOMMU: %s\n",
> >>>
> >>
> >> Best regards,
> >> baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-20 12:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 18:25 [PATCH] iommu/vt-d: debugfs: Check irq_remapping_cap before PI info dump Tina Zhang
2021-01-20 2:34 ` Lu Baolu
2021-01-20 8:41 ` Zhang, Tina
2021-01-20 11:37 ` Lu Baolu
2021-01-20 12:21 ` Zhang, Tina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).