xen_hvm_callback_vector

xen_hvm_callback_vector

[Jeremy Fitzhardinge]

 Should entry_64.S's xen_hvm_callback_vector be calling
xen_evtchn_do_upcall or xen_hvm_evtchn_do_upcall?  If not the hvm
version, when does that get used?

    J

Re: xen_hvm_callback_vector

[Ian Campbell]

On Fri, 2010-08-27 at 23:57 +0100, Jeremy Fitzhardinge wrote:
> Should entry_64.S's xen_hvm_callback_vector be calling
> xen_evtchn_do_upcall or xen_hvm_evtchn_do_upcall?
> If not the hvm version, when does that get used?

xen_evtchn_do_upcall is used for the callback vector interrupt injection
(and regular PV obviously).

xen_hvm_evtchn_do_upcall is used by the old-style PCI IRQ injection
route in platform-pci.c if the callback vector is not available.

So entry_64.S should be using xen_evtchn_do_upcall.

Also, I think smp_xen_hvm_callback_vector is simply a redundant
duplicate of xen_evtchn_do_upcall which could be removed.

Ian.

Re: xen_hvm_callback_vector

[Jeremy Fitzhardinge]

 On 08/28/2010 11:19 AM, Ian Campbell wrote:
> xen_evtchn_do_upcall is used for the callback vector interrupt injection
> (and regular PV obviously).
>
> xen_hvm_evtchn_do_upcall is used by the old-style PCI IRQ injection
> route in platform-pci.c if the callback vector is not available.
>
> So entry_64.S should be using xen_evtchn_do_upcall.
>
> Also, I think smp_xen_hvm_callback_vector is simply a redundant
> duplicate of xen_evtchn_do_upcall which could be removed.

smp_xen_hvm_callback_vector is just missing, so I think it should be
using BUILD_INTERRUPT3 with the third arg being xen_evtchn_do_upcall.

    J

Re: xen_hvm_callback_vector

[Ian Campbell]

On Sat, 2010-08-28 at 22:40 +0100, Jeremy Fitzhardinge wrote:
> On 08/28/2010 11:19 AM, Ian Campbell wrote:
> > xen_evtchn_do_upcall is used for the callback vector interrupt injection
> > (and regular PV obviously).
> >
> > xen_hvm_evtchn_do_upcall is used by the old-style PCI IRQ injection
> > route in platform-pci.c if the callback vector is not available.
> >
> > So entry_64.S should be using xen_evtchn_do_upcall.
> >
> > Also, I think smp_xen_hvm_callback_vector is simply a redundant
> > duplicate of xen_evtchn_do_upcall which could be removed.
> 
> smp_xen_hvm_callback_vector is just missing,

It's in drivers/xen/events.c, in the stable-2.6.32.x branch at least.

> so I think it should be
> using BUILD_INTERRUPT3 with the third arg being xen_evtchn_do_upcall.

I agree.

Ian.

Re: xen_hvm_callback_vector

[Stefano Stabellini]

On Sat, 28 Aug 2010, Jeremy Fitzhardinge wrote:
>  On 08/28/2010 11:19 AM, Ian Campbell wrote:
> > xen_evtchn_do_upcall is used for the callback vector interrupt injection
> > (and regular PV obviously).
> >
> > xen_hvm_evtchn_do_upcall is used by the old-style PCI IRQ injection
> > route in platform-pci.c if the callback vector is not available.
> >
> > So entry_64.S should be using xen_evtchn_do_upcall.
> >
> > Also, I think smp_xen_hvm_callback_vector is simply a redundant
> > duplicate of xen_evtchn_do_upcall which could be removed.

That's true, in fact I removed it in my tree.
It should be removed in the upstream version too, I just forgot to
send an update to Jeremy for the stable tree.

In my tree in entry_64 I have

apicinterrupt XEN_HVM_EVTCHN_CALLBACK \
	xen_hvm_callback_vector xen_evtchn_do_upcall

where xen_evtchn_do_upcall is the same callback that PV guests use and
smp_xen_hvm_callback_vector has been removed from drivers/xen/events.c
too.


> 
> smp_xen_hvm_callback_vector is just missing, so I think it should be
> using BUILD_INTERRUPT3 with the third arg being xen_evtchn_do_upcall.

BUILD_INTERRUPT3 is an x86_32 only MACRO, that's why I am using
apicinterrupt instead.

Re: xen_hvm_callback_vector

[Jeremy Fitzhardinge]

 On 08/30/2010 02:37 AM, Stefano Stabellini wrote:
>> smp_xen_hvm_callback_vector is just missing, so I think it should be
>> using BUILD_INTERRUPT3 with the third arg being xen_evtchn_do_upcall.
> BUILD_INTERRUPT3 is an x86_32 only MACRO, that's why I am using
> apicinterrupt instead.

