[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