[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Wed, Dec 03, 2008 at 08:58:36AM +0000, Jan Beulich wrote:
> >>> Isaku Yamahata <yamahata@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.