On 15.02.22 10:11, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
>
> @@ -171,8 +173,24 @@ static int __init apply_map(struct domain *d, const
> struct pci_dev *pdev,
> struct map_data data = { .d = d, .map = true };
> int rc;
>
> + ASSERT(rw_is_write_locked(&d->vpci_rwlock));
> +
> while ( (rc = rangeset_consume_ranges(mem, map_range, &data)) ==
> -ERESTART )
> + {
> + /*
> + * FIXME: apply_map is called from dom0 specific init code when
> + * system_state < SYS_STATE_active, so there is no race condition
> + * possible between this code and vpci_process_pending. So, neither
> + * vpci_process_pending may try to acquire the lock in read mode and
> + * also destroy pdev->vpci in its error path nor pdev may be disposed
> + * yet. This means that it is not required to check if the relevant
> + * pdev->vpci still exists after re-acquiring the lock.
> + */
> I'm not sure why you need to mention vpci_process_pending here:
> apply_map and defer_map are mutually exclusive, so given the current
> code it's impossible to get in a situation where apply_map is called
> while there's pending work on the vCPU (ie: v->vpci.mem != NULL).
>
> Also there's no need for a FIXME tag: the current approach doesn't
> require any fixes unless we start using apply_map in a different
> context.
>
> Hence I think the comment should be along the lines of:
>
> /*
> * It's safe to drop and reacquire the lock in this context without
> * risking pdev disappearing because devices cannot be removed until the
> * initial domain has been started.
> */
This sounds good, will use this
>
> Thanks, Roger.
