Re: [Xen-ia64-devel] Re: [patch 14/15] ia64: kexec: Only map PAL when making EFI, PAL or SAL calls

[Simon Horman <horms@xxxxxxxxxxxx>]

On Wed, Jul 16, 2008 at 11:37:41AM +0900, Isaku Yamahata wrote:
> On Wed, Jul 16, 2008 at 12:20:54PM +1000, Simon Horman wrote:
> > Move PAL code from the Xen identity mapped region to the
> > EFI identity mapped region, which overlaps with guest virtual space.
> > 
> > Make sure that PAL memory is only pinned into the TLB when making
> > EFI, PAL or SAL calls.
> > 
> > This seems to be nice as it provides a symmetrical approach to
> > mapping an unmapping pal code.
> > 
> > However it would be just as safe, and arguably simpler just to map
> > the PAL code (once?) when the EFI RR is active - for instance very
> > early on in boot, or when calling XEN_EFI_RR_ENTER. In other words,
> > unpining is XEN_EFI_RR_LEAVE shouldn't be neccessary, as the EFI RR
> > should protect the memory from unwanted accesses by guests (or
> > the hypevisor for that matter).
> > 
> > This patch is mostly the work of Yamahata-san.
> > 
> > Cc: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
> > Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> > 
> > Tue, 15 Jul 2008 13:06:42 +1000
> > * Remove spurious change of ptr.i to ptr.d
> >   just above .vpd_not_mapped in ia64_do_tlb_purge.
> > 
> > Wed, 16 Jul 2008 12:16:51 +1000
> > * Unpin PAL in efi_map_pal_code() to ensure that it isn't present
> >   when pinned. Thanks to Yamahata-san.
> > 
> > Index: xen-unstable.hg/xen/arch/ia64/linux-xen/efi.c
> > ===================================================================
> > --- xen-unstable.hg.orig/xen/arch/ia64/linux-xen/efi.c      2008-07-16 
> > 11:53:25.000000000 +1000
> > +++ xen-unstable.hg/xen/arch/ia64/linux-xen/efi.c   2008-07-16 
> > 12:15:17.000000000 +1000
> > @@ -424,7 +424,7 @@ efi_get_pal_addr (void)
> >                     md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
> >                     vaddr & mask, (vaddr & mask) + IA64_GRANULE_SIZE);
> >  #endif
> > -           return __va(md->phys_addr);
> > +           return __va_efi(md->phys_addr);
> >     }
> >     printk(KERN_WARNING "%s: no PAL-code memory-descriptor found\n",
> >            __FUNCTION__);
> > @@ -432,7 +432,7 @@ efi_get_pal_addr (void)
> >  }
> >  
> >  #ifdef XEN
> > -void *pal_vaddr = 0;
> > +static void *pal_vaddr = 0;
> >  
> >  void *
> >  efi_get_pal_addr(void)
> > @@ -443,24 +443,53 @@ efi_get_pal_addr(void)
> >  }
> >  #endif
> >  
> > +#ifdef XEN
> > +static void
> > +__efi_unmap_pal_code (void *pal_vaddr)
> > +{
> > +   ia64_ptr(0x1, GRANULEROUNDDOWN((unsigned long)pal_vaddr),
> > +            IA64_GRANULE_SHIFT);
> > +}
> > +
> >  void
> > -efi_map_pal_code (void)
> > +efi_unmap_pal_code (void)
> >  {
> > -#ifdef XEN
> > -   u64 psr;
> > -   (void)efi_get_pal_addr();
> > -#else
> >     void *pal_vaddr = efi_get_pal_addr ();
> >     u64 psr;
> >  
> >     if (!pal_vaddr)
> >             return;
> > +
> > +   /*
> > +    * Cannot write to CRx with PSR.ic=1
> > +    */
> > +   psr = ia64_clear_ic();
> > +   __efi_unmap_pal_code(pal_vaddr);
> > +   ia64_ptr(0x1, GRANULEROUNDDOWN((unsigned long)pal_vaddr),
> > +            IA64_GRANULE_SHIFT);
> > +   ia64_set_psr(psr);              /* restore psr */
> > +   ia64_srlz_i();
> > +}
> 
> Forgot to delete ia64_ptr()?

Yes, I will remove it.

[snip]

> > Index: xen-unstable.hg/xen/include/asm-ia64/linux-xen/linux/efi.h
> > ===================================================================
> > --- xen-unstable.hg.orig/xen/include/asm-ia64/linux-xen/linux/efi.h 
> > 2008-07-16 11:53:24.000000000 +1000
> > +++ xen-unstable.hg/xen/include/asm-ia64/linux-xen/linux/efi.h      
> > 2008-07-16 11:54:29.000000000 +1000
> > @@ -24,10 +24,6 @@
> >  #include <asm/page.h>
> >  #include <asm/system.h>
> >  
> > -#ifdef XEN
> > -extern void * pal_vaddr;
> > -#endif
> > -
> >  #define EFI_SUCCESS                0
> >  #define EFI_LOAD_ERROR          ( 1 | (1UL << (BITS_PER_LONG-1)))
> >  #define EFI_INVALID_PARAMETER      ( 2 | (1UL << (BITS_PER_LONG-1)))
> > @@ -302,6 +298,9 @@ efi_guid_unparse(efi_guid_t *guid, char 
> >  extern void efi_init (void);
> >  extern void *efi_get_pal_addr (void);
> >  extern void efi_map_pal_code (void);
> > +#ifdef XEN
> > +extern void efi_unmap_pal_code (void);
> > +#endif
> >  extern void efi_map_memmap(void);
> >  extern void efi_memmap_walk (efi_freemem_callback_t callback, void *arg);
> >  extern void efi_gettimeofday (struct timespec *ts);
> > @@ -471,9 +470,11 @@ struct efi_generic_dev_path {
> >     rr7 = ia64_get_rr(7UL << 61);                   \
> >     set_one_rr_efi(6UL << 61, XEN_EFI_RR);          \
> >     set_one_rr_efi(7UL << 61, XEN_EFI_RR);          \
> > +   efi_map_pal_code();                             \
> >  } while (0)
> >  
> >  #define XEN_EFI_RR_LEAVE(rr6, rr7) do {                    \
> > +   efi_unmap_pal_code();                           \
> >     set_one_rr_efi(6UL << 61, rr6);                 \
> >     set_one_rr_efi(7UL << 61, rr7);                 \
> >  } while (0)
> > Index: xen-unstable.hg/xen/include/asm-ia64/vmx_vcpu.h
> 
> I think XEN_EFI_RR_LEAVE() should call efi_unmap_pal_code()
> conditionally with rr7 check because in nested firmware
> calling case, itr[IA64_TR_PALCODE] should not be purged.
> Otherwise it returns back to firmware, undesirable things would happen.

Very good point. I will put the check on rr7 back in.

[snip]

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel