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.

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

Reply via email to