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

2009-01-06 Thread Isaku Yamahata
applied, thanks.

On Tue, Jan 06, 2009 at 03:56:07PM +0800, Cui, Dexuan wrote:
> Isaku Yamahata wrote:
> > On Mon, Jan 05, 2009 at 05:39:52PM +0800, Cui, Dexuan wrote:
> >> Isaku Yamahata wrote:
> >>> On x86 case p2m_lock/unlock() avoids the race, but ia64 doesn't
> >>> have such lock. At this moment, the only HVM domain would be
> >>> supported. 
> >> OK, I understand we can't support pv iommu before resolving the
> >> lockless p2m issue. 
> >> 
> >>> The issue is dom0 case. I suppose it can be supported by mapping
> >>> all the pages except xen pages at boot time and not iommu
> >>> mapping/unmapping because those pages are already mapped to dom0
> >>> by intel_iommu_domain_init().
> >> I think actually we do this.
> >> For the special pv guest Dom0, I think there is no issue here because
> >>  dom0->need_iommu is actually always 0 (when Dom0 boots up and xen
> >> assigns all the devices to Dom0, xen doesn't invoke assign_device(),
> >> and invoking iommu_domain_init()/intel_iommu_domain_init() doesn't
> >> cause need_iommu(dom0) to be 1).   
> > 
> > Okay, I missed that.
> 
> Hi Isaku, so could you please check in the attached patch (the same as the 
> old one I posted and updated the comment) first?
> 
> For the calc_dom0_size() and the memory hole issues, we can fix them in a 
> later patch.
> 
> Thanks,
> -- Dexuan
> 


> ___
> Xen-ia64-devel mailing list
> Xen-ia64-devel@lists.xensource.com
> http://lists.xensource.com/xen-ia64-devel

-- 
yamahata

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


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

2009-01-05 Thread Cui, Dexuan
Isaku Yamahata wrote:
> On Mon, Jan 05, 2009 at 05:39:52PM +0800, Cui, Dexuan wrote:
>> Isaku Yamahata wrote:
>>> On x86 case p2m_lock/unlock() avoids the race, but ia64 doesn't
>>> have such lock. At this moment, the only HVM domain would be
>>> supported. 
>> OK, I understand we can't support pv iommu before resolving the
>> lockless p2m issue. 
>> 
>>> The issue is dom0 case. I suppose it can be supported by mapping
>>> all the pages except xen pages at boot time and not iommu
>>> mapping/unmapping because those pages are already mapped to dom0
>>> by intel_iommu_domain_init().
>> I think actually we do this.
>> For the special pv guest Dom0, I think there is no issue here because
>>  dom0->need_iommu is actually always 0 (when Dom0 boots up and xen
>> assigns all the devices to Dom0, xen doesn't invoke assign_device(),
>> and invoking iommu_domain_init()/intel_iommu_domain_init() doesn't
>> cause need_iommu(dom0) to be 1).   
> 
> Okay, I missed that.

Hi Isaku, so could you please check in the attached patch (the same as the old 
one I posted and updated the comment) first?

For the calc_dom0_size() and the memory hole issues, we can fix them in a later 
patch.

Thanks,
-- Dexuan



fix_vtd.patch
Description: fix_vtd.patch
___
Xen-ia64-devel mailing list
Xen-ia64-devel@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel

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

2009-01-05 Thread Isaku Yamahata
On Mon, Jan 05, 2009 at 05:39:52PM +0800, Cui, Dexuan wrote:
> Isaku Yamahata wrote:
> > On x86 case p2m_lock/unlock() avoids the race, but ia64 doesn't have
> > such lock.
> > At this moment, the only HVM domain would be supported.
> OK, I understand we can't support pv iommu before resolving the lockless p2m 
> issue.
> 
> > The issue is dom0 case. I suppose it can be supported by mapping
> > all the pages except xen pages at boot time and not iommu
> > mapping/unmapping because those pages are already mapped to dom0
> > by intel_iommu_domain_init().
> I think actually we do this.
> For the special pv guest Dom0, I think there is no issue here because
>  dom0->need_iommu is actually always 0 (when Dom0 boots up and xen assigns 
> all the devices to Dom0, xen doesn't invoke assign_device(), and invoking 
> iommu_domain_init()/intel_iommu_domain_init() doesn't cause need_iommu(dom0) 
> to be 1).

Okay, I missed that.
-- 
yamahata

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


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

2009-01-05 Thread Cui, Dexuan
Isaku Yamahata wrote:
> On x86 case p2m_lock/unlock() avoids the race, but ia64 doesn't have
> such lock.
> At this moment, the only HVM domain would be supported.
OK, I understand we can't support pv iommu before resolving the lockless p2m 
issue.