My original problem was that my 32-bit builds were failing because
smp_xen_hvm_callback_vector was missing.  I wonder if its the collision
of two sets of patches.

    J

Re: xen_hvm_callback_vector

[Stefano Stabellini]

On Mon, 30 Aug 2010, Jeremy Fitzhardinge wrote:
>  On 08/30/2010 02:37 AM, Stefano Stabellini wrote:
> >> smp_xen_hvm_callback_vector is just missing, so I think it should be
> >> using BUILD_INTERRUPT3 with the third arg being xen_evtchn_do_upcall.
> > BUILD_INTERRUPT3 is an x86_32 only MACRO, that's why I am using
> > apicinterrupt instead.
> 
> My original problem was that my 32-bit builds were failing because
> smp_xen_hvm_callback_vector was missing.  I wonder if its the collision
> of two sets of patches.
 
I think you are just missing a patch like this one:

---

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 1bf0911..a626344 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1365,7 +1365,7 @@ ENTRY(xen_failsafe_callback)
 END(xen_failsafe_callback)
 
 apicinterrupt XEN_HVM_EVTCHN_CALLBACK \
-	xen_hvm_callback_vector smp_xen_hvm_callback_vector
+	xen_hvm_callback_vector xen_evtchn_do_upcall
 
 #endif /* CONFIG_XEN */
 
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index b4a00bf..743e08f 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1414,21 +1414,6 @@ int xen_set_callback_via(uint64_t via)
 }
 EXPORT_SYMBOL_GPL(xen_set_callback_via);
 
-void smp_xen_hvm_callback_vector(struct pt_regs *regs)
-{
-	struct pt_regs *old_regs = set_irq_regs(regs);
-
-	exit_idle();
-
-	irq_enter();
-
-	__xen_evtchn_do_upcall(regs);
-
-	irq_exit();
-
-	set_irq_regs(old_regs);
-}
-
 /* Vector callbacks are better than PCI interrupts to receive event
  * channel notifications because we can receive vector callbacks on any
  * vcpu and we don't need PCI support or APIC interactions. */

Re: xen_hvm_callback_vector

[Stefano Stabellini]

On Mon, 30 Aug 2010, Stefano Stabellini wrote:
> On Mon, 30 Aug 2010, Jeremy Fitzhardinge wrote:
> >  On 08/30/2010 02:37 AM, Stefano Stabellini wrote:
> > >> smp_xen_hvm_callback_vector is just missing, so I think it should be
> > >> using BUILD_INTERRUPT3 with the third arg being xen_evtchn_do_upcall.
> > > BUILD_INTERRUPT3 is an x86_32 only MACRO, that's why I am using
> > > apicinterrupt instead.
> > 
> > My original problem was that my 32-bit builds were failing because
> > smp_xen_hvm_callback_vector was missing.  I wonder if its the collision
> > of two sets of patches.
>  
> I think you are just missing a patch like this one:
> 

sorry I missed a very import chunk of the patch, appending again:

diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 21feb03..7764118 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1088,7 +1088,8 @@ ENTRY(xen_failsafe_callback)
 .previous
 ENDPROC(xen_failsafe_callback)
 
-BUILD_INTERRUPT(xen_hvm_callback_vector, XEN_HVM_EVTCHN_CALLBACK)
+BUILD_INTERRUPT3(xen_hvm_callback_vector, XEN_HVM_EVTCHN_CALLBACK,
+		xen_evtchn_do_upcall)
 
 #endif	/* CONFIG_XEN */
 
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 1bf0911..a626344 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1365,7 +1365,7 @@ ENTRY(xen_failsafe_callback)
 END(xen_failsafe_callback)
 
 apicinterrupt XEN_HVM_EVTCHN_CALLBACK \
-	xen_hvm_callback_vector smp_xen_hvm_callback_vector
+	xen_hvm_callback_vector xen_evtchn_do_upcall
 
 #endif /* CONFIG_XEN */
 
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index b4a00bf..743e08f 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1414,21 +1414,6 @@ int xen_set_callback_via(uint64_t via)
 }
 EXPORT_SYMBOL_GPL(xen_set_callback_via);
 
-void smp_xen_hvm_callback_vector(struct pt_regs *regs)
-{
-	struct pt_regs *old_regs = set_irq_regs(regs);
-
-	exit_idle();
-
-	irq_enter();
-
-	__xen_evtchn_do_upcall(regs);
-
-	irq_exit();
-
-	set_irq_regs(old_regs);
-}
-
 /* Vector callbacks are better than PCI interrupts to receive event
  * channel notifications because we can receive vector callbacks on any
  * vcpu and we don't need PCI support or APIC interactions. */