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)

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]

>>> Isaku Yamahata <yamahata@valinux.co.jp> 03.12.08 10:20 >>>
>Yes, you're correct. In fact I had the patch which you suggested,
>but I was hesitated to change the x86 implementation so that
>I had changed it to use virt_to_bus() on x86.
>
>
>
>evtchn, physdev: fix pirq_eoi_mfn for IA64 support.
>
>On ia64, global variables aren't in identity mapping area (i.e. kaddr)
>so that there is no relationship between its virtual address and
>its physical address. Thus virt_to_bus() can't be applied to them.
>So introduce arbitrary_virt_to_bus() to wrap arch dependent function
>and make use of it.

I needed to look into this again, because the use of arbitary_virt_to_machine()
in drivers/xen/core/evtchn.c fails to build for me (and I can't see how the
2.6.18 tree would build for x86 either, as I can't see how asm/pgtable.h
would get included: it doesn't get included in any of my 2.6.16, 2.6.22,
2.6.25, and 2.6.27 based trees). Perhaps there's a configuration
dependency, but that would then be wrong. And I'm hesitant to include
asm/pgtable.h explicitly in that file, as it really shouldn't need it.

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.

Jan

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)

[Isaku Yamahata]

On Mon, Dec 08, 2008 at 01:36:06PM +0000, Jan Beulich wrote:
> >>> Isaku Yamahata <yamahata@valinux.co.jp> 03.12.08 10:20 >>>
> >Yes, you're correct. In fact I had the patch which you suggested,
> >but I was hesitated to change the x86 implementation so that
> >I had changed it to use virt_to_bus() on x86.
> >
> >
> >
> >evtchn, physdev: fix pirq_eoi_mfn for IA64 support.
> >
> >On ia64, global variables aren't in identity mapping area (i.e. kaddr)
> >so that there is no relationship between its virtual address and
> >its physical address. Thus virt_to_bus() can't be applied to them.
> >So introduce arbitrary_virt_to_bus() to wrap arch dependent function
> >and make use of it.
> 
> I needed to look into this again, because the use of arbitary_virt_to_machine()
> in drivers/xen/core/evtchn.c fails to build for me (and I can't see how the
> 2.6.18 tree would build for x86 either, as I can't see how asm/pgtable.h
> would get included: it doesn't get included in any of my 2.6.16, 2.6.22,
> 2.6.25, and 2.6.27 based trees). Perhaps there's a configuration
> dependency, but that would then be wrong. And I'm hesitant to include
> asm/pgtable.h explicitly in that file, as it really shouldn't need it.

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...)


> 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.
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()?

-- 
yamahata

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]

>>> 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

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)

[Isaku Yamahata]

On Tue, Dec 09, 2008 at 10:04:55AM +0000, Jan Beulich wrote:
> >>> 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()).

You are the first person to pass the kernel symbol address
to virt_to_machine() in arch independent code.
Is there any necessity to allocate pirq_needs_eoi statically?
(except it did before)
If no, can we allocate the pages for them dynamically?
Then the issue will go away.


> >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).

Introduce ptep_to_machine() or something like that?

-- 
yamahata

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]

>>> Isaku Yamahata <yamahata@valinux.co.jp> 09.12.08 11:43 >>>
>You are the first person to pass the kernel symbol address
>to virt_to_machine() in arch independent code.
>Is there any necessity to allocate pirq_needs_eoi statically?
>(except it did before)

Perhaps not - avoiding the possible allocation failure (-> BUG()) and the
extra indirection were the main reasons I kept it allocated statically.

>If no, can we allocate the pages for them dynamically?
>Then the issue will go away.

Indeed.

>Introduce ptep_to_machine() or something like that?

Yes, if that name isn't ambiguous in some way.

Jan

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)

[Keir Fraser]

On 09/12/2008 10:54, "Jan Beulich" <jbeulich@novell.com> wrote:

>>>> Isaku Yamahata <yamahata@valinux.co.jp> 09.12.08 11:43 >>>
>> You are the first person to pass the kernel symbol address
>> to virt_to_machine() in arch independent code.
>> Is there any necessity to allocate pirq_needs_eoi statically?
>> (except it did before)
> 
> Perhaps not - avoiding the possible allocation failure (-> BUG()) and the
> extra indirection were the main reasons I kept it allocated statically.
> 
>> If no, can we allocate the pages for them dynamically?
>> Then the issue will go away.
> 
> Indeed.

Yes please. Just do this and be done. It's a one-off start-of-day allocation
which, if it fails, means you're screwed anyway.

 -- Keir

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)

[Isaku Yamahata]

On Tue, Dec 09, 2008 at 11:06:58AM +0000, Keir Fraser wrote:
> On 09/12/2008 10:54, "Jan Beulich" <jbeulich@novell.com> wrote:
> 
> >>>> Isaku Yamahata <yamahata@valinux.co.jp> 09.12.08 11:43 >>>
> >> You are the first person to pass the kernel symbol address
> >> to virt_to_machine() in arch independent code.
> >> Is there any necessity to allocate pirq_needs_eoi statically?
> >> (except it did before)
> > 
> > Perhaps not - avoiding the possible allocation failure (-> BUG()) and the
> > extra indirection were the main reasons I kept it allocated statically.
> > 
> >> If no, can we allocate the pages for them dynamically?
> >> Then the issue will go away.
> > 
> > Indeed.
> 
> Yes please. Just do this and be done. It's a one-off start-of-day allocation
> which, if it fails, means you're screwed anyway.

Here is.

evtchn: allocate pirq_needs_eoi bitmap dynamically.

allocating pirq_needs_eoi statically causes an address conversion
issue between ia64 and x86 because ia64 kernel symbol address
doesn't lay in 1:1 mapping area which can't be apssed to virt_to_machine().
So allocate it dynamically to work around it.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff --git a/drivers/xen/core/evtchn.c b/drivers/xen/core/evtchn.c
--- a/drivers/xen/core/evtchn.c
+++ b/drivers/xen/core/evtchn.c
@@ -31,6 +31,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/bootmem.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/sched.h>
@@ -755,8 +756,7 @@ static struct hw_interrupt_type dynirq_t
 
 /* Bitmap indicating which PIRQs require Xen to be notified on unmask. */
 static int pirq_eoi_does_unmask;
-static DECLARE_BITMAP(pirq_needs_eoi, ALIGN(NR_PIRQS, PAGE_SIZE * 8))
-	__attribute__ ((__section__(".bss.page_aligned"), __aligned__(PAGE_SIZE)));
+static unsigned long *pirq_needs_eoi;
 
 static void pirq_unmask_and_notify(unsigned int evtchn, unsigned int irq)
 {
@@ -1041,8 +1041,7 @@ void irq_resume(void)
 	if (pirq_eoi_does_unmask) {
 		struct physdev_pirq_eoi_gmfn eoi_gmfn;
 
-		eoi_gmfn.gmfn = arbitrary_virt_to_machine(pirq_needs_eoi)
-			>> PAGE_SHIFT;
+		eoi_gmfn.gmfn = virt_to_machine(pirq_needs_eoi) >> PAGE_SHIFT;
 		if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn, &eoi_gmfn))
 			BUG();
 	}
@@ -1137,9 +1136,12 @@ void __init xen_init_IRQ(void)
 
 	init_evtchn_cpu_bindings();
 
+	pirq_needs_eoi = alloc_bootmem_pages(PAGE_SIZE);
+	memset(pirq_needs_eoi, 0, PAGE_SIZE);
+	if (pirq_needs_eoi == NULL)
+		panic("failed to allocate a page for event channel.");
 	BUG_ON(!bitmap_empty(pirq_needs_eoi, PAGE_SIZE * 8));
-	eoi_gmfn.gmfn = arbitrary_virt_to_machine(pirq_needs_eoi)
-		>> PAGE_SHIFT;
+	eoi_gmfn.gmfn = virt_to_machine(pirq_needs_eoi) >> PAGE_SHIFT;
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn, &eoi_gmfn) == 0)
 		pirq_eoi_does_unmask = 1;
 


