Stefano Stabellini
2012-Jun-22 16:08 UTC
[PATCH 0/5] xen/arm: dom1 PV console up and running
Hi all,
this patch series is based upon Ian''s "arm: boot a dom1 to
"Calibrating
delay loop" then hang"
(http://marc.info/?l=xen-devel&m=133856539418794)
and my "xen/arm: event channels and shared_info page"
(http://marc.info/?l=xen-devel&m=133796319005683) patches series.
It fixes few bugs and implements few missing pieces needed to get the
first PV console up and running. With this patch series applied I can
fully boot a Dom1 guest out of an initramfs and connect to its pv
console in dom0, using xenconsole.
Regarding the console and xenstore pfns, I have used the HVM way of
doing things (speaking in x86 gergo) because they fit naturally in our
scenario.
The corresponding Linux side patch series is going to be posted soon.
Stefano Stabellini (5):
xen/arm: implement do_hvm_op for ARM
xen/arm: gic and vgic fixes
xen/arm: disable the event optimization in the gic
libxc/arm: allocate xenstore and console pages
xcbuild: add console and xenstore support
tools/libxc/xc_dom_arm.c | 32 +++++++++++++++-------
tools/xcutils/Makefile | 4 +-
tools/xcutils/xcbuild.c | 58 +++++++++++++++++++++++++++++++++++++++-
xen/arch/arm/Makefile | 1 +
xen/arch/arm/gic.c | 48 ++-------------------------------
xen/arch/arm/hvm.c | 60 ++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/traps.c | 1 +
xen/arch/arm/vgic.c | 3 +-
xen/include/asm-arm/domain.h | 7 +++++
9 files changed, 154 insertions(+), 60 deletions(-)
Cheers,
Stefano
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/Makefile | 1 +
xen/arch/arm/hvm.c | 60 ++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/traps.c | 1 +
xen/include/asm-arm/domain.h | 7 +++++
4 files changed, 69 insertions(+), 0 deletions(-)
create mode 100644 xen/arch/arm/hvm.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 5a87ba6..634b620 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -26,6 +26,7 @@ obj-y += traps.o
obj-y += vgic.o
obj-y += vtimer.o
obj-y += vpl011.o
+obj-y += hvm.o
#obj-bin-y += ....o
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
new file mode 100644
index 0000000..c11378d
--- /dev/null
+++ b/xen/arch/arm/hvm.c
@@ -0,0 +1,60 @@
+#include <xen/config.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <xen/sched.h>
+
+#include <public/xen.h>
+#include <public/hvm/params.h>
+#include <public/hvm/hvm_op.h>
+
+#include <asm/hypercall.h>
+
+long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
+
+{
+ long rc = 0;
+
+ switch ( op )
+ {
+ case HVMOP_set_param:
+ case HVMOP_get_param:
+ {
+ struct xen_hvm_param a;
+ struct domain *d;
+
+ if ( copy_from_guest(&a, arg, 1) )
+ return -EFAULT;
+
+ if ( a.index >= HVM_NR_PARAMS )
+ return -EINVAL;
+
+ rc = rcu_lock_target_domain_by_id(a.domid, &d);
+ if ( rc != 0 )
+ return rc;
+
+ if ( op == HVMOP_set_param )
+ {
+ d->arch.hvm_domain.params[a.index] = a.value;
+ }
+ else
+ {
+ a.value = d->arch.hvm_domain.params[a.index];
+ rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+ }
+
+ rcu_unlock_domain(d);
+ break;
+ }
+
+ default:
+ {
+ printk("%s: Bad HVM op %ld.\n", __func__, op);
+ rc = -ENOSYS;
+ break;
+ }
+ }
+
+ return rc;
+}
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ec74298..3900545 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -430,6 +430,7 @@ static arm_hypercall_t *arm_hypercall_table[] = {
HYPERCALL(memory_op),
HYPERCALL(physdev_op),
HYPERCALL(sysctl),
+ HYPERCALL(hvm_op),
};
static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 2b14545..114a8f6 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -5,6 +5,7 @@
#include <xen/cache.h>
#include <asm/page.h>
#include <asm/p2m.h>
+#include <public/hvm/params.h>
/* Represents state corresponding to a block of 32 interrupts */
struct vgic_irq_rank {
@@ -28,9 +29,15 @@ struct pending_irq
struct list_head lr_queue;
};
+struct hvm_domain
+{
+ uint64_t params[HVM_NR_PARAMS];
+} __cacheline_aligned;
+
struct arch_domain
{
struct p2m_domain p2m;
+ struct hvm_domain hvm_domain;
struct {
/*
--
1.7.2.5
- the last argument of find_next_bit is the maximum number of bits, not
the maximum number of bytes;
- use list_for_each_entry_safe instead of list_for_each_entry in
gic_restore_pending_irqs because we are actually deleting entries in the
loop.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/gic.c | 8 ++++----
xen/arch/arm/vgic.c | 3 +--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 87713c1..8190f84 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -480,13 +480,13 @@ out:
static void gic_restore_pending_irqs(struct vcpu *v)
{
int i;
- struct pending_irq *p;
+ struct pending_irq *p, *t;
/* check for new pending irqs */
if ( list_empty(&v->arch.vgic.lr_pending) )
return;
- list_for_each_entry ( p, &v->arch.vgic.lr_pending, lr_queue )
+ list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue
)
{
i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
if ( i >= nr_lrs ) return;
@@ -593,7 +593,7 @@ static void events_maintenance(struct vcpu *v)
if (!already_pending && gic.event_mask != 0) {
spin_lock_irq(&gic.lock);
while ((i = find_next_bit((const long unsigned int *)
&gic.event_mask,
- sizeof(uint64_t), i)) < sizeof(uint64_t)) {
+ 64, i)) < 64) {
GICH[GICH_LR + i] = 0;
clear_bit(i, &gic.lr_mask);
@@ -615,7 +615,7 @@ static void maintenance_interrupt(int irq, void *dev_id,
struct cpu_user_regs *r
events_maintenance(v);
while ((i = find_next_bit((const long unsigned int *) &eisr,
- sizeof(eisr), i)) < sizeof(eisr)) {
+ 64, i)) < 64) {
struct pending_irq *p;
spin_lock_irq(&gic.lock);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 653e8e5..b383e01 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -355,8 +355,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int
n)
unsigned int irq;
int i = 0;
- while ( (i = find_next_bit((const long unsigned int *) &r,
- sizeof(uint32_t), i)) < sizeof(uint32_t) ) {
+ while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) <
32 ) {
irq = i + (32 * n);
p = irq_to_pending(v, irq);
if ( !list_empty(&p->inflight) )
--
1.7.2.5
Stefano Stabellini
2012-Jun-22 16:09 UTC
[PATCH 3/5] xen/arm: disable the event optimization in the gic
Currently we have an optimization in the GIC driver that allows us not
to request maintenance interrupts for Xen events. The basic idea is that
every time we are about to inject a new interrupt or event into a guest,
we check whether we can remove some old event irqs from one or more LRs.
We can do this because we know when a guest finishes processing event
notifications: it sets the evtchn_upcall_pending bit to 0.
However it is unsafe: the guest resets evtchn_upcall_pending before
EOI''ing the event irq, therefore a small window of time exists when Xen
could jump in and remove the event irq from the LR register before the
guest has EOI''ed it.
This is not a good practice according to the GIC manual.
Remove the optimization for now.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/gic.c | 42 ------------------------------------------
1 files changed, 0 insertions(+), 42 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 8190f84..c6cee4b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -37,7 +37,6 @@
+ (GIC_CR_OFFSET & 0xfff)))
#define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH) \
+ (GIC_HR_OFFSET & 0xfff)))
-static void events_maintenance(struct vcpu *v);
static void gic_restore_pending_irqs(struct vcpu *v);
/* Global state */
@@ -48,7 +47,6 @@ static struct {
unsigned int lines;
unsigned int cpus;
spinlock_t lock;
- uint64_t event_mask;
uint64_t lr_mask;
} gic;
@@ -62,7 +60,6 @@ void gic_save_state(struct vcpu *v)
for ( i=0; i<nr_lrs; i++)
v->arch.gic_lr[i] = GICH[GICH_LR + i];
v->arch.lr_mask = gic.lr_mask;
- v->arch.event_mask = gic.event_mask;
/* Disable until next VCPU scheduled */
GICH[GICH_HCR] = 0;
isb();
@@ -76,7 +73,6 @@ void gic_restore_state(struct vcpu *v)
return;
gic.lr_mask = v->arch.lr_mask;
- gic.event_mask = v->arch.event_mask;
for ( i=0; i<nr_lrs; i++)
GICH[GICH_LR + i] = v->arch.gic_lr[i];
GICH[GICH_HCR] = GICH_HCR_EN;
@@ -318,7 +314,6 @@ int __init gic_init(void)
gic_hyp_init();
gic.lr_mask = 0ULL;
- gic.event_mask = 0ULL;
spin_unlock(&gic.lock);
@@ -421,9 +416,6 @@ static inline void gic_set_lr(int lr, unsigned int
virtual_irq,
BUG_ON(lr > nr_lrs);
- if (virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK && nr_lrs > 1)
- maintenance_int = 0;
-
GICH[GICH_LR + lr] = state |
maintenance_int |
((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
@@ -436,11 +428,6 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int
virtual_irq,
int i;
struct pending_irq *iter, *n;
- if ( v->is_running )
- {
- events_maintenance(v);
- }
-
spin_lock_irq(&gic.lock);
if ( v->is_running &&
list_empty(&v->arch.vgic.lr_pending) )
@@ -448,8 +435,6 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int
virtual_irq,
i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
if (i < nr_lrs) {
set_bit(i, &gic.lr_mask);
- if ( virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK )
- set_bit(i, &gic.event_mask);
gic_set_lr(i, virtual_irq, state, priority);
goto out;
}
@@ -494,8 +479,6 @@ static void gic_restore_pending_irqs(struct vcpu *v)
gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
list_del_init(&p->lr_queue);
set_bit(i, &gic.lr_mask);
- if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK )
- set_bit(i, &gic.event_mask);
}
}
@@ -584,27 +567,6 @@ void gicv_setup(struct domain *d)
GIC_BASE_ADDRESS + GIC_VR_OFFSET);
}
-static void events_maintenance(struct vcpu *v)
-{
- int i = 0;
- int already_pending = test_bit(0,
- (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
-
- if (!already_pending && gic.event_mask != 0) {
- spin_lock_irq(&gic.lock);
- while ((i = find_next_bit((const long unsigned int *)
&gic.event_mask,
- 64, i)) < 64) {
-
- GICH[GICH_LR + i] = 0;
- clear_bit(i, &gic.lr_mask);
- clear_bit(i, &gic.event_mask);
-
- i++;
- }
- spin_unlock_irq(&gic.lock);
- }
-}
-
static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs
*regs)
{
int i = 0, virq;
@@ -612,8 +574,6 @@ static void maintenance_interrupt(int irq, void *dev_id,
struct cpu_user_regs *r
struct vcpu *v = current;
uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) <<
32);
- events_maintenance(v);
-
while ((i = find_next_bit((const long unsigned int *) &eisr,
64, i)) < 64) {
struct pending_irq *p;
@@ -629,8 +589,6 @@ static void maintenance_interrupt(int irq, void *dev_id,
struct cpu_user_regs *r
gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
list_del_init(&p->lr_queue);
set_bit(i, &gic.lr_mask);
- if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK )
- set_bit(i, &gic.event_mask);
} else {
gic_inject_irq_stop();
}
--
1.7.2.5
Stefano Stabellini
2012-Jun-22 16:09 UTC
[PATCH 4/5] libxc/arm: allocate xenstore and console pages
Allocate two additional pages at the end of the guest physical memory
for xenstore and console.
Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding
values.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
tools/libxc/xc_dom_arm.c | 32 ++++++++++++++++++++++----------
1 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index bb86139..df2eefe 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -25,6 +25,10 @@
#include "xg_private.h"
#include "xc_dom.h"
+#define NR_MAGIC_PAGES 2
+#define CONSOLE_PFN_OFFSET 0
+#define XENSTORE_PFN_OFFSET 1
+
/* ------------------------------------------------------------------------ */
/*
* arm guests are hybrid and start off with paging disabled, therefore no
@@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
static int alloc_magic_pages(struct xc_dom_image *dom)
{
DOMPRINTF_CALLED(dom->xch);
- /* XXX
- * dom->p2m_guest
- * dom->start_info_pfn
- * dom->xenstore_pfn
- * dom->console_pfn
- */
return 0;
}
@@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
{
int rc;
xen_pfn_t pfn, allocsz, i;
+ xen_pfn_t store_pfn, console_pfn;
fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase
%"PRI_xen_pfn"\n", __func__,
dom->total_pages, dom->rambase_pfn);
dom->shadow_enabled = 1;
- dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) *
dom->total_pages);
+ dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) *
(dom->total_pages + NR_MAGIC_PAGES));
fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for
%"PRI_xen_pfn" pages\n", __func__,
dom->rambase_pfn, dom->total_pages );
/* setup initial p2m */
- for ( pfn = 0; pfn < dom->total_pages; pfn++ )
+ for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ )
dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
fprintf(stderr, "%s: init''d p2m_host[0] =
%"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]);
@@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
/* allocate guest memory */
for ( i = rc = allocsz = 0;
- (i < dom->total_pages) && !rc;
+ (i < dom->total_pages + NR_MAGIC_PAGES) && !rc;
i += allocsz )
{
- allocsz = dom->total_pages - i;
+ allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;
if ( allocsz > 1024*1024 )
allocsz = 1024*1024;
fprintf(stderr, "alloc %"PRI_xen_pfn" at offset
%"PRI_xen_pfn" which is %"PRI_xen_pfn"\n",
@@ -168,6 +167,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
fprintf(stderr, "%s: popl''d
p2m_host[%"PRI_xen_pfn"] = %"PRI_xen_pfn"\n",
__func__, dom->total_pages-1,
dom->p2m_host[dom->total_pages-1]);
+ console_pfn = dom->rambase_pfn + dom->total_pages +
CONSOLE_PFN_OFFSET;
+ store_pfn = dom->rambase_pfn + dom->total_pages +
XENSTORE_PFN_OFFSET;
+
+ xc_clear_domain_page(dom->xch, dom->guest_domid, console_pfn);
+ xc_clear_domain_page(dom->xch, dom->guest_domid, store_pfn);
+ xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN,
+ console_pfn);
+ xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN,
+ store_pfn);
+
+ fprintf(stderr, "%s: console_pfn=%"PRI_xen_pfn"
xenstore_pfn=%"PRI_xen_pfn"\n",
+ __func__, console_pfn, store_pfn);
+
return 0;
}
--
1.7.2.5
Stefano Stabellini
2012-Jun-22 16:09 UTC
[PATCH 5/5] xcbuild: add console and xenstore support
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
tools/xcutils/Makefile | 4 +-
tools/xcutils/xcbuild.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 59 insertions(+), 3 deletions(-)
diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile
index 92c5a68..c88f286 100644
--- a/tools/xcutils/Makefile
+++ b/tools/xcutils/Makefile
@@ -20,7 +20,7 @@ CFLAGS_xc_save.o := $(CFLAGS_libxenctrl)
$(CFLAGS_libxenguest) $(CFLAGS_libxe
CFLAGS_readnotes.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
CFLAGS_lsevtchn.o := $(CFLAGS_libxenctrl)
CFLAGS_xcversion.o := $(CFLAGS_libxenctrl)
-CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
+CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
$(CFLAGS_libxenstore)
.PHONY: all
all: build
@@ -35,7 +35,7 @@ xc_save: xc_save.o
$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest)
$(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
xcbuild: xcbuild.o
- $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest)
$(APPEND_LDFLAGS)
+ $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest)
$(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
readnotes: readnotes.o
$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest)
$(APPEND_LDFLAGS)
diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c
index a70c3ca..7b5c0f8 100644
--- a/tools/xcutils/xcbuild.c
+++ b/tools/xcutils/xcbuild.c
@@ -1,6 +1,7 @@
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <errno.h>
@@ -8,6 +9,7 @@
//#include <xenguest.h>
#include <xentoollog.h>
#include <xc_dom.h>
+#include <xenstore.h>
int main(int argc, char **argv)
{
@@ -15,11 +17,19 @@ int main(int argc, char **argv)
xc_interface *xch;
int rv;
const char *image;
- uint32_t domid;
+ uint32_t domid = 1;
xen_domain_handle_t handle;
int maxmem = 128; /* MB */ //atoi(argv[2]);
int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */
struct xc_dom_image *dom;
+ unsigned long console_mfn;
+ unsigned long console_port;
+ unsigned long store_mfn;
+ unsigned long store_port;
+ struct xs_handle *xs;
+ char *dom_path;
+ char path[256];
+ char val[256];
image = (argc < 2) ? "guest.img" : argv[1];
printf("Image: %s\n", image);
@@ -103,6 +113,52 @@ int main(int argc, char **argv)
if (rv) return rv;
rv = xc_dom_build_image(dom);
if (rv) return rv;
+
+ xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn);
+ console_port = xc_evtchn_alloc_unbound(xch, domid, 0);
+ xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port);
+
+ xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn);
+ store_port = xc_evtchn_alloc_unbound(xch, domid, 0);
+ xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port);
+ xs = xs_daemon_open();
+ if (xs == NULL) {
+ printf("Could not contact XenStore");
+ return errno;
+ }
+ dom_path = xs_get_domain_path(xs, domid);
+
+ snprintf(path, sizeof(path), "%s/console/port", dom_path);
+ snprintf(val, sizeof(val), "%lu", console_port);
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+ snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path);
+ snprintf(val, sizeof(val), "%lu", console_mfn);
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+ snprintf(path, sizeof(path), "%s/console/type", dom_path);
+ snprintf(val, sizeof(val), "xenconsoled");
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+ snprintf(path, sizeof(path), "%s/console/output", dom_path);
+ snprintf(val, sizeof(val), "pty");
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+ snprintf(path, sizeof(path), "%s/console/limit", dom_path);
+ snprintf(val, sizeof(val), "%d", 1048576);
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+ snprintf(path, sizeof(path), "%s/store/port", dom_path);
+ snprintf(val, sizeof(val), "%lu", store_port);
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+ snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path);
+ snprintf(val, sizeof(val), "%lu", store_mfn);
+ xs_write(xs, XBT_NULL, path, val, strlen(val));
+ xs_introduce_domain(xs, domid, store_mfn, store_port);
+
+ xs_daemon_close(xs);
+
rv = xc_dom_boot_image(dom);
if (rv) return rv;
--
1.7.2.5
Ian Campbell
2012-Jun-26 13:50 UTC
Re: [PATCH 5/5] xcbuild: add console and xenstore support
Do you not have libxl and friends up and running sufficiently to use? This xcbuild was really just intended to tide us over until that stuff worked, not to be an actual thing... On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > tools/xcutils/Makefile | 4 +- > tools/xcutils/xcbuild.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 59 insertions(+), 3 deletions(-) > > diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile > index 92c5a68..c88f286 100644 > --- a/tools/xcutils/Makefile > +++ b/tools/xcutils/Makefile > @@ -20,7 +20,7 @@ CFLAGS_xc_save.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe > CFLAGS_readnotes.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > CFLAGS_lsevtchn.o := $(CFLAGS_libxenctrl) > CFLAGS_xcversion.o := $(CFLAGS_libxenctrl) > -CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > +CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) > > .PHONY: all > all: build > @@ -35,7 +35,7 @@ xc_save: xc_save.o > $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) > > xcbuild: xcbuild.o > - $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) > + $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) > > readnotes: readnotes.o > $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) > diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c > index a70c3ca..7b5c0f8 100644 > --- a/tools/xcutils/xcbuild.c > +++ b/tools/xcutils/xcbuild.c > @@ -1,6 +1,7 @@ > #include <unistd.h> > #include <stdio.h> > #include <stdlib.h> > +#include <string.h> > > #include <errno.h> > > @@ -8,6 +9,7 @@ > //#include <xenguest.h> > #include <xentoollog.h> > #include <xc_dom.h> > +#include <xenstore.h> > > int main(int argc, char **argv) > { > @@ -15,11 +17,19 @@ int main(int argc, char **argv) > xc_interface *xch; > int rv; > const char *image; > - uint32_t domid; > + uint32_t domid = 1; > xen_domain_handle_t handle; > int maxmem = 128; /* MB */ //atoi(argv[2]); > int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */ > struct xc_dom_image *dom; > + unsigned long console_mfn; > + unsigned long console_port; > + unsigned long store_mfn; > + unsigned long store_port; > + struct xs_handle *xs; > + char *dom_path; > + char path[256]; > + char val[256]; > > image = (argc < 2) ? "guest.img" : argv[1]; > printf("Image: %s\n", image); > @@ -103,6 +113,52 @@ int main(int argc, char **argv) > if (rv) return rv; > rv = xc_dom_build_image(dom); > if (rv) return rv; > + > + xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn); > + console_port = xc_evtchn_alloc_unbound(xch, domid, 0); > + xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port); > + > + xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn); > + store_port = xc_evtchn_alloc_unbound(xch, domid, 0); > + xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port); > + xs = xs_daemon_open(); > + if (xs == NULL) { > + printf("Could not contact XenStore"); > + return errno; > + } > + dom_path = xs_get_domain_path(xs, domid); > + > + snprintf(path, sizeof(path), "%s/console/port", dom_path); > + snprintf(val, sizeof(val), "%lu", console_port); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + > + snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path); > + snprintf(val, sizeof(val), "%lu", console_mfn); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + > + snprintf(path, sizeof(path), "%s/console/type", dom_path); > + snprintf(val, sizeof(val), "xenconsoled"); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + > + snprintf(path, sizeof(path), "%s/console/output", dom_path); > + snprintf(val, sizeof(val), "pty"); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + > + snprintf(path, sizeof(path), "%s/console/limit", dom_path); > + snprintf(val, sizeof(val), "%d", 1048576); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + > + snprintf(path, sizeof(path), "%s/store/port", dom_path); > + snprintf(val, sizeof(val), "%lu", store_port); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + > + snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path); > + snprintf(val, sizeof(val), "%lu", store_mfn); > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > + xs_introduce_domain(xs, domid, store_mfn, store_port); > + > + xs_daemon_close(xs); > + > rv = xc_dom_boot_image(dom); > if (rv) return rv; >
We need to device how hybrid our hybrid arm guests are going to be. The particular hvm params you are using here (evtchn port etc) typically live in start_info for a PV guest. In principal we could define a start info for ARM too but that leaves the question of how the guest can find it (which loops up back to hvm_params...). Looking at the other stuff in start_info, it has stuff like modules (aka ramdisks) and command lines which ARM guest get via the normal ARM boot protocol stuff (i.e. the domain builder does it) and a bunch of stuff which seems to only make sense for proper-PV guests. So maybe HVM PARAM is the right answer? sinfo does have flags which contains stuff like SIF_INITIAL_DOMAIN. Might we want that? On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/hvm.c | 60 ++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/traps.c | 1 + > xen/include/asm-arm/domain.h | 7 +++++ > 4 files changed, 69 insertions(+), 0 deletions(-) > create mode 100644 xen/arch/arm/hvm.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 5a87ba6..634b620 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -26,6 +26,7 @@ obj-y += traps.o > obj-y += vgic.o > obj-y += vtimer.o > obj-y += vpl011.o > +obj-y += hvm.o > > #obj-bin-y += ....o > > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > new file mode 100644 > index 0000000..c11378d > --- /dev/null > +++ b/xen/arch/arm/hvm.c > @@ -0,0 +1,60 @@ > +#include <xen/config.h> > +#include <xen/init.h> > +#include <xen/lib.h> > +#include <xen/errno.h> > +#include <xen/guest_access.h> > +#include <xen/sched.h> > + > +#include <public/xen.h> > +#include <public/hvm/params.h> > +#include <public/hvm/hvm_op.h> > + > +#include <asm/hypercall.h> > + > +long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg) > + > +{ > + long rc = 0; > + > + switch ( op ) > + { > + case HVMOP_set_param: > + case HVMOP_get_param: > + { > + struct xen_hvm_param a; > + struct domain *d; > + > + if ( copy_from_guest(&a, arg, 1) ) > + return -EFAULT; > + > + if ( a.index >= HVM_NR_PARAMS ) > + return -EINVAL; > + > + rc = rcu_lock_target_domain_by_id(a.domid, &d); > + if ( rc != 0 ) > + return rc; > + > + if ( op == HVMOP_set_param ) > + { > + d->arch.hvm_domain.params[a.index] = a.value; > + } > + else > + { > + a.value = d->arch.hvm_domain.params[a.index]; > + rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; > + } > + > + rcu_unlock_domain(d); > + break; > + } > + > + default: > + { > + printk("%s: Bad HVM op %ld.\n", __func__, op); > + rc = -ENOSYS; > + break; > + } > + } > + > + return rc; > +} > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index ec74298..3900545 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -430,6 +430,7 @@ static arm_hypercall_t *arm_hypercall_table[] = { > HYPERCALL(memory_op), > HYPERCALL(physdev_op), > HYPERCALL(sysctl), > + HYPERCALL(hvm_op), > }; > > static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 2b14545..114a8f6 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -5,6 +5,7 @@ > #include <xen/cache.h> > #include <asm/page.h> > #include <asm/p2m.h> > +#include <public/hvm/params.h> > > /* Represents state corresponding to a block of 32 interrupts */ > struct vgic_irq_rank { > @@ -28,9 +29,15 @@ struct pending_irq > struct list_head lr_queue; > }; > > +struct hvm_domain > +{ > + uint64_t params[HVM_NR_PARAMS]; > +} __cacheline_aligned; > + > struct arch_domain > { > struct p2m_domain p2m; > + struct hvm_domain hvm_domain; > > struct { > /*
On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:> - the last argument of find_next_bit is the maximum number of bits, not > the maximum number of bytes;I guess 8*sizeof(...) is pretty ugly too so just using the hardcoded numbers is acceptable in this case.> > - use list_for_each_entry_safe instead of list_for_each_entry in > gic_restore_pending_irqs because we are actually deleting entries in the > loop. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/arch/arm/gic.c | 8 ++++---- > xen/arch/arm/vgic.c | 3 +-- > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 87713c1..8190f84 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -480,13 +480,13 @@ out: > static void gic_restore_pending_irqs(struct vcpu *v) > { > int i; > - struct pending_irq *p; > + struct pending_irq *p, *t; > > /* check for new pending irqs */ > if ( list_empty(&v->arch.vgic.lr_pending) ) > return; > > - list_for_each_entry ( p, &v->arch.vgic.lr_pending, lr_queue ) > + list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue ) > { > i = find_first_zero_bit(&gic.lr_mask, nr_lrs); > if ( i >= nr_lrs ) return; > @@ -593,7 +593,7 @@ static void events_maintenance(struct vcpu *v) > if (!already_pending && gic.event_mask != 0) { > spin_lock_irq(&gic.lock); > while ((i = find_next_bit((const long unsigned int *) &gic.event_mask, > - sizeof(uint64_t), i)) < sizeof(uint64_t)) { > + 64, i)) < 64) { > > GICH[GICH_LR + i] = 0; > clear_bit(i, &gic.lr_mask); > @@ -615,7 +615,7 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r > events_maintenance(v); > > while ((i = find_next_bit((const long unsigned int *) &eisr, > - sizeof(eisr), i)) < sizeof(eisr)) { > + 64, i)) < 64) { > struct pending_irq *p; > > spin_lock_irq(&gic.lock); > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 653e8e5..b383e01 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -355,8 +355,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) > unsigned int irq; > int i = 0; > > - while ( (i = find_next_bit((const long unsigned int *) &r, > - sizeof(uint32_t), i)) < sizeof(uint32_t) ) { > + while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 ) { > irq = i + (32 * n); > p = irq_to_pending(v, irq); > if ( !list_empty(&p->inflight) )
Ian Campbell
2012-Jun-26 14:29 UTC
Re: [PATCH 3/5] xen/arm: disable the event optimization in the gic
On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:> Currently we have an optimization in the GIC driver that allows us not > to request maintenance interrupts for Xen events. The basic idea is that > every time we are about to inject a new interrupt or event into a guest, > we check whether we can remove some old event irqs from one or more LRs. > We can do this because we know when a guest finishes processing event > notifications: it sets the evtchn_upcall_pending bit to 0. > > However it is unsafe: the guest resets evtchn_upcall_pending before > EOI''ing the event irq, therefore a small window of time exists when Xen > could jump in and remove the event irq from the LR register before the > guest has EOI''ed it. > > This is not a good practice according to the GIC manual. > Remove the optimization for now. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > xen/arch/arm/gic.c | 42 ------------------------------------------ > 1 files changed, 0 insertions(+), 42 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 8190f84..c6cee4b 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -37,7 +37,6 @@ > + (GIC_CR_OFFSET & 0xfff))) > #define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH) \ > + (GIC_HR_OFFSET & 0xfff))) > -static void events_maintenance(struct vcpu *v); > static void gic_restore_pending_irqs(struct vcpu *v); > > /* Global state */ > @@ -48,7 +47,6 @@ static struct { > unsigned int lines; > unsigned int cpus; > spinlock_t lock; > - uint64_t event_mask; > uint64_t lr_mask; > } gic; > > @@ -62,7 +60,6 @@ void gic_save_state(struct vcpu *v) > for ( i=0; i<nr_lrs; i++) > v->arch.gic_lr[i] = GICH[GICH_LR + i]; > v->arch.lr_mask = gic.lr_mask; > - v->arch.event_mask = gic.event_mask; > /* Disable until next VCPU scheduled */ > GICH[GICH_HCR] = 0; > isb(); > @@ -76,7 +73,6 @@ void gic_restore_state(struct vcpu *v) > return; > > gic.lr_mask = v->arch.lr_mask; > - gic.event_mask = v->arch.event_mask; > for ( i=0; i<nr_lrs; i++) > GICH[GICH_LR + i] = v->arch.gic_lr[i]; > GICH[GICH_HCR] = GICH_HCR_EN; > @@ -318,7 +314,6 @@ int __init gic_init(void) > gic_hyp_init(); > > gic.lr_mask = 0ULL; > - gic.event_mask = 0ULL; > > spin_unlock(&gic.lock); > > @@ -421,9 +416,6 @@ static inline void gic_set_lr(int lr, unsigned int virtual_irq, > > BUG_ON(lr > nr_lrs); > > - if (virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK && nr_lrs > 1) > - maintenance_int = 0; > - > GICH[GICH_LR + lr] = state | > maintenance_int | > ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) | > @@ -436,11 +428,6 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq, > int i; > struct pending_irq *iter, *n; > > - if ( v->is_running ) > - { > - events_maintenance(v); > - } > - > spin_lock_irq(&gic.lock); > > if ( v->is_running && list_empty(&v->arch.vgic.lr_pending) ) > @@ -448,8 +435,6 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq, > i = find_first_zero_bit(&gic.lr_mask, nr_lrs); > if (i < nr_lrs) { > set_bit(i, &gic.lr_mask); > - if ( virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK ) > - set_bit(i, &gic.event_mask); > gic_set_lr(i, virtual_irq, state, priority); > goto out; > } > @@ -494,8 +479,6 @@ static void gic_restore_pending_irqs(struct vcpu *v) > gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority); > list_del_init(&p->lr_queue); > set_bit(i, &gic.lr_mask); > - if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK ) > - set_bit(i, &gic.event_mask); > } > > } > @@ -584,27 +567,6 @@ void gicv_setup(struct domain *d) > GIC_BASE_ADDRESS + GIC_VR_OFFSET); > } > > -static void events_maintenance(struct vcpu *v) > -{ > - int i = 0; > - int already_pending = test_bit(0, > - (unsigned long *)&vcpu_info(v, evtchn_upcall_pending)); > - > - if (!already_pending && gic.event_mask != 0) { > - spin_lock_irq(&gic.lock); > - while ((i = find_next_bit((const long unsigned int *) &gic.event_mask, > - 64, i)) < 64) { > - > - GICH[GICH_LR + i] = 0; > - clear_bit(i, &gic.lr_mask); > - clear_bit(i, &gic.event_mask); > - > - i++; > - } > - spin_unlock_irq(&gic.lock); > - } > -} > - > static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) > { > int i = 0, virq; > @@ -612,8 +574,6 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r > struct vcpu *v = current; > uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32); > > - events_maintenance(v); > - > while ((i = find_next_bit((const long unsigned int *) &eisr, > 64, i)) < 64) { > struct pending_irq *p; > @@ -629,8 +589,6 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r > gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority); > list_del_init(&p->lr_queue); > set_bit(i, &gic.lr_mask); > - if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK ) > - set_bit(i, &gic.event_mask); > } else { > gic_inject_irq_stop(); > }
Ian Campbell
2012-Jun-26 14:34 UTC
Re: [PATCH 4/5] libxc/arm: allocate xenstore and console pages
On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:> Allocate two additional pages at the end of the guest physical memory > for xenstore and console. > Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding > values. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > tools/libxc/xc_dom_arm.c | 32 ++++++++++++++++++++++---------- > 1 files changed, 22 insertions(+), 10 deletions(-) > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > index bb86139..df2eefe 100644 > --- a/tools/libxc/xc_dom_arm.c > +++ b/tools/libxc/xc_dom_arm.c > @@ -25,6 +25,10 @@ > #include "xg_private.h" > #include "xc_dom.h" > > +#define NR_MAGIC_PAGES 2 > +#define CONSOLE_PFN_OFFSET 0 > +#define XENSTORE_PFN_OFFSET 1 > + > /* ------------------------------------------------------------------------ */ > /* > * arm guests are hybrid and start off with paging disabled, therefore no > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom) > static int alloc_magic_pages(struct xc_dom_image *dom) > { > DOMPRINTF_CALLED(dom->xch); > - /* XXX > - * dom->p2m_guest > - * dom->start_info_pfn > - * dom->xenstore_pfn > - * dom->console_pfn > - */ > return 0; > } > > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom) > { > int rc; > xen_pfn_t pfn, allocsz, i; > + xen_pfn_t store_pfn, console_pfn; > > fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__, > dom->total_pages, dom->rambase_pfn); > > dom->shadow_enabled = 1; > > - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages); > + dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES)); > > fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__, > dom->rambase_pfn, dom->total_pages ); > /* setup initial p2m */ > - for ( pfn = 0; pfn < dom->total_pages; pfn++ ) > + for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ ) > dom->p2m_host[pfn] = pfn + dom->rambase_pfn; > > fprintf(stderr, "%s: init''d p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]); > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > /* allocate guest memory */ > for ( i = rc = allocsz = 0; > - (i < dom->total_pages) && !rc; > + (i < dom->total_pages + NR_MAGIC_PAGES) && !rc; > i += allocsz ) > { > - allocsz = dom->total_pages - i; > + allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;All these "+ NR_MAGIC_PAGES" are a bit troublesome looking. Do these pages need to be in p2m_host or would it be fine to just insert them into the guest p2m individually outside the main allocation logic? Otherwise perhaps simply int total_pages = dom->total_pages + NR... and use throughout?> if ( allocsz > 1024*1024 ) > allocsz = 1024*1024; > fprintf(stderr, "alloc %"PRI_xen_pfn" at offset %"PRI_xen_pfn" which is %"PRI_xen_pfn"\n", > @@ -168,6 +167,19 @@ int arch_setup_meminit(struct xc_dom_image *dom) > fprintf(stderr, "%s: popl''d p2m_host[%"PRI_xen_pfn"] = %"PRI_xen_pfn"\n", > __func__, dom->total_pages-1, dom->p2m_host[dom->total_pages-1]);I really need to scrub the debug printfs from my patch...> > + console_pfn = dom->rambase_pfn + dom->total_pages + CONSOLE_PFN_OFFSET; > + store_pfn = dom->rambase_pfn + dom->total_pages + XENSTORE_PFN_OFFSET; > + > + xc_clear_domain_page(dom->xch, dom->guest_domid, console_pfn); > + xc_clear_domain_page(dom->xch, dom->guest_domid, store_pfn); > + xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN, > + console_pfn); > + xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN, > + store_pfn); > + > + fprintf(stderr, "%s: console_pfn=%"PRI_xen_pfn" xenstore_pfn=%"PRI_xen_pfn"\n", > + __func__, console_pfn, store_pfn);... and so do you ;-)
On Tue, 26 Jun 2012, Ian Campbell wrote:> On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote: > > - the last argument of find_next_bit is the maximum number of bits, not > > the maximum number of bytes; > > I guess 8*sizeof(...) is pretty ugly too so just using the hardcoded > numbers is acceptable in this case.Yeah, that''s what I thought.
Stefano Stabellini
2012-Jun-26 17:58 UTC
Re: [PATCH 5/5] xcbuild: add console and xenstore support
On Tue, 26 Jun 2012, Ian Campbell wrote:> Do you not have libxl and friends up and running sufficiently to use? > This xcbuild was really just intended to tide us over until that stuff > worked, not to be an actual thing...xl/libxl works well enough for basic commands like "xl list", however in order to support something like "xl create" I expect that some more refactoring is needed as well as code motion to arch specific files. Considering that we are in code freeze right now, I thought it wouldn''t be acceptable for 4.2, so I didn''t even try.> On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote: > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > tools/xcutils/Makefile | 4 +- > > tools/xcutils/xcbuild.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++- > > 2 files changed, 59 insertions(+), 3 deletions(-) > > > > diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile > > index 92c5a68..c88f286 100644 > > --- a/tools/xcutils/Makefile > > +++ b/tools/xcutils/Makefile > > @@ -20,7 +20,7 @@ CFLAGS_xc_save.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe > > CFLAGS_readnotes.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > > CFLAGS_lsevtchn.o := $(CFLAGS_libxenctrl) > > CFLAGS_xcversion.o := $(CFLAGS_libxenctrl) > > -CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > > +CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) > > > > .PHONY: all > > all: build > > @@ -35,7 +35,7 @@ xc_save: xc_save.o > > $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) > > > > xcbuild: xcbuild.o > > - $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) > > + $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) > > > > readnotes: readnotes.o > > $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) > > diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c > > index a70c3ca..7b5c0f8 100644 > > --- a/tools/xcutils/xcbuild.c > > +++ b/tools/xcutils/xcbuild.c > > @@ -1,6 +1,7 @@ > > #include <unistd.h> > > #include <stdio.h> > > #include <stdlib.h> > > +#include <string.h> > > > > #include <errno.h> > > > > @@ -8,6 +9,7 @@ > > //#include <xenguest.h> > > #include <xentoollog.h> > > #include <xc_dom.h> > > +#include <xenstore.h> > > > > int main(int argc, char **argv) > > { > > @@ -15,11 +17,19 @@ int main(int argc, char **argv) > > xc_interface *xch; > > int rv; > > const char *image; > > - uint32_t domid; > > + uint32_t domid = 1; > > xen_domain_handle_t handle; > > int maxmem = 128; /* MB */ //atoi(argv[2]); > > int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */ > > struct xc_dom_image *dom; > > + unsigned long console_mfn; > > + unsigned long console_port; > > + unsigned long store_mfn; > > + unsigned long store_port; > > + struct xs_handle *xs; > > + char *dom_path; > > + char path[256]; > > + char val[256]; > > > > image = (argc < 2) ? "guest.img" : argv[1]; > > printf("Image: %s\n", image); > > @@ -103,6 +113,52 @@ int main(int argc, char **argv) > > if (rv) return rv; > > rv = xc_dom_build_image(dom); > > if (rv) return rv; > > + > > + xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn); > > + console_port = xc_evtchn_alloc_unbound(xch, domid, 0); > > + xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port); > > + > > + xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn); > > + store_port = xc_evtchn_alloc_unbound(xch, domid, 0); > > + xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port); > > + xs = xs_daemon_open(); > > + if (xs == NULL) { > > + printf("Could not contact XenStore"); > > + return errno; > > + } > > + dom_path = xs_get_domain_path(xs, domid); > > + > > + snprintf(path, sizeof(path), "%s/console/port", dom_path); > > + snprintf(val, sizeof(val), "%lu", console_port); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + > > + snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path); > > + snprintf(val, sizeof(val), "%lu", console_mfn); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + > > + snprintf(path, sizeof(path), "%s/console/type", dom_path); > > + snprintf(val, sizeof(val), "xenconsoled"); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + > > + snprintf(path, sizeof(path), "%s/console/output", dom_path); > > + snprintf(val, sizeof(val), "pty"); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + > > + snprintf(path, sizeof(path), "%s/console/limit", dom_path); > > + snprintf(val, sizeof(val), "%d", 1048576); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + > > + snprintf(path, sizeof(path), "%s/store/port", dom_path); > > + snprintf(val, sizeof(val), "%lu", store_port); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + > > + snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path); > > + snprintf(val, sizeof(val), "%lu", store_mfn); > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > + xs_introduce_domain(xs, domid, store_mfn, store_port); > > + > > + xs_daemon_close(xs); > > + > > rv = xc_dom_boot_image(dom); > > if (rv) return rv; > > > > >
Stefano Stabellini
2012-Jun-26 18:05 UTC
Re: [PATCH 4/5] libxc/arm: allocate xenstore and console pages
On Tue, 26 Jun 2012, Ian Campbell wrote:> On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote: > > Allocate two additional pages at the end of the guest physical memory > > for xenstore and console. > > Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding > > values. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > tools/libxc/xc_dom_arm.c | 32 ++++++++++++++++++++++---------- > > 1 files changed, 22 insertions(+), 10 deletions(-) > > > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > > index bb86139..df2eefe 100644 > > --- a/tools/libxc/xc_dom_arm.c > > +++ b/tools/libxc/xc_dom_arm.c > > @@ -25,6 +25,10 @@ > > #include "xg_private.h" > > #include "xc_dom.h" > > > > +#define NR_MAGIC_PAGES 2 > > +#define CONSOLE_PFN_OFFSET 0 > > +#define XENSTORE_PFN_OFFSET 1 > > + > > /* ------------------------------------------------------------------------ */ > > /* > > * arm guests are hybrid and start off with paging disabled, therefore no > > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom) > > static int alloc_magic_pages(struct xc_dom_image *dom) > > { > > DOMPRINTF_CALLED(dom->xch); > > - /* XXX > > - * dom->p2m_guest > > - * dom->start_info_pfn > > - * dom->xenstore_pfn > > - * dom->console_pfn > > - */ > > return 0; > > } > > > > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > { > > int rc; > > xen_pfn_t pfn, allocsz, i; > > + xen_pfn_t store_pfn, console_pfn; > > > > fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__, > > dom->total_pages, dom->rambase_pfn); > > > > dom->shadow_enabled = 1; > > > > - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages); > > + dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES)); > > > > fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__, > > dom->rambase_pfn, dom->total_pages ); > > /* setup initial p2m */ > > - for ( pfn = 0; pfn < dom->total_pages; pfn++ ) > > + for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ ) > > dom->p2m_host[pfn] = pfn + dom->rambase_pfn; > > > > fprintf(stderr, "%s: init''d p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]); > > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > > > /* allocate guest memory */ > > for ( i = rc = allocsz = 0; > > - (i < dom->total_pages) && !rc; > > + (i < dom->total_pages + NR_MAGIC_PAGES) && !rc; > > i += allocsz ) > > { > > - allocsz = dom->total_pages - i; > > + allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i; > > All these "+ NR_MAGIC_PAGES" are a bit troublesome looking. > > Do these pages need to be in p2m_host or would it be fine to just insert > them into the guest p2m individually outside the main allocation logic?I think it makes sense for them to be in p2m_host. In fact if we try to allocate them later, wouldn''t we have the problem of having to extend the guest p2m? We might as well do it here.> Otherwise perhaps simply int total_pages = dom->total_pages + NR... and > use throughout?Yes, I can do that.> > if ( allocsz > 1024*1024 ) > > allocsz = 1024*1024; > > fprintf(stderr, "alloc %"PRI_xen_pfn" at offset %"PRI_xen_pfn" which is %"PRI_xen_pfn"\n", > > @@ -168,6 +167,19 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > fprintf(stderr, "%s: popl''d p2m_host[%"PRI_xen_pfn"] = %"PRI_xen_pfn"\n", > > __func__, dom->total_pages-1, dom->p2m_host[dom->total_pages-1]); > > I really need to scrub the debug printfs from my patch... > > > > > + console_pfn = dom->rambase_pfn + dom->total_pages + CONSOLE_PFN_OFFSET; > > + store_pfn = dom->rambase_pfn + dom->total_pages + XENSTORE_PFN_OFFSET; > > + > > + xc_clear_domain_page(dom->xch, dom->guest_domid, console_pfn); > > + xc_clear_domain_page(dom->xch, dom->guest_domid, store_pfn); > > + xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN, > > + console_pfn); > > + xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN, > > + store_pfn); > > + > > + fprintf(stderr, "%s: console_pfn=%"PRI_xen_pfn" xenstore_pfn=%"PRI_xen_pfn"\n", > > + __func__, console_pfn, store_pfn); > > ... and so do you ;-)OK :)
Stefano Stabellini
2012-Jun-26 18:29 UTC
Re: [PATCH 1/5] xen/arm: implement do_hvm_op for ARM
On Tue, 26 Jun 2012, Ian Campbell wrote:> We need to device how hybrid our hybrid arm guests are going to be.Right.> The particular hvm params you are using here (evtchn port etc) typically > live in start_info for a PV guest. In principal we could define a start > info for ARM too but that leaves the question of how the guest can find > it (which loops up back to hvm_params...).One way would be to introduce a new XENMAPSPACE, like we have done for XENMAPSPACE_shared_info. Then the guest would use a XENMEM_add_to_physmap to map it.> Looking at the other stuff in start_info, it has stuff like modules (aka > ramdisks) and command lines which ARM guest get via the normal ARM boot > protocol stuff (i.e. the domain builder does it) and a bunch of stuff > which seems to only make sense for proper-PV guests. > > So maybe HVM PARAM is the right answer?Yes, that is one of the reasons why I preferred introducing hvm_op rather than a new XENMAPSPACE: we don''t need anything else from start_info aside from the parameters already provided through HVMOP_get_param. On the other hand hvm_op can be useful for other things, for example one day we might use the HVM_PARAM_IOREQ_PFN parameter. Most importantly, from the Linux side of things, acting like a PV on HVM kernel is a perfect fit, the changes required are down to a minimum.> sinfo does have flags which contains stuff like SIF_INITIAL_DOMAIN. > Might we want that?We don''t need it thanks to XENFEAT_dom0, see 1340381685-22529-3-git-send-email-stefano.stabellini@eu.citrix.com
Ian Campbell
2012-Jun-27 08:55 UTC
Re: [PATCH 5/5] xcbuild: add console and xenstore support
On Tue, 2012-06-26 at 18:58 +0100, Stefano Stabellini wrote:> On Tue, 26 Jun 2012, Ian Campbell wrote: > > Do you not have libxl and friends up and running sufficiently to use? > > This xcbuild was really just intended to tide us over until that stuff > > worked, not to be an actual thing... > > xl/libxl works well enough for basic commands like "xl list", however > in order to support something like "xl create" I expect that some more > refactoring is needed as well as code motion to arch specific files. > Considering that we are in code freeze right now, I thought it wouldn''t > be acceptable for 4.2, so I didn''t even try.Sounds reasonable. It might still be interesting to run xl create and see what actually breaks... I don''t plan to commit xcbuild, but I could maintain a dev branch somewhere with such code in it for people to pull in to their local tree, if that sounds useful?> > On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote: > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > --- > > > tools/xcutils/Makefile | 4 +- > > > tools/xcutils/xcbuild.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++- > > > 2 files changed, 59 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile > > > index 92c5a68..c88f286 100644 > > > --- a/tools/xcutils/Makefile > > > +++ b/tools/xcutils/Makefile > > > @@ -20,7 +20,7 @@ CFLAGS_xc_save.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe > > > CFLAGS_readnotes.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > > > CFLAGS_lsevtchn.o := $(CFLAGS_libxenctrl) > > > CFLAGS_xcversion.o := $(CFLAGS_libxenctrl) > > > -CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > > > +CFLAGS_xcbuild.o := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) > > > > > > .PHONY: all > > > all: build > > > @@ -35,7 +35,7 @@ xc_save: xc_save.o > > > $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) > > > > > > xcbuild: xcbuild.o > > > - $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) > > > + $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) > > > > > > readnotes: readnotes.o > > > $(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS) > > > diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c > > > index a70c3ca..7b5c0f8 100644 > > > --- a/tools/xcutils/xcbuild.c > > > +++ b/tools/xcutils/xcbuild.c > > > @@ -1,6 +1,7 @@ > > > #include <unistd.h> > > > #include <stdio.h> > > > #include <stdlib.h> > > > +#include <string.h> > > > > > > #include <errno.h> > > > > > > @@ -8,6 +9,7 @@ > > > //#include <xenguest.h> > > > #include <xentoollog.h> > > > #include <xc_dom.h> > > > +#include <xenstore.h> > > > > > > int main(int argc, char **argv) > > > { > > > @@ -15,11 +17,19 @@ int main(int argc, char **argv) > > > xc_interface *xch; > > > int rv; > > > const char *image; > > > - uint32_t domid; > > > + uint32_t domid = 1; > > > xen_domain_handle_t handle; > > > int maxmem = 128; /* MB */ //atoi(argv[2]); > > > int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */ > > > struct xc_dom_image *dom; > > > + unsigned long console_mfn; > > > + unsigned long console_port; > > > + unsigned long store_mfn; > > > + unsigned long store_port; > > > + struct xs_handle *xs; > > > + char *dom_path; > > > + char path[256]; > > > + char val[256]; > > > > > > image = (argc < 2) ? "guest.img" : argv[1]; > > > printf("Image: %s\n", image); > > > @@ -103,6 +113,52 @@ int main(int argc, char **argv) > > > if (rv) return rv; > > > rv = xc_dom_build_image(dom); > > > if (rv) return rv; > > > + > > > + xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn); > > > + console_port = xc_evtchn_alloc_unbound(xch, domid, 0); > > > + xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port); > > > + > > > + xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn); > > > + store_port = xc_evtchn_alloc_unbound(xch, domid, 0); > > > + xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port); > > > + xs = xs_daemon_open(); > > > + if (xs == NULL) { > > > + printf("Could not contact XenStore"); > > > + return errno; > > > + } > > > + dom_path = xs_get_domain_path(xs, domid); > > > + > > > + snprintf(path, sizeof(path), "%s/console/port", dom_path); > > > + snprintf(val, sizeof(val), "%lu", console_port); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + > > > + snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path); > > > + snprintf(val, sizeof(val), "%lu", console_mfn); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + > > > + snprintf(path, sizeof(path), "%s/console/type", dom_path); > > > + snprintf(val, sizeof(val), "xenconsoled"); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + > > > + snprintf(path, sizeof(path), "%s/console/output", dom_path); > > > + snprintf(val, sizeof(val), "pty"); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + > > > + snprintf(path, sizeof(path), "%s/console/limit", dom_path); > > > + snprintf(val, sizeof(val), "%d", 1048576); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + > > > + snprintf(path, sizeof(path), "%s/store/port", dom_path); > > > + snprintf(val, sizeof(val), "%lu", store_port); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + > > > + snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path); > > > + snprintf(val, sizeof(val), "%lu", store_mfn); > > > + xs_write(xs, XBT_NULL, path, val, strlen(val)); > > > + xs_introduce_domain(xs, domid, store_mfn, store_port); > > > + > > > + xs_daemon_close(xs); > > > + > > > rv = xc_dom_boot_image(dom); > > > if (rv) return rv; > > > > > > > > >
Ian Campbell
2012-Jun-27 08:59 UTC
Re: [PATCH 4/5] libxc/arm: allocate xenstore and console pages
On Tue, 2012-06-26 at 19:05 +0100, Stefano Stabellini wrote:> On Tue, 26 Jun 2012, Ian Campbell wrote: > > On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote: > > > Allocate two additional pages at the end of the guest physical memory > > > for xenstore and console. > > > Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding > > > values. > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > --- > > > tools/libxc/xc_dom_arm.c | 32 ++++++++++++++++++++++---------- > > > 1 files changed, 22 insertions(+), 10 deletions(-) > > > > > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > > > index bb86139..df2eefe 100644 > > > --- a/tools/libxc/xc_dom_arm.c > > > +++ b/tools/libxc/xc_dom_arm.c > > > @@ -25,6 +25,10 @@ > > > #include "xg_private.h" > > > #include "xc_dom.h" > > > > > > +#define NR_MAGIC_PAGES 2 > > > +#define CONSOLE_PFN_OFFSET 0 > > > +#define XENSTORE_PFN_OFFSET 1 > > > + > > > /* ------------------------------------------------------------------------ */ > > > /* > > > * arm guests are hybrid and start off with paging disabled, therefore no > > > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom) > > > static int alloc_magic_pages(struct xc_dom_image *dom) > > > { > > > DOMPRINTF_CALLED(dom->xch); > > > - /* XXX > > > - * dom->p2m_guest > > > - * dom->start_info_pfn > > > - * dom->xenstore_pfn > > > - * dom->console_pfn > > > - */ > > > return 0; > > > } > > > > > > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > > { > > > int rc; > > > xen_pfn_t pfn, allocsz, i; > > > + xen_pfn_t store_pfn, console_pfn; > > > > > > fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__, > > > dom->total_pages, dom->rambase_pfn); > > > > > > dom->shadow_enabled = 1; > > > > > > - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages); > > > + dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES)); > > > > > > fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__, > > > dom->rambase_pfn, dom->total_pages ); > > > /* setup initial p2m */ > > > - for ( pfn = 0; pfn < dom->total_pages; pfn++ ) > > > + for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ ) > > > dom->p2m_host[pfn] = pfn + dom->rambase_pfn; > > > > > > fprintf(stderr, "%s: init''d p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]); > > > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > > > > > /* allocate guest memory */ > > > for ( i = rc = allocsz = 0; > > > - (i < dom->total_pages) && !rc; > > > + (i < dom->total_pages + NR_MAGIC_PAGES) && !rc; > > > i += allocsz ) > > > { > > > - allocsz = dom->total_pages - i; > > > + allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i; > > > > All these "+ NR_MAGIC_PAGES" are a bit troublesome looking. > > > > Do these pages need to be in p2m_host or would it be fine to just insert > > them into the guest p2m individually outside the main allocation logic? > > I think it makes sense for them to be in p2m_host. In fact if we try to > allocate them later, wouldn''t we have the problem of having to extend > the guest p2m? We might as well do it here.The actual guest p2m is internal to the hypervisor so we never see it at the tools layer. I''m unsure if we need these magic pages in p2m_host. If we remember the gfn of the magic pages that''s just as useful as remembering the offset in p2m_host and using p2m_host[offset]? Ian.
On Tue, 2012-06-26 at 19:29 +0100, Stefano Stabellini wrote:> On Tue, 26 Jun 2012, Ian Campbell wrote: > > We need to device how hybrid our hybrid arm guests are going to be. > > Right. > > > > The particular hvm params you are using here (evtchn port etc) typically > > live in start_info for a PV guest. In principal we could define a start > > info for ARM too but that leaves the question of how the guest can find > > it (which loops up back to hvm_params...). > > One way would be to introduce a new XENMAPSPACE, like we have done for > XENMAPSPACE_shared_info. Then the guest would use a > XENMEM_add_to_physmap to map it. > > > > Looking at the other stuff in start_info, it has stuff like modules (aka > > ramdisks) and command lines which ARM guest get via the normal ARM boot > > protocol stuff (i.e. the domain builder does it) and a bunch of stuff > > which seems to only make sense for proper-PV guests. > > > > So maybe HVM PARAM is the right answer? > > Yes, that is one of the reasons why I preferred introducing hvm_op > rather than a new XENMAPSPACE: we don''t need anything else from > start_info aside from the parameters already provided through > HVMOP_get_param. > On the other hand hvm_op can be useful for other things, for example one > day we might use the HVM_PARAM_IOREQ_PFN parameter.That would be in future if/when we support proper "HVM" (or "PVHVM") ARM guests i.e. ones which appear to the guest like a particular physical platform, rather than our virtual hybrid platform, and therefore have a QEMU providing a DM for that hardware?> Most importantly, from the Linux side of things, acting like a PV on HVM > kernel is a perfect fit, the changes required are down to a minimum.OK.> > sinfo does have flags which contains stuff like SIF_INITIAL_DOMAIN. > > Might we want that? > > We don''t need it thanks to XENFEAT_dom0, see > 1340381685-22529-3-git-send-email-stefano.stabellini@eu.citrix.comGreat.
Stefano Stabellini
2012-Jun-27 11:02 UTC
Re: [PATCH 5/5] xcbuild: add console and xenstore support
On Wed, 27 Jun 2012, Ian Campbell wrote:> On Tue, 2012-06-26 at 18:58 +0100, Stefano Stabellini wrote: > > On Tue, 26 Jun 2012, Ian Campbell wrote: > > > Do you not have libxl and friends up and running sufficiently to use? > > > This xcbuild was really just intended to tide us over until that stuff > > > worked, not to be an actual thing... > > > > xl/libxl works well enough for basic commands like "xl list", however > > in order to support something like "xl create" I expect that some more > > refactoring is needed as well as code motion to arch specific files. > > Considering that we are in code freeze right now, I thought it wouldn''t > > be acceptable for 4.2, so I didn''t even try. > > Sounds reasonable. > > It might still be interesting to run xl create and see what actually > breaks... > > I don''t plan to commit xcbuild, but I could maintain a dev branch > somewhere with such code in it for people to pull in to their local > tree, if that sounds useful?Yep.
Stefano Stabellini
2012-Jun-27 11:04 UTC
Re: [PATCH 1/5] xen/arm: implement do_hvm_op for ARM
On Wed, 27 Jun 2012, Ian Campbell wrote:> On Tue, 2012-06-26 at 19:29 +0100, Stefano Stabellini wrote: > > On Tue, 26 Jun 2012, Ian Campbell wrote: > > > We need to device how hybrid our hybrid arm guests are going to be. > > > > Right. > > > > > > > The particular hvm params you are using here (evtchn port etc) typically > > > live in start_info for a PV guest. In principal we could define a start > > > info for ARM too but that leaves the question of how the guest can find > > > it (which loops up back to hvm_params...). > > > > One way would be to introduce a new XENMAPSPACE, like we have done for > > XENMAPSPACE_shared_info. Then the guest would use a > > XENMEM_add_to_physmap to map it. > > > > > > > Looking at the other stuff in start_info, it has stuff like modules (aka > > > ramdisks) and command lines which ARM guest get via the normal ARM boot > > > protocol stuff (i.e. the domain builder does it) and a bunch of stuff > > > which seems to only make sense for proper-PV guests. > > > > > > So maybe HVM PARAM is the right answer? > > > > Yes, that is one of the reasons why I preferred introducing hvm_op > > rather than a new XENMAPSPACE: we don''t need anything else from > > start_info aside from the parameters already provided through > > HVMOP_get_param. > > On the other hand hvm_op can be useful for other things, for example one > > day we might use the HVM_PARAM_IOREQ_PFN parameter. > > That would be in future if/when we support proper "HVM" (or "PVHVM") ARM > guests i.e. ones which appear to the guest like a particular physical > platform, rather than our virtual hybrid platform, and therefore have a > QEMU providing a DM for that hardware?Yes, that is what HVM_PARAM_IOREQ_PFN would be for.
Stefano Stabellini
2012-Jun-27 12:42 UTC
Re: [PATCH 4/5] libxc/arm: allocate xenstore and console pages
On Wed, 27 Jun 2012, Ian Campbell wrote:> On Tue, 2012-06-26 at 19:05 +0100, Stefano Stabellini wrote: > > On Tue, 26 Jun 2012, Ian Campbell wrote: > > > On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote: > > > > Allocate two additional pages at the end of the guest physical memory > > > > for xenstore and console. > > > > Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding > > > > values. > > > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > --- > > > > tools/libxc/xc_dom_arm.c | 32 ++++++++++++++++++++++---------- > > > > 1 files changed, 22 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > > > > index bb86139..df2eefe 100644 > > > > --- a/tools/libxc/xc_dom_arm.c > > > > +++ b/tools/libxc/xc_dom_arm.c > > > > @@ -25,6 +25,10 @@ > > > > #include "xg_private.h" > > > > #include "xc_dom.h" > > > > > > > > +#define NR_MAGIC_PAGES 2 > > > > +#define CONSOLE_PFN_OFFSET 0 > > > > +#define XENSTORE_PFN_OFFSET 1 > > > > + > > > > /* ------------------------------------------------------------------------ */ > > > > /* > > > > * arm guests are hybrid and start off with paging disabled, therefore no > > > > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom) > > > > static int alloc_magic_pages(struct xc_dom_image *dom) > > > > { > > > > DOMPRINTF_CALLED(dom->xch); > > > > - /* XXX > > > > - * dom->p2m_guest > > > > - * dom->start_info_pfn > > > > - * dom->xenstore_pfn > > > > - * dom->console_pfn > > > > - */ > > > > return 0; > > > > } > > > > > > > > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > > > { > > > > int rc; > > > > xen_pfn_t pfn, allocsz, i; > > > > + xen_pfn_t store_pfn, console_pfn; > > > > > > > > fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__, > > > > dom->total_pages, dom->rambase_pfn); > > > > > > > > dom->shadow_enabled = 1; > > > > > > > > - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages); > > > > + dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES)); > > > > > > > > fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__, > > > > dom->rambase_pfn, dom->total_pages ); > > > > /* setup initial p2m */ > > > > - for ( pfn = 0; pfn < dom->total_pages; pfn++ ) > > > > + for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ ) > > > > dom->p2m_host[pfn] = pfn + dom->rambase_pfn; > > > > > > > > fprintf(stderr, "%s: init''d p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]); > > > > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom) > > > > > > > > /* allocate guest memory */ > > > > for ( i = rc = allocsz = 0; > > > > - (i < dom->total_pages) && !rc; > > > > + (i < dom->total_pages + NR_MAGIC_PAGES) && !rc; > > > > i += allocsz ) > > > > { > > > > - allocsz = dom->total_pages - i; > > > > + allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i; > > > > > > All these "+ NR_MAGIC_PAGES" are a bit troublesome looking. > > > > > > Do these pages need to be in p2m_host or would it be fine to just insert > > > them into the guest p2m individually outside the main allocation logic? > > > > I think it makes sense for them to be in p2m_host. In fact if we try to > > allocate them later, wouldn''t we have the problem of having to extend > > the guest p2m? We might as well do it here. > > The actual guest p2m is internal to the hypervisor so we never see it at > the tools layer. > > I''m unsure if we need these magic pages in p2m_host. If we remember the > gfn of the magic pages that''s just as useful as remembering the offset > in p2m_host and using p2m_host[offset]?I think that you are right: it is better not to add them to p2m_host. --- libxc/arm: allocate xenstore and console pages Allocate two additional pages at the end of the guest physical memory for xenstore and console. Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding values. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c index bb86139..724e7ad 100644 --- a/tools/libxc/xc_dom_arm.c +++ b/tools/libxc/xc_dom_arm.c @@ -25,6 +25,10 @@ #include "xg_private.h" #include "xc_dom.h" +#define NR_MAGIC_PAGES 2 +#define CONSOLE_PFN_OFFSET 0 +#define XENSTORE_PFN_OFFSET 1 + /* ------------------------------------------------------------------------ */ /* * arm guests are hybrid and start off with paging disabled, therefore no @@ -46,13 +50,33 @@ static int setup_pgtables_arm(struct xc_dom_image *dom) static int alloc_magic_pages(struct xc_dom_image *dom) { + int rc, i, allocsz; + xen_pfn_t store_pfn, console_pfn, p2m[NR_MAGIC_PAGES]; + DOMPRINTF_CALLED(dom->xch); - /* XXX - * dom->p2m_guest - * dom->start_info_pfn - * dom->xenstore_pfn - * dom->console_pfn - */ + + for (i = 0; i < NR_MAGIC_PAGES; i++) + p2m[i] = dom->rambase_pfn + dom->total_pages + i; + + for ( i = rc = allocsz = 0; + (i < NR_MAGIC_PAGES) && !rc; + i += allocsz) { + allocsz = NR_MAGIC_PAGES - i; + rc = xc_domain_populate_physmap_exact( + dom->xch, dom->guest_domid, allocsz, + 0, 0, &p2m[i]); + } + + console_pfn = dom->rambase_pfn + dom->total_pages + CONSOLE_PFN_OFFSET; + store_pfn = dom->rambase_pfn + dom->total_pages + XENSTORE_PFN_OFFSET; + + xc_clear_domain_page(dom->xch, dom->guest_domid, console_pfn); + xc_clear_domain_page(dom->xch, dom->guest_domid, store_pfn); + xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN, + console_pfn); + xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN, + store_pfn); + return 0; }