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

[Muli Ben-Yehuda <muli@xxxxxxxxxx>]

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?

> 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