On Tue, Jul 15, 2008 at 11:26:29AM +0900, Isaku Yamahata wrote:
> On Mon, Jul 14, 2008 at 10:22:19PM +1000, Simon Horman wrote:
[snip]
> > > --- a/xen/include/asm-ia64/linux-xen/linux/efi.h Mon Jul 14 19:31:54
> > > 2008 +0900
> > > +++ b/xen/include/asm-ia64/linux-xen/linux/efi.h Mon Jul 14 19:33:20
> > > 2008 +0900
> > > @@ -25,6 +25,7 @@
> > > #include <asm/system.h>
> > >
> > > #ifdef XEN
> > > +#include <asm/meminit.h> /* GRANULEROUNDDOWN */
> > > extern void * pal_vaddr;
> > > #endif
> > >
> > > @@ -474,6 +475,10 @@
> > > } while (0)
> > >
> > > #define XEN_EFI_RR_RESTORE(rr6, rr7) do { \
> > > + ia64_ptr(0x1 /*I*/, \
> > > + GRANULEROUNDDOWN( \
> > > + (unsigned long)pal_vaddr), \
> > > + IA64_GRANULE_SHIFT); \
> > > set_one_rr_efi(6UL << 61, rr6); \
> > > set_one_rr_efi(7UL << 61, rr7); \
> >
> > I don't think this is quite right because ia64_new_rr7_efi
> > (via set_one_rr_efi()) will just reinsert the pal_vaddr.
> >
> > I think it might be better to make things a bit more symmetrical.
> > Pin pal_vaddr in XEN_EFI_RR_SAVE after calling set_one_rr_efi(),
> > unpin it in XEN_EFI_RR_RESTORE (as above) and not touch it at all in
> > set_one_rr_efi().
> >
>
> Agreed. I also came up that it should be more symmetorical.
> It should that ia64_new_rr7_efi() shouldn't pin down pal code and
> the macros should be something like
>
> #define XEN_EFI_RR_SAVE
> set_one_rr_efi(rr6)
> set_one_rr_efi(rr7)
> pin down pal code
> ia64_ptr()
> ia64_itr()
> Probably efi_map_pal_code() can be stolen.
> Maybe inline function?
>
> #define XEN_EFI_RR_RESTORE
> purge pinning pal code
> ia64_ptr(...)
> set_one_rr_efi(rr6)
> set_one_rr_efi(rr7)
Yes, that is pretty much what I was thinking.
It seemed nice to create a efi_unmap_pal_code() as well.
I sent a new series this morning, which includes this patch.
Let me know what you think.
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home