[Xen-ia64-devel] Re: [patch 5/5] IA64: Kexec: Use a separate RID for EFI

[Alex Williamson <alex.williamson@xxxxxx>]

On Mon, 2007-10-22 at 11:49 +0900, Simon Horman wrote:
> The patch does seem to work, in the sense that the EFI mappings work.
> I have not stress tested it to see if domains can still do nefarious
> things. I would appreciate a review of this.

Hi Simon,

   This generally looks ok, but I'd like to get an ack from Isaku and
Tristan.

> The patch is rather long, this is almost entirely due to
> duplicating the PAL, EFI and SAL macros in order to squeeze
> in the RID changes and wrap them in #if XEN. It would be shorter
> if I didn't bother with #if XEN. I'm happy to do that if its
> the prefered solution.

   #ifdef XEN is simply for us to try to track changes from upstream so
we can re-base in the future.  I think anything that makes it obvious
what changes are for Xen would be sufficient.  Since this is only used
in very tight code, perhaps we could get away w/ something like:

#ifdef XEN
#define XEN_EFI_RR_SAVE() do {          \
        unsigned long rr6, rr7;         \
        rr6 = ia64_get_rr(6);           \
        rr7 = ia64_get_rr(7);           \
        ia64_set_rr(6, EFI_RID);        \
        ia64_set_rr(7, EFI_RID);        \
        ia64_srlz_d()

#define XEN_EFI_RR_RESTORE()            \
        ia64_set_rr(6, rr6);            \
        ia64_set_rr(7, rr7);            \
        ia64_srlz_d();                  \
        } while (0)
#else
#define XEN_EFI_RR_SAVE()       do {} while (0)
#define XEN_EFI_RR_RESTORE()    do {} while (0)
#endif

> The patch also includes some header foo. This is basically
> because pal.h needs to know about GRANULE_SIZE, and thus needs
> pgtable.h. But without the header manipulation I added a loop
> is formed. I can break this out into another patch if need be.
> I'm also happy to examine alternative solutions to this problem,
> but to be frank, the headers are a mess.

   I'd prefer a separate patch to make it clear what's happening.  Also,
is this the reason for the duplicate EFI_RID definitions?  I don't like
that ivt.S has it's own definition.  Perhaps including pal.h got missed
there, although I'm not sure pal.h is the appropriate place to define
EFI_RID, EFI_RR_RESTORE and EFI_RR_SAVE.  Thanks,

        Alex
-- 
Alex Williamson                             HP Open Source & Linux Org.


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