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 10:16:56AM +0900, Isaku Yamahata wrote:
> Some comments below
> 
> On Tue, Jul 15, 2008 at 04:25:45PM +1000, Simon Horman wrote:
> [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-15 16:11:00.000000000 +1000
> > +++ xen-unstable.hg/xen/include/asm-ia64/linux-xen/linux/efi.h      
> > 2008-07-15 16:16:56.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);
> > @@ -469,13 +468,19 @@ struct efi_generic_dev_path {
> >  #define XEN_EFI_RR_ENTER(rr6, rr7) do {                    \
> >     rr6 = ia64_get_rr(6UL << 61);                   \
> >     rr7 = ia64_get_rr(7UL << 61);                   \
> > -   set_one_rr_efi(6UL << 61, XEN_EFI_RR);          \
> > -   set_one_rr_efi(7UL << 61, XEN_EFI_RR);          \
> > +   if (rr7 != XEN_EFI_RR) {                        \
> > +           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 {                    \
> > -   set_one_rr_efi(6UL << 61, rr6);                 \
> > -   set_one_rr_efi(7UL << 61, rr7);                 \
> > +   if (rr7 != XEN_EFI_RR) {                        \
> > +           efi_unmap_pal_code();                   \
> > +           set_one_rr_efi(6UL << 61, rr6);         \
> > +           set_one_rr_efi(7UL << 61, rr7);         \
> > +   }                                               \
> >  } while (0)
> >  
> >  #else
> 
> I think that rr7 check in XEN_EFI_RR_LEAVE is necessary.
> On the other hand, the rr7 check in XEN_EFI_RR_ENTRY might cause
> problems because there is a window between setting rr[7] = XEN_EFI_RR
> and itr[IA64_TR_PALCODE].
> The window can be closed by always setting rr's and calling
> efi_unmap_pal_code() and efi_map_pal_code() in XEN_EFI_RR_ENTRY().
> 
> I may be too paranoiac, though.

Better to be paranoid than broken. I will incoporate
your idea of unpining before pinning in efi_map_pal_code()

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