[PATCH 2/2] linux/x86: use shared page indicating the need for an EOI notification

[PATCH 2/2] linux/x86: use shared page indicating the need for an EOI notification

[Jan Beulich]

As usual, written and tested on 2.6.27.7 and made apply to the 2.6.18
tree without further testing.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: head-2008-11-25/drivers/xen/core/evtchn.c
===================================================================
--- head-2008-11-25.orig/drivers/xen/core/evtchn.c	2008-11-26 15:59:04.000000000 +0100
+++ head-2008-11-25/drivers/xen/core/evtchn.c	2008-11-26 16:00:52.000000000 +0100
@@ -130,9 +130,6 @@ DEFINE_PER_CPU(int, ipi_to_irq[NR_IPIS])
 /* Reference counts for bindings to IRQs. */
 static int irq_bindcount[NR_IRQS];
 
-/* Bitmap indicating which PIRQs require Xen to be notified on unmask. */
-static DECLARE_BITMAP(pirq_needs_eoi, NR_PIRQS);
-
 #ifdef CONFIG_SMP
 
 static u8 cpu_evtchn[NR_EVENT_CHANNELS];
@@ -786,16 +783,48 @@ static struct irq_chip dynirq_chip = {
 	.retrigger = resend_irq_on_evtchn,
 };
 
-static inline void pirq_unmask_notify(int irq)
+/* Bitmap indicating which PIRQs require Xen to be notified on unmask. */
+static bool 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 void pirq_unmask_and_notify(unsigned int evtchn, unsigned int irq)
 {
 	struct physdev_eoi eoi = { .irq = evtchn_get_xen_pirq(irq) };
-	if (unlikely(test_bit(irq - PIRQ_BASE, pirq_needs_eoi)))
-		VOID(HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi));
+
+	if (pirq_eoi_does_unmask) {
+		if (test_bit(eoi.irq, pirq_needs_eoi))
+			VOID(HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi));
+		else
+			unmask_evtchn(evtchn);
+	} else if (test_bit(irq - PIRQ_BASE, pirq_needs_eoi)) {
+		if (smp_processor_id() != cpu_from_evtchn(evtchn)) {
+			struct evtchn_unmask unmask = { .port = evtchn };
+			struct multicall_entry mcl[2];
+
+			mcl[0].op = __HYPERVISOR_event_channel_op;
+			mcl[0].args[0] = EVTCHNOP_unmask;
+			mcl[0].args[1] = (unsigned long)&unmask;
+			mcl[1].op = __HYPERVISOR_physdev_op;
+			mcl[1].args[0] = PHYSDEVOP_eoi;
+			mcl[1].args[1] = (unsigned long)&eoi;
+
+			if (HYPERVISOR_multicall(mcl, 2))
+				BUG();
+		} else {
+			unmask_evtchn(evtchn);
+			VOID(HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi));
+		}
+	} else
+		unmask_evtchn(evtchn);
 }
 
 static inline void pirq_query_unmask(int irq)
 {
 	struct physdev_irq_status_query irq_status;
+
+	if (pirq_eoi_does_unmask)
+		return;
 	irq_status.irq = evtchn_get_xen_pirq(irq);
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_irq_status_query, &irq_status))
 		irq_status.flags = 0;
@@ -836,8 +864,7 @@ static unsigned int startup_pirq(unsigne
 	irq_info[irq] = mk_irq_info(IRQT_PIRQ, bind_pirq.pirq, evtchn);
 
  out:
-	unmask_evtchn(evtchn);
-	pirq_unmask_notify(irq);
+	pirq_unmask_and_notify(evtchn, irq);
 
 	return 0;
 }
@@ -889,10 +916,8 @@ static void end_pirq(unsigned int irq)
 	if ((irq_desc[irq].status & (IRQ_DISABLED|IRQ_PENDING)) ==
 	    (IRQ_DISABLED|IRQ_PENDING)) {
 		shutdown_pirq(irq);
-	} else if (VALID_EVTCHN(evtchn)) {
-		unmask_evtchn(evtchn);
-		pirq_unmask_notify(irq);
-	}
+	} else if (VALID_EVTCHN(evtchn))
+		pirq_unmask_and_notify(evtchn, irq);
 }
 
 static struct hw_interrupt_type pirq_type = {
@@ -1045,6 +1070,14 @@ static int evtchn_resume(struct sys_devi
 
 	init_evtchn_cpu_bindings();
 
+	if (pirq_eoi_does_unmask) {
+		struct physdev_pirq_eoi_mfn eoi_mfn;
+
+		eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+		if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn))
+			BUG();
+	}
+
 	/* New event-channel space is not 'live' yet. */
 	for (evtchn = 0; evtchn < NR_EVENT_CHANNELS; evtchn++)
 		mask_evtchn(evtchn);
@@ -1167,6 +1200,7 @@ int evtchn_get_xen_pirq(int irq)
 void __init xen_init_IRQ(void)
 {
 	unsigned int i;
+	struct physdev_pirq_eoi_mfn eoi_mfn;
 
 	init_evtchn_cpu_bindings();
 
@@ -1179,6 +1213,11 @@ void __init xen_init_IRQ(void)
 
 	init_evtchn_cpu_bindings();
 
+	BUG_ON(!bitmap_empty(pirq_needs_eoi, PAGE_SIZE * 8));
+	eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+	if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn) == 0)
+		pirq_eoi_does_unmask = true;
+
 	/* No event channels are 'live' right now. */
 	for (i = 0; i < NR_EVENT_CHANNELS; i++)
 		mask_evtchn(i);
Index: head-2008-11-25/include/xen/interface/physdev.h
===================================================================
--- head-2008-11-25.orig/include/xen/interface/physdev.h	2008-11-26 15:59:04.000000000 +0100
+++ head-2008-11-25/include/xen/interface/physdev.h	2008-11-24 15:16:10.000000000 +0100
@@ -41,6 +41,21 @@ typedef struct physdev_eoi physdev_eoi_t
 DEFINE_XEN_GUEST_HANDLE(physdev_eoi_t);
 
 /*
+ * Register a shared page for the hypervisor to indicate whether the guest
+ * must issue PHYSDEVOP_eoi. The semantics of PHYSDEVOP_eoi change slightly
+ * once the guest used this function in that the associated event channel
+ * will automatically get unmasked. The page registered is used as a bit
+ * array indexed by Xen's PIRQ value.
+ */
+#define PHYSDEVOP_pirq_eoi_mfn          17
+struct physdev_pirq_eoi_mfn {
+    /* IN */
+    xen_pfn_t mfn;
+};
+typedef struct physdev_pirq_eoi_mfn physdev_pirq_eoi_mfn_t;
+DEFINE_XEN_GUEST_HANDLE(physdev_pirq_eoi_mfn_t);
+
+/*
  * Query the status of an IRQ line.
  * @arg == pointer to physdev_irq_status_query structure.
  */

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

Hi Jan. Thank you for taking care of not breaking existing code.
However there is an ia64 specific issue in this patch.
Here is the patch to fix it.

And I have an issue:
MFN is passed from a guest to the VMM to indicate a page in guest.
However I think GMFN should be used, instead of MFN like
grant table, xenoprof and other hypercalls.
I'll post two patches to rename the related stuff.



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.

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
@@ -1032,6 +1032,11 @@ static void restore_cpu_ipis(unsigned in
 	}
 }
 
+static void physdev_set_pirq_eoi_mfn(struct physdev_pirq_eoi_mfn *eoi_mfn)
+{
+	eoi_mfn->mfn = arbitrary_virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+}
+
 void irq_resume(void)
 {
 	unsigned int cpu, irq, evtchn;
@@ -1041,7 +1046,7 @@ void irq_resume(void)
 	if (pirq_eoi_does_unmask) {
 		struct physdev_pirq_eoi_mfn eoi_mfn;
 
-		eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+		physdev_set_pirq_eoi_mfn(&eoi_mfn);
 		if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn))
 			BUG();
 	}
@@ -1137,7 +1142,7 @@ void __init xen_init_IRQ(void)
 	init_evtchn_cpu_bindings();
 
 	BUG_ON(!bitmap_empty(pirq_needs_eoi, PAGE_SIZE * 8));
