[PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Tim Deegan]

Xenheap pages will always have an extra typecount, taken in
share_xen_page_with_guest(), which doesn't come from a shadow PTE.
Adjust the warning in sh_remove_all_mappings() to account for it.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/mm/shadow/common.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 4b576ac..adffa06 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2437,10 +2437,13 @@ int sh_remove_all_mappings(struct vcpu *v, mfn_t gmfn)
     {
         /* Don't complain if we're in HVM and there are some extra mappings: 
          * The qemu helper process has an untyped mapping of this dom's RAM 
-         * and the HVM restore program takes another. */
+         * and the HVM restore program takes another.
+         * Also allow one typed refcount for xenheap pages, to match
+         * share_xen_page_with_guest(). */
         if ( !(shadow_mode_external(v->domain)
                && (page->count_info & PGC_count_mask) <= 3
-               && (page->u.inuse.type_info & PGT_count_mask) == 0) )
+               && ((page->u.inuse.type_info & PGT_count_mask)
+                   == !!is_xen_heap_page(page))) )
         {
             SHADOW_ERROR("can't find all mappings of mfn %lx: "
                           "c=%08lx t=%08lx\n", mfn_x(gmfn), 
-- 
1.7.10.4

Re: [PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Andrew Cooper]

On 28/03/13 10:35, Tim Deegan wrote:
> Xenheap pages will always have an extra typecount, taken in
> share_xen_page_with_guest(), which doesn't come from a shadow PTE.
> Adjust the warning in sh_remove_all_mappings() to account for it.
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Tim Deegan <tim@xen.org>

Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/arch/x86/mm/shadow/common.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
> index 4b576ac..adffa06 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -2437,10 +2437,13 @@ int sh_remove_all_mappings(struct vcpu *v, mfn_t gmfn)
>      {
>          /* Don't complain if we're in HVM and there are some extra mappings: 
>           * The qemu helper process has an untyped mapping of this dom's RAM 
> -         * and the HVM restore program takes another. */
> +         * and the HVM restore program takes another.
> +         * Also allow one typed refcount for xenheap pages, to match
> +         * share_xen_page_with_guest(). */
>          if ( !(shadow_mode_external(v->domain)
>                 && (page->count_info & PGC_count_mask) <= 3
> -               && (page->u.inuse.type_info & PGT_count_mask) == 0) )
> +               && ((page->u.inuse.type_info & PGT_count_mask)
> +                   == !!is_xen_heap_page(page))) )
>          {
>              SHADOW_ERROR("can't find all mappings of mfn %lx: "
>                            "c=%08lx t=%08lx\n", mfn_x(gmfn), 

Re: [PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Jan Beulich]

>>> On 28.03.13 at 11:35, Tim Deegan <tim@xen.org> wrote:
> Xenheap pages will always have an extra typecount, taken in
> share_xen_page_with_guest(), which doesn't come from a shadow PTE.
> Adjust the warning in sh_remove_all_mappings() to account for it.
> 
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Tim Deegan <tim@xen.org>

I suppose this ought to go into 4.2.2 and 4.1.5 as well?

Jan

Re: [PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Tim Deegan]

At 09:11 +0100 on 05 Apr (1365153095), Jan Beulich wrote:
> >>> On 28.03.13 at 11:35, Tim Deegan <tim@xen.org> wrote:
> > Xenheap pages will always have an extra typecount, taken in
> > share_xen_page_with_guest(), which doesn't come from a shadow PTE.
> > Adjust the warning in sh_remove_all_mappings() to account for it.
> > 
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Tim Deegan <tim@xen.org>
> 
> I suppose this ought to go into 4.2.2 and 4.1.5 as well?

Could do -- it's just suppressing a printk, so not a particularly urgent
fix AFAICT.

Tim.

Re: [PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Jan Beulich]

>>> On 05.04.13 at 10:30, Tim Deegan <tim@xen.org> wrote:
> At 09:11 +0100 on 05 Apr (1365153095), Jan Beulich wrote:
>> >>> On 28.03.13 at 11:35, Tim Deegan <tim@xen.org> wrote:
>> > Xenheap pages will always have an extra typecount, taken in
>> > share_xen_page_with_guest(), which doesn't come from a shadow PTE.
>> > Adjust the warning in sh_remove_all_mappings() to account for it.
>> > 
>> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> > Signed-off-by: Tim Deegan <tim@xen.org>
>> 
>> I suppose this ought to go into 4.2.2 and 4.1.5 as well?
> 
> Could do -- it's just suppressing a printk, so not a particularly urgent
> fix AFAICT.

But the message, if present when inspecting some random log,
looks worrying, so getting rid of the false positive is likely going
to help assessing the health of systems/guests.

Jan

Re: [PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Tim Deegan]

At 09:37 +0100 on 05 Apr (1365154638), Jan Beulich wrote:
> >>> On 05.04.13 at 10:30, Tim Deegan <tim@xen.org> wrote:
> > At 09:11 +0100 on 05 Apr (1365153095), Jan Beulich wrote:
> >> >>> On 28.03.13 at 11:35, Tim Deegan <tim@xen.org> wrote:
> >> > Xenheap pages will always have an extra typecount, taken in
> >> > share_xen_page_with_guest(), which doesn't come from a shadow PTE.
> >> > Adjust the warning in sh_remove_all_mappings() to account for it.
> >> > 
> >> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> > Signed-off-by: Tim Deegan <tim@xen.org>
> >> 
> >> I suppose this ought to go into 4.2.2 and 4.1.5 as well?
> > 
> > Could do -- it's just suppressing a printk, so not a particularly urgent
> > fix AFAICT.
> 
> But the message, if present when inspecting some random log,
> looks worrying, so getting rid of the false positive is likely going
> to help assessing the health of systems/guests.

Sure.  In any case the patch is harmless, so there should be no risk to
putting it in.

Tim.

Re: [PATCH] x86/mm/shadow: spurious warning when unmapping xenheap pages.

[Andrew Cooper]

On 05/04/13 09:46, Tim Deegan wrote:
> At 09:37 +0100 on 05 Apr (1365154638), Jan Beulich wrote:
>>>>> On 05.04.13 at 10:30, Tim Deegan <tim@xen.org> wrote:
>>> At 09:11 +0100 on 05 Apr (1365153095), Jan Beulich wrote:
>>>>>>> On 28.03.13 at 11:35, Tim Deegan <tim@xen.org> wrote:
>>>>> Xenheap pages will always have an extra typecount, taken in
>>>>> share_xen_page_with_guest(), which doesn't come from a shadow PTE.
>>>>> Adjust the warning in sh_remove_all_mappings() to account for it.
>>>>>
>>>>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> Signed-off-by: Tim Deegan <tim@xen.org>
>>>> I suppose this ought to go into 4.2.2 and 4.1.5 as well?
>>> Could do -- it's just suppressing a printk, so not a particularly urgent
>>> fix AFAICT.
>> But the message, if present when inspecting some random log,
>> looks worrying, so getting rid of the false positive is likely going
>> to help assessing the health of systems/guests.
> Sure.  In any case the patch is harmless, so there should be no risk to
> putting it in.
>
> Tim.

The reason I started debugging it was because it is an scary message for
what turns out to be a safe action.

So please backport :)

~Andrew

>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel