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

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

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.
-- 
yamahata

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