> From: Jan Beulich <jbeul...@suse.com>
> Sent: Friday, November 12, 2021 5:49 PM
>
> - Correct struct field type.
> - Use unsigned int when that suffices.
> - Eliminate a (badly typed) local variable from
> context_set_domain_id().
> - Don't use -EFAULT inappropriately.
> - Move set_bit() such that it won't be done redundantly.
> - Constification.
> - Reduce scope of some variables.
> - Coding style.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Kevin Tian <kevin.t...@intel.com>
>
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -62,10 +62,10 @@ static struct tasklet vtd_fault_tasklet;
> static int setup_hwdom_device(u8 devfn, struct pci_dev *);
> static void setup_hwdom_rmrr(struct domain *d);
>
> -static int domain_iommu_domid(struct domain *d,
> - struct vtd_iommu *iommu)
> +static int domain_iommu_domid(const struct domain *d,
> + const struct vtd_iommu *iommu)
> {
> - unsigned long nr_dom, i;
> + unsigned int nr_dom, i;
>
> nr_dom = cap_ndoms(iommu->cap);
> i = find_first_bit(iommu->domid_bitmap, nr_dom);
> @@ -74,7 +74,7 @@ static int domain_iommu_domid(struct dom
> if ( iommu->domid_map[i] == d->domain_id )
> return i;
>
> - i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
> + i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
> }
>
> if ( !d->is_dying )
> @@ -88,61 +88,52 @@ static int domain_iommu_domid(struct dom
> #define DID_FIELD_WIDTH 16
> #define DID_HIGH_OFFSET 8
> static int context_set_domain_id(struct context_entry *context,
> - struct domain *d,
> + const struct domain *d,
> struct vtd_iommu *iommu)
> {
> - unsigned long nr_dom, i;
> - int found = 0;
> + unsigned int nr_dom, i;
>
> ASSERT(spin_is_locked(&iommu->lock));
>
> nr_dom = cap_ndoms(iommu->cap);
> i = find_first_bit(iommu->domid_bitmap, nr_dom);
> - while ( i < nr_dom )
> - {
> - if ( iommu->domid_map[i] == d->domain_id )
> - {
> - found = 1;
> - break;
> - }
> - i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
> - }
> + while ( i < nr_dom && iommu->domid_map[i] != d->domain_id )
> + i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
>
> - if ( found == 0 )
> + if ( i >= nr_dom )
> {
> i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
> if ( i >= nr_dom )
> {
> dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain ids\n");
> - return -EFAULT;
> + return -EBUSY;
> }
> iommu->domid_map[i] = d->domain_id;
> + set_bit(i, iommu->domid_bitmap);
> }
>
> - set_bit(i, iommu->domid_bitmap);
> context->hi |= (i & ((1 << DID_FIELD_WIDTH) - 1)) << DID_HIGH_OFFSET;
> return 0;
> }
>
> -static int context_get_domain_id(struct context_entry *context,
> - struct vtd_iommu *iommu)
> +static int context_get_domain_id(const struct context_entry *context,
> + const struct vtd_iommu *iommu)
> {
> - unsigned long dom_index, nr_dom;
> int domid = -1;
>
> - if (iommu && context)
> + if ( iommu && context )
> {
> - nr_dom = cap_ndoms(iommu->cap);
> -
> - dom_index = context_domain_id(*context);
> + unsigned int nr_dom = cap_ndoms(iommu->cap);
> + unsigned int dom_index = context_domain_id(*context);
>
> if ( dom_index < nr_dom && iommu->domid_map )
> domid = iommu->domid_map[dom_index];
> else
> dprintk(XENLOG_DEBUG VTDPREFIX,
> - "dom_index %lu exceeds nr_dom %lu or iommu has no
> domid_map\n",
> + "dom_index %u exceeds nr_dom %u or iommu has no
> domid_map\n",
> dom_index, nr_dom);
> }
> +
> return domid;
> }
>
> @@ -1302,7 +1293,7 @@ int __init iommu_alloc(struct acpi_drhd_
> if ( !iommu->domid_bitmap )
> return -ENOMEM;
>
> - iommu->domid_map = xzalloc_array(u16, nr_dom);
> + iommu->domid_map = xzalloc_array(domid_t, nr_dom);
> if ( !iommu->domid_map )
> return -ENOMEM;
>
> @@ -1477,11 +1468,12 @@ int domain_context_mapping_one(
> spin_unlock(&hd->arch.mapping_lock);
> }
>
> - if ( context_set_domain_id(context, domain, iommu) )
> + rc = context_set_domain_id(context, domain, iommu);
> + if ( rc )
> {
> spin_unlock(&iommu->lock);
> unmap_vtd_domain_page(context_entries);
> - return -EFAULT;
> + return rc;
> }
>
> context_set_address_width(*context, level_to_agaw(iommu-
> >nr_pt_levels));
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -505,7 +505,7 @@ struct vtd_iommu {
>
> struct list_head ats_devices;
> unsigned long *domid_bitmap; /* domain id bitmap */
> - u16 *domid_map; /* domain id mapping array */
> + domid_t *domid_map; /* domain id mapping array */
> uint32_t version;
> };
>