> The issue is dom0 case. I suppose it can be supported by mapping
> all the pages except xen pages at boot time and not iommu
> mapping/unmapping because those pages are already mapped to dom0
> by intel_iommu_domain_init().
I think actually we do this.
For the special pv guest Dom0, I think there is no issue here because
 dom0->need_iommu is actually always 0 (when Dom0 boots up and xen assigns all 
the devices to Dom0, xen doesn't invoke assign_device(), and invoking 
iommu_domain_init()/intel_iommu_domain_init() doesn't cause need_iommu(dom0) to 
be 1).

>>> intel_iommu_domain_init() and dom0 memory size
>>>   calc_dom0_size() in xen/arch/ia64/domain.c calculates default dom0
>>>   memory size. You should take memory for iommu page table
>>>   into account because the memory size for iommu page table
>>>   wouldn't   be neglectable. probably iommu_pages = (max phys addr)
>>>   / PTRS_PER_PTE_4K + (some spare) where PTRS_PER_PTE_4K = (1 <<
>>> (PAGE_SHIFT_4K - 3)) 
>> Now, in intel_iommu_domain_init(), with respect to iommu mapping,
>> Xen maps all the pages for Dom0 except for the pages used by Xen
>> itself.  
>> Do you mean xen should only maps the page owned actually by Dom0? 
>> -- for instance, you're saying xen should not map the iommu page
>> tables? -- since in Dom0 normally drivers don't touch iommu
>> pagetables at all, looks the current code  is OK?   
> 
> No. I meant that calc_dom0_size() should be updated.
> It calculates the maximum memory size which can be passed to dom0
> safely. Without dom0_mem_size Xen VMM tries to give dom0 the maximum
> memory size which is a common use case.
> 
> On the other hand, it isn't uncommon that ia64 machine has several
> hundred Giga bytes, so memory size for VT-d table would reach tens or
> hundreds megabytes which can't be neglectable compared to xen heap
> size. Memory for the VT-d table size should be taken into acount
> in calc_dom0_size().

I'll look into this.

> 
>>> intel_iommu_domain_init() and sparse memory.
>>>   To be honest, I'm not sure how it matters in practice.
>>>   On ia64 memory might be located sparsely. So iommu page table
>>>   should also sparse instead of [0, max_page] - (xen page).
>>>   You want to use efi_memmap_walk() instead of for loop.
>> Thanks for pointing this out!
>> So my understanding is: in the current intel_iommu_domain_init(),
>> since xen judges by the 'max_page', actually some pages at the high
>> address(possible in the middle or at the end) are not mapped while
>> they should have been mapped?   
> 
> On ia64 machine there might be a big hole so that mapping all the
> range [0, max_page] would cause lack of memory. Off course, it
> depends on what kind of ia64 box you use.
> Probably we can skip this issue and address it later if the issue
> arose.

I'll look into this.

Thanks!

-- Dexuan


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


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

2009-01-04 Thread Isaku Yamahata
Hi. Sorry for delayed reply.

On Thu, Dec 25, 2008 at 10:14:09PM +0800, Cui, Dexuan wrote:
> Isaku Yamahata wrote:
> > On Wed, Dec 24, 2008 at 01:11:03PM +0800, Cui, Dexuan wrote:
> >> Isaku Yamahata wrote:
>  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
> >> Oh, thanks for the reminder. Here I neglected this.
> >> 
> >> Do you mean this:
> >> if ( is_hvm_domain(d) )
> >> if(iommu_domain_init(d) != 0)
> >> goto fail_iommu;
> >> This is also not ok since we must ensure iommu_domain_init() is
> >> invoked for Dom0 -- we need the function invoked to enable DMA
> >> remapping.  
> >> 
> >> So how about changing the logic to:
> >> if ( (d->domain_id == 0) || is_hvm_domain(d) )
> >> if(iommu_domain_init(d) != 0)
> >> goto fail_iommu;
> >> 
> >> If you agree this, I'll post a new patch.
> > 
> > Do you mean if ( d->domain_id == 0 ) clause in
> > the function, intel_iommu_domain_init()?
> Yes. 
> 
> > Is iommu map/unmap for dom0 is necessary?
> >   intel_iommu_domain_init() maps all the pages excect ones xen uses
> >   to dom0. I suppose this is what you want.
> Yes.
> When Dom0 boots up, we assign all the devices to it, so it needs the 1:1 VT-d 
> pagetables mapping.
> 
> >   However later pages is mapped/unmapped even for dom0 because
> I suppose you mean the balloon driver and the grant table operations. Correct?

That's right.


> >   need_iommu(dom0) returns true due ot iommu_domain_init(dom0).
> >   Since dom0 is PV, so iommu mapping/unmapping causes race on ia64.
> In the cases of balloon and granttable, the iommu mapping/unmapping would 
> cause race on IA64?
> Sorry, I know few about the lockless p2m table now. I'm trying to understand 
> more.

