Re: [Xen-devel] [PATCH RFC 01/12] x86/paging: introduce paging_set_allocation

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, Aug 02, 2016 at 11:47:22AM +0200, Roger Pau Monne wrote:
> On Fri, Jul 29, 2016 at 05:47:24PM +0100, Andrew Cooper wrote:
> > On 29/07/16 17:28, Roger Pau Monne wrote:
> > > diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
> > > index 107fc8c..1b270df 100644
> > > --- a/xen/arch/x86/mm/paging.c
> > > +++ b/xen/arch/x86/mm/paging.c
> > > @@ -953,6 +953,22 @@ void paging_write_p2m_entry(struct p2m_domain *p2m, 
> > > unsigned long gfn,
> > >          safe_write_pte(p, new);
> > >  }
> > >  
> > > +int paging_set_allocation(struct domain *d, unsigned long pages)
> > > +{
> > > +    int rc;
> > > +
> > > +    ASSERT(paging_mode_enabled(d));
> > > +
> > > +    paging_lock(d);
> > > +    if ( hap_enabled(d) )
> > > +        rc = hap_set_allocation(d, pages, NULL);
> > > +    else
> > > +        rc = sh_set_allocation(d, pages, NULL);
> > 
> > (without looking at the rest of the series) Your NMI is probably a
> > watchdog timeout from this call, as passing NULL means "non-preemptible".
> 
> I don't think so, the NMI I saw happened while the guest was booting.
> 
> > As this is for the construction of dom0, it would be better to take a
> > preemptible pointer, loop in construct_dom0(), with a
> > process_pending_softirqs() call in between.
> 
> Now fixed.

Hm, I have to stand corrected, using hypercall_preempt_check (as 
any of the *_set_allocation function use), is not safe at this point:

(XEN) ----[ Xen-4.8-unstable  x86_64  debug=y  Tainted:    C  ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82d08022fd47>] 
hap.c#local_events_need_delivery+0x27/0x40
(XEN) RFLAGS: 0000000000010246   CONTEXT: hypervisor
(XEN) rax: 0000000000000000   rbx: ffff83023f5a5000   rcx: ffff82d080312900
(XEN) rdx: 0000000000000001   rsi: ffff83023f5a56c8   rdi: ffff8300b213d000
(XEN) rbp: ffff82d080307cc8   rsp: ffff82d080307cc8   r8:  0180000000000000
(XEN) r9:  0000000000000000   r10: 0000000000247000   r11: ffff82d08029a5b0
(XEN) r12: 0000000000000011   r13: 00000000000023ac   r14: ffff82d080307d4c
(XEN) r15: ffff83023f5a56c8   cr0: 000000008005003b   cr4: 00000000001526e0
(XEN) cr3: 00000000b20fc000   cr2: 0000000000000000
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: e008
(XEN) Xen code around <ffff82d08022fd47> 
(hap.c#local_events_need_delivery+0x27/0x40):
(XEN)  0d ad fa ff 48 8b 47 08 <80> 38 00 74 09 80 78 01 00 0f 94 c0 eb 02 31 c0
(XEN) Xen stack trace from rsp=ffff82d080307cc8:
(XEN)    ffff82d080307d08 ffff82d08022fc47 0000000000000000 ffff83023f5a5000
(XEN)    ffff83023f5a5648 0000000000000000 ffff82d080307d4c 0000000000002400
(XEN)    ffff82d080307d38 ffff82d08020008c 00000000000ffffd ffff8300b1efd000
(XEN)    ffff83023f5a5000 ffff82d080307d4c ffff82d080307d78 ffff82d0802cad30
(XEN)    0000000000203000 ffff83023f5a5000 ffff82d0802bf860 0000000000000000
(XEN)    0000000000000001 ffff83000008bef0 ffff82d080307de8 ffff82d0802c91e0
(XEN)    ffff82d080307de8 ffff82d080143900 ffff82d080307de8 0000000000000000
(XEN)    ffff83000008bf00 ffff82d0802eb480 ffff82d080307dc4 ffff82d08028b1cd
(XEN)    ffff83000008bf00 0000000000000000 0000000000000001 ffff83023f5a5000
(XEN)    ffff82d080307f08 ffff82d0802bf0c9 0000000000000000 0000000000000000
(XEN)    0000000000000000 ffff82d080307f18 ffff83000008bee0 0000000000000001
(XEN)    0000000000000001 0000000000000001 0000000000000000 0000000000100000
(XEN)    0000000000000001 0000000000247000 ffff83000008bef4 0000000000100000
(XEN)    ffff830100000000 0000000000247001 0000000000000014 0000000100000000
(XEN)    ffff8300ffffffec ffff83000008bef0 ffff82d0802e0640 ffff83000008bfb0
(XEN)    0000000000000000 0000000000000000 0000000000000111 0000000800000000
(XEN)    000000010000006e 0000000000000003 00000000000002f8 0000000000000000
(XEN)    00000000ad5c0bd0 0000000000000000 0000000000000001 0000000000000008
(XEN)    0000000000000000 ffff82d080100073 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN) Xen call trace:
(XEN)    [<ffff82d08022fd47>] hap.c#local_events_need_delivery+0x27/0x40
(XEN)    [<ffff82d08022fc47>] hap_set_allocation+0x107/0x130
(XEN)    [<ffff82d08020008c>] paging_set_allocation+0x4c/0x80
(XEN)    [<ffff82d0802cad30>] domain_build.c#hvm_setup_p2m+0x70/0x1a0
(XEN)    [<ffff82d0802c91e0>] domain_build.c#construct_dom0_hvm+0x60/0x120
(XEN)    [<ffff82d0802bf0c9>] __start_xen+0x1ea9/0x23a0
(XEN)    [<ffff82d080100073>] __high_start+0x53/0x60
(XEN)
(XEN) Pagetable walk from 0000000000000000:
(XEN)  L4[0x000] = 0000000245233063 ffffffffffffffff
(XEN)  L3[0x000] = 0000000245232063 ffffffffffffffff
(XEN)  L2[0x000] = 0000000245231063 ffffffffffffffff
(XEN)  L1[0x000] = 0000000000000000 ffffffffffffffff
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=0000]
(XEN) Faulting linear address: 0000000000000000
(XEN) ****************************************

I've tried setting current to d->vcpu[0], but that just makes the call to 
hypercall_preempt_check crash in some scheduler assert. In any case, I've 
added the preempt parameter to the paging_set_allocation function, but I 
don't plan to use it in the domain builder for the time being. Does that 
sound right?

Roger.

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