RE: [patch 12/20] genirq/msi: Make descriptor freeing domain aware

Tian, Kevin
2022-11-18T08:17:31Z

> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Friday, November 11, 2022 9:57 PM
>
> int msi_domain_insert_msi_desc(struct device *dev, unsigned int domid,
> struct msi_desc *init_desc);
> /**
> - * msi_insert_msi_desc - Allocate and initialize a MSI descriptor in the
> default domain
> + * msi_insert_msi_desc - Allocate and initialize a MSI descriptor in the
> default irqdomain
> + *

belong to last patch

> +/**
> + * struct msi_ctrl - MSI internal management control structure
> + * @domid: ID of the domain on which management operations should
> be done
> + * @first: First (hardware) slot index to operate on
> + * @last: Last (hardware) slot index to operate on
> + */
> +struct msi_ctrl {
> + unsigned int domid;
> + unsigned int first;
> + unsigned int last;
> +};
> +

this really contains the range information. what about msi_range and
then msi_range_valid()?

> +static void msi_domain_free_descs(struct device *dev, struct msi_ctrl *ctrl)
> {
> struct xarray *xa = &dev->msi.data->__store;
> struct msi_desc *desc;
> unsigned long idx;
> + int base;
> +
> + lockdep_assert_held(&dev->msi.data->mutex);
>
> - if (WARN_ON_ONCE(first_index >= MSI_MAX_INDEX || last_index >=
> MSI_MAX_INDEX))
> + if (!msi_ctrl_valid(dev, ctrl))
> return;
>
> - lockdep_assert_held(&dev->msi.data->mutex);
> + base = msi_get_domain_base_index(dev, ctrl->domid);
> + if (base < 0)
> + return;

What about putting domid checks in msi_ctrl_valid() then here could
be a simple calculation on domid * MSI_XA_DOMAIN_SIZE.

domid is part of msi_ctrl. then it sound reasonable to validate it
together with first/last.