On Fri, 2015-07-10 at 13:40 +0800, Chen, Tiejun wrote:
> >> tools/libxl/libxl_dom.c | 5 +++
> >> tools/libxl/libxl_internal.h | 24 +++++++++++++
> >> tools/libxl/libxl_x86.c | 83
> >> ++++++++++++++++++++++++++++++++++++++++++++
> > ...
> >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> >> index 62ef120..41da479 100644
> >> --- a/tools/libxl/libxl_dom.c
> >> +++ b/tools/libxl/libxl_dom.c
> >> @@ -1004,6 +1004,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
> >> goto out;
> >> }
> >>
> >> + if (libxl__domain_construct_e820(gc, d_config, domid, &args)) {
> >> + LOG(ERROR, "setting domain memory map failed");
> >> + goto out;
> >> + }
> >
> > This is platform-independent code, isn't it ? In which case this will
> > break the build on ARM, I think.
> >
> > Would an ARM maintainer please confirm.
> >
>
> I think you're right. I should make this specific to arch since here
> we're talking e820.
>
> So I tried to refactor this patch,
This approach looks like it should work, and I think given the point in
the release it would be acceptable for 4.6.
However long term I think it might make sense to try and reuse one of
the existing libxl__arch hooks, i.e.
libxl__arch_domain_init_hw_description or
libxl__arch_domain_finalise_hw_description. On ARM these are to do with
setting the Device Tree Blob, which included the memory map, so it is
somewhat morally equivalent to configuring the e820 on x86, I think.
Those hooks are only called from libxl__build_pv today, but calling them
from libxl__build_hvm seems like it would be good too.
In particular I think a call to
libxl__arch_domain_finalise_hw_description could be inserted just before
xc_hvm_build, which is similar to PV where it precedes
xc_dom_build_image, and is where you would want to setup the e820.
libxl__arch_domain_init_hw_description I think would still be a NOP on
x86, but it should probably go either just after the call to
libxl__domain_firmware.
Tiejun, would you be willing to commit to refactoring this and the
issues which Ian raised in response to #11 and #16 a subsequent clean up
series? I don't think it would even need to wait for the freeze to be
over to be posted (although it may need to wait to be applied).
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
