Re: [PATCH] fix ia64 breakage with PHYSDEVOP_pirq_eoi_mfn (was Re: [PATCH 2/2] linux/x86: use shared page indicating the need for an EOI notification)

["Jan Beulich" <jbeulich@novell.com>]

>>> Isaku Yamahata <yamahata@valinux.co.jp> 09.12.08 04:40 >>>
>Sorry for breakage.
>How about moving arbitrary_virt_to_machine() from pgtable.h to maddr.h?
>(Yes, this is a work around. and you wouldn't like it...)

No, that wouldn't work for x86 either, because the macro uses
lookup_address(), which in turn is also only declared in pgtable.h. I
wouldn't really mind moving arbitrary_virt_to_machine(), but it would
then require duplicating (not moving) the lookup_address() declaration.

>> Looking at how ia64 defines virt_to_machine() at present I would be
>> inclined to say that all current users (tpmfront, blktap, and gntdev) of
>> that macro don't get what they expect, and the implementation you
>> added for arbitary_virt_to_machine() really ought to be the one for
>> virt_to_machine(), given your description above.
>
>Looking the x86 virt_to_machine definition, virt_to_machine()
>assumes the passed address in the straight mapping area.
>So the ia64 assumption is same to x86.

Not exactly: Addresses of kernel objects *can* be passed to
virt_to_machine() on x86 (minus a supposed compiler issue demanding
the special __pa_symbol() to be used on x86-64 - I'm trying to find out
how relevant this still is), but they can't be on ia64. This is what seemed
wrong to me. But otoh as I understand it you can't pass kernel
addresses through __pa() either, but (to my surprise) ia64 apparently
has no problem with this wrt architecture independent code (but making
necessary work-arounds like paddr_vmcoreinfo_note()).

>Hmm, ia64 and x86_64 have nothing to do with highmem,
>but x86_32 has to deal with highmem. So x86_32 with highmem
>seems to have the issue you described above.
>If ptep which is passed to virt_to_machine is highmem,
>I don't see how it works. So all virt_to_machine() shouldn't
>be changed to arbitrary_virt_to_machine()?

Actually, looking at it a second time, tpmfront uses the result of the result
of __get_free_page() here, so the address is always in the 1:1 mapping.

But I think you're quite right about the HIGHPTE implications on blktap and
gntdev - these ought to be fixed, perhaps indeed by using
arbitrary_virt_to_machine() there (but I'd want to make this conditional
upon the HIGHPTE config option, so to not affect performance of other
configurations: possibly this ought to be an architecture-defined macro
like ptep_virt_to_machine(), as I wouldn't want to place an x86-specific
conditional in there that would risk breaking any future supported
architecture without explicit notice).

Jan