Re: [PATCH 2/2] iommu/vt-d: Use device rbtree in iopf reporting path

[Baolu Lu]

On 2024/2/19 14:54, Ethan Zhao wrote:
> On 2/15/2024 3:22 PM, Lu Baolu wrote:
> > The existing IO page fault handler currently locates the PCI device by
> > calling pci_get_domain_bus_and_slot(). This function searches the list
> > of all PCI devices until the desired device is found. To improve lookup
> > efficiency, a helper function named device_rbtree_find() is introduced
> > to search for the device within the rbtree. Replace
> > pci_get_domain_bus_and_slot() in the IO page fault handling path.
> >
> > Co-developed-by: Huang Jiaqing <jiaqing.huang@xxxxxxxxx>
> > Signed-off-by: Huang Jiaqing <jiaqing.huang@xxxxxxxxx>
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/iommu/intel/iommu.h |  1 +
> >   drivers/iommu/intel/iommu.c | 29 +++++++++++++++++++++++++++++
> >   drivers/iommu/intel/svm.c   | 14 ++++++--------
> >   3 files changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> > index 54eeaa8e35a9..f13c228924f8 100644
> > --- a/drivers/iommu/intel/iommu.h
> > +++ b/drivers/iommu/intel/iommu.h
> > @@ -1081,6 +1081,7 @@ void free_pgtable_page(void *vaddr);
> >   void iommu_flush_write_buffer(struct intel_iommu *iommu);
> >   struct iommu_domain *intel_nested_domain_alloc(struct iommu_domain 
> > *parent,
> >                              const struct iommu_user_data *user_data);
> > +struct device *device_rbtree_find(struct intel_iommu *iommu, u16 rid);
> >   #ifdef CONFIG_INTEL_IOMMU_SVM
> >   void intel_svm_check(struct intel_iommu *iommu);
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 09009d96e553..d92c680bcc96 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -120,6 +120,35 @@ static int device_rid_cmp(struct rb_node *lhs, 
> > const struct rb_node *rhs)
> >       return device_rid_cmp_key(&key, rhs);
> >   }
> > +/*
> > + * Looks up an IOMMU-probed device using its source ID.
> > + *
> > + * If the device is found:
> > + *  - Increments its reference count.
> > + *  - Returns a pointer to the device.
> > + *  - The caller must call put_device() after using the pointer.
> > + *
> > + * If the device is not found, returns NULL.
> > + */
> > +struct device *device_rbtree_find(struct intel_iommu *iommu, u16 rid)
> > +{
> > +    struct device_domain_info *info;
> > +    struct device *dev = NULL;
> > +    struct rb_node *node;
> > +    unsigned long flags;
> > +
> > +    spin_lock_irqsave(&iommu->device_rbtree_lock, flags);
>
> Though per iommu device rbtree isn't a big tree, given already holds 
> spin_lock
> why still needs irq off ?

I want it to be usable not only in the normal execution flow, but also
in the interrupt context, such as for the DMA remapping fault
(unrecoverable) reporting path.

Best regards,
baolu