Yes. That is why the first ia64 VT-d patches doesn't enable VT-d
for PV domains by not calling iommu_domain_init().
On x86 case p2m_lock/unlock() avoids the race, but ia64 doesn't have such
lock.
At this moment, the only HVM domain would be supported.
The issue is dom0 case. I suppose it can be supported by mapping
all the pages except xen pages at boot time and not iommu
mapping/unmapping because those pages are already mapped to dom0
by intel_iommu_domain_init().


> >   Only setting up iommu tables at the dom0 creation is necessary,
> Could you please explain more about the this? I can't get the point.
> 
> >   all "if ( iommu_enabled && (is_hvm_domain(d) || need_iommu(d)) )"
> >   would be "if ( iommu_enabled && is_hvm_domain(d) && need_iommu(d))
> > )" 
> Am I missing somthing?
> #define need_iommu(d)((d)->need_iommu && !(d)->is_hvm)
> So,
> iommu_enabled && is_hvm_domain(d) && need_iommu(d)
> is undoubtedly false. :-)

Ah sorry. I missed d->is_hvm. Please forget this sentence.


> > intel_iommu_domain_init() and dom0 memory size
> >   calc_dom0_size() in xen/arch/ia64/domain.c calculates default dom0
> >   memory size. You should take memory for iommu page table
> >   into account because the memory size for iommu page table wouldn't
> >   be neglectable.
> >   probably iommu_pages = (max phys addr) / PTRS_PER_PTE_4K + (some
> >   spare) where PTRS_PER_PTE_4K = (1 << (PAGE_SHIFT_4K - 3))
> Now, in intel_iommu_domain_init(), with respect to iommu mapping, Xen maps 
> all the pages for Dom0 except for the pages used by Xen itself.
> Do you mean xen should only maps the page owned actually by Dom0?  -- for 
> instance, you're saying xen should not map the iommu page tables? -- since in 
> Dom0 normally drivers don't touch iommu pagetables at all, looks the current 
> code  is OK?

No. I meant that calc_dom0_size() should be updated.
It calculates the maximum memory size which can be passed to dom0 safely.
Without dom0_mem_size Xen VMM tries to give dom0 the maximum memory size
which is a common use case.

On the other hand, it isn't uncommon that ia64 machine has several
hundred Giga bytes, so memory size for VT-d table would reach tens or
hundreds megabytes which can't be neglectable compared to xen heap size.
Memory for the VT-d table size should be taken into acount
in calc_dom0_size().


> > intel_iommu_domain_init() and sparse memory.
> >   To be honest, I'm not sure how i

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

2008-12-25 Thread Cui, Dexuan
Isaku Yamahata wrote:
> On Wed, Dec 24, 2008 at 01:11:03PM +0800, Cui, Dexuan wrote:
>> Isaku Yamahata wrote:
 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
>> Oh, thanks for the reminder. Here I neglected this.
>> 
>> Do you mean this:
>> if ( is_hvm_domain(d) )
>> if(iommu_domain_init(d) != 0)
>> goto fail_iommu;
>> This is also not ok since we must ensure iommu_domain_init() is
>> invoked for Dom0 -- we need the function invoked to enable DMA
>> remapping.  
>> 
>> So how about changing the logic to:
>> if ( (d->domain_id == 0) || is_hvm_domain(d) )
>> if(iommu_domain_init(d) != 0)
>> goto fail_iommu;
>> 
>> If you agree this, I'll post a new patch.
> 
> Do you mean if ( d->domain_id == 0 ) clause in
> the function, intel_iommu_domain_init()?
Yes. 

> Is iommu map/unmap for dom0 is necessary?
>   intel_iommu_domain_init() maps all the pages excect ones xen uses
>   to dom0. I suppose this is what you want.
Yes.
When Dom0 boots up, we assign all the devices to it, so it needs the 1:1 VT-d 
pagetables mapping.

>   However later pages is mapped/unmapped even for dom0 because
I suppose you mean the balloon driver and the grant table operations. Correct?

>   need_iommu(dom0) returns true due ot iommu_domain_init(dom0).
>   Since dom0 is PV, so iommu mapping/unmapping causes race on ia64.
In the cases of balloon and granttable, the iommu mapping/unmapping would cause 
race on IA64?
Sorry, I know few about the lockless p2m table now. I'm trying to understand 
more.

>   Only setting up iommu tables at the dom0 creation is necessary,
Could you please explain more about the this? I can't get the point.

>   all "if ( iommu_enabled && (is_hvm_domain(d) || need_iommu(d)) )"
>   would be "if ( iommu_enabled && is_hvm_domain(d) && need_iommu(d))
> )" 
Am I missing somthing?
#define need_iommu(d)((d)->need_iommu && !(d)->is_hvm)
So,
iommu_enabled && is_hvm_domain(d) && need_iommu(d)
is undoubtedly false. :-)

