| Message ID | 20200930104108.35969-5-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/intr: introduce EOI callbacks and fix vPT | expand |
> -----Original Message----- > From: Roger Pau Monne <roger.pau@citrix.com> > Sent: 30 September 2020 11:41 > To: xen-devel@lists.xenproject.org > Cc: Roger Pau Monne <roger.pau@citrix.com>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper > <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org> > Subject: [PATCH v2 04/11] x86/vmsi: use the newly introduced EOI callbacks > > Remove the unconditional call to hvm_dpci_msi_eoi in vlapic_handle_EOI > and instead use the newly introduced EOI callback mechanism in order > to register a callback for MSI vectors injected from passed through > devices. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/hvm/vlapic.c | 2 -- > xen/arch/x86/hvm/vmsi.c | 36 ++++++++++++++++++++++-------------- > xen/drivers/passthrough/io.c | 2 +- > xen/include/asm-x86/hvm/io.h | 2 +- > 4 files changed, 24 insertions(+), 18 deletions(-) > > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c > index 38c62a02e6..8a18b33428 100644 > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -496,8 +496,6 @@ void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) > if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) > vioapic_update_EOI(vector); > > - hvm_dpci_msi_eoi(vector); > - > spin_lock_irqsave(&vlapic->callback_lock, flags); > callback = vlapic->callbacks[index].callback; > vlapic->callbacks[index].callback = NULL; > diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c > index 7ca19353ab..e192c4c6da 100644 > --- a/xen/arch/x86/hvm/vmsi.c > +++ b/xen/arch/x86/hvm/vmsi.c > @@ -44,11 +44,9 @@ > #include <asm/event.h> > #include <asm/io_apic.h> > > -static void vmsi_inj_irq( > - struct vlapic *target, > - uint8_t vector, > - uint8_t trig_mode, > - uint8_t delivery_mode) > +static void vmsi_inj_irq(struct vlapic *target, uint8_t vector, > + uint8_t trig_mode, uint8_t delivery_mode, > + vlapic_eoi_callback_t *callback, void *data) > { > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "vmsi_inj_irq: vec %02x trig %d dm %d\n", > vector, trig_mode, delivery_mode); > @@ -57,17 +55,17 @@ static void vmsi_inj_irq( > { > case dest_Fixed: > case dest_LowestPrio: > - vlapic_set_irq(target, vector, trig_mode); > + vlapic_set_irq_callback(target, vector, trig_mode, callback, data); > break; > default: > BUG(); > } > } > > -int vmsi_deliver( > - struct domain *d, int vector, > - uint8_t dest, uint8_t dest_mode, > - uint8_t delivery_mode, uint8_t trig_mode) > +static int vmsi_deliver_callback(struct domain *d, int vector, uint8_t dest, > + uint8_t dest_mode, uint8_t delivery_mode, > + uint8_t trig_mode, > + vlapic_eoi_callback_t *callback, void *data) > { > struct vlapic *target; > struct vcpu *v; > @@ -78,7 +76,8 @@ int vmsi_deliver( > target = vlapic_lowest_prio(d, NULL, 0, dest, dest_mode); > if ( target != NULL ) > { > - vmsi_inj_irq(target, vector, trig_mode, delivery_mode); > + vmsi_inj_irq(target, vector, trig_mode, delivery_mode, callback, > + data); > break; > } > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "null MSI round robin: vector=%02x\n", > @@ -89,8 +88,8 @@ int vmsi_deliver( > for_each_vcpu ( d, v ) > if ( vlapic_match_dest(vcpu_vlapic(v), NULL, > 0, dest, dest_mode) ) > - vmsi_inj_irq(vcpu_vlapic(v), vector, > - trig_mode, delivery_mode); > + vmsi_inj_irq(vcpu_vlapic(v), vector, trig_mode, delivery_mode, > + callback, data); > break; > > default: > @@ -103,6 +102,14 @@ int vmsi_deliver( > return 0; > } > > + Stray blank line > +int vmsi_deliver(struct domain *d, int vector, uint8_t dest, uint8_t dest_mode, > + uint8_t delivery_mode, uint8_t trig_mode) > +{ > + return vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, > + trig_mode, NULL, NULL); > +} > + > void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) > { > uint32_t flags = pirq_dpci->gmsi.gflags; > @@ -119,7 +126,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) > > ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI); > > - vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); > + vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, trig_mode, > + hvm_dpci_msi_eoi, NULL); I think there are more efficiencies possible here. E.g. if is_hardware_domain(d) is true then hvm_dpci_msi_eoi() will do nothing, so no point in setting up the callback in that case. Paul > } > > /* Return value, -1 : multi-dests, non-negative value: dest_vcpu_id */ > diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c > index 536e91ad76..bff0f6628a 100644 > --- a/xen/drivers/passthrough/io.c > +++ b/xen/drivers/passthrough/io.c > @@ -874,7 +874,7 @@ static int _hvm_dpci_msi_eoi(struct domain *d, > return 0; > } > > -void hvm_dpci_msi_eoi(unsigned int vector) > +void hvm_dpci_msi_eoi(unsigned int vector, void *data) > { > struct domain *d = current->domain; > > diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h > index b05f619435..759ee486af 100644 > --- a/xen/include/asm-x86/hvm/io.h > +++ b/xen/include/asm-x86/hvm/io.h > @@ -158,7 +158,7 @@ struct hvm_hw_stdvga { > void stdvga_init(struct domain *d); > void stdvga_deinit(struct domain *d); > > -extern void hvm_dpci_msi_eoi(unsigned int vector); > +void hvm_dpci_msi_eoi(unsigned int vector, void *data); > > /* Decode a PCI port IO access into a bus/slot/func/reg. */ > unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr, > -- > 2.28.0
On Wed, Sep 30, 2020 at 12:57:31PM +0100, Paul Durrant wrote: > > -----Original Message----- > > From: Roger Pau Monne <roger.pau@citrix.com> > > Sent: 30 September 2020 11:41 > > To: xen-devel@lists.xenproject.org > > Cc: Roger Pau Monne <roger.pau@citrix.com>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper > > <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org> > > Subject: [PATCH v2 04/11] x86/vmsi: use the newly introduced EOI callbacks > > > > Remove the unconditional call to hvm_dpci_msi_eoi in vlapic_handle_EOI > > and instead use the newly introduced EOI callback mechanism in order > > to register a callback for MSI vectors injected from passed through > > devices. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > xen/arch/x86/hvm/vlapic.c | 2 -- > > xen/arch/x86/hvm/vmsi.c | 36 ++++++++++++++++++++++-------------- > > xen/drivers/passthrough/io.c | 2 +- > > xen/include/asm-x86/hvm/io.h | 2 +- > > 4 files changed, 24 insertions(+), 18 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c > > index 38c62a02e6..8a18b33428 100644 > > --- a/xen/arch/x86/hvm/vlapic.c > > +++ b/xen/arch/x86/hvm/vlapic.c > > @@ -496,8 +496,6 @@ void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) > > if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) > > vioapic_update_EOI(vector); > > > > - hvm_dpci_msi_eoi(vector); > > - > > spin_lock_irqsave(&vlapic->callback_lock, flags); > > callback = vlapic->callbacks[index].callback; > > vlapic->callbacks[index].callback = NULL; > > diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c > > index 7ca19353ab..e192c4c6da 100644 > > --- a/xen/arch/x86/hvm/vmsi.c > > +++ b/xen/arch/x86/hvm/vmsi.c > > @@ -44,11 +44,9 @@ > > #include <asm/event.h> > > #include <asm/io_apic.h> > > > > -static void vmsi_inj_irq( > > - struct vlapic *target, > > - uint8_t vector, > > - uint8_t trig_mode, > > - uint8_t delivery_mode) > > +static void vmsi_inj_irq(struct vlapic *target, uint8_t vector, > > + uint8_t trig_mode, uint8_t delivery_mode, > > + vlapic_eoi_callback_t *callback, void *data) > > { > > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "vmsi_inj_irq: vec %02x trig %d dm %d\n", > > vector, trig_mode, delivery_mode); > > @@ -57,17 +55,17 @@ static void vmsi_inj_irq( > > { > > case dest_Fixed: > > case dest_LowestPrio: > > - vlapic_set_irq(target, vector, trig_mode); > > + vlapic_set_irq_callback(target, vector, trig_mode, callback, data); > > break; > > default: > > BUG(); > > } > > } > > > > -int vmsi_deliver( > > - struct domain *d, int vector, > > - uint8_t dest, uint8_t dest_mode, > > - uint8_t delivery_mode, uint8_t trig_mode) > > +static int vmsi_deliver_callback(struct domain *d, int vector, uint8_t dest, > > + uint8_t dest_mode, uint8_t delivery_mode, > > + uint8_t trig_mode, > > + vlapic_eoi_callback_t *callback, void *data) > > { > > struct vlapic *target; > > struct vcpu *v; > > @@ -78,7 +76,8 @@ int vmsi_deliver( > > target = vlapic_lowest_prio(d, NULL, 0, dest, dest_mode); > > if ( target != NULL ) > > { > > - vmsi_inj_irq(target, vector, trig_mode, delivery_mode); > > + vmsi_inj_irq(target, vector, trig_mode, delivery_mode, callback, > > + data); > > break; > > } > > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "null MSI round robin: vector=%02x\n", > > @@ -89,8 +88,8 @@ int vmsi_deliver( > > for_each_vcpu ( d, v ) > > if ( vlapic_match_dest(vcpu_vlapic(v), NULL, > > 0, dest, dest_mode) ) > > - vmsi_inj_irq(vcpu_vlapic(v), vector, > > - trig_mode, delivery_mode); > > + vmsi_inj_irq(vcpu_vlapic(v), vector, trig_mode, delivery_mode, > > + callback, data); > > break; > > > > default: > > @@ -103,6 +102,14 @@ int vmsi_deliver( > > return 0; > > } > > > > + > > Stray blank line > > > +int vmsi_deliver(struct domain *d, int vector, uint8_t dest, uint8_t dest_mode, > > + uint8_t delivery_mode, uint8_t trig_mode) > > +{ > > + return vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, > > + trig_mode, NULL, NULL); > > +} > > + > > void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) > > { > > uint32_t flags = pirq_dpci->gmsi.gflags; > > @@ -119,7 +126,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) > > > > ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI); > > > > - vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); > > + vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, trig_mode, > > + hvm_dpci_msi_eoi, NULL); > > I think there are more efficiencies possible here. E.g. if is_hardware_domain(d) is true then hvm_dpci_msi_eoi() will do nothing, so no point in setting up the callback in that case. No, I don't think that's true, hvm_dpci_msi_eoi will execute the call to pt_pirq_iterate and _hvm_dpci_msi_eoi for the hardware domain, because MSI vectors need to be acked from the physical lapic even in the hardware domain case. This was fixed by commit 7b653a245ff. Thanks, Roger.
On 30.09.2020 12:41, Roger Pau Monne wrote: > Remove the unconditional call to hvm_dpci_msi_eoi in vlapic_handle_EOI > and instead use the newly introduced EOI callback mechanism in order > to register a callback for MSI vectors injected from passed through > devices. What I'm kind of missing here is a word on why this is an improvement: After all ... > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -496,8 +496,6 @@ void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) > if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) > vioapic_update_EOI(vector); > > - hvm_dpci_msi_eoi(vector); ... you're exchanging this direct call for a more complex model with an indirect one (to the same function). > @@ -119,7 +126,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) > > ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI); > > - vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); > + vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, trig_mode, > + hvm_dpci_msi_eoi, NULL); > } While I agree with your reply to Paul regarding Dom0, I still think the entire if() in hvm_dpci_msi_eoi() should be converted into a conditional here. There's no point registering the callback if it's not going to do anything. However, looking further, the "!hvm_domain_irq(d)->dpci && !is_hardware_domain(d)" can be simply dropped altogether, right away. It's now fulfilled by the identical check at the top of hvm_dirq_assist(), thus guarding the sole call site of this function. The !is_iommu_enabled(d) is slightly more involved to prove, but it should also be possible to simply drop. What might help here is a separate change to suppress opening of HVM_DPCI_SOFTIRQ when there's no IOMMU in the system, as then it becomes obvious that this part of the condition is guaranteed by hvm_do_IRQ_dpci(), being the only site where the softirq can get raised (apart from the softirq handler itself). To sum up - the call above can probably stay as is, but the callback can be simplified as a result of the change. > --- a/xen/drivers/passthrough/io.c > +++ b/xen/drivers/passthrough/io.c > @@ -874,7 +874,7 @@ static int _hvm_dpci_msi_eoi(struct domain *d, > return 0; > } > > -void hvm_dpci_msi_eoi(unsigned int vector) > +void hvm_dpci_msi_eoi(unsigned int vector, void *data) > { > struct domain *d = current->domain; Instead of passing NULL for data and latching d from current, how about you make the registration pass d to more easily use here? Jan
On Fri, Oct 02, 2020 at 05:25:34PM +0200, Jan Beulich wrote: > On 30.09.2020 12:41, Roger Pau Monne wrote: > > Remove the unconditional call to hvm_dpci_msi_eoi in vlapic_handle_EOI > > and instead use the newly introduced EOI callback mechanism in order > > to register a callback for MSI vectors injected from passed through > > devices. > > What I'm kind of missing here is a word on why this is an improvement: > After all ... > > > --- a/xen/arch/x86/hvm/vlapic.c > > +++ b/xen/arch/x86/hvm/vlapic.c > > @@ -496,8 +496,6 @@ void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) > > if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) > > vioapic_update_EOI(vector); > > > > - hvm_dpci_msi_eoi(vector); > > ... you're exchanging this direct call for a more complex model with > an indirect one (to the same function). Sure. But this direct call will be made for each vlapic EOI, while my added callback will only be executed if the vector was injected by thee vmsi code, and hence will remove pointless calls to hvm_dpci_msi_eoi. It's IMO not feasible to be adding hardcoded calls to vlapic_handle_EOI for each possible subsystem or emulated device that wants to be notified of EOIs, hence we need some kind of generic framework to achieve this. > > @@ -119,7 +126,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) > > > > ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI); > > > > - vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); > > + vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, trig_mode, > > + hvm_dpci_msi_eoi, NULL); > > } > > While I agree with your reply to Paul regarding Dom0, I still think > the entire if() in hvm_dpci_msi_eoi() should be converted into a > conditional here. There's no point registering the callback if it's > not going to do anything. > > However, looking further, the "!hvm_domain_irq(d)->dpci && > !is_hardware_domain(d)" can be simply dropped altogether, right away. > It's now fulfilled by the identical check at the top of > hvm_dirq_assist(), thus guarding the sole call site of this function. > > The !is_iommu_enabled(d) is slightly more involved to prove, but it > should also be possible to simply drop. What might help here is a > separate change to suppress opening of HVM_DPCI_SOFTIRQ when there's > no IOMMU in the system, as then it becomes obvious that this part of > the condition is guaranteed by hvm_do_IRQ_dpci(), being the only > site where the softirq can get raised (apart from the softirq > handler itself). > > To sum up - the call above can probably stay as is, but the callback > can be simplified as a result of the change. Yes, I agree. Would you be fine with converting the check in the callback into an assert, or would you rather have it removed completely? > > --- a/xen/drivers/passthrough/io.c > > +++ b/xen/drivers/passthrough/io.c > > @@ -874,7 +874,7 @@ static int _hvm_dpci_msi_eoi(struct domain *d, > > return 0; > > } > > > > -void hvm_dpci_msi_eoi(unsigned int vector) > > +void hvm_dpci_msi_eoi(unsigned int vector, void *data) > > { > > struct domain *d = current->domain; > > Instead of passing NULL for data and latching d from current, how > about you make the registration pass d to more easily use here? Yes, I think that's fine - we already have the domain pointer in vmsi_deliver_callback so it could be passed as the callback private data. Thanks, Roger.
On 13.10.2020 16:47, Roger Pau Monné wrote: > On Fri, Oct 02, 2020 at 05:25:34PM +0200, Jan Beulich wrote: >> On 30.09.2020 12:41, Roger Pau Monne wrote: >>> @@ -119,7 +126,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) >>> >>> ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI); >>> >>> - vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); >>> + vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, trig_mode, >>> + hvm_dpci_msi_eoi, NULL); >>> } >> >> While I agree with your reply to Paul regarding Dom0, I still think >> the entire if() in hvm_dpci_msi_eoi() should be converted into a >> conditional here. There's no point registering the callback if it's >> not going to do anything. >> >> However, looking further, the "!hvm_domain_irq(d)->dpci && >> !is_hardware_domain(d)" can be simply dropped altogether, right away. >> It's now fulfilled by the identical check at the top of >> hvm_dirq_assist(), thus guarding the sole call site of this function. >> >> The !is_iommu_enabled(d) is slightly more involved to prove, but it >> should also be possible to simply drop. What might help here is a >> separate change to suppress opening of HVM_DPCI_SOFTIRQ when there's >> no IOMMU in the system, as then it becomes obvious that this part of >> the condition is guaranteed by hvm_do_IRQ_dpci(), being the only >> site where the softirq can get raised (apart from the softirq >> handler itself). >> >> To sum up - the call above can probably stay as is, but the callback >> can be simplified as a result of the change. > > Yes, I agree. Would you be fine with converting the check in the > callback into an assert, or would you rather have it removed > completely? Either way is fine with me, I think. Jan
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 38c62a02e6..8a18b33428 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -496,8 +496,6 @@ void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) vioapic_update_EOI(vector); - hvm_dpci_msi_eoi(vector); - spin_lock_irqsave(&vlapic->callback_lock, flags); callback = vlapic->callbacks[index].callback; vlapic->callbacks[index].callback = NULL; diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c index 7ca19353ab..e192c4c6da 100644 --- a/xen/arch/x86/hvm/vmsi.c +++ b/xen/arch/x86/hvm/vmsi.c @@ -44,11 +44,9 @@ #include <asm/event.h> #include <asm/io_apic.h> -static void vmsi_inj_irq( - struct vlapic *target, - uint8_t vector, - uint8_t trig_mode, - uint8_t delivery_mode) +static void vmsi_inj_irq(struct vlapic *target, uint8_t vector, + uint8_t trig_mode, uint8_t delivery_mode, + vlapic_eoi_callback_t *callback, void *data) { HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "vmsi_inj_irq: vec %02x trig %d dm %d\n", vector, trig_mode, delivery_mode); @@ -57,17 +55,17 @@ static void vmsi_inj_irq( { case dest_Fixed: case dest_LowestPrio: - vlapic_set_irq(target, vector, trig_mode); + vlapic_set_irq_callback(target, vector, trig_mode, callback, data); break; default: BUG(); } } -int vmsi_deliver( - struct domain *d, int vector, - uint8_t dest, uint8_t dest_mode, - uint8_t delivery_mode, uint8_t trig_mode) +static int vmsi_deliver_callback(struct domain *d, int vector, uint8_t dest, + uint8_t dest_mode, uint8_t delivery_mode, + uint8_t trig_mode, + vlapic_eoi_callback_t *callback, void *data) { struct vlapic *target; struct vcpu *v; @@ -78,7 +76,8 @@ int vmsi_deliver( target = vlapic_lowest_prio(d, NULL, 0, dest, dest_mode); if ( target != NULL ) { - vmsi_inj_irq(target, vector, trig_mode, delivery_mode); + vmsi_inj_irq(target, vector, trig_mode, delivery_mode, callback, + data); break; } HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "null MSI round robin: vector=%02x\n", @@ -89,8 +88,8 @@ int vmsi_deliver( for_each_vcpu ( d, v ) if ( vlapic_match_dest(vcpu_vlapic(v), NULL, 0, dest, dest_mode) ) - vmsi_inj_irq(vcpu_vlapic(v), vector, - trig_mode, delivery_mode); + vmsi_inj_irq(vcpu_vlapic(v), vector, trig_mode, delivery_mode, + callback, data); break; default: @@ -103,6 +102,14 @@ int vmsi_deliver( return 0; } + +int vmsi_deliver(struct domain *d, int vector, uint8_t dest, uint8_t dest_mode, + uint8_t delivery_mode, uint8_t trig_mode) +{ + return vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, + trig_mode, NULL, NULL); +} + void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) { uint32_t flags = pirq_dpci->gmsi.gflags; @@ -119,7 +126,8 @@ void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci) ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI); - vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); + vmsi_deliver_callback(d, vector, dest, dest_mode, delivery_mode, trig_mode, + hvm_dpci_msi_eoi, NULL); } /* Return value, -1 : multi-dests, non-negative value: dest_vcpu_id */ diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c index 536e91ad76..bff0f6628a 100644 --- a/xen/drivers/passthrough/io.c +++ b/xen/drivers/passthrough/io.c @@ -874,7 +874,7 @@ static int _hvm_dpci_msi_eoi(struct domain *d, return 0; } -void hvm_dpci_msi_eoi(unsigned int vector) +void hvm_dpci_msi_eoi(unsigned int vector, void *data) { struct domain *d = current->domain; diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h index b05f619435..759ee486af 100644 --- a/xen/include/asm-x86/hvm/io.h +++ b/xen/include/asm-x86/hvm/io.h @@ -158,7 +158,7 @@ struct hvm_hw_stdvga { void stdvga_init(struct domain *d); void stdvga_deinit(struct domain *d); -extern void hvm_dpci_msi_eoi(unsigned int vector); +void hvm_dpci_msi_eoi(unsigned int vector, void *data); /* Decode a PCI port IO access into a bus/slot/func/reg. */ unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr,
Remove the unconditional call to hvm_dpci_msi_eoi in vlapic_handle_EOI and instead use the newly introduced EOI callback mechanism in order to register a callback for MSI vectors injected from passed through devices. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/hvm/vlapic.c | 2 -- xen/arch/x86/hvm/vmsi.c | 36 ++++++++++++++++++++++-------------- xen/drivers/passthrough/io.c | 2 +- xen/include/asm-x86/hvm/io.h | 2 +- 4 files changed, 24 insertions(+), 18 deletions(-)