* [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest
@ 2013-12-09 12:13 Ian Campbell
2013-12-09 15:14 ` Julien Grall
2013-12-09 18:34 ` Julien Grall
0 siblings, 2 replies; 5+ messages in thread
From: Ian Campbell @ 2013-12-09 12:13 UTC (permalink / raw)
To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini
This is a generic interface which is supposed to return the number of bytes
which were not copied. Make it so.
Update the incorrect callers prepare_dtb, decode_thumb{2} and
xenmem_add_to_physmap_range.
In the xenmem_add_to_physmap_range case, observe that we are not propagating
errors from xenmem_add_to_physmap_one and do so.
In the decode_thumb case and an emacs magic block to decode.c
Make the flush_dcache parameter to the helper an int while at it.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix xenmem_add_to_physmap_range, which was assuming -errno return codes.
Fix raw_copy_from_guest and raw_clear_guest too
---
xen/arch/arm/decode.c | 22 ++++++++++++++--------
xen/arch/arm/domain_build.c | 8 ++++----
xen/arch/arm/guestcopy.c | 20 +++++++-------------
xen/arch/arm/mm.c | 23 +++++++++++++++++------
4 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index abe9f26..8880c39 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -36,12 +36,10 @@ static void update_dabt(struct hsr_dabt *dabt, int reg,
static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw1)
{
uint16_t hw2;
- int rc;
uint16_t rt;
- rc = raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2));
- if ( rc )
- return rc;
+ if ( raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2)) )
+ return -EFAULT;
rt = (hw2 >> 12) & 0x7;
@@ -88,11 +86,9 @@ bad_thumb2:
static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
{
uint16_t instr;
- int rc;
- rc = raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr));
- if ( rc )
- return rc;
+ if ( raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr)) )
+ return -EFAULT;
switch ( instr >> 12 )
{
@@ -163,3 +159,13 @@ int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt *dabt)
return 1;
}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 0269294..73a7cff 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -907,15 +907,15 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
static void dtb_load(struct kernel_info *kinfo)
{
void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
- unsigned long rc;
+ unsigned long left;
printk("Loading dom0 DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
kinfo->dtb_paddr, kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
- rc = raw_copy_to_guest_flush_dcache(dtb_virt, kinfo->fdt,
+ left = raw_copy_to_guest_flush_dcache(dtb_virt, kinfo->fdt,
fdt_totalsize(kinfo->fdt));
- if ( rc != 0 )
- panic("Unable to copy the DTB to dom0 memory (rc = %lu)", rc);
+ if ( left != 0 )
+ panic("Unable to copy the DTB to dom0 memory (left = %lu bytes)", left);
xfree(kinfo->fdt);
}
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 08800a4..bd0a355 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -6,21 +6,19 @@
#include <asm/guest_access.h>
static unsigned long raw_copy_to_guest_helper(void *to, const void *from,
- unsigned len, unsigned flush_dcache)
+ unsigned len, int flush_dcache)
{
/* XXX needs to handle faults */
unsigned offset = (vaddr_t)to & ~PAGE_MASK;
while ( len )
{
- int rc;
paddr_t g;
void *p;
unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
- rc = gvirt_to_maddr((vaddr_t) to, &g);
- if ( rc )
- return rc;
+ if ( gvirt_to_maddr((vaddr_t) to, &g) )
+ return len;
p = map_domain_page(g>>PAGE_SHIFT);
p += offset;
@@ -56,14 +54,12 @@ unsigned long raw_clear_guest(void *to, unsigned len)
while ( len )
{
- int rc;
paddr_t g;
void *p;
unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
- rc = gvirt_to_maddr((vaddr_t) to, &g);
- if ( rc )
- return rc;
+ if ( gvirt_to_maddr((vaddr_t) to, &g) )
+ return len;
p = map_domain_page(g>>PAGE_SHIFT);
p += offset;
@@ -84,14 +80,12 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned le
while ( len )
{
- int rc;
paddr_t g;
void *p;
unsigned size = min(len, (unsigned)(PAGE_SIZE - offset));
- rc = gvirt_to_maddr((vaddr_t) from & PAGE_MASK, &g);
- if ( rc )
- return rc;
+ if ( gvirt_to_maddr((vaddr_t) from & PAGE_MASK, &g) )
+ return len;
p = map_domain_page(g>>PAGE_SHIFT);
p += ((vaddr_t)from & (~PAGE_MASK));
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 399e546..26ca588 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1067,22 +1067,33 @@ static int xenmem_add_to_physmap_range(struct domain *d,
xen_ulong_t idx;
xen_pfn_t gpfn;
- rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1, 1);
- if ( rc < 0 )
+ if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs,
+ xatpr->size-1, 1)) )
+ {
+ rc = -EFAULT;
goto out;
+ }
- rc = copy_from_guest_offset(&gpfn, xatpr->gpfns, xatpr->size-1, 1);
- if ( rc < 0 )
+ if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns,
+ xatpr->size-1, 1)) )
+ {
+ rc = -EFAULT;
goto out;
+ }
rc = xenmem_add_to_physmap_one(d, xatpr->space,
xatpr->foreign_domid,
idx, gpfn);
-
- rc = copy_to_guest_offset(xatpr->errs, xatpr->size-1, &rc, 1);
if ( rc < 0 )
goto out;
+ if ( unlikely(copy_to_guest_offset(xatpr->errs,
+ xatpr->size-1, &rc, 1)) );
+ {
+ rc = -EFAULT;
+ goto out;
+ }
+
xatpr->size--;
/* Check for continuation if it's not the last interation */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest
2013-12-09 12:13 [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest Ian Campbell
@ 2013-12-09 15:14 ` Julien Grall
2013-12-09 15:48 ` Ian Campbell
2013-12-09 18:34 ` Julien Grall
1 sibling, 1 reply; 5+ messages in thread
From: Julien Grall @ 2013-12-09 15:14 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, tim, xen-devel
On 12/09/2013 12:13 PM, Ian Campbell wrote:
> This is a generic interface which is supposed to return the number of bytes
> which were not copied. Make it so.
>
> Update the incorrect callers prepare_dtb, decode_thumb{2} and
> xenmem_add_to_physmap_range.
>
> In the xenmem_add_to_physmap_range case, observe that we are not propagating
> errors from xenmem_add_to_physmap_one and do so.
>
> In the decode_thumb case and an emacs magic block to decode.c
>
> Make the flush_dcache parameter to the helper an int while at it.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
> ---
> v2: Fix xenmem_add_to_physmap_range, which was assuming -errno return codes.
> Fix raw_copy_from_guest and raw_clear_guest too
> ---
> xen/arch/arm/decode.c | 22 ++++++++++++++--------
> xen/arch/arm/domain_build.c | 8 ++++----
> xen/arch/arm/guestcopy.c | 20 +++++++-------------
> xen/arch/arm/mm.c | 23 +++++++++++++++++------
> 4 files changed, 42 insertions(+), 31 deletions(-)
>
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index abe9f26..8880c39 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -36,12 +36,10 @@ static void update_dabt(struct hsr_dabt *dabt, int reg,
> static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw1)
> {
> uint16_t hw2;
> - int rc;
> uint16_t rt;
>
> - rc = raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2));
> - if ( rc )
> - return rc;
> + if ( raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2)) )
> + return -EFAULT;
>
> rt = (hw2 >> 12) & 0x7;
>
> @@ -88,11 +86,9 @@ bad_thumb2:
> static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
> {
> uint16_t instr;
> - int rc;
>
> - rc = raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr));
> - if ( rc )
> - return rc;
> + if ( raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr)) )
> + return -EFAULT;
>
> switch ( instr >> 12 )
> {
> @@ -163,3 +159,13 @@ int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt *dabt)
>
> return 1;
> }
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 0269294..73a7cff 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -907,15 +907,15 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
> static void dtb_load(struct kernel_info *kinfo)
> {
> void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
> - unsigned long rc;
> + unsigned long left;
>
> printk("Loading dom0 DTB to 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
> kinfo->dtb_paddr, kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
>
> - rc = raw_copy_to_guest_flush_dcache(dtb_virt, kinfo->fdt,
> + left = raw_copy_to_guest_flush_dcache(dtb_virt, kinfo->fdt,
> fdt_totalsize(kinfo->fdt));
> - if ( rc != 0 )
> - panic("Unable to copy the DTB to dom0 memory (rc = %lu)", rc);
> + if ( left != 0 )
> + panic("Unable to copy the DTB to dom0 memory (left = %lu bytes)", left);
> xfree(kinfo->fdt);
> }
>
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index 08800a4..bd0a355 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -6,21 +6,19 @@
> #include <asm/guest_access.h>
>
> static unsigned long raw_copy_to_guest_helper(void *to, const void *from,
> - unsigned len, unsigned flush_dcache)
> + unsigned len, int flush_dcache)
> {
> /* XXX needs to handle faults */
> unsigned offset = (vaddr_t)to & ~PAGE_MASK;
>
> while ( len )
> {
> - int rc;
> paddr_t g;
> void *p;
> unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
>
> - rc = gvirt_to_maddr((vaddr_t) to, &g);
> - if ( rc )
> - return rc;
> + if ( gvirt_to_maddr((vaddr_t) to, &g) )
> + return len;
>
> p = map_domain_page(g>>PAGE_SHIFT);
> p += offset;
> @@ -56,14 +54,12 @@ unsigned long raw_clear_guest(void *to, unsigned len)
>
> while ( len )
> {
> - int rc;
> paddr_t g;
> void *p;
> unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
>
> - rc = gvirt_to_maddr((vaddr_t) to, &g);
> - if ( rc )
> - return rc;
> + if ( gvirt_to_maddr((vaddr_t) to, &g) )
> + return len;
>
> p = map_domain_page(g>>PAGE_SHIFT);
> p += offset;
> @@ -84,14 +80,12 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned le
>
> while ( len )
> {
> - int rc;
> paddr_t g;
> void *p;
> unsigned size = min(len, (unsigned)(PAGE_SIZE - offset));
>
> - rc = gvirt_to_maddr((vaddr_t) from & PAGE_MASK, &g);
> - if ( rc )
> - return rc;
> + if ( gvirt_to_maddr((vaddr_t) from & PAGE_MASK, &g) )
> + return len;
>
> p = map_domain_page(g>>PAGE_SHIFT);
> p += ((vaddr_t)from & (~PAGE_MASK));
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 399e546..26ca588 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1067,22 +1067,33 @@ static int xenmem_add_to_physmap_range(struct domain *d,
> xen_ulong_t idx;
> xen_pfn_t gpfn;
>
> - rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1, 1);
> - if ( rc < 0 )
> + if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs,
> + xatpr->size-1, 1)) )
> + {
> + rc = -EFAULT;
> goto out;
> + }
>
> - rc = copy_from_guest_offset(&gpfn, xatpr->gpfns, xatpr->size-1, 1);
> - if ( rc < 0 )
> + if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns,
> + xatpr->size-1, 1)) )
> + {
> + rc = -EFAULT;
> goto out;
> + }
>
> rc = xenmem_add_to_physmap_one(d, xatpr->space,
> xatpr->foreign_domid,
> idx, gpfn);
> -
> - rc = copy_to_guest_offset(xatpr->errs, xatpr->size-1, &rc, 1);
> if ( rc < 0 )
> goto out;
>
> + if ( unlikely(copy_to_guest_offset(xatpr->errs,
> + xatpr->size-1, &rc, 1)) );
> + {
> + rc = -EFAULT;
> + goto out;
> + }
> +
> xatpr->size--;
>
> /* Check for continuation if it's not the last interation */
>
--
Julien Grall
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest
2013-12-09 15:14 ` Julien Grall
@ 2013-12-09 15:48 ` Ian Campbell
0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2013-12-09 15:48 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, tim, xen-devel
On Mon, 2013-12-09 at 15:14 +0000, Julien Grall wrote:
> On 12/09/2013 12:13 PM, Ian Campbell wrote:
> > This is a generic interface which is supposed to return the number of bytes
> > which were not copied. Make it so.
> >
> > Update the incorrect callers prepare_dtb, decode_thumb{2} and
> > xenmem_add_to_physmap_range.
> >
> > In the xenmem_add_to_physmap_range case, observe that we are not propagating
> > errors from xenmem_add_to_physmap_one and do so.
> >
> > In the decode_thumb case and an emacs magic block to decode.c
> >
> > Make the flush_dcache parameter to the helper an int while at it.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest
2013-12-09 12:13 [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest Ian Campbell
2013-12-09 15:14 ` Julien Grall
@ 2013-12-09 18:34 ` Julien Grall
2013-12-10 9:29 ` Ian Campbell
1 sibling, 1 reply; 5+ messages in thread
From: Julien Grall @ 2013-12-09 18:34 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, tim, xen-devel
On 12/09/2013 12:13 PM, Ian Campbell wrote:
> This is a generic interface which is supposed to return the number of bytes
> which were not copied. Make it so.
>
> Update the incorrect callers prepare_dtb, decode_thumb{2} and
> xenmem_add_to_physmap_range.
>
> In the xenmem_add_to_physmap_range case, observe that we are not propagating
> errors from xenmem_add_to_physmap_one and do so.
>
> In the decode_thumb case and an emacs magic block to decode.c
>
> Make the flush_dcache parameter to the helper an int while at it.
Actually this patch is buggy, I can't anymore create a guest on midway. I get lots of:
Failed to map pfn to mfn rc:-14:0 pfn:3efd8 mfn:802a3
Failed to map pfn to mfn rc:-14:0 pfn:3e019 mfn:802a4
Failed to map pfn to mfn rc:-14:0 pfn:3ed92 mfn:802a5
...
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v2: Fix xenmem_add_to_physmap_range, which was assuming -errno return codes.
> Fix raw_copy_from_guest and raw_clear_guest too
> ---
[..]
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 399e546..26ca588 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1067,22 +1067,33 @@ static int xenmem_add_to_physmap_range(struct domain *d,
> xen_ulong_t idx;
> xen_pfn_t gpfn;
>
> - rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1, 1);
> - if ( rc < 0 )
> + if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs,
> + xatpr->size-1, 1)) )
> + {
> + rc = -EFAULT;
> goto out;
> + }
>
> - rc = copy_from_guest_offset(&gpfn, xatpr->gpfns, xatpr->size-1, 1);
> - if ( rc < 0 )
> + if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns,
> + xatpr->size-1, 1)) )
> + {
> + rc = -EFAULT;
> goto out;
> + }
>
> rc = xenmem_add_to_physmap_one(d, xatpr->space,
> xatpr->foreign_domid,
> idx, gpfn);
> -
> - rc = copy_to_guest_offset(xatpr->errs, xatpr->size-1, &rc, 1);
> if ( rc < 0 )
> goto out;
copy_to_guest_offset was fine before the "if ( rc < 0 )" because we want to copy the error in the xatpr->errs.
> + if ( unlikely(copy_to_guest_offset(xatpr->errs,
> + xatpr->size-1, &rc, 1)) );
You forgot to remote the ';' at the end.
Here a patch to fix this commit
======================================================================================
commit ce9b426051d742e116d43cbd0e15dc47f5fab88b
Author: Julien Grall <julien.grall@linaro.org>
Date: Mon Dec 9 18:29:50 2013 +0000
xen/arm: Fix regression after commit d963923
The commit d963923 "xen: arm: correct return value of
raw_copy_{to/from}_guest_*, raw_clear_guest" doesn't permit to boot guest
on Xen ARM.
Also we want to get the right rc in the error arrays, so move back
copy_to_guest function to the previous place.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6e6d7d4..4598866 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1108,16 +1108,17 @@ static int xenmem_add_to_physmap_range(struct domain *d,
rc = xenmem_add_to_physmap_one(d, xatpr->space,
xatpr->foreign_domid,
idx, gpfn);
- if ( rc < 0 )
- goto out;
if ( unlikely(copy_to_guest_offset(xatpr->errs,
- xatpr->size-1, &rc, 1)) );
+ xatpr->size-1, &rc, 1)) )
{
rc = -EFAULT;
goto out;
}
+ if ( rc < 0 )
+ goto out;
+
xatpr->size--;
/* Check for continuation if it's not the last interation */
--
Julien Grall
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest
2013-12-09 18:34 ` Julien Grall
@ 2013-12-10 9:29 ` Ian Campbell
0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2013-12-10 9:29 UTC (permalink / raw)
To: Julien Grall; +Cc: stefano.stabellini, tim, xen-devel
On Mon, 2013-12-09 at 18:34 +0000, Julien Grall wrote:
> On 12/09/2013 12:13 PM, Ian Campbell wrote:
> > This is a generic interface which is supposed to return the number of bytes
> > which were not copied. Make it so.
> >
> > Update the incorrect callers prepare_dtb, decode_thumb{2} and
> > xenmem_add_to_physmap_range.
> >
> > In the xenmem_add_to_physmap_range case, observe that we are not propagating
> > errors from xenmem_add_to_physmap_one and do so.
> >
> > In the decode_thumb case and an emacs magic block to decode.c
> >
> > Make the flush_dcache parameter to the helper an int while at it.
>
> Actually this patch is buggy, I can't anymore create a guest on midway. I get lots of:
> Failed to map pfn to mfn rc:-14:0 pfn:3efd8 mfn:802a3
> Failed to map pfn to mfn rc:-14:0 pfn:3e019 mfn:802a4
> Failed to map pfn to mfn rc:-14:0 pfn:3ed92 mfn:802a5
> ...
>
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > v2: Fix xenmem_add_to_physmap_range, which was assuming -errno return codes.
> > Fix raw_copy_from_guest and raw_clear_guest too
> > ---
>
> [..]
>
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 399e546..26ca588 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -1067,22 +1067,33 @@ static int xenmem_add_to_physmap_range(struct domain *d,
> > xen_ulong_t idx;
> > xen_pfn_t gpfn;
> >
> > - rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1, 1);
> > - if ( rc < 0 )
> > + if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs,
> > + xatpr->size-1, 1)) )
> > + {
> > + rc = -EFAULT;
> > goto out;
> > + }
> >
> > - rc = copy_from_guest_offset(&gpfn, xatpr->gpfns, xatpr->size-1, 1);
> > - if ( rc < 0 )
> > + if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns,
> > + xatpr->size-1, 1)) )
> > + {
> > + rc = -EFAULT;
> > goto out;
> > + }
> >
> > rc = xenmem_add_to_physmap_one(d, xatpr->space,
> > xatpr->foreign_domid,
> > idx, gpfn);
> > -
> > - rc = copy_to_guest_offset(xatpr->errs, xatpr->size-1, &rc, 1);
> > if ( rc < 0 )
> > goto out;
>
> copy_to_guest_offset was fine before the "if ( rc < 0 )" because we want to copy the error in the xatpr->errs.
>
> > + if ( unlikely(copy_to_guest_offset(xatpr->errs,
> > + xatpr->size-1, &rc, 1)) );
>
> You forgot to remote the ';' at the end.
Damn, that's what I get for making a last minute change to the patch!
>
> Here a patch to fix this commit
>
> ======================================================================================
>
> commit ce9b426051d742e116d43cbd0e15dc47f5fab88b
> Author: Julien Grall <julien.grall@linaro.org>
> Date: Mon Dec 9 18:29:50 2013 +0000
>
> xen/arm: Fix regression after commit d963923
>
> The commit d963923 "xen: arm: correct return value of
> raw_copy_{to/from}_guest_*, raw_clear_guest" doesn't permit to boot guest
> on Xen ARM.
>
> Also we want to get the right rc in the error arrays, so move back
> copy_to_guest function to the previous place.
Ah, I missed that the error check was in the copy_to_guest_offset and
thought it wasn't being tested at all, which is why I changed it.
I tried to clarify what was going on by expanding the commit log to:
Remove the stray semicolon from the end of the if statement.
Also we want to get the right rc in the error arrays, so we need to do the
copy_to_guest_offset before checking the rc returned by
xenmem_add_to_physmap_one.
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked + applied, thanks for catching this.
Ian.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-10 9:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-09 12:13 [PATCH v2] xen: arm: correct return value of raw_copy_{to/from}_guest_*, raw_clear_guest Ian Campbell
2013-12-09 15:14 ` Julien Grall
2013-12-09 15:48 ` Ian Campbell
2013-12-09 18:34 ` Julien Grall
2013-12-10 9:29 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.