From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page() Date: Thu, 23 Oct 2014 14:57:19 +0100 Message-ID: <5449093F.6080500@citrix.com> References: <54492111020000780004175B@mail.emea.novell.com> <5449226E020000780004176D@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0389125227699784216==" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XhIui-0008KY-8E for xen-devel@lists.xenproject.org; Thu, 23 Oct 2014 13:58:52 +0000 In-Reply-To: <5449226E020000780004176D@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============0389125227699784216== Content-Type: multipart/alternative; boundary="------------020400050000020304060101" --------------020400050000020304060101 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable On 23/10/14 14:44, Jan Beulich wrote: > In the event of a #PF while inan EFI runtime service function we "in an" > otherwise can't dump the page tables, making the analysis of the > problem more cumbersome. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/domain_page.c > +++ b/xen/arch/x86/domain_page.c > @@ -7,6 +7,7 @@ > */ > =20 > #include > +#include > #include > #include > #include > @@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr > */ > if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcp= u(v) ) > { > + unsigned long cr3; > + This will need an __maybe_unused to compile in a non-debug build. > /* If we really are idling, perform lazy context switch now. *= / > if ( (v =3D idle_vcpu[smp_processor_id()]) =3D=3D current ) > sync_local_execstate(); > /* We must now be running on the idle page table. */ > - ASSERT(read_cr3() =3D=3D __pa(idle_pg_table)); > + ASSERT((cr3 =3D read_cr3()) =3D=3D __pa(idle_pg_table) || > + (efi_enabled && cr3 =3D=3D efi_rs_page_table())); > } > =20 > return v; > --- a/xen/arch/x86/efi/stub.c > +++ b/xen/arch/x86/efi/stub.c > @@ -9,6 +9,12 @@ const bool_t efi_enabled =3D 0; > =20 > void __init efi_init_memory(void) { } > =20 > +paddr_t efi_rs_page_table(void) > +{ > + BUG(); > + return 0; Is the return strictly needed? The __builtin_unreachable() in BUG() should prevent the compiler from complaining. ~Andrew > +} > + > unsigned long efi_get_time(void) > { > BUG(); > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -98,6 +98,11 @@ void efi_rs_leave(unsigned long cr3) > stts(); > } > =20 > +paddr_t efi_rs_page_table(void) > +{ > + return virt_to_maddr(efi_l4_pgtable); > +} > + > unsigned long efi_get_time(void) > { > EFI_TIME time; > --- a/xen/include/xen/efi.h > +++ b/xen/include/xen/efi.h > @@ -28,6 +28,7 @@ struct xenpf_efi_runtime_call; > struct compat_pf_efi_runtime_call; > =20 > void efi_init_memory(void); > +paddr_t efi_rs_page_table(void); > unsigned long efi_get_time(void); > void efi_halt_system(void); > void efi_reset_system(bool_t warm); > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------020400050000020304060101 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 8bit
On 23/10/14 14:44, Jan Beulich wrote:
In the event of a #PF while inan EFI runtime service function we

"in an"

otherwise can't dump the page tables, making the analysis of the
problem more cumbersome.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -7,6 +7,7 @@
  */
 
 #include <xen/domain_page.h>
+#include <xen/efi.h>
 #include <xen/mm.h>
 #include <xen/perfc.h>
 #include <xen/pfn.h>
@@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
      */
     if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
     {
+        unsigned long cr3;
+

This will need an __maybe_unused to compile in a non-debug build.

         /* If we really are idling, perform lazy context switch now. */
         if ( (v = idle_vcpu[smp_processor_id()]) == current )
             sync_local_execstate();
         /* We must now be running on the idle page table. */
-        ASSERT(read_cr3() == __pa(idle_pg_table));
+        ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
+               (efi_enabled && cr3 == efi_rs_page_table()));
     }
 
     return v;
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
 
 void __init efi_init_memory(void) { }
 
+paddr_t efi_rs_page_table(void)
+{
+    BUG();
+    return 0;

Is the return strictly needed?  The __builtin_unreachable() in BUG() should prevent the compiler from complaining.

~Andrew

+}
+
 unsigned long efi_get_time(void)
 {
     BUG();
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -98,6 +98,11 @@ void efi_rs_leave(unsigned long cr3)
     stts();
 }
 
+paddr_t efi_rs_page_table(void)
+{
+    return virt_to_maddr(efi_l4_pgtable);
+}
+
 unsigned long efi_get_time(void)
 {
     EFI_TIME time;
--- a/xen/include/xen/efi.h
+++ b/xen/include/xen/efi.h
@@ -28,6 +28,7 @@ struct xenpf_efi_runtime_call;
 struct compat_pf_efi_runtime_call;
 
 void efi_init_memory(void);
+paddr_t efi_rs_page_table(void);
 unsigned long efi_get_time(void);
 void efi_halt_system(void);
 void efi_reset_system(bool_t warm);






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

--------------020400050000020304060101-- --===============0389125227699784216== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============0389125227699784216==--