[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 v5 10/23] ppc/pnv: Loop on the threads of the chip to
From: |
Greg Kurz |
Subject: |
Re: [PATCH for-5.0 v5 10/23] ppc/pnv: Loop on the threads of the chip to find a matching NVT |
Date: |
Wed, 20 Nov 2019 17:13:14 +0100 |
On Fri, 15 Nov 2019 17:24:23 +0100
Cédric Le Goater <address@hidden> wrote:
> CPU_FOREACH() loops on all the CPUs of the machine which is incorrect.
> Each XIVE Presenter should scan only the HW threads of the chip it
> belongs to.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
> include/hw/ppc/pnv.h | 2 ++
> hw/intc/pnv_xive.c | 63 ++++++++++++++++++++++++++------------------
> hw/ppc/pnv.c | 2 +-
> 3 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 07c56c05ad30..58f4dcc0b71d 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -150,6 +150,8 @@ typedef struct PnvChipClass {
> */
> #define PNV_CHIP_HWID(i) ((((i) & 0x3e) << 3) | ((i) & 0x1))
>
> +const char *pnv_chip_core_typename(const PnvChip *chip);
> +
> /*
> * Converts back a HW chip id to an index. This is useful to calculate
> * the MMIO addresses of some controllers which depend on the chip id.
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 087cbfbaad48..71ca4961b6b1 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -377,34 +377,45 @@ static int pnv_xive_match_nvt(XivePresenter *xptr,
> uint8_t format,
> bool cam_ignore, uint8_t priority,
> uint32_t logic_serv, XiveTCTXMatch *match)
> {
> - CPUState *cs;
> + PnvXive *xive = PNV_XIVE(xptr);
> + PnvChip *chip = xive->chip;
> + const char *typename = pnv_chip_core_typename(chip);
> + size_t typesize = object_type_get_instance_size(typename);
Ahh the same boiler plate is needed again because the cores are
stored in an void *cores array in PnvChip... which is an ugly and
fragile pattern we got rid of in spapr (commit 94ad93bd976841). We
probably don't have the same risk here because the powernv machine
doesn't support hot-unplug, but this is still ugly anyway.
I certainly don't want to hold this series any longer for the sake
of Cedric's health :) but I'll be glad to turn the void *cores array
into a PnvCore **cores as a follow-up cleanup.
Rest LGTM.
Reviewed-by: Greg Kurz <address@hidden>
> int count = 0;
> -
> - CPU_FOREACH(cs) {
> - PowerPCCPU *cpu = POWERPC_CPU(cs);
> - XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> - int ring;
> -
> - /*
> - * Check the thread context CAM lines and record matches.
> - */
> - ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk,
> nvt_idx,
> - cam_ignore, logic_serv);
> - /*
> - * Save the context and follow on to catch duplicates, that we
> - * don't support yet.
> - */
> - if (ring != -1) {
> - if (match->tctx) {
> - qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a "
> - "thread context NVT %x/%x\n",
> - nvt_blk, nvt_idx);
> - return -1;
> + int i, j;
> +
> + for (i = 0; i < chip->nr_cores; i++) {
> + PnvCore *pc = PNV_CORE(chip->cores + i * typesize);
> + CPUCore *cc = CPU_CORE(pc);
> +
> + for (j = 0; j < cc->nr_threads; j++) {
> + PowerPCCPU *cpu = pc->threads[j];
> + XiveTCTX *tctx;
> + int ring;
> +
> + tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> +
> + /*
> + * Check the thread context CAM lines and record matches.
> + */
> + ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk,
> + nvt_idx, cam_ignore,
> logic_serv);
> + /*
> + * Save the context and follow on to catch duplicates, that we
> + * don't support yet.
> + */
> + if (ring != -1) {
> + if (match->tctx) {
> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a "
> + "thread context NVT %x/%x\n",
> + nvt_blk, nvt_idx);
> + return -1;
> + }
> +
> + match->ring = ring;
> + match->tctx = tctx;
> + count++;
> }
> -
> - match->ring = ring;
> - match->tctx = tctx;
> - count++;
> }
> }
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 57f924ba0466..94c9f536413f 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -64,7 +64,7 @@
> #define INITRD_LOAD_ADDR 0x60000000
> #define INITRD_MAX_SIZE (256 * MiB)
>
> -static const char *pnv_chip_core_typename(const PnvChip *o)
> +const char *pnv_chip_core_typename(const PnvChip *o)
> {
> const char *chip_type =
> object_class_get_name(object_get_class(OBJECT(o)));
> int len = strlen(chip_type) - strlen(PNV_CHIP_TYPE_SUFFIX);
- [PATCH for-5.0 v5 06/23] ppc/xive: Introduce OS CAM line helpers, (continued)
- [PATCH for-5.0 v5 06/23] ppc/xive: Introduce OS CAM line helpers, Cédric Le Goater, 2019/11/15
- [PATCH for-5.0 v5 07/23] ppc/xive: Check V bit in TM_PULL_POOL_CTX, Cédric Le Goater, 2019/11/15
- [PATCH for-5.0 v5 08/23] ppc/xive: Introduce a XivePresenter interface, Cédric Le Goater, 2019/11/15
- [PATCH for-5.0 v5 09/23] ppc/xive: Implement the XivePresenter interface, Cédric Le Goater, 2019/11/15
- [PATCH for-5.0 v5 10/23] ppc/pnv: Loop on the threads of the chip to find a matching NVT, Cédric Le Goater, 2019/11/15
- Re: [PATCH for-5.0 v5 10/23] ppc/pnv: Loop on the threads of the chip to find a matching NVT,
Greg Kurz <=
- [PATCH for-5.0 v5 11/23] ppc/pnv: Introduce a pnv_xive_is_cpu_enabled() helper, Cédric Le Goater, 2019/11/15
[PATCH for-5.0 v5 12/23] ppc/xive: Introduce a XiveFabric interface, Cédric Le Goater, 2019/11/15
[PATCH for-5.0 v5 13/23] ppc/pnv: Implement the XiveFabric interface, Cédric Le Goater, 2019/11/15