From: Konrad Rzeszutek Wilk <konrad@kernel.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: sstabellini@kernel.org, andrew.cooper3@citrix.com,
Blaise Boscaccy <blaise.boscaccy@oracle.com>,
ross.lagerwall@citrix.com, julien.grall@arm.com,
Vegard Nossum <vegard.nossum@oracle.com>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v3 14/17] livepatch/x86/arm: arch/x86/mm: generalize do_page_walk() and implement arch_livepatch_lookup_mfn
Date: Tue, 12 Sep 2017 20:23:07 -0400 [thread overview]
Message-ID: <20170913002306.GB6632@localhost.localdomain> (raw)
In-Reply-To: <59B8114A020000780017A40A@prv-mh.provo.novell.com>
On Tue, Sep 12, 2017 at 08:54:34AM -0600, Jan Beulich wrote:
> >>> On 12.09.17 at 02:37, <konrad@kernel.org> wrote:
> > With this change we can use _do_page_walk() to implement
> > arch_livepatch_lookup_mfn() which can be used to find out
> > vmap virtual addresses (as under x86 virt_to_mfn won't work
> > for vmap, but it does for arm!).
>
> How about using domain_page_map_to_mfn() instead?
It is very colorfull:
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 667573c..f90d4c8 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -10,6 +10,7 @@
#include <xen/vmap.h>
#include <xen/livepatch_elf.h>
#include <xen/livepatch.h>
+#include <xen/domain_page.h>
#include <asm/nmi.h>
#include <asm/livepatch.h>
@@ -17,11 +18,18 @@
mfn_t arch_livepatch_lookup_mfn(unsigned long addr)
{
unsigned long cr3 = read_cr3() >> PAGE_SHIFT;
+ mfn_t r, r2;
+ uint64_t mfn;
if ( !mfn_valid(_mfn(cr3)) )
return INVALID_MFN;
- return _do_page_walk(cr3, addr);
+ r = _do_page_walk(cr3, addr);
+ mfn = domain_page_map_to_mfn((void *)addr);
+ r2 = _mfn(mfn);
+ WARN_ON( !mfn_eq(r, r2) );
+
+ return r;
}
void arch_livepatch_revive(void)
..
(XEN) livepatch.c:1450: livepatch: xen_hello_world: CPU1 - IPIing the other 3 CPUs
Applying xen_hello_world... (XEN) livepatch: xen_hello_world: Applying 1 functions
(XEN) Assertion 'va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END' failed at domain_page.c:349
(XEN) ----[ Xen-4.10-unstable x86_64 debug=y Tainted: C ]----
(XEN) CPU: 1
(XEN) RIP: e008:[<ffff82d080281d68>] domain_page_map_to_mfn+0x98/0xc8
(XEN) RFLAGS: 0000000000010012 CONTEXT: hypervisor
(XEN) rax: 000000d040200000 rbx: 000000000009e400 rcx: 0000000000000000
(XEN) rdx: 0000001080200000 rsi: 000000009e677000 rdi: ffff82d080200000
(XEN) rbp: ffff84842789fe18 rsp: ffff84842789fe18 r8: ffff8484278d83c0
(XEN) r9: 0000000000000004 r10: 0000000000000001 r11: 0000000000000001
(XEN) r12: ffff82d080200000 r13: ffff84843e220ab0 r14: 00000009b3af6068
(XEN) r15: 00000000ffffffff cr0: 000000008005003b cr4: 00000000000426e0
(XEN) cr3: 000000009e677000 cr2: ffff8800078d4328
(XEN) ds: 002b es: 002b fs: 0000 gs: 0000 ss: e010 cs: e008
(XEN) Xen code around <ffff82d080281d68> (domain_page_map_to_mfn+0x98/0xc8):
(XEN) 48 3d ff ff ff 1f 76 04 <0f> 0b eb fe 48 c1 e7 10 48 c1 ef 1c 48 b8 00 00
(XEN) Xen stack trace from rsp=ffff84842789fe18:
(XEN) ffff84842789fe38 ffff82d080286d05 0000000000000001 ffff82d08060b180
(XEN) ffff84842789fe88 ffff82d08021b8b6 ffff84842789fe58 ffff82d0802395a5
(XEN) 000000000000000d ffff84843e220bf0 ffff84843e220ab0 ffff84843e220bf0
(XEN) ffff84843e220ab0 00000009b3af6068 ffff84842789feb8 ffff82d08021bc38
(XEN) 0000000000000001 0000000000000001 ffff84843e220ab0 ffff84843e220ab0
(XEN) ffff84842789ff08 ffff82d08021befa ffff84842789fed8 0000000000000246
(XEN) 0000000000000000 ffff84809e9a5000 0000000000000001 ffff84809e6de000
(XEN) ffff84842788c000 00000000ffffffff ffff84842789fd78 ffff82d08027d4e4
(XEN) ffff8800394b6000 ffff8800394b6002 0000000000000000 0000000000000000
(XEN) ffffc900005f3e60 0000000000000001 0000000000000246 0000000000000000
(XEN) 0000000000000000 000000009b810048 0000000000000000 ffffffff810013aa
(XEN) 0000000000000001 deadbeefdeadf00d deadbeefdeadf00d 0000010000000000
(XEN) ffffffff810013aa 000000000000e033 0000000000000246 ffffc900005f3e48
(XEN) 000000000000e02b 42f0e7438e1e7fc8 06c64a65b2da83f9 6ddeafb16f2755f2
(XEN) 97e5fcb4dae8d065 ea89d67a00000001 ffff84809e9a5000 000001b3b6129680
(XEN) 00000000000426e0
(XEN) Xen call trace:
(XEN) [<ffff82d080281d68>] domain_page_map_to_mfn+0x98/0xc8
(XEN) [<ffff82d080286d05>] arch_livepatch_lookup_mfn+0x46/0x61
(XEN) [<ffff82d08021b8b6>] livepatch.c#livepatch_quiesce+0x45/0x1e4
(XEN) [<ffff82d08021bc38>] livepatch.c#apply_payload+0x3f/0x109
(XEN) [<ffff82d08021befa>] check_for_livepatch_work+0x1f8/0x395
(XEN) [<ffff82d08027d4e4>] domain.c#continue_idle_domain+0x1b/0x22
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 1:
(XEN) Assertion 'va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END' failed at domain_page.c:349
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...
Which is due to:
287 start = (void *)xen_virt_end;
288 end = (void *)(XEN_VIRT_END - NR_CPUS * PAGE_SIZE);
289
290 BUG_ON(end <= start);
291
292 vm_init_type(VMAP_XEN, start, end);
If I modify it a bit:
diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index 3432a854dd..a95e95b372 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -344,6 +344,11 @@ unsigned long domain_page_map_to_mfn(const void *ptr)
pl1e = virt_to_xen_l1e(va);
BUG_ON(!pl1e);
}
+ else if ( va >= xen_virt_end && va < (XEN_VIRT_END - NR_CPUS * PAGE_SIZE) )
+ {
+ pl1e = virt_to_xen_l1e(va);
+ BUG_ON(!pl1e);
+ }
else
{
ASSERT(va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END);
And then some extra debug:
(XEN) ptr=0xffff82004000d000, DIRECTMAP_VIRT_START=0xffff848000000000, VMAP_VIRT_START=0xffff82c000000000, VMAP_VIRT_END=0xffff82d000000000, xen_virt_end=0xffff82d080600000, end=0xffff82d0bfe00000
(XEN) ptr=0xffff820040014000, DIRECTMAP_VIRT_START=0xffff848000000000, VMAP_VIRT_START=0xffff82c000000000, VMAP_VIRT(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Assertion 'va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END' failed at domain_page.c:358
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...
I will take a look at it later this week, but regardless it will require
some tweaking as well. Do you prefer the domain_page_map_to_mfn path
in instead of walk_do_page one?
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-09-13 0:23 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-12 0:37 [PATCH v3] Livepatching patch set for 4.10 Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 01/17] livepatch: Expand check for safe_for_reapply if livepatch has only .rodata Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 02/17] livepatch: Tighten alignment checks Konrad Rzeszutek Wilk
2017-09-12 14:28 ` Jan Beulich
2017-09-12 0:37 ` [PATCH v3 03/17] livepatch: Include sizes when an mismatch occurs Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 04/17] xen/livepatch/ARM32: Don't load and crash on livepatches loaded with wrong text alignment Konrad Rzeszutek Wilk
2017-09-14 11:36 ` Julien Grall
2017-09-12 0:37 ` [PATCH v3 05/17] alternative/x86/arm32: Align altinstructions (and altinstr_replacement) sections Konrad Rzeszutek Wilk
2017-09-12 14:40 ` Jan Beulich
2017-09-12 0:37 ` [PATCH v3 06/17] xen/livepatch/x86/arm32: Force .livepatch.depends section to be uint32_t aligned Konrad Rzeszutek Wilk
2017-09-14 12:27 ` Julien Grall
2017-09-19 0:32 ` Konrad Rzeszutek Wilk
2017-09-19 11:05 ` Julien Grall
2017-09-20 14:01 ` Wei Liu
2017-09-20 21:17 ` Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 07/17] livepatch/arm/x86: Strip note_depends symbol from test-cases Konrad Rzeszutek Wilk
2017-09-12 14:48 ` Jan Beulich
2017-09-12 23:46 ` Konrad Rzeszutek Wilk
2017-09-13 8:51 ` Jan Beulich
2017-09-13 16:28 ` Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 08/17] livepatch/tests: Make sure all .livepatch.funcs sections are read-only Konrad Rzeszutek Wilk
2017-09-12 14:49 ` Jan Beulich
2017-09-19 0:36 ` Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 09/17] livepatch/arm[32, 64]: Modify livepatch_funcs Konrad Rzeszutek Wilk
2017-09-14 13:20 ` Julien Grall
2017-09-19 0:35 ` Konrad Rzeszutek Wilk
2017-09-19 11:09 ` Julien Grall
2017-09-12 0:37 ` [PATCH v3 10/17] livepatch: Declare live patching as a supported feature Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 11/17] livepatch/x86/arm[32, 64]: Use common vmap code for applying Konrad Rzeszutek Wilk
2017-09-12 14:50 ` Andrew Cooper
2017-09-12 0:37 ` [PATCH v3 12/17] livepatch/x86/arm[32, 64]: Unify arch_livepatch_revert Konrad Rzeszutek Wilk
2017-09-14 13:23 ` Julien Grall
2017-09-12 0:37 ` [PATCH v3 13/17] livepatch: Expand spin_debug_disable in [apply|revert]_payload Konrad Rzeszutek Wilk
2017-09-14 13:47 ` Julien Grall
2017-09-12 0:37 ` [PATCH v3 14/17] livepatch/x86/arm: arch/x86/mm: generalize do_page_walk() and implement arch_livepatch_lookup_mfn Konrad Rzeszutek Wilk
2017-09-12 14:54 ` Jan Beulich
2017-09-13 0:23 ` Konrad Rzeszutek Wilk [this message]
2017-09-13 8:54 ` Jan Beulich
2017-09-14 13:54 ` Julien Grall
2017-09-12 0:37 ` [PATCH v3 15/17] livepatch/x86/arm: Utilize the arch_livepatch_lookup_mfn Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 16/17] livepatch: Add local and global symbol resolution Konrad Rzeszutek Wilk
2017-09-12 0:37 ` [PATCH v3 17/17] livepatch: Add xen_local_symbols test-case Konrad Rzeszutek Wilk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170913002306.GB6632@localhost.localdomain \
--to=konrad@kernel.org \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=blaise.boscaccy@oracle.com \
--cc=julien.grall@arm.com \
--cc=ross.lagerwall@citrix.com \
--cc=sstabellini@kernel.org \
--cc=vegard.nossum@oracle.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).