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

Thomas Gleixner
2022-11-18T12:22:43Z

On Fri, Nov 18 2022 at 08:17, Kevin Tian wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>> /**
>> - * 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.