-	eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+	physdev_set_pirq_eoi_mfn(&eoi_mfn);
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn) == 0)
 		pirq_eoi_does_unmask = 1;
 
diff --git a/include/asm-i386/mach-xen/asm/io.h b/include/asm-i386/mach-xen/asm/io.h
--- a/include/asm-i386/mach-xen/asm/io.h
+++ b/include/asm-i386/mach-xen/asm/io.h
@@ -163,6 +163,7 @@ extern void bt_iounmap(void *addr, unsig
  */
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) virt_to_bus(_x)
 
 /*
  * readX/writeX() are used to access memory mapped devices. On some
diff --git a/include/asm-ia64/io.h b/include/asm-ia64/io.h
--- a/include/asm-ia64/io.h
+++ b/include/asm-ia64/io.h
@@ -105,6 +105,9 @@ extern int valid_mmap_phys_addr_range (u
 	phys_to_virt(machine_to_phys_for_dma(bus))
 #define virt_to_bus(virt)	\
 	phys_to_machine_for_dma(virt_to_phys(virt))
+#define arbitrary_virt_to_bus(virt)	\
+	phys_to_machine_for_dma(virt_to_phys(ia64_imva(virt)))
+
 #define page_to_bus(page)	\
 	phys_to_machine_for_dma(page_to_pseudophys(page))
 



-- 
yamahata

[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 03:07 >>>
>Hi Jan. Thank you for taking care of not breaking existing code.
>However there is an ia64 specific issue in this patch.
>Here is the patch to fix it.

I'm sorry for that, I really tried to not break ia64.

>And I have an issue:
>MFN is passed from a guest to the VMM to indicate a page in guest.
>However I think GMFN should be used, instead of MFN like
>grant table, xenoprof and other hypercalls.
>I'll post two patches to rename the related stuff.

Hmm, I know too little about ia64 Xen to understand the significance of
that difference.

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

The same applies to x86-64, but virt_to_bus() (or rather the underlying
virt_to_phys()) is prepared to deal with that situation. So it rather sounds
like a shortcoming of the ia64 variant to me...

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 Wed, Dec 03, 2008 at 07:58:56AM +0000, Jan Beulich wrote:
> >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.
> 
> The same applies to x86-64, but virt_to_bus() (or rather the underlying
> virt_to_phys()) is prepared to deal with that situation. So it rather sounds
> like a shortcoming of the ia64 variant to me...

Oh I forgot the x86-64 case.

virt_to_bus() is intended only for virtual address of the kernel 
identity mapping area, I think.
virt_to_bus() can't be used to the vmalloc area, for example.

On the other hand, there is no guarantee that global variables
lay in the kernel identity mapping area.
On i386 and x86-64, the kernel global variables happen to
be in kaddr, but it isn't the case on ia64 nor more generally
for global variables of the kernel modules which are allocated
from the vmalloc area.
So I think virt_to_bus() shouldn't be used for global variables.


Here is the updated one which includes the x86-64 changes.

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.

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
@@ -1032,6 +1032,11 @@ static void restore_cpu_ipis(unsigned in
 	}
 }
 
+static void physdev_set_pirq_eoi_mfn(struct physdev_pirq_eoi_mfn *eoi_mfn)
+{
+	eoi_mfn->mfn = arbitrary_virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+}
+
 void irq_resume(void)
 {
 	unsigned int cpu, irq, evtchn;
@@ -1041,7 +1046,7 @@ void irq_resume(void)
 	if (pirq_eoi_does_unmask) {
 		struct physdev_pirq_eoi_mfn eoi_mfn;
 
-		eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+		physdev_set_pirq_eoi_mfn(&eoi_mfn);
 		if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn))
 			BUG();
 	}
@@ -1137,7 +1142,7 @@ void __init xen_init_IRQ(void)
 	init_evtchn_cpu_bindings();
 
 	BUG_ON(!bitmap_empty(pirq_needs_eoi, PAGE_SIZE * 8));
-	eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+	physdev_set_pirq_eoi_mfn(&eoi_mfn);
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn) == 0)
 		pirq_eoi_does_unmask = 1;
 
diff --git a/include/asm-i386/mach-xen/asm/io.h b/include/asm-i386/mach-xen/asm/io.h
--- a/include/asm-i386/mach-xen/asm/io.h
+++ b/include/asm-i386/mach-xen/asm/io.h
@@ -163,6 +163,7 @@ extern void bt_iounmap(void *addr, unsig
  */
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) virt_to_bus(_x)
 
 /*
  * readX/writeX() are used to access memory mapped devices. On some
diff --git a/include/asm-ia64/io.h b/include/asm-ia64/io.h
--- a/include/asm-ia64/io.h
+++ b/include/asm-ia64/io.h
@@ -105,6 +105,9 @@ extern int valid_mmap_phys_addr_range (u
 	phys_to_virt(machine_to_phys_for_dma(bus))
 #define virt_to_bus(virt)	\
 	phys_to_machine_for_dma(virt_to_phys(virt))
+#define arbitrary_virt_to_bus(virt)	\
+	phys_to_machine_for_dma(virt_to_phys(ia64_imva(virt)))
+
 #define page_to_bus(page)	\
 	phys_to_machine_for_dma(page_to_pseudophys(page))
 
diff --git a/include/asm-x86_64/mach-xen/asm/io.h b/include/asm-x86_64/mach-xen/asm/io.h
--- a/include/asm-x86_64/mach-xen/asm/io.h
+++ b/include/asm-x86_64/mach-xen/asm/io.h
@@ -122,6 +122,7 @@ static inline void * phys_to_virt(unsign
 
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) virt_to_bus(_x)
 #endif
 
 /*
@@ -179,6 +180,7 @@ extern void iounmap(volatile void __iome
  */
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) virt_to_bus(_x)
 
 /*
  * readX/writeX() are used to access memory mapped devices. On some



-- 
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> 03.12.08 09:44 >>>
>On Wed, Dec 03, 2008 at 07:58:56AM +0000, Jan Beulich wrote:
>> >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.
>> 
>> The same applies to x86-64, but virt_to_bus() (or rather the underlying
>> virt_to_phys()) is prepared to deal with that situation. So it rather sounds
>> like a shortcoming of the ia64 variant to me...
>
>Oh I forgot the x86-64 case.
>
>virt_to_bus() is intended only for virtual address of the kernel 
>identity mapping area, I think.
>virt_to_bus() can't be used to the vmalloc area, for example.
>
>On the other hand, there is no guarantee that global variables
>lay in the kernel identity mapping area.
>On i386 and x86-64, the kernel global variables happen to
>be in kaddr, but it isn't the case on ia64 nor more generally
>for global variables of the kernel modules which are allocated
>from the vmalloc area.
>So I think virt_to_bus() shouldn't be used for global variables.

You seem to contradict yourself then: The variable we're talking about
is not in module space, so the vmalloc() argument wouldn't apply. If
however you think it is relevant, then you can't implement
arbitrary_virt_to_bus() on x86 by simply using virt_to_bus() - and even
without considering that aspect, the name on x86 doesn't hold what it
promises. So I think you either need to properly implement it for x86
(by using arbitary_virt_to_machine - and then you could simply use that
name on ia64 and don't change anything for x86), or you should abstract
out that aspect in evtchn.c.

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 Wed, Dec 03, 2008 at 08:58:36AM +0000, Jan Beulich wrote:
> >>> Isaku Yamahata <yamahata@valinux.co.jp> 03.12.08 09:44 >>>
> >On Wed, Dec 03, 2008 at 07:58:56AM +0000, Jan Beulich wrote:
> >> >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.
> >> 
> >> The same applies to x86-64, but virt_to_bus() (or rather the underlying
> >> virt_to_phys()) is prepared to deal with that situation. So it rather sounds
> >> like a shortcoming of the ia64 variant to me...
> >
> >Oh I forgot the x86-64 case.
> >
> >virt_to_bus() is intended only for virtual address of the kernel 
> >identity mapping area, I think.
> >virt_to_bus() can't be used to the vmalloc area, for example.
> >
> >On the other hand, there is no guarantee that global variables
> >lay in the kernel identity mapping area.
> >On i386 and x86-64, the kernel global variables happen to
> >be in kaddr, but it isn't the case on ia64 nor more generally
> >for global variables of the kernel modules which are allocated
> >from the vmalloc area.
> >So I think virt_to_bus() shouldn't be used for global variables.
> 
> You seem to contradict yourself then: The variable we're talking about
> is not in module space, so the vmalloc() argument wouldn't apply. If
> however you think it is relevant, then you can't implement
> arbitrary_virt_to_bus() on x86 by simply using virt_to_bus() - and even
> without considering that aspect, the name on x86 doesn't hold what it
> promises. So I think you either need to properly implement it for x86
> (by using arbitary_virt_to_machine - and then you could simply use that
> name on ia64 and don't change anything for x86), or you should abstract
> out that aspect in evtchn.c.

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.

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
@@ -1032,6 +1032,11 @@ static void restore_cpu_ipis(unsigned in
 	}
 }
 
+static void physdev_set_pirq_eoi_mfn(struct physdev_pirq_eoi_mfn *eoi_mfn)
+{
+	eoi_mfn->mfn = arbitrary_virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+}
+
 void irq_resume(void)
 {
 	unsigned int cpu, irq, evtchn;
@@ -1041,7 +1046,7 @@ void irq_resume(void)
 	if (pirq_eoi_does_unmask) {
 		struct physdev_pirq_eoi_mfn eoi_mfn;
 
-		eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+		physdev_set_pirq_eoi_mfn(&eoi_mfn);
 		if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn))
 			BUG();
 	}
@@ -1137,7 +1142,7 @@ void __init xen_init_IRQ(void)
 	init_evtchn_cpu_bindings();
 
 	BUG_ON(!bitmap_empty(pirq_needs_eoi, PAGE_SIZE * 8));
-	eoi_mfn.mfn = virt_to_bus(pirq_needs_eoi) >> PAGE_SHIFT;
+	physdev_set_pirq_eoi_mfn(&eoi_mfn);
 	if (HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_mfn, &eoi_mfn) == 0)
 		pirq_eoi_does_unmask = 1;
 
diff --git a/include/asm-i386/mach-xen/asm/io.h b/include/asm-i386/mach-xen/asm/io.h
--- a/include/asm-i386/mach-xen/asm/io.h
+++ b/include/asm-i386/mach-xen/asm/io.h
@@ -163,6 +163,7 @@ extern void bt_iounmap(void *addr, unsig
  */
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) arbitrary_virt_to_machine(_x)
 
 /*
  * readX/writeX() are used to access memory mapped devices. On some
diff --git a/include/asm-ia64/io.h b/include/asm-ia64/io.h
--- a/include/asm-ia64/io.h
+++ b/include/asm-ia64/io.h
@@ -105,6 +105,9 @@ extern int valid_mmap_phys_addr_range (u
 	phys_to_virt(machine_to_phys_for_dma(bus))
 #define virt_to_bus(virt)	\
 	phys_to_machine_for_dma(virt_to_phys(virt))
+#define arbitrary_virt_to_bus(virt)	\
+	phys_to_machine_for_dma(virt_to_phys(ia64_imva(virt)))
+
 #define page_to_bus(page)	\
 	phys_to_machine_for_dma(page_to_pseudophys(page))
 
diff --git a/include/asm-x86_64/mach-xen/asm/io.h b/include/asm-x86_64/mach-xen/asm/io.h
--- a/include/asm-x86_64/mach-xen/asm/io.h
+++ b/include/asm-x86_64/mach-xen/asm/io.h
@@ -122,6 +122,7 @@ static inline void * phys_to_virt(unsign
 
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) arbitrary_virt_to_machine(_x)
 #endif
 
 /*
@@ -179,6 +180,7 @@ extern void iounmap(volatile void __iome
  */
 #define virt_to_bus(_x) phys_to_machine(__pa(_x))
 #define bus_to_virt(_x) __va(machine_to_phys(_x))
