Instead of switching off the sources, set their state to PENDING to
possibly catch a hotplug event occuring while the VM is stopped. At
resume, check the previous state and if an interrupt was queued,
generate a trigger.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/intc/spapr_xive_kvm.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index 99a829fb3f60..64d160babb26 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -500,8 +500,16 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
if (running) {
for (i = 0; i < xsrc->nr_irqs; i++) {
uint8_t pq = xive_source_esb_get(xsrc, i);
- if (xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8)) != 0x1) {
- error_report("XIVE: IRQ %d has an invalid state", i);
+ uint8_t old_pq;
+
+ old_pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8));
+
+ /*
+ * If an interrupt was queued (hotplug event) while VM was
+ * stopped, generate a trigger.
+ */
+ if (pq == XIVE_ESB_RESET && old_pq == XIVE_ESB_QUEUED) {
+ xive_esb_trigger(xsrc, i);
}
}
@@ -515,7 +523,15 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
* migration is in progress.
*/
for (i = 0; i < xsrc->nr_irqs; i++) {
- uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_01);
+ uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
+
+ /*
+ * PQ is set to PENDING to possibly catch a hotplug event
+ * occuring while the VM is stopped.
+ */
+ if (pq != XIVE_ESB_OFF) {
+ pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_10);
+ }
xive_source_esb_set(xsrc, i, pq);
}
--
2.20.1
On Fri, Feb 22, 2019 at 02:13:22PM +0100, Cédric Le Goater wrote:
> Instead of switching off the sources, set their state to PENDING to
> possibly catch a hotplug event occuring while the VM is stopped. At
> resume, check the previous state and if an interrupt was queued,
> generate a trigger.
First, I think it would be better to fold this fix into the patch
introducing the state change handlers.
Second, IIUC this would handle any instance of an irq being triggered
while the VM is stopped. Hotplug interrupts is one obvious case of
that, but I'm not sure its the only one. VFIO devices could interrupt
while the VM is stopped, I think. Maybe even emulated devices
depending on how their synchronization with the cpu run state works.
There might be other cases. Does that sound right to you?
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/intc/spapr_xive_kvm.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 99a829fb3f60..64d160babb26 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -500,8 +500,16 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
> if (running) {
> for (i = 0; i < xsrc->nr_irqs; i++) {
> uint8_t pq = xive_source_esb_get(xsrc, i);
> - if (xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8)) != 0x1) {
> - error_report("XIVE: IRQ %d has an invalid state", i);
> + uint8_t old_pq;
> +
> + old_pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8));
> +
> + /*
> + * If an interrupt was queued (hotplug event) while VM was
> + * stopped, generate a trigger.
> + */
> + if (pq == XIVE_ESB_RESET && old_pq == XIVE_ESB_QUEUED) {
> + xive_esb_trigger(xsrc, i);
> }
> }
>
> @@ -515,7 +523,15 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
> * migration is in progress.
> */
> for (i = 0; i < xsrc->nr_irqs; i++) {
> - uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_01);
> + uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
> +
> + /*
> + * PQ is set to PENDING to possibly catch a hotplug event
> + * occuring while the VM is stopped.
> + */
> + if (pq != XIVE_ESB_OFF) {
> + pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_10);
> + }
> xive_source_esb_set(xsrc, i, pq);
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 2/26/19 5:17 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 02:13:22PM +0100, Cédric Le Goater wrote:
>> Instead of switching off the sources, set their state to PENDING to
>> possibly catch a hotplug event occuring while the VM is stopped. At
>> resume, check the previous state and if an interrupt was queued,
>> generate a trigger.
>
> First, I think it would be better to fold this fix into the patch
> introducing the state change handlers.>
> Second, IIUC this would handle any instance of an irq being triggered
> while the VM is stopped.
yes.
> Hotplug interrupts is one obvious case of that, but I'm not sure its
> the only one.
Do we really need to support device hotplug when the VM is stopped ?
Is that a libvirt requirement ?
> VFIO devices could interrupt while the VM is stopped, I think.
If the guest has configured and mapped the IRQs, I would say yes.
> Maybe even emulated devices
> depending on how their synchronization with the cpu run state works.
The console is one example.
> There might be other cases. Does that sound right to you?
yes.
Supporting interrupts while a VM is stopped seems like a weird
test scenario to me. Should we or should we not ?
Thanks,
C.
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> hw/intc/spapr_xive_kvm.c | 22 +++++++++++++++++++---
>> 1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
>> index 99a829fb3f60..64d160babb26 100644
>> --- a/hw/intc/spapr_xive_kvm.c
>> +++ b/hw/intc/spapr_xive_kvm.c
>> @@ -500,8 +500,16 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
>> if (running) {
>> for (i = 0; i < xsrc->nr_irqs; i++) {
>> uint8_t pq = xive_source_esb_get(xsrc, i);
>> - if (xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8)) != 0x1) {
>> - error_report("XIVE: IRQ %d has an invalid state", i);
>> + uint8_t old_pq;
>> +
>> + old_pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8));
>> +
>> + /*
>> + * If an interrupt was queued (hotplug event) while VM was
>> + * stopped, generate a trigger.
>> + */
>> + if (pq == XIVE_ESB_RESET && old_pq == XIVE_ESB_QUEUED) {
>> + xive_esb_trigger(xsrc, i);
>> }
>> }
>>
>> @@ -515,7 +523,15 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
>> * migration is in progress.
>> */
>> for (i = 0; i < xsrc->nr_irqs; i++) {
>> - uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_01);
>> + uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
>> +
>> + /*
>> + * PQ is set to PENDING to possibly catch a hotplug event
>> + * occuring while the VM is stopped.
>> + */
>> + if (pq != XIVE_ESB_OFF) {
>> + pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_10);
>> + }
>> xive_source_esb_set(xsrc, i, pq);
>> }
>>
>
On Mon, Mar 11, 2019 at 09:59:32PM +0100, Cédric Le Goater wrote: > On 2/26/19 5:17 AM, David Gibson wrote: > > On Fri, Feb 22, 2019 at 02:13:22PM +0100, Cédric Le Goater wrote: > >> Instead of switching off the sources, set their state to PENDING to > >> possibly catch a hotplug event occuring while the VM is stopped. At > >> resume, check the previous state and if an interrupt was queued, > >> generate a trigger. > > > > First, I think it would be better to fold this fix into the patch > > introducing the state change handlers.> > > Second, IIUC this would handle any instance of an irq being triggered > > while the VM is stopped. > > yes. > > > Hotplug interrupts is one obvious case of that, but I'm not sure its > > the only one. > > Do we really need to support device hotplug when the VM is stopped ? > Is that a libvirt requirement ? I'm not actually sure, but I think it's the right thing to do. Interrupts are asynchronous by nature, so it makes sense that they can occur while the machine is otherwise stopped. > > VFIO devices could interrupt while the VM is stopped, I think. > > If the guest has configured and mapped the IRQs, I would say yes. > > > Maybe even emulated devices > > depending on how their synchronization with the cpu run state works. > > The console is one example. > > > There might be other cases. Does that sound right to you? > > yes. > > Supporting interrupts while a VM is stopped seems like a weird > test scenario to me. Should we or should we not ? I think we should. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
© 2016 - 2025 Red Hat, Inc.