-- 
yamahata

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)

[Isaku Yamahata]

Sorry I sent out the old one. Here is the corrected one.
NULL check must be before its use.

evtchn: allocate pirq_needs_eoi bitmap dynamically.

allocating pirq_needs_eoi statically causes an address conversion
issue between ia64 and x86 because ia64 kernel symbol address
doesn't lay in 1:1 mapping area which can't be apssed to virt_to_machine().
So allocate it dynamically to work around it.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff --git a/drivers/xen/core/evtchn.c b/drivers/xen/core/evtchn.c
--- a/drivers/xen/core/evtchn.c
+++ b/drivers/xen/core/evtchn.c
@@ -31,6 +31,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/bootmem.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/sched.h>
@@ -755,8 +756,7 @@ static struct hw_interrupt_type dynirq_t
 
 /* Bitmap indicating which PIRQs require Xen to be notified on unmask. */
 static int pirq_eoi_does_unmask;
-static DECLARE_BITMAP(pirq_needs_eoi, ALIGN(NR_PIRQS, PAGE_SIZE * 8))
-	__attribute__ ((__section__(".bss.page_aligned"), __aligned__(PAGE_SIZE)));
+static unsigned long *pirq_needs_eoi;
 
 static void pirq_unmask_and_notify(unsigned int evtchn, unsigned int irq)
 {
@@ -1041,8 +1041,7 @@ void irq_resume(void)
 	if (pirq_eoi_does_unmask) {
 		struct physdev_pirq_eoi_gmfn eoi_gmfn;
 
-		eoi_gmfn.gmfn = arbitrary_virt_to_machine(pirq_needs_eoi)
-			>> PAGE_SHIFT;
+		eoi_gmfn.gmfn = virt_to_machine(pirq_needs_eoi) >> PAGE_SHIFT;
 		if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn, &eoi_gmfn))
 			BUG();
 	}
@@ -1137,9 +1136,12 @@ void __init xen_init_IRQ(void)
 
 	init_evtchn_cpu_bindings();
 
+	pirq_needs_eoi = alloc_bootmem_pages(PAGE_SIZE);
+	if (pirq_needs_eoi == NULL)
+		panic("failed to allocate a page for event channel.");
+	memset(pirq_needs_eoi, 0, PAGE_SIZE);
 	BUG_ON(!bitmap_empty(pirq_needs_eoi, PAGE_SIZE * 8));
-	eoi_gmfn.gmfn = arbitrary_virt_to_machine(pirq_needs_eoi)
-		>> PAGE_SHIFT;
+	eoi_gmfn.gmfn = virt_to_machine(pirq_needs_eoi) >> PAGE_SHIFT;
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn, &eoi_gmfn) == 0)
 		pirq_eoi_does_unmask = 1;
 


-- 
yamahata

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

[Jan Beulich]

>>> Isaku Yamahata <yamahata@valinux.co.jp> 10.12.08 05:16 >>>
>Sorry I sent out the old one. Here is the corrected one.
>NULL check must be before its use.

Not really - alloc_bootmem() etc panic for themselves unless you use the
_nopanic variants.
Also, alloc_bootmem() etc zero the allocated memory, so no need for
memset(), and the subsequent BUG_ON() can obviously go away.
And finally, PAGE_SIZE isn't correct, you should use the size originally
used, just slightly modified:

BITS_TO_LONGS(ALIGN(NR_PIRQS, PAGE_SIZE * 8))

I was about to put together a patch for this myself...

Jan

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

[Keir Fraser]

On 10/12/2008 09:21, "Jan Beulich" <jbeulich@novell.com> wrote:

>>>> Isaku Yamahata <yamahata@valinux.co.jp> 10.12.08 05:16 >>>
>> Sorry I sent out the old one. Here is the corrected one.
>> NULL check must be before its use.
> 
> Not really - alloc_bootmem() etc panic for themselves unless you use the
> _nopanic variants.
> Also, alloc_bootmem() etc zero the allocated memory, so no need for
> memset(), and the subsequent BUG_ON() can obviously go away.
> And finally, PAGE_SIZE isn't correct, you should use the size originally
> used, just slightly modified:
> 
> BITS_TO_LONGS(ALIGN(NR_PIRQS, PAGE_SIZE * 8))
> 
> I was about to put together a patch for this myself...

