On Fri, Nov 18 2022 at 08:17, Kevin Tian wrote: >> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx> >> /** >> - * 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 Yes. >> +/** >> + * 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()? It's range plus domain id and later it gains nirqs. So its awkward in any case. >> +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. Let me look at that.