WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains

To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 5/6] xen-gntdev: Support mapping in HVM domains
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Tue, 18 Jan 2011 11:09:54 -0500
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "jeremy@xxxxxxxx" <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "konrad.wilk@xxxxxxxxxx" <konrad.wilk@xxxxxxxxxx>
Delivery-date: Tue, 18 Jan 2011 08:11:02 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1101171410150.7277@kaball-desktop>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: National Security Agency
References: <1295019976-605-1-git-send-email-dgdegra@xxxxxxxxxxxxx> <1295019976-605-6-git-send-email-dgdegra@xxxxxxxxxxxxx> <alpine.DEB.2.00.1101171410150.7277@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc13 Thunderbird/3.1.7
On 01/17/2011 09:28 AM, Stefano Stabellini wrote:
> On Fri, 14 Jan 2011, Daniel De Graaf wrote:
>> HVM does not allow direct PTE modification, so instead we request
>> that Xen change its internal p2m mappings on the allocated pages and
>> map the memory into userspace normally.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
>> ---
>>  drivers/xen/gntdev.c      |  117 
>> +++++++++++++++++++++++++++++++--------------
>>  drivers/xen/grant-table.c |    6 ++
>>  2 files changed, 87 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
>> index 8a12857..1931657 100644
>> --- a/drivers/xen/gntdev.c
>> +++ b/drivers/xen/gntdev.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/sched.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/slab.h>
>> +#include <linux/highmem.h>
>>  
>>  #include <xen/xen.h>
>>  #include <xen/grant_table.h>
>> @@ -52,6 +53,8 @@ MODULE_PARM_DESC(limit, "Maximum number of grants that may 
>> be mapped by "
>>  
>>  static atomic_t pages_mapped = ATOMIC_INIT(0);
>>  
>> +static int use_ptemod = 0;
>> +
>>  struct gntdev_priv {
>>      struct list_head maps;
>>      /* lock protects maps from concurrent changes */
>> @@ -184,6 +187,9 @@ static void gntdev_put_map(struct grant_map *map)
>>  
>>      atomic_sub(map->count, &pages_mapped);
>>  
>> +    if (!use_ptemod)
>> +            unmap_grant_pages(map, 0, map->count);
>> +
>>      for (i = 0; i < map->count; i++) {
>>              if (map->pages[i])
>>                      __free_page(map->pages[i]);
>> @@ -212,9 +218,12 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
>>  static int map_grant_pages(struct grant_map *map)
>>  {
>>      int i, flags, err = 0;
>> +    phys_addr_t addr;
>>      struct gnttab_map_grant_ref* map_ops = NULL;
>>  
>> -    flags = GNTMAP_host_map | GNTMAP_application_map | GNTMAP_contains_pte;
>> +    flags = GNTMAP_host_map;
>> +    if (use_ptemod)
>> +            flags |= GNTMAP_application_map | GNTMAP_contains_pte;
>>      if (map->is_ro)
>>              flags |= GNTMAP_readonly;
>>  
>> @@ -224,7 +233,11 @@ static int map_grant_pages(struct grant_map *map)
>>              goto out;
>>  
>>      for(i=0; i < map->count; i++) {
>> -            gnttab_set_map_op(&map_ops[i], map->pginfo[i].pte_maddr, flags,
>> +            if (use_ptemod)
>> +                    addr = map->pginfo[i].pte_maddr;
>> +            else
>> +                    addr = 
>> (phys_addr_t)pfn_to_kaddr(page_to_pfn(map->pages[i]));
>> +            gnttab_set_map_op(&map_ops[i], addr, flags,
>>                                map->pginfo[i].target.ref,
>>                                map->pginfo[i].target.domid);
>>      }
> 
> If I am not mistaken you are asking Xen to modify the virtual kernel
> mappings of map->pages, but it is not enough to have correctly working
> userspace mappings of map->pages: you need gnttab_map_refs to correctly
> fix the p2m and add an entry to m2p_override too.
> However in the last gntdev patch series I sent, requests with
> !GNTMAP_contains_pte are not added to m2p_override and the p2m entries
> of map->pages are not updated either.
> 
> Therefore you probably need this (untested) patch to make it work:
> 

It looks like this patch only applies if using gnttab_map_refs without
GNTMAP_contains_pte on PV guests; my patch only uses !GNTMAP_contains_pte
on HVM, so it doesn't need this change.

I think this patch would be needed if we wanted to remove GNTMAP_contains_pte
from the PV case, which might make the code cleaner (it would remove the
dependency on the MM notifier, and allow multiple mmap() of the device in PV).
I didn't want to change the working PV case when adding HVM support, so I
left that code mostly alone.

Just from quickly looking at the patch, since it uses current->active_mm,
it seems that it might still have issues with multiple processes mapping
the area. I would assume the update would be purely to the p2m table, not
involving any page tables (since the pages should not yet be mapped).

> ---
> 
> Add support for !GNTMAP_contains_pte mappings to gnttab_map_refs
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> 
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 9ef54eb..7d6d2ba 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -459,12 +459,38 @@ int gnttab_map_refs(struct gnttab_map_grant_ref 
> *map_ops,
>               return ret;
>  
>       for (i = 0; i < count; i++) {
> -             /* m2p override only supported for GNTMAP_contains_pte mappings 
> */
> -             if (!(map_ops[i].flags & GNTMAP_contains_pte))
> -                     continue;
> -             pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> -                             (map_ops[i].host_addr & ~PAGE_MASK));
> -             mfn = pte_mfn(*pte);
> +             if ((map_ops[i].flags & GNTMAP_contains_pte)) {
> +                     pte = (pte_t *) 
> (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> +                                     (map_ops[i].host_addr & ~PAGE_MASK));
> +                     mfn = pte_mfn(*pte);
> +             } else {
> +                     pgd_t *pgd;
> +                     pud_t *pud;
> +                     pmd_t *pmd;
> +                     unsigned long addr = map_ops[i].host_addr;
> +                     pgd = pgd_offset(current->active_mm, addr);
> +                     if (pgd_none(*pgd)) {
> +                             printk(KERN_WARNING "invalid pgd found, skip 
> address=%lx\n", addr);
> +                             continue;
> +                     }
> +                     pud = pud_offset(pgd, addr);
> +                     if (pud_none(*pud)) {
> +                             printk(KERN_WARNING "invalid pud found, skip 
> address=%lx\n", addr);
> +                             continue;
> +                     }
> +                     pmd = pmd_offset(pud, addr);
> +                     if (pmd_none(*pmd)) {
> +                             printk(KERN_WARNING "invalid pmd found, skip 
> address=%lx\n", addr);
> +                             continue;
> +                     }
> +                     pte = pte_offset_map(pmd, addr);
> +                     if (pte_none(*pte)) {
> +                             printk(KERN_WARNING "invalid pte found, skip 
> address=%lx\n", addr);
> +                             continue;
> +                     }
> +                     mfn = pte_mfn(*pte);
> +                     pte_unmap(pte);
> +             }
>               ret = m2p_add_override(mfn, pages[i]);
>               if (ret)
>                       return ret;

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

<Prev in Thread] Current Thread [Next in Thread>