Can you just fix up Isaku's patch and then we'll collect a fresh sign-off
from him too?

 -- Keir

Re: [PATCH] fix ia64 breakage with PHYSDEVOP_pirq_eoi_mfn (wasRe:[PATCH 2/2] linux/x86: use shared page indicatingthe need foran EOI notification)

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 10.12.08 11:07 >>>
>On 10/12/2008 09:21, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>>>>> Isaku Yamahata <yamahata@valinux.co.jp> 10.12.08 05:16 >>>
>>> Sorry I sent out the old one. Here is the corrected one.
>>> NULL check must be before its use.
>> 
>> Not really - alloc_bootmem() etc panic for themselves unless you use the
>> _nopanic variants.
>> Also, alloc_bootmem() etc zero the allocated memory, so no need for
>> memset(), and the subsequent BUG_ON() can obviously go away.
>> And finally, PAGE_SIZE isn't correct, you should use the size originally
>> used, just slightly modified:
>> 
>> BITS_TO_LONGS(ALIGN(NR_PIRQS, PAGE_SIZE * 8))
>> 
>> I was about to put together a patch for this myself...
>
>Can you just fix up Isaku's patch and then we'll collect a fresh sign-off
>from him too?

Yes, I'm in the process of doing this (here and for the other one) - my
suggestions actually needed some further refinement, and getting the
HIGHPTE case of the other patch to build is actually non-trivial.

Jan

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)

[Isaku Yamahata]

On Tue, Dec 09, 2008 at 10:54:57AM +0000, Jan Beulich wrote:
> >Introduce ptep_to_machine() or something like that?
> Yes, if that name isn't ambiguous in some way.

Like this?
To be honest, I haven't tested the patch with highpte yet.

blktap, gntdev: fix highpte handling.

In case of highpte, virt_to_machine() can't be used.
Introduce ptep_to_machine() and use it.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff --git a/drivers/xen/blktap/blktap.c b/drivers/xen/blktap/blktap.c
--- a/drivers/xen/blktap/blktap.c
+++ b/drivers/xen/blktap/blktap.c
@@ -54,6 +54,7 @@
 #include <linux/poll.h>
 #include <linux/delay.h>
 #include <asm/tlbflush.h>
+#include <asm/pgtable.h>
 
 #define MAX_TAP_DEV 256     /*the maximum number of tapdisk ring devices    */
 #define MAX_DEV_NAME 100    /*the max tapdisk ring device name e.g. blktap0 */
