| Message ID | 20170427063203.GC11487@tungsten.ozlabs.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 27, 2017 at 04:32:03PM +1000, Sam Bobroff wrote: > Kernel commit 17d48610ae0f ("KVM: PPC: Book 3S: XICS: Implement ICS > P/Q states") added new bits to the state used by KVM IRQs. Currently, > QEMU does not preserve these bits, so migrating (or otherwise saving > and restoring) the guest state causes the P and Q bits to be cleared. > > Clearing the P bit has no effect, because the kernel will set it based > on other data, but the loss of a set Q bit will cause a lost > interrupt. > > This patch preserves the P and Q bits, correcting the problem. > > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com> So, I've applied this to ppc-for-2.10, because AFAICT it makes some situations better, and can't make any worse. However, it bothers me that this is effectively adding two bits to the TCG XICS state, but isn't putting any TCG logic to handle them correctly. I'm rather hoping corresponding TCG logic is coming. I don't believe TCG<->KVM migration works at present, but I don't think there's a theoretical reason it couldn't, and I'd certainly prefer not to introduce reasons for it not to work. > --- > hw/intc/xics_kvm.c | 12 ++++++++++++ > include/hw/ppc/xics.h | 2 ++ > 2 files changed, 14 insertions(+) > > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 03c1fc77cb..dd93531ae3 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -229,6 +229,12 @@ static void ics_get_kvm_state(ICSState *ics) > | XICS_STATUS_REJECTED; > } > } > + if (state & KVM_XICS_PRESENTED) { > + irq->status |= XICS_STATUS_PRESENTED; > + } > + if (state & KVM_XICS_QUEUED) { > + irq->status |= XICS_STATUS_QUEUED; > + } > } > } > > @@ -266,6 +272,12 @@ static int ics_set_kvm_state(ICSState *ics, int version_id) > state |= KVM_XICS_PENDING; > } > } > + if (irq->status & XICS_STATUS_PRESENTED) { > + state |= KVM_XICS_PRESENTED; > + } > + if (irq->status & XICS_STATUS_QUEUED) { > + state |= KVM_XICS_QUEUED; > + } > > ret = ioctl(kernel_xics_fd, KVM_SET_DEVICE_ATTR, &attr); > if (ret != 0) { > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 9a5e715fe5..36c79eb45c 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -131,6 +131,8 @@ struct ICSIRQState { > #define XICS_STATUS_SENT 0x2 > #define XICS_STATUS_REJECTED 0x4 > #define XICS_STATUS_MASKED_PENDING 0x8 > +#define XICS_STATUS_PRESENTED 0x10 > +#define XICS_STATUS_QUEUED 0x20 > uint8_t status; > /* (flags & XICS_FLAGS_IRQ_MASK) == 0 means the interrupt is not allocated */ > #define XICS_FLAGS_IRQ_LSI 0x1
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 03c1fc77cb..dd93531ae3 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -229,6 +229,12 @@ static void ics_get_kvm_state(ICSState *ics) | XICS_STATUS_REJECTED; } } + if (state & KVM_XICS_PRESENTED) { + irq->status |= XICS_STATUS_PRESENTED; + } + if (state & KVM_XICS_QUEUED) { + irq->status |= XICS_STATUS_QUEUED; + } } } @@ -266,6 +272,12 @@ static int ics_set_kvm_state(ICSState *ics, int version_id) state |= KVM_XICS_PENDING; } } + if (irq->status & XICS_STATUS_PRESENTED) { + state |= KVM_XICS_PRESENTED; + } + if (irq->status & XICS_STATUS_QUEUED) { + state |= KVM_XICS_QUEUED; + } ret = ioctl(kernel_xics_fd, KVM_SET_DEVICE_ATTR, &attr); if (ret != 0) { diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 9a5e715fe5..36c79eb45c 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -131,6 +131,8 @@ struct ICSIRQState { #define XICS_STATUS_SENT 0x2 #define XICS_STATUS_REJECTED 0x4 #define XICS_STATUS_MASKED_PENDING 0x8 +#define XICS_STATUS_PRESENTED 0x10 +#define XICS_STATUS_QUEUED 0x20 uint8_t status; /* (flags & XICS_FLAGS_IRQ_MASK) == 0 means the interrupt is not allocated */ #define XICS_FLAGS_IRQ_LSI 0x1
Kernel commit 17d48610ae0f ("KVM: PPC: Book 3S: XICS: Implement ICS P/Q states") added new bits to the state used by KVM IRQs. Currently, QEMU does not preserve these bits, so migrating (or otherwise saving and restoring) the guest state causes the P and Q bits to be cleared. Clearing the P bit has no effect, because the kernel will set it based on other data, but the loss of a set Q bit will cause a lost interrupt. This patch preserves the P and Q bits, correcting the problem. Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com> --- hw/intc/xics_kvm.c | 12 ++++++++++++ include/hw/ppc/xics.h | 2 ++ 2 files changed, 14 insertions(+)