| Message ID | 20200812124709.4165-4-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/vlapic: implement EOI callbacks | expand |
> -----Original Message----- > From: Roger Pau Monne <roger.pau@citrix.com> > Sent: 12 August 2020 13:47 > 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 3/5] 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 | 4 +++- > xen/include/asm-x86/hvm/io.h | 2 +- > 4 files changed, 26 insertions(+), 18 deletions(-) > > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c > index 7369be468b..3b3b3d7621 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) > else if ( has_viridian_synic(d) ) > viridian_synic_ack_sint(v, vector); > > - hvm_dpci_msi_eoi(d, vector); > - > spin_lock_irqsave(&vlapic->callback_lock, flags); > callback = vlapic->callbacks[vector].callback; > data = vlapic->callbacks[vector].data; > 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 6b1305a3e5..3793029b29 100644 > --- a/xen/drivers/passthrough/io.c > +++ b/xen/drivers/passthrough/io.c > @@ -874,8 +874,10 @@ static int _hvm_dpci_msi_eoi(struct domain *d, > return 0; > } > > -void hvm_dpci_msi_eoi(struct domain *d, int vector) > +void hvm_dpci_msi_eoi(struct vcpu *v, unsigned int vector, void *data) > { > + struct domain *d = v->domain; > + Could we actually drop the vcpu parameter here... i.e. is there any case where this code will be invoked with v != current? > if ( !is_iommu_enabled(d) || > (!hvm_domain_irq(d)->dpci && !is_hardware_domain(d)) ) > return; > diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h > index 558426b772..450c7c8acb 100644 > --- a/xen/include/asm-x86/hvm/io.h > +++ b/xen/include/asm-x86/hvm/io.h > @@ -159,7 +159,7 @@ struct hvm_hw_stdvga { > void stdvga_init(struct domain *d); > void stdvga_deinit(struct domain *d); > > -extern void hvm_dpci_msi_eoi(struct domain *d, int vector); > +void hvm_dpci_msi_eoi(struct vcpu *v, 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 Thu, Aug 13, 2020 at 09:19:30AM +0100, Paul Durrant wrote: > > -----Original Message----- > > From: Roger Pau Monne <roger.pau@citrix.com> > > Sent: 12 August 2020 13:47 > > 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 3/5] 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 | 4 +++- > > xen/include/asm-x86/hvm/io.h | 2 +- > > 4 files changed, 26 insertions(+), 18 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c > > index 7369be468b..3b3b3d7621 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) > > else if ( has_viridian_synic(d) ) > > viridian_synic_ack_sint(v, vector); > > > > - hvm_dpci_msi_eoi(d, vector); > > - > > spin_lock_irqsave(&vlapic->callback_lock, flags); > > callback = vlapic->callbacks[vector].callback; > > data = vlapic->callbacks[vector].data; > > 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 6b1305a3e5..3793029b29 100644 > > --- a/xen/drivers/passthrough/io.c > > +++ b/xen/drivers/passthrough/io.c > > @@ -874,8 +874,10 @@ static int _hvm_dpci_msi_eoi(struct domain *d, > > return 0; > > } > > > > -void hvm_dpci_msi_eoi(struct domain *d, int vector) > > +void hvm_dpci_msi_eoi(struct vcpu *v, unsigned int vector, void *data) > > { > > + struct domain *d = v->domain; > > + > > Could we actually drop the vcpu parameter here... i.e. is there any case where this code will be invoked with v != current? viridian_synic_wrmsr seems to call vlapic_EOI_set without enforcing v == current (as it seems to be fine being called from v != current as long as it's not running). There's also a call to vlapic_EOI_set in vlapic_has_pending_irq that I'm not sure won't be called with v != current. In a normal hardware architecture I would say the EOI can only be performed from the same CPU, and hence v == current, on Xen however I'm not sure if any of the assists that we provide would allow for the EOI to be performed from a different vCPU. I can prepare a pre-patch to change the functions called from vlapic_handle_EOI to not take a domain or vcpu parameter. Thanks, Roger.
On 12.08.2020 14:47, 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. In patch 2 you merely invoke the callback when the EOI arrived, but you don't clear the callback (unless I've missed something). Here you register the callback once per triggering of the IRQ, without clearing it from the callback itself either. Why is it correct / safe to keep the callback registered? What about conflicting callbacks for shared vectors? Jan
On Mon, Aug 24, 2020 at 04:06:31PM +0200, Jan Beulich wrote: > On 12.08.2020 14:47, 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. > > In patch 2 you merely invoke the callback when the EOI arrived, > but you don't clear the callback (unless I've missed something). > Here you register the callback once per triggering of the IRQ, > without clearing it from the callback itself either. It gets cleared on the next call to vlapic_set_irq_callback, or set to a new callback value if the passed callback is not NULL. > Why is it > correct / safe to keep the callback registered? The next vector injected is going to clear it, so should be safe, as no vector can be injected without calling vlapic_set_irq_callback. > What about > conflicting callbacks for shared vectors? In this callback model for vlapic only the last injected vector callback would get executed. It's my understanding that lapic vectors cannot be safely shared unless there's a higher level interrupt controller (ie: an IO-APIC) that does the sharing. I have further patches that also add EOI callbacks to vIO-APIC and 8259A PICs, so that all interrupt controllers can handle EOI callbacks. Thanks, Roger.
On 24.08.2020 16:44, Roger Pau Monné wrote: > On Mon, Aug 24, 2020 at 04:06:31PM +0200, Jan Beulich wrote: >> On 12.08.2020 14:47, 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. >> >> In patch 2 you merely invoke the callback when the EOI arrived, >> but you don't clear the callback (unless I've missed something). >> Here you register the callback once per triggering of the IRQ, >> without clearing it from the callback itself either. > > It gets cleared on the next call to vlapic_set_irq_callback, or set to > a new callback value if the passed callback is not NULL. > >> Why is it >> correct / safe to keep the callback registered? > > The next vector injected is going to clear it, so should be safe, as > no vector can be injected without calling vlapic_set_irq_callback. But what about duplicate EOIs, even if just by bug or erratum? I notice, for example, that VMX'es VMEXIT handler directly calls vlapic_handle_EOI(). I'd find it more safe if an unexpected EOI didn't get any callback invoked. >> What about >> conflicting callbacks for shared vectors? > > In this callback model for vlapic only the last injected vector > callback would get executed. It's my understanding that lapic > vectors cannot be safely shared unless there's a higher level > interrupt controller (ie: an IO-APIC) that does the sharing. As said on a different, but pretty recent thread: I do think sharing is possible if devices and drivers meet certain criteria (in particular have a way to determine which of the devices actually require service). It's just not something one would normally do. But iirc such a model was used in good old DOS to overcome the 15 IRQs limit (of which quite a few had fixed purposes, so for add-in devices there were only very few left). Jan
On Mon, Aug 24, 2020 at 06:07:28PM +0200, Jan Beulich wrote: > On 24.08.2020 16:44, Roger Pau Monné wrote: > > On Mon, Aug 24, 2020 at 04:06:31PM +0200, Jan Beulich wrote: > >> On 12.08.2020 14:47, 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. > >> > >> In patch 2 you merely invoke the callback when the EOI arrived, > >> but you don't clear the callback (unless I've missed something). > >> Here you register the callback once per triggering of the IRQ, > >> without clearing it from the callback itself either. > > > > It gets cleared on the next call to vlapic_set_irq_callback, or set to > > a new callback value if the passed callback is not NULL. > > > >> Why is it > >> correct / safe to keep the callback registered? > > > > The next vector injected is going to clear it, so should be safe, as > > no vector can be injected without calling vlapic_set_irq_callback. > > But what about duplicate EOIs, even if just by bug or erratum? > I notice, for example, that VMX'es VMEXIT handler directly > calls vlapic_handle_EOI(). Yes, but that's expected and required, because when using VMX LAPIC virtualization you get an explicit vmexit (EXIT_REASON_EOI_INDUCED) on EOI of requested vectors by using the EOI exit bitmap (vmx_update_eoi_exit_bitmap). > I'd find it more safe if an > unexpected EOI didn't get any callback invoked. OK, the callback can be certainly cleared in vlapic_handle_EOI. > > >> What about > >> conflicting callbacks for shared vectors? > > > > In this callback model for vlapic only the last injected vector > > callback would get executed. It's my understanding that lapic > > vectors cannot be safely shared unless there's a higher level > > interrupt controller (ie: an IO-APIC) that does the sharing. > > As said on a different, but pretty recent thread: I do think > sharing is possible if devices and drivers meet certain criteria > (in particular have a way to determine which of the devices > actually require service). It's just not something one would > normally do. But iirc such a model was used in good old DOS to > overcome the 15 IRQs limit (of which quite a few had fixed > purposes, so for add-in devices there were only very few left). So my callback model for the vIO-APIC/vPIC is different from the one used for the vlapic. In that case callers must use a helper to register/remove a callback for GSIs, and a single GSI can have multiple callbacks attached. Such interface works well with the vIO-APIC/vPIC because interrupts from devices are statically assigned a GSI, and you only need to register the callback when the device is instantiated. For vlapic OTOH this would be more complex, as a guest might decide to change MSI vectors constantly and thus require a non-trivial amount of registrations/removals of callbacks. I was assuming that any sane OS wouldn't share a lapic vector for multiple devices, and that hence we could get away without having to implement multiple per-vector callback support. Would you be fine with clearing the callback in vlapic_handle_EOI and then vlapic_set_irq_callback complaining if it finds there's a previous callback different than the one provided already set for the to be injected vector? Thanks, Roger.
On 24.08.2020 18:39, Roger Pau Monné wrote: > Would you be fine with clearing the callback in vlapic_handle_EOI and Yes - as said, I'd prefer such a model. > then vlapic_set_irq_callback complaining if it finds there's a > previous callback different than the one provided already set for the > to be injected vector? For debugging purposes this may indeed be helpful, albeit I'll admit that initially I didn't consider this as necessary. Jan
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 7369be468b..3b3b3d7621 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) else if ( has_viridian_synic(d) ) viridian_synic_ack_sint(v, vector); - hvm_dpci_msi_eoi(d, vector); - spin_lock_irqsave(&vlapic->callback_lock, flags); callback = vlapic->callbacks[vector].callback; data = vlapic->callbacks[vector].data; 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 6b1305a3e5..3793029b29 100644 --- a/xen/drivers/passthrough/io.c +++ b/xen/drivers/passthrough/io.c @@ -874,8 +874,10 @@ static int _hvm_dpci_msi_eoi(struct domain *d, return 0; } -void hvm_dpci_msi_eoi(struct domain *d, int vector) +void hvm_dpci_msi_eoi(struct vcpu *v, unsigned int vector, void *data) { + struct domain *d = v->domain; + if ( !is_iommu_enabled(d) || (!hvm_domain_irq(d)->dpci && !is_hardware_domain(d)) ) return; diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h index 558426b772..450c7c8acb 100644 --- a/xen/include/asm-x86/hvm/io.h +++ b/xen/include/asm-x86/hvm/io.h @@ -159,7 +159,7 @@ struct hvm_hw_stdvga { void stdvga_init(struct domain *d); void stdvga_deinit(struct domain *d); -extern void hvm_dpci_msi_eoi(struct domain *d, int vector); +void hvm_dpci_msi_eoi(struct vcpu *v, 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 | 4 +++- xen/include/asm-x86/hvm/io.h | 2 +- 4 files changed, 26 insertions(+), 18 deletions(-)