@@ -364,7 +365,7 @@ static pte_t blktap_clear_pte(struct vm_
 		BUG_ON(xen_feature(XENFEAT_auto_translated_physmap));
 
 		copy = *ptep;
-		gnttab_set_unmap_op(&unmap[count], virt_to_machine(ptep), 
+		gnttab_set_unmap_op(&unmap[count], ptep_to_machine(ptep), 
 				    GNTMAP_host_map 
 				    | GNTMAP_application_map 
 				    | GNTMAP_contains_pte,
diff --git a/drivers/xen/gntdev/gntdev.c b/drivers/xen/gntdev/gntdev.c
--- a/drivers/xen/gntdev/gntdev.c
+++ b/drivers/xen/gntdev/gntdev.c
@@ -34,7 +34,7 @@
 
 #include <linux/types.h>
 #include <xen/public/gntdev.h>
-
+#include <asm/pgtable.h>
 
 #define DRIVER_AUTHOR "Derek G. Murray <Derek.Murray@cl.cam.ac.uk>"
 #define DRIVER_DESC   "User-space granted page access driver"
@@ -769,7 +769,7 @@ static pte_t gntdev_clear_pte(struct vm_
 		    GNTDEV_INVALID_HANDLE && 
 		    !xen_feature(XENFEAT_auto_translated_physmap)) {
 			/* NOT USING SHADOW PAGE TABLES. */
-			gnttab_set_unmap_op(&op, virt_to_machine(ptep), 
+			gnttab_set_unmap_op(&op, ptep_to_machine(ptep), 
 					    GNTMAP_contains_pte,
 					    private_data->grants[slot_index]
 					    .u.valid.user_handle);
diff --git a/include/asm-i386/mach-xen/asm/pgtable.h b/include/asm-i386/mach-xen/asm/pgtable.h
--- a/include/asm-i386/mach-xen/asm/pgtable.h
+++ b/include/asm-i386/mach-xen/asm/pgtable.h
@@ -488,6 +488,17 @@ void make_pages_writable(void *va, unsig
 	(((maddr_t)pte_mfn(*virt_to_ptep(va)) << PAGE_SHIFT)		\
 	 | ((unsigned long)(va) & (PAGE_SIZE - 1)))
 
+#ifdef CONFIG_HIGHPTE
+#define ptep_to_machine(ptep)						\
+({									\
+	(unsigned long)(ptep) >= (unsigned long)high_memory?		\
+		arbitrary_virt_to_machine(ptep) :			\
+		virt_to_machine(ptep);					\
+})
+#else
+#define ptep_to_machine(ptep)	virt_to_machine(ptep)
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 #ifdef CONFIG_FLATMEM
diff --git a/include/asm-ia64/maddr.h b/include/asm-ia64/maddr.h
--- a/include/asm-ia64/maddr.h
+++ b/include/asm-ia64/maddr.h
@@ -100,6 +100,7 @@ mfn_to_local_pfn(unsigned long mfn)
 #define virt_to_mfn(virt) (__pa(virt) >> PAGE_SHIFT)
 #define virt_to_machine(virt) __pa(virt) /* for tpmfront.c */
 #define arbitrary_virt_to_machine(virt) virt_to_machine(ia64_imva(virt))
+#define ptep_to_machine(virt) virt_to_machine(virt)
 
 #define set_phys_to_machine(pfn, mfn) do { } while (0)
 
diff --git a/include/asm-x86_64/mach-xen/asm/pgtable.h b/include/asm-x86_64/mach-xen/asm/pgtable.h
--- a/include/asm-x86_64/mach-xen/asm/pgtable.h
+++ b/include/asm-x86_64/mach-xen/asm/pgtable.h
@@ -30,6 +30,8 @@ extern pte_t *lookup_address(unsigned lo
 #define arbitrary_virt_to_machine(va)					\
 	(((maddr_t)pte_mfn(*virt_to_ptep(va)) << PAGE_SHIFT)		\
 	 | ((unsigned long)(va) & (PAGE_SIZE - 1)))
+
+#define ptep_to_machine(ptep)	virt_to_machine(ptep)
 #endif
 
 extern pud_t level3_kernel_pgt[512];


-- 
yamahata

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]

>>> Isaku Yamahata <yamahata@valinux.co.jp> 10.12.08 05:09 >>>
>Like this?

Almost.

>--- a/include/asm-i386/mach-xen/asm/pgtable.h
>+++ b/include/asm-i386/mach-xen/asm/pgtable.h
>@@ -488,6 +488,17 @@ void make_pages_writable(void *va, unsig
> 	(((maddr_t)pte_mfn(*virt_to_ptep(va)) << PAGE_SHIFT)		\
> 	 | ((unsigned long)(va) & (PAGE_SIZE - 1)))
> 
>+#ifdef CONFIG_HIGHPTE
>+#define ptep_to_machine(ptep)						\
>+({									\
>+	(unsigned long)(ptep) >= (unsigned long)high_memory?		\
>+		arbitrary_virt_to_machine(ptep) :			\
>+		virt_to_machine(ptep);					\
>+})

I was intending to make use of kmap_atomic_to_page() here. Due to the
check at the beginning of that function, it would even be possible to get
away without a conditional in the highpte case of the macro:

#define ptep_to_machine(ptep) page_to_phys(kmap_atomic_to_page(ptep))

page_to_phys() and page_to_bus() could be used here interchangeably due
to them being identical on x86.

Jan