xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Xu Zhang <xzhang@cs.uic.edu>
To: Samuel Thibault <samuel.thibault@ens-lyon.org>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	gm281@cam.ac.uk
Subject: Re: Nested events in 64bit mini-OS
Date: Tue, 13 Nov 2012 19:49:27 -0600	[thread overview]
Message-ID: <50A2F8A7.5000202@cs.uic.edu> (raw)
In-Reply-To: <509FC5F6.4050506@cs.uic.edu>


>>> +    .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.
After a second thought, it seems to me such "expansion" of 
hypercall_page into fixup table is not necessary (and wrong). Insead, we 
can and need to mask out events before we doing a hypercall iret. I 
think it is safe (and must, see below) to do this because:
1. if event is disabled: doesn't hurt to mask it again;
2. if event is enabled: we disable event, and jumps to hypercall_page to 
make a hypercall iret, which eventually calls do_iret:

In do_iret, line 309:
      /* Restore upcall mask from supplied EFLAGS.IF. */
      vcpu_info(v, evtchn_upcall_mask) = !(iret_saved.rflags & 
X86_EFLAGS_IF);

No matter what the current value of upcall mask is, do_iret always 
retores it to the negation of rflags.IF saved on stack. In other words, 
it is safe to mask upcall mask before hypercall iret. And in fact, I am 
afraid it's necessary that we make it a must, because even one can guard 
against nested events in guest OS, but can never do so on hypercall page 
(because the code is in Xen). We should never jump to hypercall_page 
iret with events enabled. So we have to set upcall mask before the jump. 
I guess that's one of the reason why Xen always restores upcall mask 
from stack.


>> 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. :-)
>
>
Correct me if I am wrong, I think hypercall_page is mapped at runtime to 
guest OS by Xen. It's not actually part of the critical section of guest 
OS, at least not at compile time. So having a fixup table for the code 
at hypercall_page would cause such check to fail (mismatch). Following 
the discussion above, we could easily avoid such fixup table by mask out 
the events.

I don't know if this make sense at all. :-)

  reply	other threads:[~2012-11-14  1:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-23 22:43 Nested events in 64bit mini-OS Xu Zhang
2012-10-25 20:56 ` Samuel Thibault
2012-11-06  5:56   ` Xu Zhang
2012-11-10 13:52     ` Samuel Thibault
2012-11-11 15:36       ` Xu Zhang
2012-11-14  1:49         ` Xu Zhang [this message]
2012-11-18 17:43           ` Samuel Thibault
2012-11-19 10:22             ` Ian Campbell
2012-11-19 10:21       ` Ian Campbell

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=50A2F8A7.5000202@cs.uic.edu \
    --to=xzhang@cs.uic.edu \
    --cc=gm281@cam.ac.uk \
    --cc=samuel.thibault@ens-lyon.org \
    --cc=xen-devel@lists.xen.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).