* [PATCH for-4.10 0/2] xen/arm: Add more debug in get_page_from_gva
@ 2017-11-15 19:34 Julien Grall
2017-11-15 19:34 ` [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr Julien Grall
2017-11-15 19:34 ` [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva Julien Grall
0 siblings, 2 replies; 9+ messages in thread
From: Julien Grall @ 2017-11-15 19:34 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, Julien Grall
Hi all,
It looks like get_page_from_gva intermittenly fails on the Arndale (see [1])
leading to Dom0 crashing.
At the moment it is a bit hard to know why given the hypervisor does not
provide much information.
This series add more debug in get_page_from_gva to hopefully narrow down
the issue.
I think the 2 patches are good candidate for Xen 4.10 as this may help fixing
a dom0 crash on the Arndale.
Cheers,
[1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
Julien Grall (2):
xen/arm: mm: Change the return value of gvirt_to_maddr
xen/arm: p2m: Add more debug in get_page_from_gva
xen/arch/arm/domain_build.c | 8 ++++----
xen/arch/arm/kernel.c | 8 ++++----
xen/arch/arm/p2m.c | 19 ++++++++++++++++---
xen/include/asm-arm/mm.h | 9 +++++++--
4 files changed, 31 insertions(+), 13 deletions(-)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr
2017-11-15 19:34 [PATCH for-4.10 0/2] xen/arm: Add more debug in get_page_from_gva Julien Grall
@ 2017-11-15 19:34 ` Julien Grall
2017-11-16 1:35 ` Stefano Stabellini
2017-11-15 19:34 ` [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva Julien Grall
1 sibling, 1 reply; 9+ messages in thread
From: Julien Grall @ 2017-11-15 19:34 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, Julien Grall
Currently, gvirt_to_maddr return -EFAULT when the translation failed.
It might be useful to return the PAR_EL1 (Physical Address Register)
in such a case to get a better idea of the reason.
So modify the return value to use 0 on sucess or return the PAR on
failure.
The callers are modified to reflect the change of the return value.
Note that with the change in gvirt_to_maddr, ma needs to be initialized
to avoid GCC been confused (i.e value may be unitialized) with the new
construction.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/domain_build.c | 8 ++++----
xen/arch/arm/kernel.c | 8 ++++----
xen/arch/arm/p2m.c | 6 +++---
xen/include/asm-arm/mm.h | 9 +++++++--
4 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index bf29299707..c74f4dd69d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2002,15 +2002,15 @@ static void initrd_load(struct kernel_info *kinfo)
for ( offs = 0; offs < len; )
{
- int rc;
- paddr_t s, l, ma;
+ uint64_t par;
+ paddr_t s, l, ma = 0;
void *dst;
s = offs & ~PAGE_MASK;
l = min(PAGE_SIZE - s, len);
- rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
- if ( rc )
+ par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
+ if ( par )
{
panic("Unable to translate guest address");
return;
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index c2755a9ab9..a6c6413712 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -167,15 +167,15 @@ static void kernel_zimage_load(struct kernel_info *info)
paddr, load_addr, load_addr + len);
for ( offs = 0; offs < len; )
{
- int rc;
- paddr_t s, l, ma;
+ uint64_t par;
+ paddr_t s, l, ma = 0;
void *dst;
s = offs & ~PAGE_MASK;
l = min(PAGE_SIZE - s, len);
- rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
- if ( rc )
+ par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
+ if ( par )
{
panic("Unable to map translate guest address");
return;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 68b488997d..f6b3d8e421 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1414,7 +1414,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
struct p2m_domain *p2m = p2m_get_hostp2m(d);
struct page_info *page = NULL;
paddr_t maddr = 0;
- int rc;
+ uint64_t par;
/*
* XXX: To support a different vCPU, we would need to load the
@@ -1425,9 +1425,9 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
p2m_read_lock(p2m);
- rc = gvirt_to_maddr(va, &maddr, flags);
+ par = gvirt_to_maddr(va, &maddr, flags);
- if ( rc )
+ if ( par )
goto err;
if ( !mfn_valid(maddr_to_mfn(maddr)) )
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index cd6dfb54b9..ad2f2a43dc 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -266,11 +266,16 @@ static inline void *maddr_to_virt(paddr_t ma)
}
#endif
-static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
+/*
+ * Translate a guest virtual address to a machine address.
+ * Return the fault information if the translation has failed else 0.
+ */
+static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
+ unsigned int flags)
{
uint64_t par = gva_to_ma_par(va, flags);
if ( par & PAR_F )
- return -EFAULT;
+ return par;
*pa = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
return 0;
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva
2017-11-15 19:34 [PATCH for-4.10 0/2] xen/arm: Add more debug in get_page_from_gva Julien Grall
2017-11-15 19:34 ` [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr Julien Grall
@ 2017-11-15 19:34 ` Julien Grall
2017-11-15 19:43 ` Andrew Cooper
2017-11-16 1:36 ` Stefano Stabellini
1 sibling, 2 replies; 9+ messages in thread
From: Julien Grall @ 2017-11-15 19:34 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, Julien Grall
The function get_page_from_gva is used by copy_*_guest helpers to
translate a guest virtual address to a machine physical address and take
reference on the page.
There are a couple of errors path that will return the same value making
difficult to know the exact error. Add more debug in each error patch
only for debug-build.
This should help narrowing down the intermittent failure with the
hypercall GNTTABOP_copy (see [1]).
[1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/p2m.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f6b3d8e421..417609ede2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
par = gvirt_to_maddr(va, &maddr, flags);
if ( par )
+ {
+ dprintk(XENLOG_G_DEBUG,
+ "%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx par=%#"PRIx64"\n",
+ v, va, flags, par);
goto err;
+ }
if ( !mfn_valid(maddr_to_mfn(maddr)) )
+ {
+ dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
+ v, mfn_x(maddr_to_mfn(maddr)));
goto err;
+ }
page = mfn_to_page(maddr_to_mfn(maddr));
ASSERT(page);
if ( unlikely(!get_page(page, d)) )
+ {
+ dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN %#"PRI_mfn"\n",
+ v, mfn_x(maddr_to_mfn(maddr)));
page = NULL;
+ }
err:
if ( !page && p2m->mem_access_enabled )
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva
2017-11-15 19:34 ` [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva Julien Grall
@ 2017-11-15 19:43 ` Andrew Cooper
2017-11-15 21:42 ` Julien Grall
2017-11-16 1:36 ` Stefano Stabellini
1 sibling, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2017-11-15 19:43 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: sstabellini
On 15/11/17 19:34, Julien Grall wrote:
> The function get_page_from_gva is used by copy_*_guest helpers to
> translate a guest virtual address to a machine physical address and take
> reference on the page.
>
> There are a couple of errors path that will return the same value making
> difficult to know the exact error. Add more debug in each error patch
> only for debug-build.
>
> This should help narrowing down the intermittent failure with the
> hypercall GNTTABOP_copy (see [1]).
>
> [1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
> xen/arch/arm/p2m.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index f6b3d8e421..417609ede2 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
> par = gvirt_to_maddr(va, &maddr, flags);
>
> if ( par )
> + {
> + dprintk(XENLOG_G_DEBUG,
> + "%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx par=%#"PRIx64"\n",
> + v, va, flags, par);
Given the long round-trip time on debugging output, how about trying to
dump the guest and/or second stage table walk?
~Andrew
> goto err;
> + }
>
> if ( !mfn_valid(maddr_to_mfn(maddr)) )
> + {
> + dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
> + v, mfn_x(maddr_to_mfn(maddr)));
> goto err;
> + }
>
> page = mfn_to_page(maddr_to_mfn(maddr));
> ASSERT(page);
>
> if ( unlikely(!get_page(page, d)) )
> + {
> + dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN %#"PRI_mfn"\n",
> + v, mfn_x(maddr_to_mfn(maddr)));
> page = NULL;
> + }
>
> err:
> if ( !page && p2m->mem_access_enabled )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva
2017-11-15 19:43 ` Andrew Cooper
@ 2017-11-15 21:42 ` Julien Grall
0 siblings, 0 replies; 9+ messages in thread
From: Julien Grall @ 2017-11-15 21:42 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: sstabellini
Hi Andrew,
On 11/15/2017 07:43 PM, Andrew Cooper wrote:
> On 15/11/17 19:34, Julien Grall wrote:
>> The function get_page_from_gva is used by copy_*_guest helpers to
>> translate a guest virtual address to a machine physical address and take
>> reference on the page.
>>
>> There are a couple of errors path that will return the same value making
>> difficult to know the exact error. Add more debug in each error patch
>> only for debug-build.
>>
>> This should help narrowing down the intermittent failure with the
>> hypercall GNTTABOP_copy (see [1]).
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>> xen/arch/arm/p2m.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index f6b3d8e421..417609ede2 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>> par = gvirt_to_maddr(va, &maddr, flags);
>>
>> if ( par )
>> + {
>> + dprintk(XENLOG_G_DEBUG,
>> + "%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx par=%#"PRIx64"\n",
>> + v, va, flags, par);
>
> Given the long round-trip time on debugging output, how about trying to
> dump the guest and/or second stage table walk?
I thought about it, however at the moment dump_s1_guest_walk() is very
minimal and would be add much value here. Thought, Now that we have code
to do first-stage walk (see guest_walk_tables), we might be able to get
a better dump here. Thought I am not sure it would be 4.10 material.
However, I think we could try to translate the guest VA to a guest PA
using hardware instruction and then do the second-stage walk using
dump_p2m_lookup.
Let me have a look.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr
2017-11-15 19:34 ` [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr Julien Grall
@ 2017-11-16 1:35 ` Stefano Stabellini
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Stabellini @ 2017-11-16 1:35 UTC (permalink / raw)
To: Julien Grall; +Cc: sstabellini, xen-devel
On Wed, 15 Nov 2017, Julien Grall wrote:
> Currently, gvirt_to_maddr return -EFAULT when the translation failed.
> It might be useful to return the PAR_EL1 (Physical Address Register)
> in such a case to get a better idea of the reason.
>
> So modify the return value to use 0 on sucess or return the PAR on
^ success
> failure.
>
> The callers are modified to reflect the change of the return value.
>
> Note that with the change in gvirt_to_maddr, ma needs to be initialized
> to avoid GCC been confused (i.e value may be unitialized) with the new
^ uninitialized
> construction.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
I fixed on commit
> ---
> xen/arch/arm/domain_build.c | 8 ++++----
> xen/arch/arm/kernel.c | 8 ++++----
> xen/arch/arm/p2m.c | 6 +++---
> xen/include/asm-arm/mm.h | 9 +++++++--
> 4 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index bf29299707..c74f4dd69d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2002,15 +2002,15 @@ static void initrd_load(struct kernel_info *kinfo)
>
> for ( offs = 0; offs < len; )
> {
> - int rc;
> - paddr_t s, l, ma;
> + uint64_t par;
> + paddr_t s, l, ma = 0;
> void *dst;
>
> s = offs & ~PAGE_MASK;
> l = min(PAGE_SIZE - s, len);
>
> - rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> - if ( rc )
> + par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> + if ( par )
> {
> panic("Unable to translate guest address");
> return;
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index c2755a9ab9..a6c6413712 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -167,15 +167,15 @@ static void kernel_zimage_load(struct kernel_info *info)
> paddr, load_addr, load_addr + len);
> for ( offs = 0; offs < len; )
> {
> - int rc;
> - paddr_t s, l, ma;
> + uint64_t par;
> + paddr_t s, l, ma = 0;
> void *dst;
>
> s = offs & ~PAGE_MASK;
> l = min(PAGE_SIZE - s, len);
>
> - rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> - if ( rc )
> + par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> + if ( par )
> {
> panic("Unable to map translate guest address");
> return;
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 68b488997d..f6b3d8e421 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1414,7 +1414,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> struct page_info *page = NULL;
> paddr_t maddr = 0;
> - int rc;
> + uint64_t par;
>
> /*
> * XXX: To support a different vCPU, we would need to load the
> @@ -1425,9 +1425,9 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>
> p2m_read_lock(p2m);
>
> - rc = gvirt_to_maddr(va, &maddr, flags);
> + par = gvirt_to_maddr(va, &maddr, flags);
>
> - if ( rc )
> + if ( par )
> goto err;
>
> if ( !mfn_valid(maddr_to_mfn(maddr)) )
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index cd6dfb54b9..ad2f2a43dc 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -266,11 +266,16 @@ static inline void *maddr_to_virt(paddr_t ma)
> }
> #endif
>
> -static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
> +/*
> + * Translate a guest virtual address to a machine address.
> + * Return the fault information if the translation has failed else 0.
> + */
> +static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
> + unsigned int flags)
> {
> uint64_t par = gva_to_ma_par(va, flags);
> if ( par & PAR_F )
> - return -EFAULT;
> + return par;
> *pa = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
> return 0;
> }
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva
2017-11-15 19:34 ` [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva Julien Grall
2017-11-15 19:43 ` Andrew Cooper
@ 2017-11-16 1:36 ` Stefano Stabellini
2017-11-16 8:50 ` Julien Grall
1 sibling, 1 reply; 9+ messages in thread
From: Stefano Stabellini @ 2017-11-16 1:36 UTC (permalink / raw)
To: Julien Grall; +Cc: sstabellini, xen-devel
On Wed, 15 Nov 2017, Julien Grall wrote:
> The function get_page_from_gva is used by copy_*_guest helpers to
> translate a guest virtual address to a machine physical address and take
> reference on the page.
>
> There are a couple of errors path that will return the same value making
^ paths
> difficult to know the exact error. Add more debug in each error patch
^ it difficult
> only for debug-build.
>
> This should help narrowing down the intermittent failure with the
> hypercall GNTTABOP_copy (see [1]).
>
> [1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
fixed on commit
> ---
> xen/arch/arm/p2m.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index f6b3d8e421..417609ede2 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
> par = gvirt_to_maddr(va, &maddr, flags);
>
> if ( par )
> + {
> + dprintk(XENLOG_G_DEBUG,
> + "%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx par=%#"PRIx64"\n",
> + v, va, flags, par);
> goto err;
> + }
>
> if ( !mfn_valid(maddr_to_mfn(maddr)) )
> + {
> + dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
> + v, mfn_x(maddr_to_mfn(maddr)));
> goto err;
> + }
>
> page = mfn_to_page(maddr_to_mfn(maddr));
> ASSERT(page);
>
> if ( unlikely(!get_page(page, d)) )
> + {
> + dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN %#"PRI_mfn"\n",
> + v, mfn_x(maddr_to_mfn(maddr)));
> page = NULL;
> + }
>
> err:
> if ( !page && p2m->mem_access_enabled )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva
2017-11-16 1:36 ` Stefano Stabellini
@ 2017-11-16 8:50 ` Julien Grall
2017-11-16 19:00 ` Stefano Stabellini
0 siblings, 1 reply; 9+ messages in thread
From: Julien Grall @ 2017-11-16 8:50 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Andrew Cooper, xen-devel
On 11/16/2017 01:36 AM, Stefano Stabellini wrote:
> On Wed, 15 Nov 2017, Julien Grall wrote:
>> The function get_page_from_gva is used by copy_*_guest helpers to
>> translate a guest virtual address to a machine physical address and take
>> reference on the page.
>>
>> There are a couple of errors path that will return the same value making
> ^ paths
>
>> difficult to know the exact error. Add more debug in each error patch
> ^ it difficult
>
>
>> only for debug-build.
>>
>> This should help narrowing down the intermittent failure with the
>> hypercall GNTTABOP_copy (see [1]).
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
>
> fixed on commit
I am not sure why this was merged given Andrew gave some comments...
>
>
>> ---
>> xen/arch/arm/p2m.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index f6b3d8e421..417609ede2 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>> par = gvirt_to_maddr(va, &maddr, flags);
>>
>> if ( par )
>> + {
>> + dprintk(XENLOG_G_DEBUG,
>> + "%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx par=%#"PRIx64"\n",
>> + v, va, flags, par);
>> goto err;
>> + }
>>
>> if ( !mfn_valid(maddr_to_mfn(maddr)) )
>> + {
>> + dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
>> + v, mfn_x(maddr_to_mfn(maddr)));
>> goto err;
>> + }
>>
>> page = mfn_to_page(maddr_to_mfn(maddr));
>> ASSERT(page);
>>
>> if ( unlikely(!get_page(page, d)) )
>> + {
>> + dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN %#"PRI_mfn"\n",
>> + v, mfn_x(maddr_to_mfn(maddr)));
>> page = NULL;
>> + }
>>
>> err:
>> if ( !page && p2m->mem_access_enabled )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva
2017-11-16 8:50 ` Julien Grall
@ 2017-11-16 19:00 ` Stefano Stabellini
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Stabellini @ 2017-11-16 19:00 UTC (permalink / raw)
To: Julien Grall; +Cc: Andrew Cooper, Stefano Stabellini, xen-devel
On Thu, 16 Nov 2017, Julien Grall wrote:
> On 11/16/2017 01:36 AM, Stefano Stabellini wrote:
> > On Wed, 15 Nov 2017, Julien Grall wrote:
> > > The function get_page_from_gva is used by copy_*_guest helpers to
> > > translate a guest virtual address to a machine physical address and take
> > > reference on the page.
> > >
> > > There are a couple of errors path that will return the same value making
> > ^ paths
> >
> > > difficult to know the exact error. Add more debug in each error patch
> > ^ it difficult
> >
> >
> > > only for debug-build.
> > >
> > > This should help narrowing down the intermittent failure with the
> > > hypercall GNTTABOP_copy (see [1]).
> > >
> > > [1] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00942.html
> > >
> > > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >
> > Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> >
> > fixed on commit
>
> I am not sure why this was merged given Andrew gave some comments...
Sorry Andrew! I missed your reply!
> >
> > > ---
> > > xen/arch/arm/p2m.c | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > > index f6b3d8e421..417609ede2 100644
> > > --- a/xen/arch/arm/p2m.c
> > > +++ b/xen/arch/arm/p2m.c
> > > @@ -1428,16 +1428,29 @@ struct page_info *get_page_from_gva(struct vcpu
> > > *v, vaddr_t va,
> > > par = gvirt_to_maddr(va, &maddr, flags);
> > > if ( par )
> > > + {
> > > + dprintk(XENLOG_G_DEBUG,
> > > + "%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx
> > > par=%#"PRIx64"\n",
> > > + v, va, flags, par);
> > > goto err;
> > > + }
> > > if ( !mfn_valid(maddr_to_mfn(maddr)) )
> > > + {
> > > + dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n",
> > > + v, mfn_x(maddr_to_mfn(maddr)));
> > > goto err;
> > > + }
> > > page = mfn_to_page(maddr_to_mfn(maddr));
> > > ASSERT(page);
> > > if ( unlikely(!get_page(page, d)) )
> > > + {
> > > + dprintk(XENLOG_G_DEBUG, "%pv: Failing to acquire the MFN
> > > %#"PRI_mfn"\n",
> > > + v, mfn_x(maddr_to_mfn(maddr)));
> > > page = NULL;
> > > + }
> > > err:
> > > if ( !page && p2m->mem_access_enabled )
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-16 19:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-15 19:34 [PATCH for-4.10 0/2] xen/arm: Add more debug in get_page_from_gva Julien Grall
2017-11-15 19:34 ` [PATCH for-4.10 1/2] xen/arm: mm: Change the return value of gvirt_to_maddr Julien Grall
2017-11-16 1:35 ` Stefano Stabellini
2017-11-15 19:34 ` [PATCH for-4.10 2/2] xen/arm: p2m: Add more debug in get_page_from_gva Julien Grall
2017-11-15 19:43 ` Andrew Cooper
2017-11-15 21:42 ` Julien Grall
2017-11-16 1:36 ` Stefano Stabellini
2017-11-16 8:50 ` Julien Grall
2017-11-16 19:00 ` Stefano Stabellini
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.