[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2] xen/arm:alternative: Register re-mapped Xen area as a temporary virtual region



Hi Wei,

NIT: title: please add a space between : and alternative.

On 23/03/17 09:43, Wei Chen wrote:
While I was using the alternative patching in the SErrors patch series [1].
I used a branch instruction as alternative instruction.

        ALTERNATIVE("nop",
                    "b skip_check",
                    SKIP_CHECK_PENDING_VSERROR)

unfortunately, I got a system panic message with this code:

NIT: s/unfortunately/Unfortunately/


(XEN) build-id: f64081d86e7e88504b7d00e1486f25751c004e39
(XEN) alternatives: Patching with alt table 100b9480 -> 100b9498
(XEN) Xen BUG at alternative.c:61
(XEN) ----[ Xen-4.9-unstable  arm32  debug=y   Tainted:  C   ]----
(XEN) CPU:    0
(XEN) PC:     00252b68 alternative.c#__apply_alternatives+0x128/0x1d4
(XEN) CPSR:   800000da MODE:Hypervisor
(XEN)      R0: 00000000 R1: 00000000 R2: 100b9490 R3: 100b949c
(XEN)      R4: eafeff84 R5: 00000000 R6: 100b949c R7: 10079290
(XEN)      R8: 100792ac R9: 00000001 R10:100b948c R11:002cfe04 R12:002932c0
(XEN) HYP: SP: 002cfdc4 LR: 00239128
(XEN)
(XEN)   VTCR_EL2: 80003558
(XEN)  VTTBR_EL2: 0000000000000000
(XEN)
(XEN)  SCTLR_EL2: 30cd187f
(XEN)    HCR_EL2: 000000000038663f
(XEN)  TTBR0_EL2: 00000000bff09000
(XEN)
(XEN)    ESR_EL2: 00000000
(XEN)  HPFAR_EL2: 0000000000000000
(XEN)      HDFAR: 00000000
(XEN)      HIFAR: 00000000
(XEN)
(XEN) Xen stack trace from sp=002cfdc4:
(XEN)    00000000 00294328 002e0004 00000001 10079290 002cfe14 100b9490 00000000
(XEN)    10010000 10122700 00200000 002cfe1c 00000080 00252c14 00000000 002cfe64
(XEN)    00252dd8 00000007 00000000 000bfe00 100b9480 100b9498 002cfe1c 002cfe1c
(XEN)    10010000 10122700 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 002ddf30 00000000 003113e8 0030f018 002cfe9c
(XEN)    00238914 00000002 00000000 00000000 00000000 0028b000 00000002 00293800
(XEN)    00000002 0030f238 00000002 00290640 00000001 002cfea4 002a2840 002cff54
(XEN)    002a65fc 11112131 10011142 00000000 0028d194 00000000 00000000 00000000
(XEN)    bdffb000 80000000 00000000 c0000000 00000000 00000002 00000000 c0000000
(XEN)    002b8060 00002000 002b8040 00000000 c0000000 bc000000 00000000 c0000000
(XEN)    00000000 be000000 00000000 00112701 00000000 bff12701 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000018 00000000 00000001 00000000
(XEN)    9fece000 80200000 80000000 00400000 00200550 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
(XEN) Xen call trace:
(XEN)    [<00252b68>] alternative.c#__apply_alternatives+0x128/0x1d4 (PC)
(XEN)    [<00239128>] is_active_kernel_text+0x10/0x28 (LR)
(XEN)    [<00252dd8>] alternative.c#__apply_alternatives_multi_stop+0x1c4/0x204
(XEN)    [<00238914>] stop_machine_run+0x1e8/0x254
(XEN)    [<002a2840>] apply_alternatives_all+0x38/0x54
(XEN)    [<002a65fc>] start_xen+0xcf4/0xf88
(XEN)    [<00200550>] arm32/head.o#paging+0x94/0xd8
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at alternative.c:61
(XEN) ****************************************

This panic was triggered by the BUG(); in branch_insn_requires_update.
That's because in this case the alternative patching needs to update the
offset of the branch instruction. But the new target address of the branch
instruction could not pass the check of is_active_kernel_text();

The reason is that, we haven't registered this the re-mapped Xen area as
an active kernel text virtual region. In order to update branch instruction
offset, we have to update the Xen text section. But Xen text section is
marked as read-only and we configure the hardware to not allow a region
to be writable and executable at the same time. So re-map Xen in a temporary
area for writing.

The calculation of the new target address of the branch instruction is based
on this re-mapped area. The new target address will point to a value in the
re-mapped area. We haven't registered this area as an active kernel text. So
the check of is_active_kernel_text will always return false.

We have to register the re-mapped Xen area as a virtual region temporarily to
solve this problem,

s/,/./

The commit message is much better. However, you don't explain that this is only happening when booting Xen (apply_alternatives_all) and not livepatching or else.

So someone may wonder why you only register the Xen area in apply_alternatives_all.


Notes:
This bug will affect the staging, staging-4.8 and stable-4.8 source trees.

NIT: this is not necessary to keep in the commit message after the merge. So this should go after ---.

The code looks good to me.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.