From mboxrd@z Thu Jan 1 00:00:00 1970 From: Corneliu ZUZU Subject: Re: [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events Date: Mon, 7 Mar 2016 14:49:29 +0200 Message-ID: <56DD78D9.7060501@bitdefender.com> References: <1457014210-14552-1-git-send-email-czuzu@bitdefender.com> <56DD3A2B.6060803@bitdefender.com> <56DD7654.8010405@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7079331972970158590==" Return-path: In-Reply-To: <56DD7654.8010405@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Andrew Cooper , Tamas K Lengyel Cc: Kevin Tian , Keir Fraser , Jan Beulich , Razvan Cojocaru , Xen-devel , Stefano Stabellini , Jun Nakajima List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --===============7079331972970158590== Content-Type: multipart/alternative; boundary="------------070000060101060105070607" This is a multi-part message in MIME format. --------------070000060101060105070607 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 3/7/2016 2:38 PM, Andrew Cooper wrote: > 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 Nice, thanks a bunch. Corneliu. --------------070000060101060105070607 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
On 3/7/2016 2:38 PM, Andrew Cooper wrote:
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

Nice, thanks a bunch.

Corneliu.
--------------070000060101060105070607-- --===============7079331972970158590== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============7079331972970158590==--