Re: [Xen-ia64-devel] [PATCH] Fix some IPF Xen VT-d bugs

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Tue, Dec 23, 2008 at 11:20:34AM +0800, Cui, Dexuan wrote:
> In arch_domain_create(): when xen creates Dom0, need_iommu(d) is false, so 
> iommu_domain_init() is not invoked, as a result, eventually iommu is not 
> enabled properly.
> 
> In IA64 Xen, physdev_map_pirq()/physdev_unmap_pirq() are kept dummy since we 
> don't support MSI in IA64 Xen now, but here they shouldn't return -ENOSYS 
> because xend invokes them (the x86 version of them is necessary for x86 Xen); 
> in IPF Xen if they return -ENOSYS, xend would disallow us to create IPF HVM 
> guest with devices assigned. Here They can return 0 instead.
> 
> Signed-off-by: Dexuan Cui <dexuan.cui@xxxxxxxxx>
>
> diff -r 008b68ff6095 xen/arch/ia64/xen/domain.c
> --- a/xen/arch/ia64/xen/domain.c      Tue Nov 18 10:33:55 2008 +0900
> +++ b/xen/arch/ia64/xen/domain.c      Mon Dec 15 18:41:52 2008 +0800
> @@ -602,10 +602,8 @@ int arch_domain_create(struct domain *d,
>       if ((d->arch.mm.pgd = pgd_alloc(&d->arch.mm)) == NULL)
>           goto fail_nomem;
>  
> -     if ( iommu_enabled && (is_hvm_domain(d) || need_iommu(d)) ){
> -             if(iommu_domain_init(d) != 0)
> -                     goto fail_iommu;
> -     }
> +     if(iommu_domain_init(d) != 0)
> +             goto fail_iommu;
>  
>       /*
>        * grant_table_create() can't fully initialize grant table for domain

Please don't drop is_hvm_domain(d) check.
At this moment ia64 doesn't support iommu for PV domain because
it would introduce race with the ia64 lockless p2m table.
If you want VT-d support for pv domain, the race must be resolved somehow.


> diff -r 008b68ff6095 xen/arch/ia64/xen/hypercall.c
> --- a/xen/arch/ia64/xen/hypercall.c   Tue Nov 18 10:33:55 2008 +0900
> +++ b/xen/arch/ia64/xen/hypercall.c   Mon Dec 15 18:41:52 2008 +0800
> @@ -316,16 +316,17 @@ iosapic_guest_write(
>  
>  
>  /*
> - * XXX We don't support MSI for PCI passthrough, so just return ENOSYS
> + * XXX We don't support MSI for PCI passthrough at present, so make the
> + * following 2 functions dummy for now.
>   */
>  static int physdev_map_pirq(struct physdev_map_pirq *map)
>  {
> -     return -ENOSYS;
> +     return 0;
>  }
>  
>  static int physdev_unmap_pirq(struct physdev_unmap_pirq *unmap)
>  {
> -     return -ENOSYS;
> +     return 0;
>  }

Please add comments about why 0 instead of -ENOSYS.
The same sentence in the commit log is okay.

thanks,
-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel