Hi all,
this patch series implements the basic mechanisms needed by the grant
table to work properly.
With this patch series applied and the corresponding Linux side patch
series, I am able to boot a guest out of a PV disk.
Stefano Stabellini (4):
xen/arm: set paging_mode_translate and paging_mode_external
xen/arm: implement page reference and grant table functions needed by
grant_table.c
xen/arm: create_p2m_entries should not call free_domheap_page
xen/arm: grant table
xen/arch/arm/domain.c | 3 +
xen/arch/arm/dummy.S | 9 ---
xen/arch/arm/mm.c | 139 ++++++++++++++++++++++++++++++++++++++-
xen/arch/arm/p2m.c | 82 +++++++++++++++--------
xen/arch/arm/traps.c | 1 +
xen/include/asm-arm/paging.h | 4 +-
xen/include/public/hvm/params.h | 4 +-
7 files changed, 200 insertions(+), 42 deletions(-)
Cheers,
Stefano
Stefano Stabellini
2012-Jul-04 11:05 UTC
[PATCH 1/4] xen/arm: set paging_mode_translate and paging_mode_external
On ARM, given the kind of guests we support, it makes sense to set paging_mode_translate and paging_mode_external by default. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/include/asm-arm/paging.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/include/asm-arm/paging.h b/xen/include/asm-arm/paging.h index 3d7dd95..fe1f203 100644 --- a/xen/include/asm-arm/paging.h +++ b/xen/include/asm-arm/paging.h @@ -1,8 +1,8 @@ #ifndef _XEN_PAGING_H #define _XEN_PAGING_H -#define paging_mode_translate(d) (0) -#define paging_mode_external(d) (0) +#define paging_mode_translate(d) (1) +#define paging_mode_external(d) (1) #endif /* XEN_PAGING_H */ -- 1.7.2.5
Stefano Stabellini
2012-Jul-04 11:05 UTC
[PATCH 2/4] xen/arm: implement page reference and grant table functions needed by grant_table.c
The implementation is strongly "inspired" by their x86 counterparts,
except that we assume paging_mode_external and paging_mode_translate.
TODO: read_only mappings and gnttab_mark_dirty.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/dummy.S | 9 ----
xen/arch/arm/mm.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/p2m.c | 77 ++++++++++++++++++++++++-----------
3 files changed, 163 insertions(+), 33 deletions(-)
diff --git a/xen/arch/arm/dummy.S b/xen/arch/arm/dummy.S
index cab9522..baced25 100644
--- a/xen/arch/arm/dummy.S
+++ b/xen/arch/arm/dummy.S
@@ -23,18 +23,10 @@ DUMMY(arch_vcpu_reset);
NOP(update_vcpu_system_time);
/* Page Reference & Type Maintenance */
-DUMMY(get_page);
DUMMY(get_page_type);
-DUMMY(page_get_owner_and_reference);
-DUMMY(put_page);
DUMMY(put_page_type);
/* Grant Tables */
-DUMMY(create_grant_host_mapping);
-DUMMY(gnttab_clear_flag);
-DUMMY(gnttab_mark_dirty);
-DUMMY(is_iomem_page);
-DUMMY(replace_grant_host_mapping);
DUMMY(steal_page);
/* Page Offlining */
@@ -45,7 +37,6 @@ DUMMY(domain_get_maximum_gpfn);
DUMMY(domain_relinquish_resources);
DUMMY(domain_set_time_offset);
DUMMY(dom_cow);
-DUMMY(gmfn_to_mfn);
DUMMY(hypercall_create_continuation);
DUMMY(send_timer_event);
DUMMY(share_xen_page_with_privileged_guests);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 40ac176..ddf8c6c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -525,6 +525,116 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
return 0;
}
+
+struct domain *page_get_owner_and_reference(struct page_info *page)
+{
+ unsigned long x, y = page->count_info;
+
+ do {
+ x = y;
+ /*
+ * Count == 0: Page is not allocated, so we cannot take a reference.
+ * Count == -1: Reference count would wrap, which is invalid.
+ * Count == -2: Remaining unused ref is reserved for get_page_light().
+ */
+ if ( unlikely(((x + 2) & PGC_count_mask) <= 2) )
+ return NULL;
+ }
+ while ( (y = cmpxchg(&page->count_info, x, x + 1)) != x );
+
+ return page_get_owner(page);
+}
+
+void put_page(struct page_info *page)
+{
+ unsigned long nx, x, y = page->count_info;
+
+ do {
+ ASSERT((y & PGC_count_mask) != 0);
+ x = y;
+ nx = x - 1;
+ }
+ while ( unlikely((y = cmpxchg(&page->count_info, x, nx)) != x) );
+
+ if ( unlikely((nx & PGC_count_mask) == 0) )
+ {
+ free_domheap_page(page);
+ }
+}
+
+int get_page(struct page_info *page, struct domain *domain)
+{
+ struct domain *owner = page_get_owner_and_reference(page);
+
+ if ( likely(owner == domain) )
+ return 1;
+
+ if ( owner != NULL )
+ put_page(page);
+
+ return 0;
+}
+
+void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
+{
+ /*
+ * Note that this cannot be clear_bit(), as the access must be
+ * confined to the specified 2 bytes.
+ */
+ uint16_t mask = ~(1 << nr), old;
+
+ do {
+ old = *addr;
+ } while (cmpxchg(addr, old, old & mask) != old);
+}
+
+void gnttab_mark_dirty(struct domain *d, unsigned long l)
+{
+ /* XXX: mark dirty */
+}
+
+int create_grant_host_mapping(unsigned long addr, unsigned long frame,
+ unsigned int flags, unsigned int cache_flags)
+{
+ int rc;
+
+ if ( cache_flags || (flags & ~GNTMAP_readonly) != GNTMAP_host_map )
+ return GNTST_general_error;
+
+ /* XXX: read only mappings
+ if ( flags & GNTMAP_readonly )
+ p2mt = p2m_grant_map_ro;
+ else
+ p2mt = p2m_grant_map_rw;
+ */
+ rc = guest_physmap_add_page(current->domain,
+ addr >> PAGE_SHIFT, frame, 0);
+ if ( rc )
+ return GNTST_general_error;
+ else
+ return GNTST_okay;
+}
+
+int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
+ unsigned long new_addr, unsigned int flags)
+{
+ unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
+ struct domain *d = current->domain;
+
+ if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
+ return GNTST_general_error;
+
+ guest_physmap_remove_page(d, gfn, mfn, 0);
+
+ return GNTST_okay;
+}
+
+int is_iomem_page(unsigned long mfn)
+{
+ if ( !mfn_valid(mfn) )
+ return 1;
+ return 0;
+}
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 073216b..7c23b7d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -120,8 +120,14 @@ static int p2m_create_table(struct domain *d,
return 0;
}
+enum p2m_operation {
+ INSERT,
+ ALLOCATE,
+ REMOVE
+};
+
static int create_p2m_entries(struct domain *d,
- int alloc,
+ enum p2m_operation op,
paddr_t start_gpaddr,
paddr_t end_gpaddr,
paddr_t maddr,
@@ -191,25 +197,39 @@ static int create_p2m_entries(struct domain *d,
}
/* Allocate a new RAM page and attach */
- if (alloc)
- {
- struct page_info *page;
- lpae_t pte;
-
- rc = -ENOMEM;
- page = alloc_domheap_page(d, 0);
- if ( page == NULL ) {
- printk("p2m_populate_ram: failed to allocate
page\n");
- goto out;
- }
-
- pte = mfn_to_p2m_entry(page_to_mfn(page), mattr);
-
- write_pte(&third[third_table_offset(addr)], pte);
- } else {
- lpae_t pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT, mattr);
- write_pte(&third[third_table_offset(addr)], pte);
- maddr += PAGE_SIZE;
+ switch (op) {
+ case ALLOCATE:
+ {
+ struct page_info *page;
+ lpae_t pte;
+
+ rc = -ENOMEM;
+ page = alloc_domheap_page(d, 0);
+ if ( page == NULL ) {
+ printk("p2m_populate_ram: failed to allocate
page\n");
+ goto out;
+ }
+
+ pte = mfn_to_p2m_entry(page_to_mfn(page), mattr);
+
+ write_pte(&third[third_table_offset(addr)], pte);
+ }
+ break;
+ case INSERT:
+ {
+ lpae_t pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT,
mattr);
+ write_pte(&third[third_table_offset(addr)], pte);
+ maddr += PAGE_SIZE;
+ }
+ break;
+ case REMOVE:
+ {
+ lpae_t pte;
+ memset(&pte, 0x00, sizeof(pte));
+ write_pte(&third[third_table_offset(addr)], pte);
+ maddr += PAGE_SIZE;
+ }
+ break;
}
}
@@ -229,7 +249,7 @@ int p2m_populate_ram(struct domain *d,
paddr_t start,
paddr_t end)
{
- return create_p2m_entries(d, 1, start, end, 0, MATTR_MEM);
+ return create_p2m_entries(d, ALLOCATE, start, end, 0, MATTR_MEM);
}
int map_mmio_regions(struct domain *d,
@@ -237,7 +257,7 @@ int map_mmio_regions(struct domain *d,
paddr_t end_gaddr,
paddr_t maddr)
{
- return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr, MATTR_DEV);
+ return create_p2m_entries(d, INSERT, start_gaddr, end_gaddr, maddr,
MATTR_DEV);
}
int guest_physmap_add_page(struct domain *d,
@@ -245,7 +265,7 @@ int guest_physmap_add_page(struct domain *d,
unsigned long mfn,
unsigned int page_order)
{
- return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
+ return create_p2m_entries(d, INSERT, gpfn << PAGE_SHIFT,
(gpfn + (1<<page_order)) <<
PAGE_SHIFT,
mfn << PAGE_SHIFT, MATTR_MEM);
}
@@ -254,7 +274,9 @@ void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order)
{
- ASSERT(0);
+ create_p2m_entries(d, REMOVE, gpfn << PAGE_SHIFT,
+ (gpfn + (1<<page_order)) << PAGE_SHIFT,
+ mfn << PAGE_SHIFT, MATTR_MEM);
}
int p2m_alloc_table(struct domain *d)
@@ -318,6 +340,13 @@ int p2m_init(struct domain *d)
return 0;
}
+
+unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
+{
+ paddr_t p = p2m_lookup(d, gpfn << PAGE_SHIFT);
+ return p >> PAGE_SHIFT;
+}
+
/*
* Local variables:
* mode: C
--
1.7.2.5
Stefano Stabellini
2012-Jul-04 11:05 UTC
[PATCH 3/4] xen/arm: create_p2m_entries should not call free_domheap_page
The guest is entitled to use valid memory pages as pfns for grant_table
usage.
In these cases we shouldn''t call free_domheap_page to free the existing
page from create_p2m_entries, because it resets the reference counting
but the page is still allocated to the guest (even if not in the p2m
anymore) and common grant_table code is also going to call put_page on
it.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/p2m.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 7c23b7d..7ae4515 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -189,12 +189,7 @@ static int create_p2m_entries(struct domain *d,
/* else: third already valid */
if ( third[third_table_offset(addr)].p2m.valid )
- {
- /* p2m entry already present */
- free_domheap_page(
- mfn_to_page(third[third_table_offset(addr)].p2m.base));
flush_tlb_all_local();
- }
/* Allocate a new RAM page and attach */
switch (op) {
--
1.7.2.5
Implement XENMAPSPACE_grant_table and grant_table_op.
Introduce an HVM_PARAM to tell the guest where to map the grant table
(HVM_PARAM_GRANT_START_PFN), similarly to what we do with
HVM_PARAM_CONSOLE_PFN and HVM_PARAM_STORE_PFN.
However HVM_PARAM_GRANT_START_PFN is also going to be used by dom0, so
we set the parameter in Xen rather than libxc.
Using HVM_PARAM_GRANT_START_PFN removes the need for a platform pci
device.
Statically set HVM_PARAM_GRANT_START_PFN to 0xb0000000 for now.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/domain.c | 3 +++
xen/arch/arm/mm.c | 29 ++++++++++++++++++++++++++++-
xen/arch/arm/traps.c | 1 +
xen/include/public/hvm/params.h | 4 +++-
4 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ac6a30d..f33f6f1 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -341,6 +341,9 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags)
if ( (rc = domain_vgic_init(d)) != 0 )
goto fail;
+ /* XXX: select this dynamically */
+ d->arch.hvm_domain.params[HVM_PARAM_GRANT_START_PFN] = 0xb0000;
+
/* Domain 0 gets a real UART not an emulated one */
if ( d->domain_id && (rc = domain_uart0_init(d)) != 0 )
goto fail;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index ddf8c6c..43bbfef 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -25,6 +25,7 @@
#include <xen/mm.h>
#include <xen/preempt.h>
#include <xen/errno.h>
+#include <xen/grant_table.h>
#include <xen/guest_access.h>
#include <xen/domain_page.h>
#include <asm/page.h>
@@ -465,7 +466,7 @@ static int xenmem_add_to_physmap_once(
struct domain *d,
const struct xen_add_to_physmap *xatp)
{
- unsigned long mfn = 0;
+ unsigned long mfn = 0, idx = 0;
int rc;
switch ( xatp->space )
@@ -474,6 +475,32 @@ static int xenmem_add_to_physmap_once(
if ( xatp->idx == 0 )
mfn = virt_to_mfn(d->shared_info);
break;
+ case XENMAPSPACE_grant_table:
+ spin_lock(&d->grant_table->lock);
+
+ if ( d->grant_table->gt_version == 0 )
+ d->grant_table->gt_version = 1;
+
+ idx = xatp->idx;
+ if ( d->grant_table->gt_version == 2 &&
+ (xatp->idx & XENMAPIDX_grant_table_status) )
+ {
+ idx &= ~XENMAPIDX_grant_table_status;
+ if ( idx < nr_status_frames(d->grant_table) )
+ mfn = virt_to_mfn(d->grant_table->status[idx]);
+ }
+ else
+ {
+ if ( (idx >= nr_grant_frames(d->grant_table)) &&
+ (idx < max_nr_grant_frames) )
+ gnttab_grow_table(d, idx + 1);
+
+ if ( idx < nr_grant_frames(d->grant_table) )
+ mfn = virt_to_mfn(d->grant_table->shared_raw[idx]);
+ }
+
+ spin_unlock(&d->grant_table->lock);
+ break;
default:
return -ENOSYS;
}
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6e6807..3975878 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -431,6 +431,7 @@ static arm_hypercall_t *arm_hypercall_table[] = {
HYPERCALL(physdev_op),
HYPERCALL(sysctl),
HYPERCALL(hvm_op),
+ HYPERCALL(grant_table_op),
};
static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 55c1b57..a2da05d 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -147,6 +147,8 @@
#define HVM_PARAM_ACCESS_RING_PFN 28
#define HVM_PARAM_SHARING_RING_PFN 29
-#define HVM_NR_PARAMS 30
+#define HVM_PARAM_GRANT_START_PFN 30
+
+#define HVM_NR_PARAMS 31
#endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
--
1.7.2.5
Tim Deegan
2012-Jul-12 11:47 UTC
Re: [PATCH 1/4] xen/arm: set paging_mode_translate and paging_mode_external
At 12:05 +0100 on 04 Jul (1341403531), Stefano Stabellini wrote:> On ARM, given the kind of guests we support, it makes sense to set > paging_mode_translate and paging_mode_external by default. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Acked-by: Tim Deegan <tim@xen.org>
Tim Deegan
2012-Jul-12 12:06 UTC
Re: [PATCH 2/4] xen/arm: implement page reference and grant table functions needed by grant_table.c
At 12:05 +0100 on 04 Jul (1341403532), Stefano Stabellini wrote:> The implementation is strongly "inspired" by their x86 counterparts, > except that we assume paging_mode_external and paging_mode_translate. > > TODO: read_only mappings and gnttab_mark_dirty.Do we actually need these refcounts on ARM? Or was it just the typecount we thought we could do without?> +struct domain *page_get_owner_and_reference(struct page_info *page) > +{ > + unsigned long x, y = page->count_info; > + > + do { > + x = y; > + /* > + * Count == 0: Page is not allocated, so we cannot take a reference. > + * Count == -1: Reference count would wrap, which is invalid. > + * Count == -2: Remaining unused ref is reserved for get_page_light(). > + */ > + if ( unlikely(((x + 2) & PGC_count_mask) <= 2) ) > + return NULL; > + } > + while ( (y = cmpxchg(&page->count_info, x, x + 1)) != x );get_page_light() is an x86-ism, so we don''t need to leave room for it here.> +void gnttab_clear_flag(unsigned long nr, uint16_t *addr) > +{ > + /* > + * Note that this cannot be clear_bit(), as the access must be > + * confined to the specified 2 bytes. > + */ > + uint16_t mask = ~(1 << nr), old; > + > + do { > + old = *addr;A hard tab has snuck in here.> + } while (cmpxchg(addr, old, old & mask) != old);This could be done more efficiently with the underlying LRREXH/STREXH instructions but maybe not worth it? Is this on any hot paths?> +} > + > +void gnttab_mark_dirty(struct domain *d, unsigned long l) > +{ > + /* XXX: mark dirty */ > +} > + > +int create_grant_host_mapping(unsigned long addr, unsigned long frame, > + unsigned int flags, unsigned int cache_flags) > +{ > + int rc; > + > + if ( cache_flags || (flags & ~GNTMAP_readonly) != GNTMAP_host_map ) > + return GNTST_general_error; > + > + /* XXX: read only mappings > + if ( flags & GNTMAP_readonly ) > + p2mt = p2m_grant_map_ro; > + else > + p2mt = p2m_grant_map_rw; > + */ > + rc = guest_physmap_add_page(current->domain, > + addr >> PAGE_SHIFT, frame, 0);I''m a little worried about taking this in, in case we never get round to making the read-only grants read-only. Could we get away with making that an error case or are they needed right now? Cheers, Tim.
Tim Deegan
2012-Jul-12 12:09 UTC
Re: [PATCH 3/4] xen/arm: create_p2m_entries should not call free_domheap_page
At 12:05 +0100 on 04 Jul (1341403533), Stefano Stabellini wrote:> The guest is entitled to use valid memory pages as pfns for grant_table > usage. > > In these cases we shouldn''t call free_domheap_page to free the existing > page from create_p2m_entries, because it resets the reference counting > but the page is still allocated to the guest (even if not in the p2m > anymore) and common grant_table code is also going to call put_page on > it.Does this break other users? I guess this answers my earlier question about needing refcounts, anyway -- we''ll need to use them everywhere to keep the grant-table code consistent. Cheers, Tim.> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/arch/arm/p2m.c | 5 ----- > 1 files changed, 0 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 7c23b7d..7ae4515 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -189,12 +189,7 @@ static int create_p2m_entries(struct domain *d, > /* else: third already valid */ > > if ( third[third_table_offset(addr)].p2m.valid ) > - { > - /* p2m entry already present */ > - free_domheap_page( > - mfn_to_page(third[third_table_offset(addr)].p2m.base)); > flush_tlb_all_local(); > - } > > /* Allocate a new RAM page and attach */ > switch (op) { > -- > 1.7.2.5 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
At 12:05 +0100 on 04 Jul (1341403534), Stefano Stabellini wrote:> Implement XENMAPSPACE_grant_table and grant_table_op. > > Introduce an HVM_PARAM to tell the guest where to map the grant table > (HVM_PARAM_GRANT_START_PFN), similarly to what we do with > HVM_PARAM_CONSOLE_PFN and HVM_PARAM_STORE_PFN. > However HVM_PARAM_GRANT_START_PFN is also going to be used by dom0, so > we set the parameter in Xen rather than libxc. > Using HVM_PARAM_GRANT_START_PFN removes the need for a platform pci > device. > > Statically set HVM_PARAM_GRANT_START_PFN to 0xb0000000 for now. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Acked-by: Tim Deegan <tim@xen.org>
Stefano Stabellini
2012-Jul-13 15:45 UTC
Re: [PATCH 3/4] xen/arm: create_p2m_entries should not call free_domheap_page
On Thu, 12 Jul 2012, Tim Deegan wrote:> At 12:05 +0100 on 04 Jul (1341403533), Stefano Stabellini wrote: > > The guest is entitled to use valid memory pages as pfns for grant_table > > usage. > > > > In these cases we shouldn''t call free_domheap_page to free the existing > > page from create_p2m_entries, because it resets the reference counting > > but the page is still allocated to the guest (even if not in the p2m > > anymore) and common grant_table code is also going to call put_page on > > it. > > Does this break other users?I don''t think so, unless we start calling map_mmio_regions passing valid memory addresses in construct_dom0.> I guess this answers my earlier question > about needing refcounts, anyway -- we''ll need to use them everywhere > to keep the grant-table code consistent.Yeah
Stefano Stabellini
2012-Jul-13 16:16 UTC
Re: [PATCH 2/4] xen/arm: implement page reference and grant table functions needed by grant_table.c
On Thu, 12 Jul 2012, Tim Deegan wrote:> At 12:05 +0100 on 04 Jul (1341403532), Stefano Stabellini wrote: > > The implementation is strongly "inspired" by their x86 counterparts, > > except that we assume paging_mode_external and paging_mode_translate. > > > > TODO: read_only mappings and gnttab_mark_dirty. > > Do we actually need these refcounts on ARM? Or was it just the > typecount we thought we could do without?Nope, we need them for the grant_table.> > +struct domain *page_get_owner_and_reference(struct page_info *page) > > +{ > > + unsigned long x, y = page->count_info; > > + > > + do { > > + x = y; > > + /* > > + * Count == 0: Page is not allocated, so we cannot take a reference. > > + * Count == -1: Reference count would wrap, which is invalid. > > + * Count == -2: Remaining unused ref is reserved for get_page_light(). > > + */ > > + if ( unlikely(((x + 2) & PGC_count_mask) <= 2) ) > > + return NULL; > > + } > > + while ( (y = cmpxchg(&page->count_info, x, x + 1)) != x ); > > get_page_light() is an x86-ism, so we don''t need to leave room for it here.I''ll remove it> > +void gnttab_clear_flag(unsigned long nr, uint16_t *addr) > > +{ > > + /* > > + * Note that this cannot be clear_bit(), as the access must be > > + * confined to the specified 2 bytes. > > + */ > > + uint16_t mask = ~(1 << nr), old; > > + > > + do { > > + old = *addr; > > A hard tab has snuck in here.I''ll fix> > + } while (cmpxchg(addr, old, old & mask) != old); > > This could be done more efficiently with the underlying LRREXH/STREXH > instructions but maybe not worth it? Is this on any hot paths?It is called whem mapping/unmapping grant references, I don''t think it counts as hot path.> > +} > > + > > +void gnttab_mark_dirty(struct domain *d, unsigned long l) > > +{ > > + /* XXX: mark dirty */ > > +} > > + > > +int create_grant_host_mapping(unsigned long addr, unsigned long frame, > > + unsigned int flags, unsigned int cache_flags) > > +{ > > + int rc; > > + > > + if ( cache_flags || (flags & ~GNTMAP_readonly) != GNTMAP_host_map ) > > + return GNTST_general_error; > > + > > + /* XXX: read only mappings > > + if ( flags & GNTMAP_readonly ) > > + p2mt = p2m_grant_map_ro; > > + else > > + p2mt = p2m_grant_map_rw; > > + */ > > + rc = guest_physmap_add_page(current->domain, > > + addr >> PAGE_SHIFT, frame, 0); > > I''m a little worried about taking this in, in case we never get round to > making the read-only grants read-only. Could we get away with making > that an error case or are they needed right now?Good idea, I''ll return an error.