[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 09/14] ppc/xive2: Add support for MMIO operations on the N
|
From: |
Nicholas Piggin |
|
Subject: |
Re: [PATCH v2 09/14] ppc/xive2: Add support for MMIO operations on the NVPG/NVC BAR |
|
Date: |
Mon, 10 Mar 2025 15:10:47 +1000 |
On Tue Dec 10, 2024 at 10:05 AM AEST, Michael Kowal wrote:
> From: Frederic Barrat <fbarrat@linux.ibm.com>
>
> Add support for the NVPG and NVC BARs. Access to the BAR pages will
> cause backlog counter operations to either increment or decriment
> the counter.
>
> Also added qtests for the same.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
> ---
> include/hw/ppc/xive2.h | 9 ++
> include/hw/ppc/xive2_regs.h | 3 +
> tests/qtest/pnv-xive2-common.h | 1 +
> hw/intc/pnv_xive2.c | 80 +++++++++++++---
> hw/intc/xive2.c | 87 +++++++++++++++++
> tests/qtest/pnv-xive2-nvpg_bar.c | 154 +++++++++++++++++++++++++++++++
> tests/qtest/pnv-xive2-test.c | 3 +
> hw/intc/trace-events | 4 +
> tests/qtest/meson.build | 3 +-
> 9 files changed, 329 insertions(+), 15 deletions(-)
> create mode 100644 tests/qtest/pnv-xive2-nvpg_bar.c
>
> diff --git a/include/hw/ppc/xive2.h b/include/hw/ppc/xive2.h
> index fc7422fea7..c07e23e1d3 100644
> --- a/include/hw/ppc/xive2.h
> +++ b/include/hw/ppc/xive2.h
> @@ -90,6 +90,15 @@ int xive2_presenter_tctx_match(XivePresenter *xptr,
> XiveTCTX *tctx,
> uint8_t nvt_blk, uint32_t nvt_idx,
> bool cam_ignore, uint32_t logic_serv);
>
> +uint64_t xive2_presenter_nvp_backlog_op(XivePresenter *xptr,
> + uint8_t blk, uint32_t idx,
> + uint16_t offset);
> +
> +uint64_t xive2_presenter_nvgc_backlog_op(XivePresenter *xptr,
> + bool crowd,
> + uint8_t blk, uint32_t idx,
> + uint16_t offset, uint16_t val);
> +
> /*
> * XIVE2 END ESBs (POWER10)
> */
> diff --git a/include/hw/ppc/xive2_regs.h b/include/hw/ppc/xive2_regs.h
> index e88d6eab1e..9bcf7a8a6f 100644
> --- a/include/hw/ppc/xive2_regs.h
> +++ b/include/hw/ppc/xive2_regs.h
> @@ -233,4 +233,7 @@ typedef struct Xive2Nvgc {
> void xive2_nvgc_pic_print_info(Xive2Nvgc *nvgc, uint32_t nvgc_idx,
> GString *buf);
>
> +#define NVx_BACKLOG_OP PPC_BITMASK(52, 53)
> +#define NVx_BACKLOG_PRIO PPC_BITMASK(57, 59)
> +
> #endif /* PPC_XIVE2_REGS_H */
> diff --git a/tests/qtest/pnv-xive2-common.h b/tests/qtest/pnv-xive2-common.h
> index 9ae34771aa..2077c05ebc 100644
> --- a/tests/qtest/pnv-xive2-common.h
> +++ b/tests/qtest/pnv-xive2-common.h
> @@ -107,5 +107,6 @@ extern void set_end(QTestState *qts, uint32_t index,
> uint32_t nvp_index,
>
>
> void test_flush_sync_inject(QTestState *qts);
> +void test_nvpg_bar(QTestState *qts);
>
> #endif /* TEST_PNV_XIVE2_COMMON_H */
> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> index 41b727d1fb..54abfe3947 100644
> --- a/hw/intc/pnv_xive2.c
> +++ b/hw/intc/pnv_xive2.c
> @@ -2202,21 +2202,40 @@ static const MemoryRegionOps pnv_xive2_tm_ops = {
> },
> };
>
> -static uint64_t pnv_xive2_nvc_read(void *opaque, hwaddr offset,
> +static uint64_t pnv_xive2_nvc_read(void *opaque, hwaddr addr,
> unsigned size)
> {
> PnvXive2 *xive = PNV_XIVE2(opaque);
> + XivePresenter *xptr = XIVE_PRESENTER(xive);
> + uint32_t page = addr >> xive->nvpg_shift;
> + uint16_t op = addr & 0xFFF;
> + uint8_t blk = pnv_xive2_block_id(xive);
>
> - xive2_error(xive, "NVC: invalid read @%"HWADDR_PRIx, offset);
> - return -1;
> + if (size != 2) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid nvc load size %d\n",
> + size);
> + return -1;
> + }
> +
> + return xive2_presenter_nvgc_backlog_op(xptr, true, blk, page, op, 1);
> }
>
> -static void pnv_xive2_nvc_write(void *opaque, hwaddr offset,
> +static void pnv_xive2_nvc_write(void *opaque, hwaddr addr,
> uint64_t val, unsigned size)
> {
> PnvXive2 *xive = PNV_XIVE2(opaque);
> + XivePresenter *xptr = XIVE_PRESENTER(xive);
> + uint32_t page = addr >> xive->nvc_shift;
> + uint16_t op = addr & 0xFFF;
> + uint8_t blk = pnv_xive2_block_id(xive);
>
> - xive2_error(xive, "NVC: invalid write @%"HWADDR_PRIx, offset);
> + if (size != 1) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid nvc write size %d\n",
> + size);
> + return;
> + }
It would be nice to convert these accessors to _with_attrs() variants
that can report access errors like this back through the memory
subsystem. I guess that's something for the todo list rather than an
issue with this patch in particular.
> + /*
> + * op:
> + * 0b00 => increment
> + * 0b01 => decrement
> + * 0b1- => read
> + */
Could use define or enum for these like the qtest has...
> + if (op == 0b00 || op == 0b01) {
> + if (op == 0b00) {
> + count += val;
> + } else {
> + if (count > val) {
> + count -= val;
> + } else {
> + count = 0;
> + }
> + }
> + xive2_nvgc_set_backlog(&nvgc, priority, count);
> + xive2_router_write_nvgc(xrtr, crowd, blk, idx, &nvgc);
> + }
> + trace_xive_nvgc_backlog_op(crowd, blk, idx, op, priority, old_count);
> + return old_count;
> +}
> +
> +uint64_t xive2_presenter_nvp_backlog_op(XivePresenter *xptr,
> + uint8_t blk, uint32_t idx,
> + uint16_t offset)
> +{
> + Xive2Router *xrtr = XIVE2_ROUTER(xptr);
> + uint8_t priority = GETFIELD(NVx_BACKLOG_PRIO, offset);
> + uint8_t op = GETFIELD(NVx_BACKLOG_OP, offset);
> + Xive2Nvp nvp;
> + uint8_t ipb, old_ipb, rc;
> +
> + if (xive2_router_get_nvp(xrtr, blk, idx, &nvp)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: No NVP %x/%x\n", blk, idx);
> + return -1;
> + }
> + if (!xive2_nvp_is_valid(&nvp)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: Invalid NVP %x/%x\n", blk,
> idx);
> + return -1;
> + }
> +
> + old_ipb = xive_get_field32(NVP2_W2_IPB, nvp.w2);
> + ipb = old_ipb;
> + /*
> + * op:
> + * 0b00 => set priority bit
> + * 0b01 => reset priority bit
> + * 0b1- => read
> + */
> + if (op == 0b00 || op == 0b01) {
> + if (op == 0b00) {
> + ipb |= xive_priority_to_ipb(priority);
> + } else {
> + ipb &= ~xive_priority_to_ipb(priority);
> + }
> + nvp.w2 = xive_set_field32(NVP2_W2_IPB, nvp.w2, ipb);
> + xive2_router_write_nvp(xrtr, blk, idx, &nvp, 2);
> + }
> + rc = !!(old_ipb & xive_priority_to_ipb(priority));
> + trace_xive_nvp_backlog_op(blk, idx, op, priority, rc);
> + return rc;
> +}
> +
> void xive2_eas_pic_print_info(Xive2Eas *eas, uint32_t lisn, GString *buf)
> {
> if (!xive2_eas_is_valid(eas)) {
> diff --git a/tests/qtest/pnv-xive2-nvpg_bar.c
> b/tests/qtest/pnv-xive2-nvpg_bar.c
> new file mode 100644
> index 0000000000..10d4962d1e
> --- /dev/null
> +++ b/tests/qtest/pnv-xive2-nvpg_bar.c
> @@ -0,0 +1,154 @@
> +/*
> + * QTest testcase for PowerNV 10 interrupt controller (xive2)
> + * - Test NVPG BAR MMIO operations
> + *
> + * Copyright (c) 2024, IBM Corporation.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.
> + */
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +
> +#include "pnv-xive2-common.h"
> +
> +#define NVPG_BACKLOG_OP_SHIFT 10
> +#define NVPG_BACKLOG_PRIO_SHIFT 4
> +
> +#define XIVE_PRIORITY_MAX 7
> +
> +enum NVx {
> + NVP,
> + NVG,
> + NVC
> +};
> +
> +typedef enum {
> + INCR_STORE = 0b100,
> + INCR_LOAD = 0b000,
> + DECR_STORE = 0b101,
> + DECR_LOAD = 0b001,
> + READ_x = 0b010,
> + READ_y = 0b011,
> +} backlog_op;
> +
> +static uint32_t nvpg_backlog_op(QTestState *qts, backlog_op op,
> + enum NVx type, uint64_t index,
> + uint8_t priority, uint8_t delta)
> +{
> + uint64_t addr, offset;
> + uint32_t count = 0;
> +
> + switch (type) {
> + case NVP:
> + addr = XIVE_NVPG_ADDR + (index << (XIVE_PAGE_SHIFT + 1));
> + break;
> + case NVG:
> + addr = XIVE_NVPG_ADDR + (index << (XIVE_PAGE_SHIFT + 1)) +
> + (1 << XIVE_PAGE_SHIFT);
> + break;
> + case NVC:
> + addr = XIVE_NVC_ADDR + (index << XIVE_PAGE_SHIFT);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + offset = (op & 0b11) << NVPG_BACKLOG_OP_SHIFT;
> + offset |= priority << NVPG_BACKLOG_PRIO_SHIFT;
> + if (op >> 2) {
> + qtest_writeb(qts, addr + offset, delta);
> + } else {
> + count = qtest_readw(qts, addr + offset);
> + }
> + return count;
> +}
> +
> +void test_nvpg_bar(QTestState *qts)
> +{
> + uint32_t nvp_target = 0x11;
> + uint32_t group_target = 0x17; /* size 16 */
> + uint32_t vp_irq = 33, group_irq = 47;
> + uint32_t vp_end = 3, group_end = 97;
> + uint32_t vp_irq_data = 0x33333333;
> + uint32_t group_irq_data = 0x66666666;
> + uint8_t vp_priority = 0, group_priority = 5;
> + uint32_t vp_count[XIVE_PRIORITY_MAX + 1] = { 0 };
> + uint32_t group_count[XIVE_PRIORITY_MAX + 1] = { 0 };
> + uint32_t count, delta;
> + uint8_t i;
> +
> + printf("#
> ============================================================\n");
> + printf("# Testing NVPG BAR operations\n");
> +
> + set_nvg(qts, group_target, 0);
> + set_nvp(qts, nvp_target, 0x04);
> + set_nvp(qts, group_target, 0x04);
> +
> + /*
> + * Setup: trigger a VP-specific interrupt and a group interrupt
> + * so that the backlog counters are initialized to something else
> + * than 0 for at least one priority level
> + */
> + set_eas(qts, vp_irq, vp_end, vp_irq_data);
> + set_end(qts, vp_end, nvp_target, vp_priority, false /* group */);
> +
> + set_eas(qts, group_irq, group_end, group_irq_data);
> + set_end(qts, group_end, group_target, group_priority, true /* group */);
> +
> + get_esb(qts, vp_irq, XIVE_EOI_PAGE, XIVE_ESB_SET_PQ_00);
> + set_esb(qts, vp_irq, XIVE_TRIGGER_PAGE, 0, 0);
> + vp_count[vp_priority]++;
> +
> + get_esb(qts, group_irq, XIVE_EOI_PAGE, XIVE_ESB_SET_PQ_00);
> + set_esb(qts, group_irq, XIVE_TRIGGER_PAGE, 0, 0);
> + group_count[group_priority]++;
> +
> + /* check the initial counters */
> + for (i = 0; i <= XIVE_PRIORITY_MAX; i++) {
> + count = nvpg_backlog_op(qts, READ_x, NVP, nvp_target, i, 0);
> + g_assert_cmpuint(count, ==, vp_count[i]);
> +
> + count = nvpg_backlog_op(qts, READ_y, NVG, group_target, i, 0);
> + g_assert_cmpuint(count, ==, group_count[i]);
> + }
> +
> + /* do a few ops on the VP. Counter can only be 0 and 1 */
> + vp_priority = 2;
> + delta = 7;
> + nvpg_backlog_op(qts, INCR_STORE, NVP, nvp_target, vp_priority, delta);
> + vp_count[vp_priority] = 1;
> + count = nvpg_backlog_op(qts, INCR_LOAD, NVP, nvp_target, vp_priority, 0);
> + g_assert_cmpuint(count, ==, vp_count[vp_priority]);
> + count = nvpg_backlog_op(qts, READ_y, NVP, nvp_target, vp_priority, 0);
> + g_assert_cmpuint(count, ==, vp_count[vp_priority]);
> +
> + count = nvpg_backlog_op(qts, DECR_LOAD, NVP, nvp_target, vp_priority, 0);
> + g_assert_cmpuint(count, ==, vp_count[vp_priority]);
> + vp_count[vp_priority] = 0;
> + nvpg_backlog_op(qts, DECR_STORE, NVP, nvp_target, vp_priority, delta);
> + count = nvpg_backlog_op(qts, READ_x, NVP, nvp_target, vp_priority, 0);
> + g_assert_cmpuint(count, ==, vp_count[vp_priority]);
It is a bit confusing because NVP ops AFAIKS are set/clear of a priority
bit. But this has inc/dec. The comment is there and set/clear is basically
incrementing/decrementing a saturating 1-bit counter so it makes sense
but might be good to make this terminology match what the model uses.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> +
> + /* do a few ops on the group */
> + group_priority = 2;
> + delta = 9;
> + /* can't go negative */
> + nvpg_backlog_op(qts, DECR_STORE, NVG, group_target, group_priority,
> delta);
> + count = nvpg_backlog_op(qts, READ_y, NVG, group_target, group_priority,
> 0);
> + g_assert_cmpuint(count, ==, 0);
> + nvpg_backlog_op(qts, INCR_STORE, NVG, group_target, group_priority,
> delta);
> + group_count[group_priority] += delta;
> + count = nvpg_backlog_op(qts, INCR_LOAD, NVG, group_target,
> + group_priority, delta);
> + g_assert_cmpuint(count, ==, group_count[group_priority]);
> + group_count[group_priority]++;
> +
> + count = nvpg_backlog_op(qts, DECR_LOAD, NVG, group_target,
> + group_priority, delta);
> + g_assert_cmpuint(count, ==, group_count[group_priority]);
> + group_count[group_priority]--;
> + count = nvpg_backlog_op(qts, READ_x, NVG, group_target, group_priority,
> 0);
> + g_assert_cmpuint(count, ==, group_count[group_priority]);
> +}
> +
> diff --git a/tests/qtest/pnv-xive2-test.c b/tests/qtest/pnv-xive2-test.c
> index a4d06550ee..a0e9f19313 100644
> --- a/tests/qtest/pnv-xive2-test.c
> +++ b/tests/qtest/pnv-xive2-test.c
> @@ -493,6 +493,9 @@ static void test_xive(void)
> reset_state(qts);
> test_flush_sync_inject(qts);
>
> + reset_state(qts);
> + test_nvpg_bar(qts);
> +
> qtest_quit(qts);
> }
>
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 7435728c51..7f362c38b0 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -285,6 +285,10 @@ xive_tctx_tm_read(uint32_t index, uint64_t offset,
> unsigned int size, uint64_t v
> xive_presenter_notify(uint8_t nvt_blk, uint32_t nvt_idx, uint8_t ring,
> uint8_t group_level) "found NVT 0x%x/0x%x ring=0x%x group_level=%d"
> xive_end_source_read(uint8_t end_blk, uint32_t end_idx, uint64_t addr) "END
> 0x%x/0x%x @0x%"PRIx64
>
> +# xive2.c
> +xive_nvp_backlog_op(uint8_t blk, uint32_t idx, uint8_t op, uint8_t priority,
> uint8_t rc) "NVP 0x%x/0x%x operation=%d priority=%d rc=%d"
> +xive_nvgc_backlog_op(bool c, uint8_t blk, uint32_t idx, uint8_t op, uint8_t
> priority, uint32_t rc) "NVGC crowd=%d 0x%x/0x%x operation=%d priority=%d
> rc=%d"
> +
> # pnv_xive.c
> pnv_xive_ic_hw_trigger(uint64_t addr, uint64_t val) "@0x%"PRIx64"
> val=0x%"PRIx64
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index bd41c9da5f..f7da3df24b 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -348,7 +348,8 @@ qtests = {
> 'ivshmem-test': [rt, '../../contrib/ivshmem-server/ivshmem-server.c'],
> 'migration-test': migration_files,
> 'pxe-test': files('boot-sector.c'),
> - 'pnv-xive2-test': files('pnv-xive2-common.c', 'pnv-xive2-flush-sync.c'),
> + 'pnv-xive2-test': files('pnv-xive2-common.c', 'pnv-xive2-flush-sync.c',
> + 'pnv-xive2-nvpg_bar.c'),
> 'qos-test': [chardev, io, qos_test_ss.apply({}).sources()],
> 'tpm-crb-swtpm-test': [io, tpmemu_files],
> 'tpm-crb-test': [io, tpmemu_files],
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 09/14] ppc/xive2: Add support for MMIO operations on the NVPG/NVC BAR,
Nicholas Piggin <=