From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xu Zhang Subject: Re: Nested events in 64bit mini-OS Date: Sun, 11 Nov 2012 09:36:22 -0600 Message-ID: <509FC5F6.4050506@cs.uic.edu> References: <50871D90.50606@gmail.com> <20121025205653.GW5925@type.chello.at> <5098A674.5090203@cs.uic.edu> <20121110135218.GA5436@type> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20121110135218.GA5436@type> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Samuel Thibault , "xen-devel@lists.xen.org" , gm281@cam.ac.uk List-Id: xen-devel@lists.xenproject.org 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