Re: [PATCH v6 7/8] xen: mapcache: Add support for grant mappings
On Thu, 23 May 2024, Edgar E. Iglesias wrote:
> On Thu, May 23, 2024 at 9:47 AM Manos Pitsidianakis
> wrote:
> On Thu, 16 May 2024 18:48, "Edgar E. Iglesias"
> wrote:
> >From: "Edgar E. Iglesias"
> >
> >Add a second mapcache for grant mappings. The mapcache for
> >grants needs to work with XC_PAGE_SIZE granularity since
> >we can't map larger ranges than what has been granted to us.
> >
> >Like with foreign mappings (xen_memory), machines using grants
> >are expected to initialize the xen_grants MR and map it
> >into their address-map accordingly.
> >
> >Signed-off-by: Edgar E. Iglesias
> >Reviewed-by: Stefano Stabellini
> >---
> > hw/xen/xen-hvm-common.c | 12 ++-
> > hw/xen/xen-mapcache.c | 163 ++--
> > include/hw/xen/xen-hvm-common.h | 3 +
> > include/sysemu/xen.h | 7 ++
> > 4 files changed, 152 insertions(+), 33 deletions(-)
> >
> >diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
> >index a0a0252da0..b8ace1c368 100644
> >--- a/hw/xen/xen-hvm-common.c
> >+++ b/hw/xen/xen-hvm-common.c
> >@@ -10,12 +10,18 @@
> > #include "hw/boards.h"
> > #include "hw/xen/arch_hvm.h"
> >
> >-MemoryRegion xen_memory;
> >+MemoryRegion xen_memory, xen_grants;
> >
> >-/* Check for xen memory. */
> >+/* Check for any kind of xen memory, foreign mappings or grants. */
> > bool xen_mr_is_memory(MemoryRegion *mr)
> > {
> >- return mr == _memory;
> >+ return mr == _memory || mr == _grants;
> >+}
> >+
> >+/* Check specifically for grants. */
> >+bool xen_mr_is_grants(MemoryRegion *mr)
> >+{
> >+ return mr == _grants;
> > }
> >
> > void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion
> *mr,
> >diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> >index a07c47b0b1..1cbc2aeaa9 100644
> >--- a/hw/xen/xen-mapcache.c
> >+++ b/hw/xen/xen-mapcache.c
> >@@ -14,6 +14,7 @@
> >
> > #include
> >
> >+#include "hw/xen/xen-hvm-common.h"
> > #include "hw/xen/xen_native.h"
> > #include "qemu/bitmap.h"
> >
> >@@ -21,6 +22,8 @@
> > #include "sysemu/xen-mapcache.h"
> > #include "trace.h"
> >
> >+#include
> >+#include
> >
> > #if HOST_LONG_BITS == 32
> > # define MCACHE_MAX_SIZE (1UL<<31) /* 2GB Cap */
> >@@ -41,6 +44,7 @@ typedef struct MapCacheEntry {
> > unsigned long *valid_mapping;
> > uint32_t lock;
> > #define XEN_MAPCACHE_ENTRY_DUMMY (1 << 0)
> >+#define XEN_MAPCACHE_ENTRY_GRANT (1 << 1)
>
> Might we get more entry kinds in the future? (for example foreign maps).
> Maybe this could be an enum.
>
>
> Perhaps. Foreign mappings are already supported, this flag separates ordinary
> foreign mappings from grant foreign mappings.
> IMO, since this is not an external interface it's probably better to change
> it once we have a concrete use-case at hand.
>
>
> > uint8_t flags;
> > hwaddr size;
> > struct MapCacheEntry *next;
> >@@ -71,6 +75,8 @@ typedef struct MapCache {
> > } MapCache;
> >
> > static MapCache *mapcache;
> >+static MapCache *mapcache_grants;
> >+static xengnttab_handle *xen_region_gnttabdev;
> >
> > static inline void mapcache_lock(MapCache *mc)
> > {
> >@@ -131,6 +137,12 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
> void *opaque)
> > unsigned long max_mcache_size;
> > unsigned int bucket_shift;
> >
> >+ xen_region_gnttabdev = xengnttab_open(NULL, 0);
> >+ if (xen_region_gnttabdev == NULL) {
> >+ error_report("mapcache: Failed to open gnttab device");
> >+ exit(EXIT_FAILURE);
> >+ }
> >+
> > if (HOST_LONG_BITS == 32) {
> > bucket_shift = 16;
> > } else {
> >@@ -159,6 +171,15 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
> void *opaque)
> > mapcache = xen_map_cache_init_single(f, opaque,
> > bucket_shift,
> > max_mcache_size);
> >+
> >+ /*
> >+ * Grant mappings must use XC_PAGE_SIZE granularity since we can't
> >+ * map anything beyond the number of pages granted to us.
> >+ */
> >+ mapcache_grants = xen_map_cache_init_single(f, opaque,
> >+ XC_PAGE_SHIFT,
> >+ max_mcache_size);
> >+
> > setrlimit(RLIMIT_AS, _as);
> > }
> >
> >@@ -168,17
Re: [PATCH v6 7/8] xen: mapcache: Add support for grant mappings
On Thu, May 23, 2024 at 9:47 AM Manos Pitsidianakis <
manos.pitsidiana...@linaro.org> wrote:
> On Thu, 16 May 2024 18:48, "Edgar E. Iglesias"
> wrote:
> >From: "Edgar E. Iglesias"
> >
> >Add a second mapcache for grant mappings. The mapcache for
> >grants needs to work with XC_PAGE_SIZE granularity since
> >we can't map larger ranges than what has been granted to us.
> >
> >Like with foreign mappings (xen_memory), machines using grants
> >are expected to initialize the xen_grants MR and map it
> >into their address-map accordingly.
> >
> >Signed-off-by: Edgar E. Iglesias
> >Reviewed-by: Stefano Stabellini
> >---
> > hw/xen/xen-hvm-common.c | 12 ++-
> > hw/xen/xen-mapcache.c | 163 ++--
> > include/hw/xen/xen-hvm-common.h | 3 +
> > include/sysemu/xen.h| 7 ++
> > 4 files changed, 152 insertions(+), 33 deletions(-)
> >
> >diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
> >index a0a0252da0..b8ace1c368 100644
> >--- a/hw/xen/xen-hvm-common.c
> >+++ b/hw/xen/xen-hvm-common.c
> >@@ -10,12 +10,18 @@
> > #include "hw/boards.h"
> > #include "hw/xen/arch_hvm.h"
> >
> >-MemoryRegion xen_memory;
> >+MemoryRegion xen_memory, xen_grants;
> >
> >-/* Check for xen memory. */
> >+/* Check for any kind of xen memory, foreign mappings or grants. */
> > bool xen_mr_is_memory(MemoryRegion *mr)
> > {
> >-return mr == _memory;
> >+return mr == _memory || mr == _grants;
> >+}
> >+
> >+/* Check specifically for grants. */
> >+bool xen_mr_is_grants(MemoryRegion *mr)
> >+{
> >+return mr == _grants;
> > }
> >
> > void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion
> *mr,
> >diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> >index a07c47b0b1..1cbc2aeaa9 100644
> >--- a/hw/xen/xen-mapcache.c
> >+++ b/hw/xen/xen-mapcache.c
> >@@ -14,6 +14,7 @@
> >
> > #include
> >
> >+#include "hw/xen/xen-hvm-common.h"
> > #include "hw/xen/xen_native.h"
> > #include "qemu/bitmap.h"
> >
> >@@ -21,6 +22,8 @@
> > #include "sysemu/xen-mapcache.h"
> > #include "trace.h"
> >
> >+#include
> >+#include
> >
> > #if HOST_LONG_BITS == 32
> > # define MCACHE_MAX_SIZE (1UL<<31) /* 2GB Cap */
> >@@ -41,6 +44,7 @@ typedef struct MapCacheEntry {
> > unsigned long *valid_mapping;
> > uint32_t lock;
> > #define XEN_MAPCACHE_ENTRY_DUMMY (1 << 0)
> >+#define XEN_MAPCACHE_ENTRY_GRANT (1 << 1)
>
> Might we get more entry kinds in the future? (for example foreign maps).
> Maybe this could be an enum.
>
>
Perhaps. Foreign mappings are already supported, this flag separates
ordinary foreign mappings from grant foreign mappings.
IMO, since this is not an external interface it's probably better to change
it once we have a concrete use-case at hand.
> > uint8_t flags;
> > hwaddr size;
> > struct MapCacheEntry *next;
> >@@ -71,6 +75,8 @@ typedef struct MapCache {
> > } MapCache;
> >
> > static MapCache *mapcache;
> >+static MapCache *mapcache_grants;
> >+static xengnttab_handle *xen_region_gnttabdev;
> >
> > static inline void mapcache_lock(MapCache *mc)
> > {
> >@@ -131,6 +137,12 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
> void *opaque)
> > unsigned long max_mcache_size;
> > unsigned int bucket_shift;
> >
> >+xen_region_gnttabdev = xengnttab_open(NULL, 0);
> >+if (xen_region_gnttabdev == NULL) {
> >+error_report("mapcache: Failed to open gnttab device");
> >+exit(EXIT_FAILURE);
> >+}
> >+
> > if (HOST_LONG_BITS == 32) {
> > bucket_shift = 16;
> > } else {
> >@@ -159,6 +171,15 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
> void *opaque)
> > mapcache = xen_map_cache_init_single(f, opaque,
> > bucket_shift,
> > max_mcache_size);
> >+
> >+/*
> >+ * Grant mappings must use XC_PAGE_SIZE granularity since we can't
> >+ * map anything beyond the number of pages granted to us.
> >+ */
> >+mapcache_grants = xen_map_cache_init_single(f, opaque,
> >+XC_PAGE_SHIFT,
> >+max_mcache_size);
> >+
> > setrlimit(RLIMIT_AS, _as);
> > }
> >
> >@@ -168,17 +189,24 @@ static void xen_remap_bucket(MapCache *mc,
> > hwaddr size,
> > hwaddr address_index,
> > bool dummy,
> >+ bool grant,
> >+ bool is_write,
> > ram_addr_t ram_offset)
> > {
> > uint8_t *vaddr_base;
> >-xen_pfn_t *pfns;
> >+uint32_t *refs = NULL;
> >+xen_pfn_t *pfns = NULL;
> > int *err;
>
> You should use g_autofree to perform automatic cleanup on function exit
> instead of manually freeing, since the allocations should only live
> within the function call.
>
>
Sounds good, I'll do that in the next version.
> > unsigned
Re: [PATCH v6 7/8] xen: mapcache: Add support for grant mappings
On Thu, 16 May 2024 18:48, "Edgar E. Iglesias" wrote:
From: "Edgar E. Iglesias"
Add a second mapcache for grant mappings. The mapcache for
grants needs to work with XC_PAGE_SIZE granularity since
we can't map larger ranges than what has been granted to us.
Like with foreign mappings (xen_memory), machines using grants
are expected to initialize the xen_grants MR and map it
into their address-map accordingly.
Signed-off-by: Edgar E. Iglesias
Reviewed-by: Stefano Stabellini
---
hw/xen/xen-hvm-common.c | 12 ++-
hw/xen/xen-mapcache.c | 163 ++--
include/hw/xen/xen-hvm-common.h | 3 +
include/sysemu/xen.h| 7 ++
4 files changed, 152 insertions(+), 33 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index a0a0252da0..b8ace1c368 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -10,12 +10,18 @@
#include "hw/boards.h"
#include "hw/xen/arch_hvm.h"
-MemoryRegion xen_memory;
+MemoryRegion xen_memory, xen_grants;
-/* Check for xen memory. */
+/* Check for any kind of xen memory, foreign mappings or grants. */
bool xen_mr_is_memory(MemoryRegion *mr)
{
-return mr == _memory;
+return mr == _memory || mr == _grants;
+}
+
+/* Check specifically for grants. */
+bool xen_mr_is_grants(MemoryRegion *mr)
+{
+return mr == _grants;
}
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index a07c47b0b1..1cbc2aeaa9 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -14,6 +14,7 @@
#include
+#include "hw/xen/xen-hvm-common.h"
#include "hw/xen/xen_native.h"
#include "qemu/bitmap.h"
@@ -21,6 +22,8 @@
#include "sysemu/xen-mapcache.h"
#include "trace.h"
+#include
+#include
#if HOST_LONG_BITS == 32
# define MCACHE_MAX_SIZE (1UL<<31) /* 2GB Cap */
@@ -41,6 +44,7 @@ typedef struct MapCacheEntry {
unsigned long *valid_mapping;
uint32_t lock;
#define XEN_MAPCACHE_ENTRY_DUMMY (1 << 0)
+#define XEN_MAPCACHE_ENTRY_GRANT (1 << 1)
Might we get more entry kinds in the future? (for example foreign maps).
Maybe this could be an enum.
uint8_t flags;
hwaddr size;
struct MapCacheEntry *next;
@@ -71,6 +75,8 @@ typedef struct MapCache {
} MapCache;
static MapCache *mapcache;
+static MapCache *mapcache_grants;
+static xengnttab_handle *xen_region_gnttabdev;
static inline void mapcache_lock(MapCache *mc)
{
@@ -131,6 +137,12 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
unsigned long max_mcache_size;
unsigned int bucket_shift;
+xen_region_gnttabdev = xengnttab_open(NULL, 0);
+if (xen_region_gnttabdev == NULL) {
+error_report("mapcache: Failed to open gnttab device");
+exit(EXIT_FAILURE);
+}
+
if (HOST_LONG_BITS == 32) {
bucket_shift = 16;
} else {
@@ -159,6 +171,15 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
mapcache = xen_map_cache_init_single(f, opaque,
bucket_shift,
max_mcache_size);
+
+/*
+ * Grant mappings must use XC_PAGE_SIZE granularity since we can't
+ * map anything beyond the number of pages granted to us.
+ */
+mapcache_grants = xen_map_cache_init_single(f, opaque,
+XC_PAGE_SHIFT,
+max_mcache_size);
+
setrlimit(RLIMIT_AS, _as);
}
@@ -168,17 +189,24 @@ static void xen_remap_bucket(MapCache *mc,
hwaddr size,
hwaddr address_index,
bool dummy,
+ bool grant,
+ bool is_write,
ram_addr_t ram_offset)
{
uint8_t *vaddr_base;
-xen_pfn_t *pfns;
+uint32_t *refs = NULL;
+xen_pfn_t *pfns = NULL;
int *err;
You should use g_autofree to perform automatic cleanup on function exit
instead of manually freeing, since the allocations should only live
within the function call.
unsigned int i;
hwaddr nb_pfn = size >> XC_PAGE_SHIFT;
trace_xen_remap_bucket(address_index);
-pfns = g_new0(xen_pfn_t, nb_pfn);
+if (grant) {
+refs = g_new0(uint32_t, nb_pfn);
+} else {
+pfns = g_new0(xen_pfn_t, nb_pfn);
+}
err = g_new0(int, nb_pfn);
if (entry->vaddr_base != NULL) {
@@ -207,21 +235,51 @@ static void xen_remap_bucket(MapCache *mc,
g_free(entry->valid_mapping);
entry->valid_mapping = NULL;
-for (i = 0; i < nb_pfn; i++) {
-pfns[i] = (address_index << (mc->bucket_shift - XC_PAGE_SHIFT)) + i;
+if (grant) {
+hwaddr grant_base = address_index - (ram_offset >> XC_PAGE_SHIFT);
+
+for (i = 0; i < nb_pfn; i++) {
+refs[i] = grant_base + i;
+}
+} else {
+for (i = 0; i < nb_pfn; i++) {
+
[PATCH v6 7/8] xen: mapcache: Add support for grant mappings
From: "Edgar E. Iglesias"
Add a second mapcache for grant mappings. The mapcache for
grants needs to work with XC_PAGE_SIZE granularity since
we can't map larger ranges than what has been granted to us.
Like with foreign mappings (xen_memory), machines using grants
are expected to initialize the xen_grants MR and map it
into their address-map accordingly.
Signed-off-by: Edgar E. Iglesias
Reviewed-by: Stefano Stabellini
---
hw/xen/xen-hvm-common.c | 12 ++-
hw/xen/xen-mapcache.c | 163 ++--
include/hw/xen/xen-hvm-common.h | 3 +
include/sysemu/xen.h| 7 ++
4 files changed, 152 insertions(+), 33 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index a0a0252da0..b8ace1c368 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -10,12 +10,18 @@
#include "hw/boards.h"
#include "hw/xen/arch_hvm.h"
-MemoryRegion xen_memory;
+MemoryRegion xen_memory, xen_grants;
-/* Check for xen memory. */
+/* Check for any kind of xen memory, foreign mappings or grants. */
bool xen_mr_is_memory(MemoryRegion *mr)
{
-return mr == _memory;
+return mr == _memory || mr == _grants;
+}
+
+/* Check specifically for grants. */
+bool xen_mr_is_grants(MemoryRegion *mr)
+{
+return mr == _grants;
}
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index a07c47b0b1..1cbc2aeaa9 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -14,6 +14,7 @@
#include
+#include "hw/xen/xen-hvm-common.h"
#include "hw/xen/xen_native.h"
#include "qemu/bitmap.h"
@@ -21,6 +22,8 @@
#include "sysemu/xen-mapcache.h"
#include "trace.h"
+#include
+#include
#if HOST_LONG_BITS == 32
# define MCACHE_MAX_SIZE (1UL<<31) /* 2GB Cap */
@@ -41,6 +44,7 @@ typedef struct MapCacheEntry {
unsigned long *valid_mapping;
uint32_t lock;
#define XEN_MAPCACHE_ENTRY_DUMMY (1 << 0)
+#define XEN_MAPCACHE_ENTRY_GRANT (1 << 1)
uint8_t flags;
hwaddr size;
struct MapCacheEntry *next;
@@ -71,6 +75,8 @@ typedef struct MapCache {
} MapCache;
static MapCache *mapcache;
+static MapCache *mapcache_grants;
+static xengnttab_handle *xen_region_gnttabdev;
static inline void mapcache_lock(MapCache *mc)
{
@@ -131,6 +137,12 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
unsigned long max_mcache_size;
unsigned int bucket_shift;
+xen_region_gnttabdev = xengnttab_open(NULL, 0);
+if (xen_region_gnttabdev == NULL) {
+error_report("mapcache: Failed to open gnttab device");
+exit(EXIT_FAILURE);
+}
+
if (HOST_LONG_BITS == 32) {
bucket_shift = 16;
} else {
@@ -159,6 +171,15 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
mapcache = xen_map_cache_init_single(f, opaque,
bucket_shift,
max_mcache_size);
+
+/*
+ * Grant mappings must use XC_PAGE_SIZE granularity since we can't
+ * map anything beyond the number of pages granted to us.
+ */
+mapcache_grants = xen_map_cache_init_single(f, opaque,
+XC_PAGE_SHIFT,
+max_mcache_size);
+
setrlimit(RLIMIT_AS, _as);
}
@@ -168,17 +189,24 @@ static void xen_remap_bucket(MapCache *mc,
hwaddr size,
hwaddr address_index,
bool dummy,
+ bool grant,
+ bool is_write,
ram_addr_t ram_offset)
{
uint8_t *vaddr_base;
-xen_pfn_t *pfns;
+uint32_t *refs = NULL;
+xen_pfn_t *pfns = NULL;
int *err;
unsigned int i;
hwaddr nb_pfn = size >> XC_PAGE_SHIFT;
trace_xen_remap_bucket(address_index);
-pfns = g_new0(xen_pfn_t, nb_pfn);
+if (grant) {
+refs = g_new0(uint32_t, nb_pfn);
+} else {
+pfns = g_new0(xen_pfn_t, nb_pfn);
+}
err = g_new0(int, nb_pfn);
if (entry->vaddr_base != NULL) {
@@ -207,21 +235,51 @@ static void xen_remap_bucket(MapCache *mc,
g_free(entry->valid_mapping);
entry->valid_mapping = NULL;
-for (i = 0; i < nb_pfn; i++) {
-pfns[i] = (address_index << (mc->bucket_shift - XC_PAGE_SHIFT)) + i;
+if (grant) {
+hwaddr grant_base = address_index - (ram_offset >> XC_PAGE_SHIFT);
+
+for (i = 0; i < nb_pfn; i++) {
+refs[i] = grant_base + i;
+}
+} else {
+for (i = 0; i < nb_pfn; i++) {
+pfns[i] = (address_index << (mc->bucket_shift - XC_PAGE_SHIFT)) +
i;
+}
}
-/*
- * If the caller has requested the mapping at a specific address use
- * MAP_FIXED to make sure it's honored.
- */
+entry->flags &= ~XEN_MAPCACHE_ENTRY_GRANT;
+
