From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events Date: Mon, 7 Mar 2016 12:38:44 +0000 Message-ID: <56DD7654.8010405@citrix.com> References: <1457014210-14552-1-git-send-email-czuzu@bitdefender.com> <56DD3A2B.6060803@bitdefender.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7367345843848816505==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Tamas K Lengyel , Corneliu ZUZU Cc: Kevin Tian , Keir Fraser , Jan Beulich , Razvan Cojocaru , Xen-devel , Stefano Stabellini , Jun Nakajima List-Id: xen-devel@lists.xenproject.org --===============7367345843848816505== Content-Type: multipart/alternative; boundary="------------070203010504020809080306" --------------070203010504020809080306 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit On 07/03/16 09:12, Tamas K Lengyel wrote: > > > On Mon, Mar 7, 2016 at 9:22 AM, Corneliu ZUZU > wrote: > > On 3/3/2016 4:10 PM, Corneliu ZUZU wrote: > > Then, > QUESTIONS (FOR VM-EVENTS & ARM MAINTAINERS ESPECIALLY): > > Q1) [...] > > Q2) [...] > > Q3) [...] > > Q4) [...] > > > Hey all, > > I have a question relating to this part of code @ vmx_update_guest_cr: > > if ( paging_mode_hap(v->domain) ) > { > /* Manage GUEST_CR3 when CR0.PE =0. */ > uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING | > CPU_BASED_CR3_STORE_EXITING); > v->arch.hvm_vmx.exec_control &= ~cr3_ctls; > if ( !hvm_paging_enabled(v) && > !vmx_unrestricted_guest(v) ) > v->arch.hvm_vmx.exec_control |= cr3_ctls; > > /* Trap CR3 updates if CR3 memory events are enabled. */ > if ( v->domain->arch.monitor.write_ctrlreg_enabled & > monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) ) > v->arch.hvm_vmx.exec_control |= > CPU_BASED_CR3_LOAD_EXITING; > > vmx_update_cpu_exec_control(v); > } > > While trying to move the check for VM_EVENT_X86_CR3 to the > scheduling tail, a few questions came to my mind. > > 1). Tamas, Razvan, maybe you guys could clarify this. I noticed > this part of code is only executed if paging_mode_hap(v->domain). > Is EPT mandatory to monitor CR3 writes or is it just that when > shadow paging is enabled, CR3 r/w are unconditionally trapped? > > > EPT is not really required for CR3 monitoring, it just has been the > case that vm_events have been only implemented for hap-enabled > domains. AFAIK for non-hap case CR3 needs to be trapped > unconditionally, yes. Specifically, the shadow pagetable code needs to swap shadows when the guest switches cr3. > > > If the former is true, shouldn't we do a check like this in > vm_event_monitor_get_capabilities instead? > > > Yes, it should now, this code was just written before > vm_event_monitor_get_capabilities was introduced and we haven't gotten > around converting this check to it. > > > > 2). I was also wondering why CR3 load/stores are trapped if paging > is disabled for a domain. > > > Good question, I was wondering about that myself at some point but I > haven't found an answer to it. Maybe some git archaeology can help > determining when that was added and why ;) Gen1 VT-x didn't support running a guest in non-paged mode. Gen2 introduced "unrestricted-guest" which works as intended, but Gen1 has to fake non-pagad mode using identity paging. As a result, CR3 cannot be used as scratch space like it can in non-paged mode, and the guest must be prevented from moving CR3 away from the gfn set up by the domain builder in HVM_PARAM_IDENT_PT. ~Andrew --------------070203010504020809080306 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
On 07/03/16 09:12, Tamas K Lengyel wrote:


On Mon, Mar 7, 2016 at 9:22 AM, Corneliu ZUZU <czuzu@bitdefender.com> wrote:
On 3/3/2016 4:10 PM, Corneliu ZUZU wrote:
Then,
QUESTIONS (FOR VM-EVENTS & ARM MAINTAINERS ESPECIALLY):

 Q1) [...]

Q2) [...]

Q3) [...]

Q4) [...]

Hey all,

I have a question relating to this part of code @ vmx_update_guest_cr:

        if ( paging_mode_hap(v->domain) )
        {
            /* Manage GUEST_CR3 when CR0.PE=0. */
            uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
                                 CPU_BASED_CR3_STORE_EXITING);
            v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
            if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
                v->arch.hvm_vmx.exec_control |= cr3_ctls;

            /* Trap CR3 updates if CR3 memory events are enabled. */
            if ( v->domain->arch.monitor.write_ctrlreg_enabled &
                 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
                v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;

             vmx_update_cpu_exec_control(v);
        }

While trying to move the check for VM_EVENT_X86_CR3 to the scheduling tail, a few questions came to my mind.

1). Tamas, Razvan, maybe you guys could clarify this. I noticed this part of code is only executed if paging_mode_hap(v->domain). Is EPT mandatory to monitor CR3 writes or is it just that when shadow paging is enabled, CR3 r/w are unconditionally trapped?

EPT is not really required for CR3 monitoring, it just has been the case that vm_events have been only implemented for hap-enabled domains. AFAIK for non-hap case CR3 needs to be trapped unconditionally, yes.

Specifically, the shadow pagetable code needs to swap shadows when the guest switches cr3.

 
If the former is true, shouldn't we do a check like this in vm_event_monitor_get_capabilities instead?

Yes, it should now, this code was just written before vm_event_monitor_get_capabilities was introduced and we haven't gotten around converting this check to it.
 

2). I was also wondering why CR3 load/stores are trapped if paging is disabled for a domain.

Good question, I was wondering about that myself at some point but I haven't found an answer to it. Maybe some git archaeology can help determining when that was added and why ;)

Gen1 VT-x didn't support running a guest in non-paged mode.  Gen2 introduced "unrestricted-guest" which works as intended, but Gen1 has to fake non-pagad mode using identity paging.  As a result, CR3 cannot be used as scratch space like it can in non-paged mode, and the guest must be prevented from moving CR3 away from the gfn set up by the domain builder in HVM_PARAM_IDENT_PT.

~Andrew
--------------070203010504020809080306-- --===============7367345843848816505== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============7367345843848816505==--