Re: [Xen-devel] Nested events in 64bit mini-OS

[Xu Zhang <xzhang@xxxxxxxxxx>]

Samuel,

Thanks for the reply.

On 11/10/2012 07:52 AM, Samuel Thibault wrote:

> You can simply run it, giving it a blk dev (or also a fb dev), it will
> run app_main(), which keeps reading stuff, and draws squares on the fb.
>
> Ideally you should manage to trigger fixups, by stuffing a wait loop in
> the critical section.
I'll try this in the following week. Thank you for pointing it out.

> > A follow-up question is that given this fix, do you still need
> > hypercall iret?
> Actually it's worse than that, see below.
>
> > +       movq %rbx,5*8(%rsp)
> ...
> > +
> > +       # check against re-entrance
> > +       movq RIP(%rsp),%rbx
> > +       cmpq $scrit,%rbx
> > +       jb 10f
> > +       cmpq $ecrit,%rbx
> > +       jb  critical_region_fixup
> > +
> > +10:    movq RBX(%rsp),%rbx                     # restore rbx
> Do we really need to restore %rbx?  I don't think do_hypervisor_callback
> uses it.
No I don't think so. I was just being pre-cautious. :-)

> > +    .byte 0x78,0x78,0x78,0x78,0x78            # jmp    hypercall_page + 
> > (__HYPERVISOR_iret * 32)
> Here we would also need a fixup table for the code at hypercall_page!
Nice catch!

> A nicer fix would be to inline the hypercall code here.  That said, I
> have to say I don't know any detail about the NMI flag, I don't see it
> defined in the Intel manual, and I don't see it being set in the Xen
> hypervisor.  Unless somebody knows more about it, I would assume that it
> currently never happens, and simply stuff a
>
> ud2 /* TODO: fix re-entrance fixup for hypercall in NMI case (when does that 
> happen actually?) */
>
> before the jmp, to catch it if it ever does happen.
I don't know that either. I tried to rise guest domain an NMI with "xl 
trigger", but it seems that only works for HVM hosts.
> Also, it would be good to check against critical section size change, in
> case somebody e.g. changes a value, or a macro like XEN_PUT_VCPU_INFO.
> For instance, stuff right after the table:
>
>         .if (ecrit-scrit) != (critical_fixup_table_end - critical_fixup_table)
>         .error "The critical has changed, the fixup table needs updating"
>         .endif
Totally agree. And it somewhat saves the obligation of checking if the 
table size matches the one of critical section by looking at disassemble 
output. :-)
> More generally, some cleanup could go along the patch, but I'd say
> keep it as you have done, focused on only the fixup code, to have it
> as such in the repository history, and then we could clean some things
> afterwards.
>
> Samuel
I really appreciate your feedback. Thanks again, Samuel!

Xu

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