> -----Original Message-----
> From: Lu Baolu <[email protected]>
> Sent: Wednesday, January 20, 2021 7:37 PM
> To: Zhang, Tina <[email protected]>
> Cc: [email protected]; [email protected]; Joerg
> Roedel <[email protected]>; Mehta, Sohil <[email protected]>; Jacob
> Pan <[email protected]>; Sun, Yi <[email protected]>
> 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 <[email protected]>
> >> Sent: Wednesday, January 20, 2021 10:35 AM
> >> To: Zhang, Tina <[email protected]>
> >> Cc: [email protected]; [email protected]; Joerg
> >> Roedel <[email protected]>; Mehta, Sohil <[email protected]>; Jacob
> >> Pan <[email protected]>; Sun, Yi <[email protected]>
> >> 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 <[email protected]>
> >>> Cc: Joerg Roedel <[email protected]>
> >>> Cc: Sohil Mehta <[email protected]>
> >>> Cc: Jacob Pan <[email protected]>
> >>> Reported-by: Yi Sun <[email protected]>
> >>> Signed-off-by: Tina Zhang <[email protected]>
> >>> ---
> >>> 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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
