On Wed, 8 May 2019 19:19:45 +0200 Cédric Le Goater <c...@kaod.org> wrote:
> The high order bits of the address of the OS event queue is stored in > bits [4-31] of word2 of the XIVE END internal structures and the low > order bits in word3. This structure is using Big Endian ordering and > computing the value requires some simple arithmetic which happens to > be wrong. The mask removing bits [0-3] of word2 is applied to the > wrong value and the resulting address is bogus when above 64GB. > > Guests with more than 64GB of RAM will allocate pages for the OS event > queues which will reside above the 64GB limit. In this case, the XIVE > device model will wake up the CPUs in case of a notification, such as > IPIs, but the update of the event queue will be written at the wrong > place in memory. The result is uncertain as the guest memory is > trashed and IPI are not delivered. > > Introduce a helper xive_end_qaddr() to compute this value correctly in > all places where it is used. > > Signed-off-by: Cédric Le Goater <c...@kaod.org> > --- I guess this patch should have a Fixes: tag and Cc qemu-stable as well since QEMU 4.0 has the issue. Apart from that, LGTM. Reviewed-by: Greg Kurz <gr...@kaod.org> > include/hw/ppc/xive_regs.h | 6 ++++++ > hw/intc/spapr_xive.c | 3 +-- > hw/intc/xive.c | 9 +++------ > 3 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h > index bf36678a242c..1a8c5b5e64f0 100644 > --- a/include/hw/ppc/xive_regs.h > +++ b/include/hw/ppc/xive_regs.h > @@ -208,6 +208,12 @@ typedef struct XiveEND { > #define xive_end_is_backlog(end) (be32_to_cpu((end)->w0) & END_W0_BACKLOG) > #define xive_end_is_escalate(end) (be32_to_cpu((end)->w0) & > END_W0_ESCALATE_CTL) > > +static inline uint64_t xive_end_qaddr(XiveEND *end) > +{ > + return ((uint64_t) be32_to_cpu(end->w2) & 0x0fffffff) << 32 | > + be32_to_cpu(end->w3); > +} > + > /* Notification Virtual Target (NVT) */ > typedef struct XiveNVT { > uint32_t w0; > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > index 666e24e9b447..810435c30cc7 100644 > --- a/hw/intc/spapr_xive.c > +++ b/hw/intc/spapr_xive.c > @@ -1150,8 +1150,7 @@ static target_ulong h_int_get_queue_config(PowerPCCPU > *cpu, > } > > if (xive_end_is_enqueue(end)) { > - args[1] = (uint64_t) be32_to_cpu(end->w2 & 0x0fffffff) << 32 > - | be32_to_cpu(end->w3); > + args[1] = xive_end_qaddr(end); > args[2] = xive_get_field32(END_W0_QSIZE, end->w0) + 12; > } else { > args[1] = 0; > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index a0b87001da25..dcf2fcd10893 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -1042,8 +1042,7 @@ static const TypeInfo xive_source_info = { > > void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor > *mon) > { > - uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fffffff) << 32 > - | be32_to_cpu(end->w3); > + uint64_t qaddr_base = xive_end_qaddr(end); > uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0); > uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1); > uint32_t qentries = 1 << (qsize + 10); > @@ -1072,8 +1071,7 @@ void xive_end_queue_pic_print_info(XiveEND *end, > uint32_t width, Monitor *mon) > > void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon) > { > - uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fffffff) << 32 > - | be32_to_cpu(end->w3); > + uint64_t qaddr_base = xive_end_qaddr(end); > uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1); > uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1); > uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0); > @@ -1101,8 +1099,7 @@ void xive_end_pic_print_info(XiveEND *end, uint32_t > end_idx, Monitor *mon) > > static void xive_end_enqueue(XiveEND *end, uint32_t data) > { > - uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fffffff) << 32 > - | be32_to_cpu(end->w3); > + uint64_t qaddr_base = xive_end_qaddr(end); > uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0); > uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1); > uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);