+#define arbitrary_virt_to_bus(_x) arbitrary_virt_to_machine(_x)
 
 /*
  * readX/writeX() are used to access memory mapped devices. On some


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

So you really need arbitrary_virt_to_bus() along with
arbitrary_virt_to_machine()? Is that another ia64 specific need (i.e. do
the two have different meanings there)?

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 Wed, Dec 03, 2008 at 09:31:41AM +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.
> 
> So you really need arbitrary_virt_to_bus() along with
> arbitrary_virt_to_machine()? Is that another ia64 specific need (i.e. do
> the two have different meanings there)?

Yes.
On x86, both MMU and DMA is paravirtualized so that machine address and
bus address have same meaning. So sometimes bus address and machine
address are abused.
On the other hand on ia64 MMU is fully virtualized (i.e. auto translated
phsymap mode enabled) and DMA is paravirtualized.
So addresses for MMU and DMA have to be distinguished.
xxx_to_machine() is used for MMU and xxx_to_bus() is used for DMA.

I think once x86 implements auto translated mode for pv guest,
the similar issue would arise.
-- 
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)

[Keir Fraser]

On 03/12/2008 09:59, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:

> On the other hand on ia64 MMU is fully virtualized (i.e. auto translated
> phsymap mode enabled) and DMA is paravirtualized.
> So addresses for MMU and DMA have to be distinguished.
> xxx_to_machine() is used for MMU and xxx_to_bus() is used for DMA.

If you are fully virtualised then gmfn should mean gpfn, and
arbitrary_virt_to_machine() is correct, isn't it? I can't see a situation
where arbitrary_virt_to_machine() wouldn't correctly give you a gmfn (after
all, it gets you a machine address in guest context, as its name describes
:-). 

 -- 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 Wed, Dec 03, 2008 at 10:08:50AM +0000, Keir Fraser wrote:
> On 03/12/2008 09:59, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:
> 
> > On the other hand on ia64 MMU is fully virtualized (i.e. auto translated
> > phsymap mode enabled) and DMA is paravirtualized.
> > So addresses for MMU and DMA have to be distinguished.
> > xxx_to_machine() is used for MMU and xxx_to_bus() is used for DMA.
> 
> If you are fully virtualised then gmfn should mean gpfn, and
> arbitrary_virt_to_machine() is correct, isn't it? I can't see a situation
> where arbitrary_virt_to_machine() wouldn't correctly give you a gmfn (after
> all, it gets you a machine address in guest context, as its name describes
> :-). 

Oh, you are quite right.
Do you agree to rename PHYSDEVOP_pirq_eoi_mfn to PHYSDEVOP_pirq_eoi_gmfn?

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

[Keir Fraser]

On 03/12/2008 10:13, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:

>> If you are fully virtualised then gmfn should mean gpfn, and
>> arbitrary_virt_to_machine() is correct, isn't it? I can't see a situation
>> where arbitrary_virt_to_machine() wouldn't correctly give you a gmfn (after
>> all, it gets you a machine address in guest context, as its name describes
>> :-). 
> 
> Oh, you are quite right.
> Do you agree to rename PHYSDEVOP_pirq_eoi_mfn to PHYSDEVOP_pirq_eoi_gmfn?

Yes, and I'll fix up your patches myself to use a_v_t_m() always.

 -- 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 Wed, Dec 03, 2008 at 10:15:33AM +0000, Keir Fraser wrote:
> On 03/12/2008 10:13, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:
> 
> >> If you are fully virtualised then gmfn should mean gpfn, and
> >> arbitrary_virt_to_machine() is correct, isn't it? I can't see a situation
> >> where arbitrary_virt_to_machine() wouldn't correctly give you a gmfn (after
> >> all, it gets you a machine address in guest context, as its name describes
> >> :-). 
> > 
> > Oh, you are quite right.
> > Do you agree to rename PHYSDEVOP_pirq_eoi_mfn to PHYSDEVOP_pirq_eoi_gmfn?
> 
> Yes, and I'll fix up your patches myself to use a_v_t_m() always.

Thank you so much. Here is the ia64 specific part.
So far ia64 hasn't had arbitrary_virt_to_machine().

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
@@ -99,6 +99,7 @@ mfn_to_local_pfn(unsigned long mfn)
 #define mfn_to_virt(mfn) (__va((mfn) << PAGE_SHIFT))
 #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 set_phys_to_machine(pfn, mfn) do { } while (0)
 

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