* [Qemu-devel] [PATCH 0/3] spapr/xive: fixes on EQ page addresses
@ 2019-05-08 17:19 Cédric Le Goater
2019-05-08 17:19 ` [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned Cédric Le Goater
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Cédric Le Goater @ 2019-05-08 17:19 UTC (permalink / raw
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Hello
Here is a small series adding a check on the EQ page address alignment
and fixing a severe issue when addresses are above 64GB.
Thanks,
C.
Cédric Le Goater (3):
spapr/xive: EQ page should be naturally aligned
spapr/xive: fix EQ page addresses above 64GB
spapr/xive: print out the EQ page address in the monitor
include/hw/ppc/xive_regs.h | 6 ++++++
hw/intc/spapr_xive.c | 14 ++++++++++----
hw/intc/xive.c | 9 +++------
3 files changed, 19 insertions(+), 10 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned
2019-05-08 17:19 [Qemu-devel] [PATCH 0/3] spapr/xive: fixes on EQ page addresses Cédric Le Goater
@ 2019-05-08 17:19 ` Cédric Le Goater
2019-05-09 5:37 ` David Gibson
2019-05-09 8:48 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2019-05-08 17:19 ` [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB Cédric Le Goater
2019-05-08 17:19 ` [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor Cédric Le Goater
2 siblings, 2 replies; 10+ messages in thread
From: Cédric Le Goater @ 2019-05-08 17:19 UTC (permalink / raw
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
When the OS configures the EQ page in which to receive event
notifications from the XIVE interrupt controller, the page should be
naturally aligned. Add this check.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/intc/spapr_xive.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 097f88d4608d..666e24e9b447 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -993,6 +993,12 @@ static target_ulong h_int_set_queue_config(PowerPCCPU *cpu,
case 16:
case 21:
case 24:
+ if (!QEMU_IS_ALIGNED(qpage, 1ul << qsize)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "XIVE: EQ @0x%" HWADDR_PRIx
+ " is not naturally aligned with %" HWADDR_PRIx "\n",
+ qpage, 1ul << qsize);
+ return H_P4;
+ }
end.w2 = cpu_to_be32((qpage >> 32) & 0x0fffffff);
end.w3 = cpu_to_be32(qpage & 0xffffffff);
end.w0 |= cpu_to_be32(END_W0_ENQUEUE);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB
2019-05-08 17:19 [Qemu-devel] [PATCH 0/3] spapr/xive: fixes on EQ page addresses Cédric Le Goater
2019-05-08 17:19 ` [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned Cédric Le Goater
@ 2019-05-08 17:19 ` Cédric Le Goater
2019-05-09 5:40 ` David Gibson
2019-05-09 8:51 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2019-05-08 17:19 ` [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor Cédric Le Goater
2 siblings, 2 replies; 10+ messages in thread
From: Cédric Le Goater @ 2019-05-08 17:19 UTC (permalink / raw
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
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 <clg@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);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor
2019-05-08 17:19 [Qemu-devel] [PATCH 0/3] spapr/xive: fixes on EQ page addresses Cédric Le Goater
2019-05-08 17:19 ` [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned Cédric Le Goater
2019-05-08 17:19 ` [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB Cédric Le Goater
@ 2019-05-08 17:19 ` Cédric Le Goater
2019-05-09 5:40 ` David Gibson
2019-05-09 8:52 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2 siblings, 2 replies; 10+ messages in thread
From: Cédric Le Goater @ 2019-05-08 17:19 UTC (permalink / raw
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
This proved to be a useful information when debugging issues with OS
event queues allocated above 64GB.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/intc/spapr_xive.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 810435c30cc7..7faf03b1fb7c 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -120,6 +120,7 @@ static int spapr_xive_target_to_end(uint32_t target, uint8_t prio,
static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
Monitor *mon)
{
+ 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);
@@ -127,9 +128,9 @@ static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
uint32_t nvt = xive_get_field32(END_W6_NVT_INDEX, end->w6);
uint8_t priority = xive_get_field32(END_W7_F0_PRIORITY, end->w7);
- monitor_printf(mon, "%3d/%d % 6d/%5d ^%d",
+ monitor_printf(mon, "%3d/%d % 6d/%5d @%"PRIx64" ^%d",
spapr_xive_nvt_to_target(0, nvt),
- priority, qindex, qentries, qgen);
+ priority, qindex, qentries, qaddr_base, qgen);
xive_end_queue_pic_print_info(end, 6, mon);
monitor_printf(mon, "]");
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned
2019-05-08 17:19 ` [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned Cédric Le Goater
@ 2019-05-09 5:37 ` David Gibson
2019-05-09 8:48 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
1 sibling, 0 replies; 10+ messages in thread
From: David Gibson @ 2019-05-09 5:37 UTC (permalink / raw
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]
On Wed, May 08, 2019 at 07:19:44PM +0200, Cédric Le Goater wrote:
> When the OS configures the EQ page in which to receive event
> notifications from the XIVE interrupt controller, the page should be
> naturally aligned. Add this check.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Applied, thanks.
> ---
> hw/intc/spapr_xive.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 097f88d4608d..666e24e9b447 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -993,6 +993,12 @@ static target_ulong h_int_set_queue_config(PowerPCCPU *cpu,
> case 16:
> case 21:
> case 24:
> + if (!QEMU_IS_ALIGNED(qpage, 1ul << qsize)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: EQ @0x%" HWADDR_PRIx
> + " is not naturally aligned with %" HWADDR_PRIx "\n",
> + qpage, 1ul << qsize);
> + return H_P4;
> + }
> end.w2 = cpu_to_be32((qpage >> 32) & 0x0fffffff);
> end.w3 = cpu_to_be32(qpage & 0xffffffff);
> end.w0 |= cpu_to_be32(END_W0_ENQUEUE);
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB
2019-05-08 17:19 ` [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB Cédric Le Goater
@ 2019-05-09 5:40 ` David Gibson
2019-05-09 8:51 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
1 sibling, 0 replies; 10+ messages in thread
From: David Gibson @ 2019-05-09 5:40 UTC (permalink / raw
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4490 bytes --]
On Wed, May 08, 2019 at 07:19:45PM +0200, Cédric Le Goater 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 <clg@kaod.org>
Applied, thanks.
> ---
> 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);
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor
2019-05-08 17:19 ` [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor Cédric Le Goater
@ 2019-05-09 5:40 ` David Gibson
2019-05-09 8:52 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
1 sibling, 0 replies; 10+ messages in thread
From: David Gibson @ 2019-05-09 5:40 UTC (permalink / raw
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1832 bytes --]
On Wed, May 08, 2019 at 07:19:46PM +0200, Cédric Le Goater wrote:
> This proved to be a useful information when debugging issues with OS
> event queues allocated above 64GB.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Applied, thanks.
> ---
> hw/intc/spapr_xive.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 810435c30cc7..7faf03b1fb7c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -120,6 +120,7 @@ static int spapr_xive_target_to_end(uint32_t target, uint8_t prio,
> static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
> Monitor *mon)
> {
> + 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);
> @@ -127,9 +128,9 @@ static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
> uint32_t nvt = xive_get_field32(END_W6_NVT_INDEX, end->w6);
> uint8_t priority = xive_get_field32(END_W7_F0_PRIORITY, end->w7);
>
> - monitor_printf(mon, "%3d/%d % 6d/%5d ^%d",
> + monitor_printf(mon, "%3d/%d % 6d/%5d @%"PRIx64" ^%d",
> spapr_xive_nvt_to_target(0, nvt),
> - priority, qindex, qentries, qgen);
> + priority, qindex, qentries, qaddr_base, qgen);
>
> xive_end_queue_pic_print_info(end, 6, mon);
> monitor_printf(mon, "]");
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned
2019-05-08 17:19 ` [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned Cédric Le Goater
2019-05-09 5:37 ` David Gibson
@ 2019-05-09 8:48 ` Greg Kurz
1 sibling, 0 replies; 10+ messages in thread
From: Greg Kurz @ 2019-05-09 8:48 UTC (permalink / raw
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson
On Wed, 8 May 2019 19:19:44 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> When the OS configures the EQ page in which to receive event
> notifications from the XIVE interrupt controller, the page should be
> naturally aligned. Add this check.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/intc/spapr_xive.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 097f88d4608d..666e24e9b447 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -993,6 +993,12 @@ static target_ulong h_int_set_queue_config(PowerPCCPU *cpu,
> case 16:
> case 21:
> case 24:
> + if (!QEMU_IS_ALIGNED(qpage, 1ul << qsize)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: EQ @0x%" HWADDR_PRIx
> + " is not naturally aligned with %" HWADDR_PRIx "\n",
> + qpage, 1ul << qsize);
> + return H_P4;
> + }
> end.w2 = cpu_to_be32((qpage >> 32) & 0x0fffffff);
> end.w3 = cpu_to_be32(qpage & 0xffffffff);
> end.w0 |= cpu_to_be32(END_W0_ENQUEUE);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB
2019-05-08 17:19 ` [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB Cédric Le Goater
2019-05-09 5:40 ` David Gibson
@ 2019-05-09 8:51 ` Greg Kurz
1 sibling, 0 replies; 10+ messages in thread
From: Greg Kurz @ 2019-05-09 8:51 UTC (permalink / raw
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson
On Wed, 8 May 2019 19:19:45 +0200
Cédric Le Goater <clg@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 <clg@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 <groug@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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor
2019-05-08 17:19 ` [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor Cédric Le Goater
2019-05-09 5:40 ` David Gibson
@ 2019-05-09 8:52 ` Greg Kurz
1 sibling, 0 replies; 10+ messages in thread
From: Greg Kurz @ 2019-05-09 8:52 UTC (permalink / raw
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson
On Wed, 8 May 2019 19:19:46 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> This proved to be a useful information when debugging issues with OS
> event queues allocated above 64GB.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/intc/spapr_xive.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 810435c30cc7..7faf03b1fb7c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -120,6 +120,7 @@ static int spapr_xive_target_to_end(uint32_t target, uint8_t prio,
> static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
> Monitor *mon)
> {
> + 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);
> @@ -127,9 +128,9 @@ static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
> uint32_t nvt = xive_get_field32(END_W6_NVT_INDEX, end->w6);
> uint8_t priority = xive_get_field32(END_W7_F0_PRIORITY, end->w7);
>
> - monitor_printf(mon, "%3d/%d % 6d/%5d ^%d",
> + monitor_printf(mon, "%3d/%d % 6d/%5d @%"PRIx64" ^%d",
> spapr_xive_nvt_to_target(0, nvt),
> - priority, qindex, qentries, qgen);
> + priority, qindex, qentries, qaddr_base, qgen);
>
> xive_end_queue_pic_print_info(end, 6, mon);
> monitor_printf(mon, "]");
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-09 8:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-08 17:19 [Qemu-devel] [PATCH 0/3] spapr/xive: fixes on EQ page addresses Cédric Le Goater
2019-05-08 17:19 ` [Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned Cédric Le Goater
2019-05-09 5:37 ` David Gibson
2019-05-09 8:48 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2019-05-08 17:19 ` [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB Cédric Le Goater
2019-05-09 5:40 ` David Gibson
2019-05-09 8:51 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2019-05-08 17:19 ` [Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor Cédric Le Goater
2019-05-09 5:40 ` David Gibson
2019-05-09 8:52 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.