On Wed, Nov 24, 2010 at 02:38:22PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> On 11/24/2010 01:08 PM, Colin Watson wrote:
> > On Tue, Nov 23, 2010 at 04:21:14PM +0000, Colin Watson wrote:
> >> It seems to me that we should probably include the emu implementation of
> >> grub_arch_sync_caches in the kernel when building for the emu platform.
> >> Does this patch look sane?
> >>
> >> 2010-11-23  Colin Watson  <cjwat...@ubuntu.com>
> >>
> >>    * grub-core/Makefile.core.def (kernel): Add kern/emu/cache.S for emu
> >>    platforms.
> >>    (grub-emu-lite): Remove kern/emu/cache.S.
> >
> > I went ahead and committed this; Manoel Rebelo Abranches seemed keen on
> > IRC, and it certainly doesn't break anything on x86 since the object is
> > essentially empty there.
> 
> Could you try the attached patch as a replacement for yours? Not that
> your patch doesn't solve the problem but it's more annoying for maintenance.

I think you meant to reply to
http://lists.gnu.org/archive/html/grub-devel/2010-11/msg00053.html
rather than to this thread.

Anyway, this looks fine and compiles fine, although I haven't tried
booting it.  One thing, though:

> +#define grub_swap_bytes16_compile_time(x) ((((x) & 0xff) << 8) | (((x) & 
> 0xff00) >> 8))
> +#define grub_swap_bytes32_compile_time(x) ((((x) & 0xff) << 24) | (((x) & 
> 0xff00) << 8) | (((x) & 0xff0000) >> 8) | (((x) & 0xff000000) >> 24))

Should the last one perhaps be 0xff000000UL, as in the grub_swap_bytes32
implementation?  I forget the exact rules here but it would at least
seem reasonable for the code to match as closely as possible for ease of
maintenance.

Thanks,

-- 
Colin Watson                                       [cjwat...@ubuntu.com]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to