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

[Simon Horman <horms@xxxxxxxxxxxx>]

On Tue, Jul 15, 2008 at 11:43:17AM +0900, Isaku Yamahata wrote:
> Looks good mostly. Some comments below.
> 
> On Tue, Jul 15, 2008 at 10:35:29AM +1000, Simon Horman wrote:

[snip]

> > Index: xen-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S
> > ===================================================================
> > --- xen-unstable.hg.orig/xen/arch/ia64/linux-xen/mca_asm.S  2008-07-15 
> > 10:12:30.000000000 +1000
> > +++ xen-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S       2008-07-15 
> > 10:22:40.000000000 +1000
> > @@ -352,7 +352,7 @@ ia64_do_tlb_purge:
> >     ;;
> >  (p7)       br.cond.sptk .vpd_not_mapped
> >     ;;
> > -   ptr.i r16,r18
> > +   ptr.d r16,r18
> >     ;;
> >     srlz.i
> >     ;;
> 
> Accidental change?

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-15 10:12:30.000000000 +1000
> > +++ xen-unstable.hg/xen/include/asm-ia64/linux-xen/linux/efi.h      
> > 2008-07-15 10:14:10.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)
> >  
> 
> I thinks efi_unamp_pal_code() is needed right before
> efi_map_pal_code(). Otherwise machine abort may happen
> when kernel issues nested frimware call because
> the above code tries to insert the same address
> conversion twice. (Or issue ptr.i in efi_map_pal_code())

That is a good point, but perhaps we could just do something like this
instead?

#define XEN_EFI_RR_ENTER(rr6, rr7) do {                 \
        rr6 = ia64_get_rr(6UL << 61);                   \
        rr7 = ia64_get_rr(7UL << 61);                   \
        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 {                 \
        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)

[snip]

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