[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [PATCH 4/6] VT-d: tidy domid map handling


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
  • Date: Mon, 15 Nov 2021 05:51:12 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=RhIxBiYeDO8NqqFPqT47MOHEGkeEkcGoHosdRwk1VDQ=; b=OIvFfbBEaFa1NKpYgPsCD9NtZObAeTzQsvuiXYbjdH3b0OCyHRUaWXPru6JK12YDuo+5B5yhFw1TdtqrFhiLA7IwgY/TrvGaudsyvsqjhjVNpWNpzrOkjnHx4j8a7dvvmVHTDfdY2hgd5TAxBT7GSICM9K7FzHEyAqe+mi/r7w841iFUdwnWhU63nbaXD0XzhnqsOgk3OzfVuzfbUrrFo5/Jx82LUcCagsFWchwOW93wMk4Ybe9FV7nsPsOXdyQntLzUnURt2clWZtNdkn1xR7RgLRTLILFetg4XQiCaxeTFObXTS1Rp2szxbCCqIaeml0n3An8R8e/kgf26F8MNCg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ADJMyQ4NRUpHKVwcIvQTmmdFFnNxtcwtq0LhBRKcoJN29AxrKUmiL6B3X9+yrXYye5VH20dWZvqDrBMhK8QCiLHR5A6LQNvfG76IP5IMnw1v2jB8QnVd27tsVnJqzXTHhoTQDkT0jZ5clR5EoGHzK4/FodVl+X3ktMfb+VW8Ym1BHbYEonQKhsn1lXgq9nl8BpcfZ8ChJTmSllvmSRsGw7A1qdCY0dmjMFLlc0L2/+yr3Qq1X6Iok3HW79hp1vpKtUwyzZzR0U4XdFG5UGQ/rDg3U/R/GOiU0xLbUBdqOLQqd9SsNMVoIz91l7EfmCpXqln9BFnIJTC/dhOdAhUauw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com;
  • Delivery-date: Mon, 15 Nov 2021 05:51:49 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHX16qTgTKvSTsW3Uqd1am9I012FqwEGi+Q
  • Thread-topic: [PATCH 4/6] VT-d: tidy domid map handling

> From: Jan Beulich <jbeulich@xxxxxxxx>
> 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 <jbeulich@xxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> 
> --- 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;
>  };
> 


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.