Re: [PATCH 10/10] mini-os: modify grant mappings to work in PVH mode

From: Samuel Thibault <samuel.thibault@ens-lyon.org>
To: Juergen Gross <jgross@suse.com>
Cc: minios-devel@lists.xenproject.org,
	xen-devel@lists.xenproject.org, wl@xen.org
Subject: Re: [PATCH 10/10] mini-os: modify grant mappings to work in PVH mode
Date: Sun, 12 Dec 2021 01:51:00 +0100	[thread overview]
Message-ID: <20211212005100.gld3gajiwiog54rv@begin> (raw)
In-Reply-To: <20211206072337.9517-11-jgross@suse.com>

Juergen Gross, le lun. 06 déc. 2021 08:23:37 +0100, a ecrit:
> For being able to use the grant mapping interface in PVH mode some
> changes are required, as the guest needs to specify a physical address
> in the hypercall interface.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
>  gntmap.c         | 81 ++++++++++++++++++++++++++++++++++--------------
>  include/gntmap.h |  1 +
>  2 files changed, 59 insertions(+), 23 deletions(-)
> 
> diff --git a/gntmap.c b/gntmap.c
> index 7ae8fe6..126b04f 100644
> --- a/gntmap.c
> +++ b/gntmap.c
> @@ -32,6 +32,7 @@
>  
>  #include <mini-os/os.h>
>  #include <mini-os/lib.h>
> +#include <mini-os/e820.h>
>  #include <mini-os/xmalloc.h>
>  #include <errno.h>
>  #include <xen/grant_table.h>
> @@ -97,11 +98,42 @@ gntmap_set_max_grants(struct gntmap *map, int count)
>      if (map->entries == NULL)
>          return -ENOMEM;
>  
> +#ifndef CONFIG_PARAVIRT
> +    map->start_pfn = e820_get_reserved_pfns(count);
> +#endif
> +
>      memset(map->entries, 0, sizeof(struct gntmap_entry) * count);
>      map->nentries = count;
>      return 0;
>  }
>  
> +static int
> +_gntmap_unmap_grant_ref(struct gntmap *map, int idx)
> +{
> +    struct gntmap_entry *entry = map->entries + idx;
> +    struct gnttab_unmap_grant_ref op;
> +    int rc;
> +
> +#ifdef CONFIG_PARAVIRT
> +    op.host_addr    = (uint64_t) entry->host_addr;
> +#else
> +    op.host_addr    = (uint64_t)(map->start_pfn + idx) << PAGE_SHIFT;
> +#endif
> +    op.dev_bus_addr = 0;
> +    op.handle       = entry->handle;
> +
> +    rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1);
> +    if (rc != 0 || op.status != GNTST_okay) {
> +        printk("GNTTABOP_unmap_grant_ref failed: "
> +               "returned %d, status %" PRId16 "\n",
> +               rc, op.status);
> +        return rc != 0 ? rc : op.status;
> +    }
> +
> +    entry->host_addr = 0;
> +    return 0;
> +}
> +
>  static int
>  _gntmap_map_grant_ref(struct gntmap *map, int idx,
>                        unsigned long host_addr,
> @@ -112,10 +144,17 @@ _gntmap_map_grant_ref(struct gntmap *map, int idx,
>      struct gntmap_entry *entry = map->entries + idx;
>      struct gnttab_map_grant_ref op;
>      int rc;
> +#ifndef CONFIG_PARAVIRT
> +    unsigned long pfn = map->start_pfn + idx;
> +#endif
>  
>      op.ref = (grant_ref_t) ref;
>      op.dom = (domid_t) domid;
> +#ifdef CONFIG_PARAVIRT
>      op.host_addr = (uint64_t) host_addr;
> +#else
> +    op.host_addr = (uint64_t)pfn << PAGE_SHIFT; 
> +#endif
>      op.flags = GNTMAP_host_map;
>      if (!writable)
>          op.flags |= GNTMAP_readonly;
> @@ -128,31 +167,18 @@ _gntmap_map_grant_ref(struct gntmap *map, int idx,
>          return rc != 0 ? rc : op.status;
>      }
>  
> -    entry->host_addr = host_addr;
> -    entry->handle = op.handle;
> -    return 0;
> -}
> -
> -static int
> -_gntmap_unmap_grant_ref(struct gntmap *map, int idx)
> -{
> -    struct gntmap_entry *entry = map->entries + idx;
> -    struct gnttab_unmap_grant_ref op;
> -    int rc;
> -
> -    op.host_addr    = (uint64_t) entry->host_addr;
> -    op.dev_bus_addr = 0;
> -    op.handle       = entry->handle;
> -
> -    rc = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1);
> -    if (rc != 0 || op.status != GNTST_okay) {
> -        printk("GNTTABOP_unmap_grant_ref failed: "
> -               "returned %d, status %" PRId16 "\n",
> -               rc, op.status);
> -        return rc != 0 ? rc : op.status;
> +#ifndef CONFIG_PARAVIRT
> +    rc = do_map_frames(host_addr, &pfn, 1, 0, 0, DOMID_SELF, NULL,
> +                       writable ? L1_PROT : L1_PROT_RO);
> +    if ( rc )
> +    {
> +        _gntmap_unmap_grant_ref(map, idx);
> +        return rc;
>      }
> +#endif
>  
> -    entry->host_addr = 0;
> +    entry->host_addr = host_addr;
> +    entry->handle = op.handle;
>      return 0;
>  }
>  
> @@ -165,6 +191,10 @@ gntmap_munmap(struct gntmap *map, unsigned long start_address, int count)
>      DEBUG("(map=%p, start_address=%lx, count=%d)",
>             map, start_address, count);
>  
> +#ifndef CONFIG_PARAVIRT
> +    unmap_frames(start_address, count);
> +#endif
> +
>      for (i = 0; i < count; i++) {
>          idx = gntmap_find_entry(map, start_address + PAGE_SIZE * i);
>          if (idx < 0) {
> @@ -242,6 +272,11 @@ gntmap_fini(struct gntmap *map)
>              (void) _gntmap_unmap_grant_ref(map, i);
>      }
>  
> +#ifndef CONFIG_PARAVIRT
> +    e820_put_reserved_pfns(map->start_pfn, map->nentries);
> +    map->start_pfn = 0;
> +#endif
> +
>      xfree(map->entries);
>      map->entries = NULL;
>      map->nentries = 0;
> diff --git a/include/gntmap.h b/include/gntmap.h
> index fde53f3..d3d7e88 100644
> --- a/include/gntmap.h
> +++ b/include/gntmap.h
> @@ -10,6 +10,7 @@
>  struct gntmap {
>      int nentries;
>      struct gntmap_entry *entries;
> +    unsigned long start_pfn;
>  };
>  
>  int
> -- 
> 2.26.2
> 

-- 
Samuel
 Yep. Moi j'ai un clavier à une touche. 
 Par contre, ma souris a 102 boutons, c'est pas toujours pratique.
 -+- OG in: Guide du Cabaliste Usenet - Le mulot contre attaque -+-