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
|