[XenPPC] [PATCH 1/2][2nd try] xen: remove start_info_t from dom0 building
Updated:
- pass in shared_info guest physical to ofd_dom0_fixup()
- only reserve shared info page in dom0 devtree
- kill rma_addr()
- kill RMA_* defines in public header
- update xc_linux_build() to use last page of RMA as shared_info
- update xc_linux_build() to choose a console/store page
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
[EMAIL PROTECTED]
diffstat output:
tools/libxc/powerpc64/xc_linux_build.c |8 ++---
xen/arch/powerpc/domain_build.c| 47 ++---
xen/arch/powerpc/mm.c |4 +-
xen/arch/powerpc/ofd_fixup.c | 27 +++---
xen/arch/powerpc/oftree.h |3 +-
xen/include/asm-powerpc/domain.h |5 ---
xen/include/public/arch-powerpc.h |8 -
7 files changed, 39 insertions(+), 63 deletions(-)
Signed-off-by: Ryan Harper <[EMAIL PROTECTED]>
---
diff -r 20e5f508accc tools/libxc/powerpc64/xc_linux_build.c
--- a/tools/libxc/powerpc64/xc_linux_build.cTue Feb 06 13:42:19 2007 -0600
+++ b/tools/libxc/powerpc64/xc_linux_build.cThu Feb 08 12:21:02 2007 -0600
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include "flatdevtree_env.h"
#include "flatdevtree.h"
@@ -256,10 +255,9 @@ int xc_linux_build(int xc_handle,
}
/* determine shared_info, console, and store paddr */
-shared_info_paddr = (rma_pages << PAGE_SHIFT) -
-(RMA_SHARED_INFO * PAGE_SIZE);
-console_paddr = (rma_pages << PAGE_SHIFT) - (RMA_CONSOLE * PAGE_SIZE);
-store_paddr = (rma_pages << PAGE_SHIFT) - (RMA_STORE * PAGE_SIZE);
+shared_info_paddr = (rma_pages << PAGE_SHIFT) - PAGE_SIZE;
+console_paddr = shared_info_paddr - PAGE_SIZE;
+store_paddr = console_paddr - PAGE_SIZE;
/* map paddrs to mfns */
*store_mfn = page_array[(xen_pfn_t)(store_paddr >> PAGE_SHIFT)];
diff -r 20e5f508accc xen/arch/powerpc/domain_build.c
--- a/xen/arch/powerpc/domain_build.c Tue Feb 06 13:42:19 2007 -0600
+++ b/xen/arch/powerpc/domain_build.c Thu Feb 08 12:21:02 2007 -0600
@@ -115,13 +115,15 @@ int construct_dom0(struct domain *d,
uint rma_nrpages = 1 << d->arch.rma_order;
ulong rma_sz = rma_size(d->arch.rma_order);
ulong rma = page_to_maddr(d->arch.rma_page);
-start_info_t *si;
ulong eomem;
int am64 = 1;
int preempt = 0;
ulong msr;
ulong pc;
ulong r2;
+ulong mod_start = 0;
+ulong mod_len = 0;
+ulong shared_info_addr;
int vcpu;
/* Sanity! */
@@ -185,24 +187,8 @@ int construct_dom0(struct domain *d,
ASSERT( image_len < rma_sz );
-si = (start_info_t *)(rma_addr(&d->arch, RMA_START_INFO) + rma);
-printk("xen_start_info: %p\n", si);
-
-sprintf(si->magic, "xen-%i.%i-powerpc%d%s",
-xen_major_version(), xen_minor_version(), BITS_PER_LONG, "HV");
-si->flags = SIF_PRIVILEGED | SIF_INITDOMAIN;
-
-si->shared_info = ((ulong)d->shared_info) - rma;
-printk("shared_info: 0x%lx,%p\n", si->shared_info, d->shared_info);
-
-eomem = si->shared_info;
-
-/* number of pages accessible */
-si->nr_pages = rma_sz >> PAGE_SHIFT;
-
-si->pt_base = 0;
-si->nr_pt_frames = 0;
-si->mfn_list = 0;
+eomem = ((ulong)d->shared_info) - rma;
+printk("shared_info: 0x%lx,%p\n", eomem, d->shared_info);
/* OF usually sits here:
* - Linux needs it to be loaded before the vmlinux or initrd
@@ -273,15 +259,13 @@ int construct_dom0(struct domain *d,
printk("loading initrd: 0x%lx, 0x%lx\n", dst, initrd_len);
memcpy((void *)dst, (void *)initrd_start, initrd_len);
-si->mod_start = dst - rma;
-si->mod_len = image_len;
+mod_start = dst - rma;
+mod_len = image_len;
dst = ALIGN_UP(dst + initrd_len, PAGE_SIZE);
-} else {
+} else
printk("no initrd\n");
-si->mod_start = 0;
-si->mod_len = 0;
-}
+
/* it may be a function descriptor */
fdesc = (ulong *)(dsi.v_kernstart + dsi.v_kernentry + kbase);
@@ -309,12 +293,8 @@ int construct_dom0(struct domain *d,
msr = 0;
}
-v->arch.ctxt.gprs[3] = si->mod_start;
-v->arch.ctxt.gprs[4] = si->mod_len;
-
-memset(si->cmd_line, 0, sizeof(si->cmd_line));
-if ( cmdline != NULL )
-strncpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line)-1);
+v->arch.ctxt.gprs[3] = mod_start;
+v->arch.ctxt.gprs[4] = mod_len;
v->arch.ctxt.msr = msr;
v->arch.ctxt.pc = pc;
@@ -322,7 +302,10 @@ int construct_dom0(struct domain *d,
printk("DOM: pc = 0x%lx, r2 = 0x%lx\n", pc, r2);
-ofd_dom0_fixup(d, *ofh_tree + rma, si);
+/* convert xen pointer shared_info into guest physical */
+shared_info_addr = (ulong)d->shared_info - page_to_maddr(d->arch.rma_page);
+
+ofd_dom0_fixup(d, *ofh_tree + rma, cmdline, shared_info_addr);
[XenPPC] [PATCH 2/2][2nd try] linux: build start_info_t from devtree only
Updated:
- made start_info_t xsi static
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
[EMAIL PROTECTED]
diffstat output:
setup.c | 35 ---
1 files changed, 16 insertions(+), 19 deletions(-)
Signed-off-by: Ryan Harper <[EMAIL PROTECTED]>
---
diff -r a6adf094e08e arch/powerpc/platforms/xen/setup.c
--- a/arch/powerpc/platforms/xen/setup.cTue Feb 06 13:55:48 2007 -0600
+++ b/arch/powerpc/platforms/xen/setup.cThu Feb 08 17:11:45 2007 -0600
@@ -32,7 +32,7 @@ EXPORT_SYMBOL(HYPERVISOR_shared_info);
EXPORT_SYMBOL(HYPERVISOR_shared_info);
/* Raw start-of-day parameters from the hypervisor. */
-start_info_t xsi;
+static start_info_t xsi;
start_info_t *xen_start_info;
extern struct machdep_calls mach_maple_md;
@@ -88,29 +88,26 @@ static void __init xen_init_early(void)
static void __init xen_init_early(void)
{
struct device_node *xen;
- u64 *si;
DBG(" -> %s\n", __func__);
xen = of_find_node_by_path("/xen");
- si = (u64 *)get_property(xen, "start-info", NULL);
-
- /* build our own start_info_t if start-info property is not present */
- if (si != NULL) {
- xen_start_info = (start_info_t *)__va(*si);
- } else {
- struct device_node *console;
- struct device_node *store;
-
- console = of_find_node_by_path("/xen/console");
- store = of_find_node_by_path("/xen/store");
-
- xen_start_info = &xsi;
-
- /* fill out start_info_t from devtree */
- xen_start_info->shared_info = *((u64 *)get_property(xen,
- "shared-info", NULL));
+ xen_start_info = &xsi;
+
+ /* fill out start_info_t from devtree */
+ if ((char *)get_property(xen, "privileged", NULL))
+ xen_start_info->flags |= SIF_PRIVILEGED;
+ if ((char *)get_property(xen, "initdomain", NULL))
+ xen_start_info->flags |= SIF_INITDOMAIN;
+ xen_start_info->shared_info = *((u64 *)get_property(xen,
+ "shared-info", NULL));
+
+ /* only look for store and console for guest domains */
+ if (xen_start_info->flags == 0) {
+ struct device_node *console =
of_find_node_by_path("/xen/console");
+ struct device_node *store = of_find_node_by_path("/xen/store");
+
xen_start_info->store_mfn = (*((u64 *)get_property(store,
"reg", NULL))) >> PAGE_SHIFT;
xen_start_info->store_evtchn = *((u32 *)get_property(store,
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
[XenPPC] [PATCH] [RESEND] Fix up potential memory leaks introduced by xencomm
With some of the logic change from the Xencomm patch. In a few
hypercalls introduces a situation where you can potentially have memory
leaks if something fails. This patch address these issues.
Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
diff -r 37ea4cf1281a arch/powerpc/platforms/xen/hcall.c
--- a/arch/powerpc/platforms/xen/hcall.c Tue Feb 06 17:10:20 2007 -0500
+++ b/arch/powerpc/platforms/xen/hcall.c Wed Feb 07 08:19:45 2007 -0600
@@ -203,11 +203,14 @@ int HYPERVISOR_sched_op(int cmd, void *a
desc = xencomm_map_no_alloc(arg, argsize);
- if (desc == NULL)
- return -EINVAL;
-
- rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
-cmd, desc);
+ if (desc) {
+ rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
+ cmd, desc);
+ xencomm_free(desc);
+ }
+ else {
+ rc = -EINVAL;
+ }
xencomm_free(ports);
@@ -389,8 +392,8 @@ static int xenppc_privcmd_domctl(privcmd
if (copy_to_user(user_op, &kern_op, sizeof(xen_domctl_t)))
ret = -EFAULT;
- xencomm_free(desc);
out:
+ xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -463,8 +466,8 @@ static int xenppc_privcmd_sysctl(privcmd
if (copy_to_user(user_op, &kern_op, sizeof(xen_sysctl_t)))
ret = -EFAULT;
- xencomm_free(desc);
out:
+ xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -514,8 +517,8 @@ static int xenppc_privcmd_platform_op(pr
if (copy_to_user(user_op, &kern_op, sizeof(xen_platform_op_t)))
ret = -EFAULT;
- xencomm_free(desc);
out:
+ xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -546,8 +549,10 @@ int HYPERVISOR_memory_op(unsigned int cm
mop->nr_extents *
sizeof(*xen_guest_handle(mop->extent_start)));
- if (desc == NULL)
+ if (desc == NULL) {
+xencomm_free(op_desc);
return -ENOMEM;
+ }
set_xen_guest_handle(mop->extent_start,
desc);
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
[XenPPC] [xenppc-unstable] [LIBXC][POWERPC] use O_CREAT on open call for DTB_FILE
# HG changeset patch
# User Jimi Xenidis <[EMAIL PROTECTED]>
# Node ID 51ca40884ed825b0d7fdf81a1e078e32451b4d22
# Parent 20e5f508accc21f6aaf9ade60d9a5510512cb289
[LIBXC][POWERPC] use O_CREAT on open call for DTB_FILE
This fixes a bug in the creating of the flat dev tree. If open is used
and O_CREAT not is specified, it will fail if the file has not already
been created. This patch will create the file if it does not exist
already. Which will allow for DomU creation.
Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
Signed-off-by: Jimi Xenidis <[EMAIL PROTECTED]>
---
tools/libxc/powerpc64/mk_flatdevtree.c |2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -r 20e5f508accc -r 51ca40884ed8 tools/libxc/powerpc64/mk_flatdevtree.c
--- a/tools/libxc/powerpc64/mk_flatdevtree.cTue Feb 06 13:42:19 2007 -0600
+++ b/tools/libxc/powerpc64/mk_flatdevtree.cThu Feb 08 06:46:13 2007 -0500
@@ -618,7 +618,7 @@ int make_devtree(struct ft_cxt *root,
}
/* write a copy of the tree to a file */
-if ((dtb_fd = open(DTB_FILE , O_RDWR)) == -1) {
+if ((dtb_fd = open(DTB_FILE , O_CREAT|O_RDWR)) == -1) {
PERROR("%s: failed to open file %s", __func__, DTB_FILE);
goto error;
}
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
[XenPPC] [xenppc-unstable] [LIBXC][POWERPC] make sure DTB is truncated and has a decent mode
# HG changeset patch
# User Jimi Xenidis <[EMAIL PROTECTED]>
# Node ID 059beff4129bc0fc44032a95a8756c6ca89f6b21
# Parent 51ca40884ed825b0d7fdf81a1e078e32451b4d22
[LIBXC][POWERPC] make sure DTB is truncated and has a decent mode
Also, since we only write to this FD, we mau as well use creat(2)
Signed-off-by: Jimi Xenidis <[EMAIL PROTECTED]>
---
tools/libxc/powerpc64/mk_flatdevtree.c |2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -r 51ca40884ed8 -r 059beff4129b tools/libxc/powerpc64/mk_flatdevtree.c
--- a/tools/libxc/powerpc64/mk_flatdevtree.cThu Feb 08 06:46:13 2007 -0500
+++ b/tools/libxc/powerpc64/mk_flatdevtree.cThu Feb 08 07:05:26 2007 -0500
@@ -618,7 +618,7 @@ int make_devtree(struct ft_cxt *root,
}
/* write a copy of the tree to a file */
-if ((dtb_fd = open(DTB_FILE , O_CREAT|O_RDWR)) == -1) {
+if ((dtb_fd = creat(DTB_FILE, S_IRUSR | S_IWUSR)) == -1) {
PERROR("%s: failed to open file %s", __func__, DTB_FILE);
goto error;
}
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH] Fix up potential memory leaks introduced by xencomm patch
On Thu, 2007-02-08 at 07:18 -0500, Jimi Xenidis wrote:
> mostly Linux Kernel style NITS
>
> On Feb 7, 2007, at 4:26 PM, Jerone Young wrote:
>
> > With some of the logic change from the Xencomm patch. In a few
> > hypercalls introduces a situation where you can potentially have
> > memory
> > leaks if something fails. This patch address these issues.
> >
> > Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
>
> > diff -r 37ea4cf1281a arch/powerpc/platforms/xen/hcall.c
> > --- a/arch/powerpc/platforms/xen/hcall.cTue Feb 06 17:10:20 2007
> > -0500
> > +++ b/arch/powerpc/platforms/xen/hcall.cWed Feb 07 14:50:05 2007
> > -0600
> > @@ -203,11 +203,16 @@ int HYPERVISOR_sched_op(int cmd, void *a
> > desc = xencomm_map_no_alloc(arg, argsize);
> > - if (desc == NULL)
> > - return -EINVAL;
> > -
> > - rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
> > - cmd, desc);
> > + if (desc)
> > + {
>
> brace on same line
>
> > + rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
> > + cmd, desc);
> > + xencomm_free(desc);
> > + }
> > + else
> > + {
>
> no braces for single line, in fact avoid the single line by pre-
> assigning the rc, the compiler will do the right optimization anyway.
>
> > + rc = -EINVAL;
>
> I thought we agreed that xencomm_map_* failures would be ENOMEM or
> ENOSPC?
No changed it (based on your suggestion) to EINVAL. ENOSPC is really
meant for disks out of space. So this actually kind of made the mode
since, it's an invalid argument, because it is too big.
>
> > + }
> > xencomm_free(ports);
> > @@ -389,8 +394,8 @@ static int xenppc_privcmd_domctl(privcmd
> > if (copy_to_user(user_op, &kern_op, sizeof(xen_domctl_t)))
> > ret = -EFAULT;
> > - xencomm_free(desc);
> > out:
> > + xencomm_free(desc);
> > xencomm_free(op_desc);
> > return ret;
> > }
> > @@ -463,8 +468,8 @@ static int xenppc_privcmd_sysctl(privcmd
> > if (copy_to_user(user_op, &kern_op, sizeof(xen_sysctl_t)))
> > ret = -EFAULT;
> > - xencomm_free(desc);
> > out:
> > + xencomm_free(desc);
> > xencomm_free(op_desc);
> > return ret;
> > }
> > @@ -514,8 +519,8 @@ static int xenppc_privcmd_platform_op(pr
> > if (copy_to_user(user_op, &kern_op, sizeof(xen_platform_op_t)))
> > ret = -EFAULT;
> > - xencomm_free(desc);
> > out:
> > + xencomm_free(desc);
> > xencomm_free(op_desc);
> > return ret;
> > }
> > @@ -547,7 +552,10 @@ int HYPERVISOR_memory_op(unsigned int cm
> > sizeof(*xen_guest_handle(mop->extent_start)));
> > if (desc == NULL)
> > + {
>
> Curlies again
ok I'll send a new patch with all the curlie braces on the first line.
>
> > + xencomm_free(op_desc);
> > return -ENOMEM;
> > + }
> > set_xen_guest_handle(mop->extent_start,
> > desc);
>
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH 2/2] linux: build start_info_t from devtree only
On Feb 8, 2007, at 8:45 AM, Ryan Harper wrote:
* Jimi Xenidis <[EMAIL PROTECTED]> [2007-02-08 06:48]:
some comments
On Feb 7, 2007, at 6:34 PM, Ryan Harper wrote:
This patch cleans up xen_init_early() to construct a start_info_t
only
from the devtree as Patch1 fixes xen to no longer create a
start_info_t
for dom0.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
[EMAIL PROTECTED]
diffstat output:
setup.c | 33 +++--
1 files changed, 15 insertions(+), 18 deletions(-)
Signed-off-by: Ryan Harper <[EMAIL PROTECTED]>
---
diff -r a6adf094e08e arch/powerpc/platforms/xen/setup.c
--- a/arch/powerpc/platforms/xen/setup.cTue Feb 06 13:55:48 2007
-0600
+++ b/arch/powerpc/platforms/xen/setup.cWed Feb 07 11:33:10 2007
-0600
@@ -88,29 +88,26 @@ static void __init xen_init_early(void)
static void __init xen_init_early(void)
{
struct device_node *xen;
- u64 *si;
DBG(" -> %s\n", __func__);
xen = of_find_node_by_path("/xen");
- si = (u64 *)get_property(xen, "start-info", NULL);
-
- /* build our own start_info_t if start-info property is not
present */
- if (si != NULL) {
- xen_start_info = (start_info_t *)__va(*si);
- } else {
- struct device_node *console;
- struct device_node *store;
-
- console = of_find_node_by_path("/xen/console");
- store = of_find_node_by_path("/xen/store");
-
- xen_start_info = &xsi;
-
- /* fill out start_info_t from devtree */
- xen_start_info->shared_info = *((u64 *)get_property(xen,
- "shared-info", NULL));
+ xen_start_info = &xsi;
Please make xsi static in its declaration earlier in the file.
Sure. And while I wanted to access xsi vai xen_start_info,
the
xen_start_info = &xsi
seemed a bit awkward. Not sure if I should switch to xsi. Thoughts?
There is too much code referring to xen_start_info as a pointer.
perhaps making that static __xen_start_info, so its obvious that we
are just suing it for memory allocation, at some point when all
references are gone we can flatten this out.
+
+ /* fill out start_info_t from devtree */
+ if ((char *)get_property(xen, "privileged", NULL))
+ xen_start_info->flags |= SIF_PRIVILEGED;
+ if ((char *)get_property(xen, "initdomain", NULL))
+ xen_start_info->flags |= SIF_INITDOMAIN;
+ xen_start_info->shared_info = *((u64 *)get_property(xen,
+ "shared-info", NULL));
+
+ /* only look for store and console for guest domains */
Hmm, I think you need to look for them always.
You _at_least_ need the console evtchn, which may not be zero and we
create the node/prop anyway.
Hrm, you may be right. I know that dom0 boots fine with this, but
that
maybe because it defaults those values to 0. I'll kill the if().
Feel free to "panic()" more:
NOTE: this is early so a "udbg_printf(); HYPERVISOR_shutdown
(SHUTDOWN_poweroff);" will do cuz panic() is no available yet.
Yeah, good idea though none of the messages get out if our shared_info
page isn't setup correctly, which I learned during my testing of this
patch, was the value most likely to get hosed.
Oh yeah, you probably want to:
HYPERVISOR_shared_info = ...;
xen_start_info->flags |= SIF_INITDOMAIN; # or not
udbg_init_xen();
As soon as you can, before you check everything else.
If you want more info earlier you can always build with:
CONFIG_PPC_EARLY_DEBUG_XEN_DOM0
xor
CONFIG_PPC_EARLY_DEBUG_XEN_DOMU
-JX
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH 1/2] xen: remove start_info_t from dom0 building
On Feb 8, 2007, at 8:40 AM, Ryan Harper wrote: * Jimi Xenidis <[EMAIL PROTECTED]> [2007-02-08 06:30]: Just 2 things. (1) I really do not want OFD code to compute anything so please pass in the shared page address for ofd_dom0_fixup() heh, unlike how it was calculating start-info ? =) This is not "do as I say not as I do", its more like "don't do as I did". I'm constantly fixing little embarrassments that we have left behind because we understand things better and have a crisper view of how things should be. (2) I'm pretty sure the all of the #define RMA_* can go now. There are a couple places, (mm.c for shared_info) and xc_linux_build() for domU which have to pick a page to use. Don't we want those choices marked in some common place? The only page that is etched in stone the shared_info page, which is always the last RMA page. all the other pages could be anywhere and only the concern of a DomU and LibXC. So, I'd really like to see these and rma_addr() go. -JX ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH 2/2] linux: build start_info_t from devtree only
* Jimi Xenidis <[EMAIL PROTECTED]> [2007-02-08 06:48]:
> some comments
> On Feb 7, 2007, at 6:34 PM, Ryan Harper wrote:
>
> >This patch cleans up xen_init_early() to construct a start_info_t only
> >from the devtree as Patch1 fixes xen to no longer create a
> >start_info_t
> >for dom0.
> >
> >--
> >Ryan Harper
> >Software Engineer; Linux Technology Center
> >IBM Corp., Austin, Tx
> >(512) 838-9253 T/L: 678-9253
> >[EMAIL PROTECTED]
> >
> >
> >diffstat output:
> > setup.c | 33 +++--
> > 1 files changed, 15 insertions(+), 18 deletions(-)
> >
> >Signed-off-by: Ryan Harper <[EMAIL PROTECTED]>
> >---
> >diff -r a6adf094e08e arch/powerpc/platforms/xen/setup.c
> >--- a/arch/powerpc/platforms/xen/setup.c Tue Feb 06 13:55:48 2007
> >-0600
> >+++ b/arch/powerpc/platforms/xen/setup.c Wed Feb 07 11:33:10 2007
> >-0600
> >@@ -88,29 +88,26 @@ static void __init xen_init_early(void)
> > static void __init xen_init_early(void)
> > {
> > struct device_node *xen;
> >-u64 *si;
> >
> > DBG(" -> %s\n", __func__);
> >
> > xen = of_find_node_by_path("/xen");
> >
> >-si = (u64 *)get_property(xen, "start-info", NULL);
> >-
> >-/* build our own start_info_t if start-info property is not
> >present */
> >-if (si != NULL) {
> >-xen_start_info = (start_info_t *)__va(*si);
> >-} else {
> >-struct device_node *console;
> >-struct device_node *store;
> >-
> >-console = of_find_node_by_path("/xen/console");
> >-store = of_find_node_by_path("/xen/store");
> >-
> >-xen_start_info = &xsi;
> >-
> >-/* fill out start_info_t from devtree */
> >-xen_start_info->shared_info = *((u64 *)get_property(xen,
> >- "shared-info", NULL));
> >+xen_start_info = &xsi;
>
> Please make xsi static in its declaration earlier in the file.
Sure. And while I wanted to access xsi vai xen_start_info,
the
xen_start_info = &xsi
seemed a bit awkward. Not sure if I should switch to xsi. Thoughts?
> >+
> >+/* fill out start_info_t from devtree */
> >+if ((char *)get_property(xen, "privileged", NULL))
> >+xen_start_info->flags |= SIF_PRIVILEGED;
> >+if ((char *)get_property(xen, "initdomain", NULL))
> >+xen_start_info->flags |= SIF_INITDOMAIN;
> >+xen_start_info->shared_info = *((u64 *)get_property(xen,
> >+ "shared-info", NULL));
> >+
> >+/* only look for store and console for guest domains */
>
> Hmm, I think you need to look for them always.
> You _at_least_ need the console evtchn, which may not be zero and we
> create the node/prop anyway.
Hrm, you may be right. I know that dom0 boots fine with this, but that
maybe because it defaults those values to 0. I'll kill the if().
>
> Feel free to "panic()" more:
> NOTE: this is early so a "udbg_printf(); HYPERVISOR_shutdown
> (SHUTDOWN_poweroff);" will do cuz panic() is no available yet.
Yeah, good idea though none of the messages get out if our shared_info
page isn't setup correctly, which I learned during my testing of this
patch, was the value most likely to get hosed.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
[EMAIL PROTECTED]
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH 1/2] xen: remove start_info_t from dom0 building
* Jimi Xenidis <[EMAIL PROTECTED]> [2007-02-08 06:30]: > Just 2 things. > (1) I really do not want OFD code to compute anything so please pass > in the shared page address for ofd_dom0_fixup() heh, unlike how it was calculating start-info ? =) > (2) I'm pretty sure the all of the #define RMA_* can go now. There are a couple places, (mm.c for shared_info) and xc_linux_build() for domU which have to pick a page to use. Don't we want those choices marked in some common place? -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 [EMAIL PROTECTED] ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH 2/2] linux: build start_info_t from devtree only
some comments
On Feb 7, 2007, at 6:34 PM, Ryan Harper wrote:
This patch cleans up xen_init_early() to construct a start_info_t only
from the devtree as Patch1 fixes xen to no longer create a
start_info_t
for dom0.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
[EMAIL PROTECTED]
diffstat output:
setup.c | 33 +++--
1 files changed, 15 insertions(+), 18 deletions(-)
Signed-off-by: Ryan Harper <[EMAIL PROTECTED]>
---
diff -r a6adf094e08e arch/powerpc/platforms/xen/setup.c
--- a/arch/powerpc/platforms/xen/setup.c Tue Feb 06 13:55:48 2007
-0600
+++ b/arch/powerpc/platforms/xen/setup.c Wed Feb 07 11:33:10 2007
-0600
@@ -88,29 +88,26 @@ static void __init xen_init_early(void)
static void __init xen_init_early(void)
{
struct device_node *xen;
- u64 *si;
DBG(" -> %s\n", __func__);
xen = of_find_node_by_path("/xen");
- si = (u64 *)get_property(xen, "start-info", NULL);
-
- /* build our own start_info_t if start-info property is not
present */
- if (si != NULL) {
- xen_start_info = (start_info_t *)__va(*si);
- } else {
- struct device_node *console;
- struct device_node *store;
-
- console = of_find_node_by_path("/xen/console");
- store = of_find_node_by_path("/xen/store");
-
- xen_start_info = &xsi;
-
- /* fill out start_info_t from devtree */
- xen_start_info->shared_info = *((u64 *)get_property(xen,
- "shared-info", NULL));
+ xen_start_info = &xsi;
Please make xsi static in its declaration earlier in the file.
+
+ /* fill out start_info_t from devtree */
+ if ((char *)get_property(xen, "privileged", NULL))
+ xen_start_info->flags |= SIF_PRIVILEGED;
+ if ((char *)get_property(xen, "initdomain", NULL))
+ xen_start_info->flags |= SIF_INITDOMAIN;
+ xen_start_info->shared_info = *((u64 *)get_property(xen,
+ "shared-info", NULL));
+
+ /* only look for store and console for guest domains */
Hmm, I think you need to look for them always.
You _at_least_ need the console evtchn, which may not be zero and we
create the node/prop anyway.
Feel free to "panic()" more:
NOTE: this is early so a "udbg_printf(); HYPERVISOR_shutdown
(SHUTDOWN_poweroff);" will do cuz panic() is no available yet.
if (!store && !(xen_start_info->flags | SIF_INITDOMAIN) panic();
if (!console) panic();
if (get_property(console, "reg", NULL) == NULL &&
!(xen_start_info->flags | SIF_INITDOMAIN)) panic
+ if (xen_start_info->flags == 0) {
+ struct device_node *console =
of_find_node_by_path("/xen/console");
+ struct device_node *store = of_find_node_by_path("/xen/store");
+
xen_start_info->store_mfn = (*((u64 *)get_property(store,
"reg", NULL))) >> PAGE_SHIFT;
xen_start_info->store_evtchn = *((u32 *)get_property(store,
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH 1/2] xen: remove start_info_t from dom0 building
Just 2 things.
(1) I really do not want OFD code to compute anything so please pass
in the shared page address for ofd_dom0_fixup()
(2) I'm pretty sure the all of the #define RMA_* can go now.
-JX
On Feb 7, 2007, at 6:34 PM, Ryan Harper wrote:
The patch removes the construction of a start_info_t structure for
dom0,
no longer needed now that Linux and construct its own start_info_t.
Patch 2 fixes up Linux to not look for /xen/start-info and just create
one from the devtree.
Included in the patch are:
-remove the page allocated for the start_info_t (RMA_START_INFO)
-remove start_info_t from construct_dom0()
-remove start_info_t from ofd_dom0_fixup()
-remove start_info_t from ofd_xen_props()
-add new /xen property "shared-info" in dom0 devtree
-add new /xen property "privileged" in dom0 devtree
-add new /xen property "initdomain" in dom0 devtree
Test booted on JS20, simple guest domU launched successfully.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253 T/L: 678-9253
[EMAIL PROTECTED]
diffstat output:
arch/powerpc/domain_build.c | 43 +
+
arch/powerpc/ofd_fixup.c | 19 --
arch/powerpc/oftree.h |2 -
include/public/arch-powerpc.h |9 +++-
4 files changed, 29 insertions(+), 44 deletions(-)
Signed-off-by: Ryan Harper <[EMAIL PROTECTED]>
---
diff -r 20e5f508accc xen/arch/powerpc/domain_build.c
--- a/xen/arch/powerpc/domain_build.c Tue Feb 06 13:42:19 2007 -0600
+++ b/xen/arch/powerpc/domain_build.c Wed Feb 07 15:28:28 2007 -0600
@@ -115,13 +115,14 @@ int construct_dom0(struct domain *d,
uint rma_nrpages = 1 << d->arch.rma_order;
ulong rma_sz = rma_size(d->arch.rma_order);
ulong rma = page_to_maddr(d->arch.rma_page);
-start_info_t *si;
ulong eomem;
int am64 = 1;
int preempt = 0;
ulong msr;
ulong pc;
ulong r2;
+ulong mod_start = 0;
+ulong mod_len = 0;
int vcpu;
/* Sanity! */
@@ -185,24 +186,8 @@ int construct_dom0(struct domain *d,
ASSERT( image_len < rma_sz );
-si = (start_info_t *)(rma_addr(&d->arch, RMA_START_INFO) + rma);
-printk("xen_start_info: %p\n", si);
-
-sprintf(si->magic, "xen-%i.%i-powerpc%d%s",
-xen_major_version(), xen_minor_version(),
BITS_PER_LONG, "HV");
-si->flags = SIF_PRIVILEGED | SIF_INITDOMAIN;
-
-si->shared_info = ((ulong)d->shared_info) - rma;
-printk("shared_info: 0x%lx,%p\n", si->shared_info, d-
>shared_info);
-
-eomem = si->shared_info;
-
-/* number of pages accessible */
-si->nr_pages = rma_sz >> PAGE_SHIFT;
-
-si->pt_base = 0;
-si->nr_pt_frames = 0;
-si->mfn_list = 0;
+eomem = ((ulong)d->shared_info) - rma;
+printk("shared_info: 0x%lx,%p\n", eomem, d->shared_info);
/* OF usually sits here:
* - Linux needs it to be loaded before the vmlinux or initrd
@@ -273,15 +258,13 @@ int construct_dom0(struct domain *d,
printk("loading initrd: 0x%lx, 0x%lx\n", dst,
initrd_len);
memcpy((void *)dst, (void *)initrd_start, initrd_len);
-si->mod_start = dst - rma;
-si->mod_len = image_len;
+mod_start = dst - rma;
+mod_len = image_len;
dst = ALIGN_UP(dst + initrd_len, PAGE_SIZE);
-} else {
+} else
printk("no initrd\n");
-si->mod_start = 0;
-si->mod_len = 0;
-}
+
/* it may be a function descriptor */
fdesc = (ulong *)(dsi.v_kernstart + dsi.v_kernentry + kbase);
@@ -309,12 +292,8 @@ int construct_dom0(struct domain *d,
msr = 0;
}
-v->arch.ctxt.gprs[3] = si->mod_start;
-v->arch.ctxt.gprs[4] = si->mod_len;
-
-memset(si->cmd_line, 0, sizeof(si->cmd_line));
-if ( cmdline != NULL )
-strncpy((char *)si->cmd_line, cmdline, sizeof(si-
>cmd_line)-1);
+v->arch.ctxt.gprs[3] = mod_start;
+v->arch.ctxt.gprs[4] = mod_len;
v->arch.ctxt.msr = msr;
v->arch.ctxt.pc = pc;
@@ -322,7 +301,7 @@ int construct_dom0(struct domain *d,
printk("DOM: pc = 0x%lx, r2 = 0x%lx\n", pc, r2);
-ofd_dom0_fixup(d, *ofh_tree + rma, si);
+ofd_dom0_fixup(d, *ofh_tree + rma, cmdline);
set_bit(_VCPUF_initialised, &v->vcpu_flags);
diff -r 20e5f508accc xen/arch/powerpc/ofd_fixup.c
--- a/xen/arch/powerpc/ofd_fixup.c Tue Feb 06 13:42:19 2007 -0600
+++ b/xen/arch/powerpc/ofd_fixup.c Wed Feb 07 15:38:28 2007 -0600
@@ -326,7 +326,7 @@ static ofdn_t ofd_rtas_props(void *m)
}
#endif
-static ofdn_t ofd_xen_props(void *m, struct domain *d,
start_info_t *si)
+static ofdn_t ofd_xen_props(void *m, struct domain *d)
{
ofdn_t n;
static const char path[] = "/xen";
@@ -349,9 +349,16 @@ static ofdn_t ofd_xen_props(void *m, str
ASSERT(xl < sizeof (xen));
ofd_prop_add(m, n, "version", xen, xl + 1);
-val[0] = (ulong)si - page_to_maddr(d->arch.rma_page);
Re: [XenPPC] [PATCH] Fix up potential memory leaks introduced by xencomm patch
mostly Linux Kernel style NITS
On Feb 7, 2007, at 4:26 PM, Jerone Young wrote:
With some of the logic change from the Xencomm patch. In a few
hypercalls introduces a situation where you can potentially have
memory
leaks if something fails. This patch address these issues.
Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
diff -r 37ea4cf1281a arch/powerpc/platforms/xen/hcall.c
--- a/arch/powerpc/platforms/xen/hcall.c Tue Feb 06 17:10:20 2007
-0500
+++ b/arch/powerpc/platforms/xen/hcall.c Wed Feb 07 14:50:05 2007
-0600
@@ -203,11 +203,16 @@ int HYPERVISOR_sched_op(int cmd, void *a
desc = xencomm_map_no_alloc(arg, argsize);
- if (desc == NULL)
- return -EINVAL;
-
- rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
- cmd, desc);
+ if (desc)
+ {
brace on same line
+ rc = plpar_hcall_norets(XEN_MARK(__HYPERVISOR_sched_op),
+ cmd, desc);
+ xencomm_free(desc);
+ }
+ else
+ {
no braces for single line, in fact avoid the single line by pre-
assigning the rc, the compiler will do the right optimization anyway.
+ rc = -EINVAL;
I thought we agreed that xencomm_map_* failures would be ENOMEM or
ENOSPC?
+ }
xencomm_free(ports);
@@ -389,8 +394,8 @@ static int xenppc_privcmd_domctl(privcmd
if (copy_to_user(user_op, &kern_op, sizeof(xen_domctl_t)))
ret = -EFAULT;
- xencomm_free(desc);
out:
+ xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -463,8 +468,8 @@ static int xenppc_privcmd_sysctl(privcmd
if (copy_to_user(user_op, &kern_op, sizeof(xen_sysctl_t)))
ret = -EFAULT;
- xencomm_free(desc);
out:
+ xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -514,8 +519,8 @@ static int xenppc_privcmd_platform_op(pr
if (copy_to_user(user_op, &kern_op, sizeof(xen_platform_op_t)))
ret = -EFAULT;
- xencomm_free(desc);
out:
+ xencomm_free(desc);
xencomm_free(op_desc);
return ret;
}
@@ -547,7 +552,10 @@ int HYPERVISOR_memory_op(unsigned int cm
sizeof(*xen_guest_handle(mop->extent_start)));
if (desc == NULL)
+ {
Curlies again
+ xencomm_free(op_desc);
return -ENOMEM;
+ }
set_xen_guest_handle(mop->extent_start,
desc);
___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