> 
> intel_iommu_domain_init() and dom0 memory size
>   calc_dom0_size() in xen/arch/ia64/domain.c calculates default dom0
>   memory size. You should take memory for iommu page table
>   into account because the memory size for iommu page table wouldn't
>   be neglectable.
>   probably iommu_pages = (max phys addr) / PTRS_PER_PTE_4K + (some
>   spare) where PTRS_PER_PTE_4K = (1 << (PAGE_SHIFT_4K - 3))
Now, in intel_iommu_domain_init(), with respect to iommu mapping, Xen maps all 
the pages for Dom0 except for the pages used by Xen itself.
Do you mean xen should only maps the page owned actually by Dom0?  -- for 
instance, you're saying xen should not map the iommu page tables? -- since in 
Dom0 normally drivers don't touch iommu pagetables at all, looks the current 
code  is OK?

> intel_iommu_domain_init() and sparse memory.
>   To be honest, I'm not sure how it matters in practice.
>   On ia64 memory might be located sparsely. So iommu page table
>   should also sparse instead of [0, max_page] - (xen page).
>   You want to use efi_memmap_walk() instead of for loop.
Thanks for pointing this out!
So my understanding is: in the current intel_iommu_domain_init(), since xen 
judges by the 'max_page', actually some pages at the high address(possible in 
the middle or at the end) are not mapped while they should have been mapped?

Thanks,
-- Dexuan

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


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

2008-12-23 Thread Isaku Yamahata
On Wed, Dec 24, 2008 at 01:11:03PM +0800, Cui, Dexuan wrote:
> Isaku Yamahata wrote:
> >> 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
> Oh, thanks for the reminder. Here I neglected this.
> 
> Do you mean this:
> if ( is_hvm_domain(d) )
> if(iommu_domain_init(d) != 0)
> goto fail_iommu;
> This is also not ok since we must ensure iommu_domain_init() is invoked for 
> Dom0 -- we need the function invoked to enable DMA remapping.
>
> So how about changing the logic to:
> if ( (d->domain_id == 0) || is_hvm_domain(d) )
> if(iommu_domain_init(d) != 0)
> goto fail_iommu;
> 
> If you agree this, I'll post a new patch.

Do you mean if ( d->domain_id == 0 ) clause in 
he function, intel_iommu_domain_init()?

Is iommu map/unmap for dom0 is necessary?
  intel_iommu_domain_init() maps all the pages excect ones xen uses
  to dom0. I suppose this is what you want.
  However later pages is mapped/unmapped even for dom0 because
  need_iommu(dom0) returns true due ot iommu_domain_init(dom0).
  Since dom0 is PV, so iommu mapping/unmapping causes race on ia64.
  Only setting up iommu tables at the dom0 creation is necessary,
  all "if ( iommu_enabled && (is_hvm_domain(d) || need_iommu(d)) )"
  would be "if ( iommu_enabled && is_hvm_domain(d) && need_iommu(d)) )"

intel_iommu_domain_init() and dom0 memory size
  calc_dom0_size() in xen/arch/ia64/domain.c calculates default dom0
  memory size. You should take memory for iommu page table
  into account because the memory size for iommu page table wouldn't
  be neglectable.
  probably iommu_pages = (max phys addr) / PTRS_PER_PTE_4K + (some spare)
  where PTRS_PER_PTE_4K = (1 << (PAGE_SHIFT_4K - 3))

intel_iommu_domain_init() and sparse memory.
  To be honest, I'm not sure how it matters in practice.
  On ia64 memory might be located sparsely. So iommu page table
  should also sparse instead of [0, max_page] - (xen page).
  You want to use efi_memmap_walk() instead of for loop.

thanks,
-- 
yamahata

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


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

2008-12-23 Thread Cui, Dexuan
Isaku Yamahata wrote:
>> 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
Oh, thanks for the reminder. Here I neglected this.

Do you mean this:
if ( is_hvm_domain(d) )
if(iommu_domain_init(d) != 0)
goto fail_iommu;
This is also not ok since we must ensure iommu_domain_init() is invoked for 
Dom0 -- we need the function invoked to enable DMA remapping.
So how about changing the logic to:
if ( (d->domain_id == 0) || is_hvm_domain(d) )
if(iommu_domain_init(d) != 0)
goto fail_iommu;

If you agree this, I'll post a new patch.

> 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. 
At present, let us support VT-d for HVM guest first.

> 
> Please add comments about why 0 instead of -ENOSYS.
OK, I'll add it.


Thanks,
-- Dexuan


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


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

2008-12-23 Thread Isaku Yamahata
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 
>
> 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@lists.xensource.com
http://lists.xensource.com/xen-ia64-devel