RE: [Xen-devel] [VTD][patch 4/5] HVM device assignment using vt-d

["Kay, Allen M" <allen.m.kay@xxxxxxxxx>]

Current Xen's memory management does not take account of handling of
device mmio memory that is not part of the e820 table.  Most of the
changes here is do whatever it take to install mmio mapping to the
guest.

For proper handling of device mmio, there probably need some kind of
overhual in p2m and shadow code.

The changes in multi.c and private.h were originally suggested by Tim
Deegan.  Basically it is for handling memory not in e820 table.  Most of
other changes you mentioned below were of the same spirit.

For dom0 memory, we are doing identity mapping.

Allen


>-----Original Message-----
>From: Muli Ben-Yehuda [mailto:muli@xxxxxxxxxx] 
>Sent: Monday, June 04, 2007 11:21 AM
>To: Kay, Allen M
>Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [Xen-devel] [VTD][patch 4/5] HVM device 
>assignment using vt-d
>
>On Wed, May 30, 2007 at 12:10:50PM -0700, Kay, Allen M wrote:
>> vtd4.patch:
>>     - mmio handling
>> 
>> Signed-off-by: Allen Kay <allen.m.kay@xxxxxxxxx>
>> Signed-off-by: Xiaohui Xin <xiaohui.xin@xxxxxxxxx>
>>
>> diff -r 0047ce6caa2d xen/arch/x86/mm/p2m.c
>> --- a/xen/arch/x86/mm/p2m.c  Wed May 30 11:53:58 2007 -0400
>> +++ b/xen/arch/x86/mm/p2m.c  Wed May 30 11:54:58 2007 -0400
>> @@ -27,11 +27,11 @@
>>  #include <asm/page.h>
>>  #include <asm/paging.h>
>>  #include <asm/p2m.h>
>> +#include <asm/iommu.h>
>>  
>>  /* Debugging and auditing of the P2M code? */
>>  #define P2M_AUDIT     0
>>  #define P2M_DEBUGGING 1
>> -
>>  /* The P2M lock.  This protects all updates to the p2m table.
>>   * Updates are expected to be safe against concurrent reads, 
>>   * which do *not* require the lock */
>> @@ -215,14 +215,18 @@ set_p2m_entry(struct domain *d, unsigned
>>      if ( mfn_valid(mfn) )
>>          entry_content = l1e_from_pfn(mfn_x(mfn), 
>__PAGE_HYPERVISOR|_PAGE_USER);
>>      else
>> -        entry_content = l1e_empty();
>> +        entry_content = l1e_from_pfn(mfn_x(mfn),
>> +                            
>__PAGE_HYPERVISOR|_PAGE_USER|_PAGE_PCD|_PAGE_PWT);
>>  
>
>Can you please explain this change?
>

This change is for getting memory outside of e820 memory range mapped.

>> diff -r 0047ce6caa2d xen/arch/x86/mm/shadow/multi.c
>> --- a/xen/arch/x86/mm/shadow/multi.c Wed May 30 11:53:58 2007 -0400
>> +++ b/xen/arch/x86/mm/shadow/multi.c Wed May 30 11:55:31 2007 -0400
>> @@ -28,6 +28,7 @@
>>  #include <xen/sched.h>
>>  #include <xen/perfc.h>
>>  #include <xen/domain_page.h>
>> +#include <xen/iocap.h>
>>  #include <asm/page.h>
>>  #include <asm/current.h>
>>  #include <asm/shadow.h>
>> @@ -667,7 +668,8 @@ _sh_propagate(struct vcpu *v,
>>      // case of a prefetch, an invalid mfn means that we can 
>not usefully
>>      // shadow anything, and so we return early.
>>      //
>> -    if ( !mfn_valid(target_mfn) )
>> +    if ( !mfn_valid(target_mfn) && 
>> +         !iomem_access_permitted(current->domain, 
>mfn_x(target_mfn), mfn_x(target_mfn)) )
>>      {
>
>Why are the iomem_access_permitted() checks needed here?
>
>> diff -r 0047ce6caa2d xen/arch/x86/mm/shadow/private.h
>> --- a/xen/arch/x86/mm/shadow/private.h       Wed May 30 
>11:53:58 2007 -0400
>> +++ b/xen/arch/x86/mm/shadow/private.h       Wed May 30 
>11:55:49 2007 -0400
>> @@ -446,7 +446,7 @@ sh_mfn_is_a_page_table(mfn_t gmfn)
>>      struct domain *owner;
>>      unsigned long type_info;
>>  
>> -    if ( !mfn_valid(gmfn) )
>> +    if ( !mfn_valid(gmfn) || unlikely(mfn_x(gmfn) > max_page) )
>>          return 0;
>
>Is this change specific to the IOMMU code? if yes, why is it needed?
> 
>> diff -r 0047ce6caa2d xen/common/page_alloc.c
>> --- a/xen/common/page_alloc.c        Wed May 30 11:53:58 2007 -0400
>> +++ b/xen/common/page_alloc.c        Wed May 30 11:57:44 2007 -0400
>> @@ -37,6 +37,7 @@
>>  #include <xen/numa.h>
>>  #include <xen/nodemask.h>
>>  #include <asm/page.h>
>> +#include <asm/iommu.h>
>>  
>>  /*
>>   * Comma-separated list of hexadecimal page numbers 
>containing bad bytes.
>> @@ -790,6 +791,9 @@ int assign_pages(
>>          wmb(); /* Domain pointer must be visible before 
>updating refcnt. */
>>          pg[i].count_info = PGC_allocated | 1;
>>          list_add_tail(&pg[i].list, &d->page_list);
>> +
>> +        if (iommu_found() && (d == dom0))
>> +            iommu_map_page(d, page_to_mfn(&pg[i]), 
>page_to_mfn(&pg[i]));
>>      }
>
>Is gfn == mfn for every dom0 gfn? if not, the second argument above
>should be gfn.
>>  
>>      spin_unlock(&d->page_alloc_lock);
>> @@ -858,7 +862,7 @@ void free_domheap_pages(struct page_info
>>  {
>>      int            i, drop_dom_ref;
>>      struct domain *d = page_get_owner(pg);
>> -
>> + 
>
>Whitespace damage.
>
>Cheers,
>Muli
>

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