| Message ID | 1507227824-11744-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 05, 2017 at 06:23:39PM +0000, Andrew Cooper wrote: > alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its > alloc_magic_pages_hvm() counterpart. Delete a redundant comment, introduce > some newlines clarity, and remove a logically dead allocation of shared info. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > --- > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Julien Grall <julien.grall@arm.com> > --- > tools/libxc/xc_dom_x86.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c > index cb68efc..885ca1b 100644 > --- a/tools/libxc/xc_dom_x86.c > +++ b/tools/libxc/xc_dom_x86.c > @@ -534,24 +534,20 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom) > > /* ------------------------------------------------------------------------ */ > > -static int alloc_magic_pages(struct xc_dom_image *dom) > +static int alloc_magic_pages_pv(struct xc_dom_image *dom) > { > - /* allocate special pages */ > dom->start_info_pfn = xc_dom_alloc_page(dom, "start info"); > if ( dom->start_info_pfn == INVALID_PFN ) > return -1; Maybe those errors paths should set errno = ENOMEM? In any case the patch is an improvement. Thanks, Roger.
On Thu, Oct 05, 2017 at 07:23:39PM +0100, Andrew Cooper wrote: > alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its > alloc_magic_pages_hvm() counterpart. Delete a redundant comment, introduce > some newlines clarity, and remove a logically dead allocation of shared info. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
On Fri, Oct 06, 2017 at 10:26:03AM +0100, Roger Pau Monné wrote: > On Thu, Oct 05, 2017 at 06:23:39PM +0000, Andrew Cooper wrote: > > alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its > > alloc_magic_pages_hvm() counterpart. Delete a redundant comment, introduce > > some newlines clarity, and remove a logically dead allocation of shared info. > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > > --- > > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > > CC: Wei Liu <wei.liu2@citrix.com> > > CC: Julien Grall <julien.grall@arm.com> > > --- > > tools/libxc/xc_dom_x86.c | 16 ++++++---------- > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c > > index cb68efc..885ca1b 100644 > > --- a/tools/libxc/xc_dom_x86.c > > +++ b/tools/libxc/xc_dom_x86.c > > @@ -534,24 +534,20 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom) > > > > /* ------------------------------------------------------------------------ */ > > > > -static int alloc_magic_pages(struct xc_dom_image *dom) > > +static int alloc_magic_pages_pv(struct xc_dom_image *dom) > > { > > - /* allocate special pages */ > > dom->start_info_pfn = xc_dom_alloc_page(dom, "start info"); > > if ( dom->start_info_pfn == INVALID_PFN ) > > return -1; > > Maybe those errors paths should set errno = ENOMEM? I believe the actual allocation function already does that.
On Fri, Oct 06, 2017 at 09:33:52AM +0000, Wei Liu wrote: > On Fri, Oct 06, 2017 at 10:26:03AM +0100, Roger Pau Monné wrote: > > On Thu, Oct 05, 2017 at 06:23:39PM +0000, Andrew Cooper wrote: > > > alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its > > > alloc_magic_pages_hvm() counterpart. Delete a redundant comment, introduce > > > some newlines clarity, and remove a logically dead allocation of shared info. > > > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > > > > --- > > > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > > > CC: Wei Liu <wei.liu2@citrix.com> > > > CC: Julien Grall <julien.grall@arm.com> > > > --- > > > tools/libxc/xc_dom_x86.c | 16 ++++++---------- > > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > > > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c > > > index cb68efc..885ca1b 100644 > > > --- a/tools/libxc/xc_dom_x86.c > > > +++ b/tools/libxc/xc_dom_x86.c > > > @@ -534,24 +534,20 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom) > > > > > > /* ------------------------------------------------------------------------ */ > > > > > > -static int alloc_magic_pages(struct xc_dom_image *dom) > > > +static int alloc_magic_pages_pv(struct xc_dom_image *dom) > > > { > > > - /* allocate special pages */ > > > dom->start_info_pfn = xc_dom_alloc_page(dom, "start info"); > > > if ( dom->start_info_pfn == INVALID_PFN ) > > > return -1; > > > > Maybe those errors paths should set errno = ENOMEM? > > I believe the actual allocation function already does that. Doesn't seem like xc_dom_alloc_page or xc_dom_chk_alloc_pages set errno at all (or I'm not able to find it), but in any case it should be set there rather than here. libxc is a disaster in this regard I'm afraid. Roger.
On Fri, Oct 06, 2017 at 10:40:36AM +0100, Roger Pau Monné wrote: > On Fri, Oct 06, 2017 at 09:33:52AM +0000, Wei Liu wrote: > > On Fri, Oct 06, 2017 at 10:26:03AM +0100, Roger Pau Monné wrote: > > > On Thu, Oct 05, 2017 at 06:23:39PM +0000, Andrew Cooper wrote: > > > > alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its > > > > alloc_magic_pages_hvm() counterpart. Delete a redundant comment, introduce > > > > some newlines clarity, and remove a logically dead allocation of shared info. > > > > > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > > > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > > > > > > --- > > > > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > > > > CC: Wei Liu <wei.liu2@citrix.com> > > > > CC: Julien Grall <julien.grall@arm.com> > > > > --- > > > > tools/libxc/xc_dom_x86.c | 16 ++++++---------- > > > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c > > > > index cb68efc..885ca1b 100644 > > > > --- a/tools/libxc/xc_dom_x86.c > > > > +++ b/tools/libxc/xc_dom_x86.c > > > > @@ -534,24 +534,20 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom) > > > > > > > > /* ------------------------------------------------------------------------ */ > > > > > > > > -static int alloc_magic_pages(struct xc_dom_image *dom) > > > > +static int alloc_magic_pages_pv(struct xc_dom_image *dom) > > > > { > > > > - /* allocate special pages */ > > > > dom->start_info_pfn = xc_dom_alloc_page(dom, "start info"); > > > > if ( dom->start_info_pfn == INVALID_PFN ) > > > > return -1; > > > > > > Maybe those errors paths should set errno = ENOMEM? > > > > I believe the actual allocation function already does that. > > Doesn't seem like xc_dom_alloc_page or xc_dom_chk_alloc_pages set > errno at all (or I'm not able to find it), but in any case it should > be set there rather than here. libxc is a disaster in this regard I'm > afraid. I misremembered. ;-)
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c index cb68efc..885ca1b 100644 --- a/tools/libxc/xc_dom_x86.c +++ b/tools/libxc/xc_dom_x86.c @@ -534,24 +534,20 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom) /* ------------------------------------------------------------------------ */ -static int alloc_magic_pages(struct xc_dom_image *dom) +static int alloc_magic_pages_pv(struct xc_dom_image *dom) { - /* allocate special pages */ dom->start_info_pfn = xc_dom_alloc_page(dom, "start info"); if ( dom->start_info_pfn == INVALID_PFN ) return -1; + dom->xenstore_pfn = xc_dom_alloc_page(dom, "xenstore"); if ( dom->xenstore_pfn == INVALID_PFN ) return -1; + dom->console_pfn = xc_dom_alloc_page(dom, "console"); if ( dom->console_pfn == INVALID_PFN ) return -1; - if ( xc_dom_translated(dom) ) - { - dom->shared_info_pfn = xc_dom_alloc_page(dom, "shared info"); - if ( dom->shared_info_pfn == INVALID_PFN ) - return -1; - } + dom->alloc_bootstack = 1; return 0; @@ -1756,7 +1752,7 @@ static struct xc_dom_arch xc_dom_32_pae = { .sizeof_pfn = 4, .p2m_base_supported = 0, .arch_private_size = sizeof(struct xc_dom_image_x86), - .alloc_magic_pages = alloc_magic_pages, + .alloc_magic_pages = alloc_magic_pages_pv, .alloc_pgtables = alloc_pgtables_x86_32_pae, .alloc_p2m_list = alloc_p2m_list_x86_32, .setup_pgtables = setup_pgtables_x86_32_pae, @@ -1775,7 +1771,7 @@ static struct xc_dom_arch xc_dom_64 = { .sizeof_pfn = 8, .p2m_base_supported = 1, .arch_private_size = sizeof(struct xc_dom_image_x86), - .alloc_magic_pages = alloc_magic_pages, + .alloc_magic_pages = alloc_magic_pages_pv, .alloc_pgtables = alloc_pgtables_x86_64, .alloc_p2m_list = alloc_p2m_list_x86_64, .setup_pgtables = setup_pgtables_x86_64,
alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its alloc_magic_pages_hvm() counterpart. Delete a redundant comment, introduce some newlines clarity, and remove a logically dead allocation of shared info. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Julien Grall <julien.grall@arm.com> --- tools/libxc/xc_dom_x86.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)